From ee1e747c2965a8f6182e8b35ec42a0cb99cc6632 Mon Sep 17 00:00:00 2001
From: "Chris M. Hostetter" <hossman@apache.org>
Date: Tue, 11 Sep 2012 18:38:45 +0000
Subject: [PATCH] SOLR-3628: SolrInputField and SolrInputDocument are now
 consistently backed by Collections passed in to setValue/setField, and
 defensively copy values from Collections passed to addValue/addField

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1383520 13f79535-47bb-0310-9956-ffa450edef68
---
 solr/CHANGES.txt                              |  5 +++++
 .../org/apache/solr/common/SolrDocument.java  | 15 ++++++++++---
 .../apache/solr/common/SolrInputDocument.java |  6 +++--
 .../apache/solr/common/SolrInputField.java    | 19 ++++++++++++----
 .../apache/solr/common/SolrDocumentTest.java  | 22 +++++++++++++++++++
 5 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fced1988785..62770b86a7d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -140,6 +140,11 @@ Bug Fixes
 * SOLR-3518: Include final 'hits' in log information when aggregating a 
   distibuted request (Markus Jelsma via hossman)
 
+* SOLR-3628: SolrInputField and SolrInputDocument are now consistently backed 
+  by Collections passed in to setValue/setField, and defensively copy values 
+  from Collections passed to addValue/addField
+  (Tom Switzer via hossman)
+
 Other Changes
 ----------------------
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrDocument.java b/solr/solrj/src/java/org/apache/solr/common/SolrDocument.java
index fb158981325..53fe21503dc 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrDocument.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrDocument.java
@@ -105,15 +105,24 @@ public class SolrDocument implements Map<String,Object>, Iterable<Map.Entry<Stri
   }
   
   /**
-   * This will add a field to the document.  If fields already exist with this name
-   * it will append the collection
+   * This will add a field to the document.  If fields already exist with this
+   * name it will append value to the collection. If the value is Collection,
+   * each value will be added independently.
    */
   @SuppressWarnings("unchecked")
   public void addField(String name, Object value) 
   { 
     Object existing = _fields.get(name);
     if (existing == null) {
-      this.setField( name, value );
+      if( value instanceof Collection ) {
+        Collection<Object> c = new ArrayList<Object>( 3 );
+        for ( Object o : (Collection<Object>)value ) {
+          c.add(o);
+        }
+        this.setField( name, c );
+      } else {
+        this.setField( name, value );
+      }
       return;
     }
     
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrInputDocument.java b/solr/solrj/src/java/org/apache/solr/common/SolrInputDocument.java
index 660802fe5a8..99e1951552f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrInputDocument.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrInputDocument.java
@@ -126,8 +126,10 @@ public class SolrInputDocument implements Map<String,SolrInputField>, Iterable<S
   }
 
   /**
-   * Adds a field with the given name, value and boost.  If a field with the name already exists, then it is updated to
-   * the new value and boost.
+   * Adds a field with the given name, value and boost.  If a field with the
+   * name already exists, then the given value is appended to the value of that
+   * field, with the new boost. If the value is a collection, then each of its
+   * values will be added to the field.
    *
    * @param name Name of the field to add
    * @param value Value of the field
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrInputField.java b/solr/solrj/src/java/org/apache/solr/common/SolrInputField.java
index 18f06764d98..b2c599a7d48 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrInputField.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrInputField.java
@@ -41,7 +41,9 @@ public class SolrInputField implements Iterable<Object>, Serializable
   //---------------------------------------------------------------
 
   /**
-   * Set the value for a field.  Arrays will be converted to a collection.
+   * Set the value for a field.  Arrays will be converted to a collection. If
+   * a collection is given, then that collection will be used as the backing
+   * collection for the values.
    */
   public void setValue(Object v, float b) {
     boost = b;
@@ -60,13 +62,22 @@ public class SolrInputField implements Iterable<Object>, Serializable
   }
 
   /**
-   * Add values to a field.  if the added value is a collection, each value
-   * will be added individually
+   * Add values to a field.  If the added value is a collection, each value
+   * will be added individually.
    */
   @SuppressWarnings("unchecked")
   public void addValue(Object v, float b) {
     if( value == null ) {
-      setValue(v, b);
+      if ( v instanceof Collection ) {
+        Collection<Object> c = new ArrayList<Object>( 3 );
+        for ( Object o : (Collection<Object>)v ) {
+          c.add( o );
+        }
+        setValue(c, b);
+      } else {
+        setValue(v, b);
+      }
+
       return;
     }
     
diff --git a/solr/solrj/src/test/org/apache/solr/common/SolrDocumentTest.java b/solr/solrj/src/test/org/apache/solr/common/SolrDocumentTest.java
index 47b3d6e5afa..51e04f1776d 100644
--- a/solr/solrj/src/test/org/apache/solr/common/SolrDocumentTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/SolrDocumentTest.java
@@ -158,6 +158,28 @@ public class SolrDocumentTest extends LuceneTestCase
     assertFalse( doc.getFieldValuesMap().containsKey( "g" ) );
     assertFalse( doc.getFieldValueMap().keySet().contains( "g" ) );
     assertFalse( doc.getFieldValuesMap().keySet().contains( "g" ) );
+
+    // A read-only list shouldn't break addField("v", ...).
+    List<String> ro = Collections.unmodifiableList(c0);
+    doc = new SolrDocument();
+    doc.addField( "v", ro );
+
+    // This should NOT throw an UnsupportedOperationException.
+    doc.addField( "v", "asdf" );
+
+    // set field using a collection is documented to be backed by 
+    // that collection, so changes should affect it.
+    Collection<String> tmp = new ArrayList<String>(3);
+    tmp.add("one");
+    doc.setField( "collection_backed", tmp );
+    assertEquals("collection not the same", 
+                 tmp, doc.getFieldValues( "collection_backed" ));
+    tmp.add("two");
+    assertEquals("wrong size", 
+                 2, doc.getFieldValues( "collection_backed" ).size());
+    assertEquals("collection not the same", 
+                 tmp, doc.getFieldValues( "collection_backed" ));
+    
   }
    
   public void testDuplicate()