SOLR-1870: the javabin format no longer throws a ClassCastException when SolrInputDocuments contain field values that implement Iterable

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@954336 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -330,6 +330,11 @@ Bug Fixes
 
 * SOLR-1948: PatternTokenizerFactory should use parent's args (koji)
 
+* SOLR-1870: Indexing documents using the 'javabin' format no longer
+  fails with a ClassCastException whenSolrInputDocuments contain field
+  values which are Collections or other classes that implement
+  Iterable. (noble, hossman)
+
 
 Other Changes
 ----------------------

solr/src/common/org/apache/solr/common/util/JavaBinCodec.java

@@ -225,8 +225,8 @@ public class JavaBinCodec {
       writeSolrDocumentList((SolrDocumentList) val);
       return true;
     }
-    if (val instanceof List) {
+    if (val instanceof Collection) {
-      writeArray((List) val);
+      writeArray((Collection) val);
       return true;
     }
     if (val instanceof Object[]) {
@@ -390,6 +390,14 @@ public class JavaBinCodec {
     }
   }
 
+  public void writeArray(Collection coll) throws IOException {
+    writeTag(ARR, coll.size());
+    for (Object o : coll) {
+      writeVal(o);
+    }
+
+  }
+
   public void writeArray(Object[] arr) throws IOException {
     writeTag(ARR, arr.length);
     for (int i = 0; i < arr.length; i++) {

solr/src/solrj/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java

@@ -96,6 +96,11 @@ public class JavaBinUpdateRequestCodec {
     final NamedList[] namedList = new NamedList[1];
     JavaBinCodec codec = new JavaBinCodec() {
 
+      // NOTE: this only works because this is an anonymous inner class 
+      // which will only ever be used on a single stream -- if this class 
+      // is ever refactored, this will not work.
+      private boolean seenOuterMostDocIterator = false;
+        
       public NamedList readNamedList(FastInputStream dis) throws IOException {
         int sz = readSize(dis);
         NamedList nl = new NamedList();
@@ -110,8 +115,18 @@ public class JavaBinUpdateRequestCodec {
         return nl;
       }
 
-
       public List readIterator(FastInputStream fis) throws IOException {
+
+        // default behavior for reading any regular Iterator in the stream
+        if (seenOuterMostDocIterator) return super.readIterator(fis);
+
+        // special treatment for first outermost Iterator 
+        // (the list of documents)
+        seenOuterMostDocIterator = true;
+        return readOuterMostDocIterator(fis);
+      }
+
+      private List readOuterMostDocIterator(FastInputStream fis) throws IOException {
         NamedList params = (NamedList) namedList[0].getVal(0);
         updateRequest.setParams(namedListToSolrParams(params));
         if (handler == null) return super.readIterator(fis);

solr/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java

@@ -24,6 +24,10 @@ import org.junit.Test;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.ArrayList;
 
@@ -43,22 +47,33 @@ public class TestUpdateRequestCodec {
     updateRequest.deleteById("id:5");
     updateRequest.deleteByQuery("2*");
     updateRequest.deleteByQuery("1*");
+
     SolrInputDocument doc = new SolrInputDocument();
     doc.addField("id", 1);
     doc.addField("desc", "one", 2.0f);
     doc.addField("desc", "1");
     updateRequest.add(doc);
+
     doc = new SolrInputDocument();
     doc.addField("id", 2);
     doc.setDocumentBoost(10.0f);
     doc.addField("desc", "two", 3.0f);
     doc.addField("desc", "2");
     updateRequest.add(doc);
+
     doc = new SolrInputDocument();
     doc.addField("id", 3);
     doc.addField("desc", "three", 3.0f);
     doc.addField("desc", "3");
     updateRequest.add(doc);
+
+    doc = new SolrInputDocument();
+    Collection<String> foobar = new HashSet<String>();
+    foobar.add("baz1");
+    foobar.add("baz2");
+    doc.addField("foobar",foobar);
+    updateRequest.add(doc);
+
 //    updateRequest.setWaitFlush(true);
     updateRequest.deleteById("2");
     updateRequest.deleteByQuery("id:3");
@@ -69,7 +84,6 @@ public class TestUpdateRequestCodec {
     JavaBinUpdateRequestCodec.StreamingDocumentHandler handler = new JavaBinUpdateRequestCodec.StreamingDocumentHandler() {
       public void document(SolrInputDocument document, UpdateRequest req) {
         Assert.assertNotNull(req.getParams());
-//        Assert.assertEquals(Boolean.TRUE, req.getParams().getBool(UpdateParams.WAIT_FLUSH));
         docs.add(document);
       }
     };
@@ -82,20 +96,89 @@ public class TestUpdateRequestCodec {
     for (int i = 0; i < updateRequest.getDocuments().size(); i++) {
       SolrInputDocument inDoc = updateRequest.getDocuments().get(i);
       SolrInputDocument outDoc = updateUnmarshalled.getDocuments().get(i);
-      compareDocs(inDoc, outDoc);
+      compareDocs("doc#"+i, inDoc, outDoc);
     }
-    Assert.assertEquals(updateUnmarshalled.getDeleteById().get(0) , updateRequest.getDeleteById().get(0));
+    Assert.assertEquals(updateUnmarshalled.getDeleteById().get(0) , 
-    Assert.assertEquals(updateUnmarshalled.getDeleteQuery().get(0) , updateRequest.getDeleteQuery().get(0));
+                        updateRequest.getDeleteById().get(0));
+    Assert.assertEquals(updateUnmarshalled.getDeleteQuery().get(0) , 
+                        updateRequest.getDeleteQuery().get(0));
 
   }
 
-  private void compareDocs(SolrInputDocument docA, SolrInputDocument docB) {
+  @Test
-    Assert.assertEquals(docA.getDocumentBoost(), docB.getDocumentBoost());
+  public void testIteratable() throws IOException {
-    for (String s : docA.getFieldNames()) {
+    final List<String> values = new ArrayList<String>();
-      SolrInputField fldA = docA.getField(s);
+    values.add("iterItem1");
-      SolrInputField fldB = docB.getField(s);
+    values.add("iterItem2");
-      Assert.assertEquals(fldA.getValue(), fldB.getValue());
+
-      Assert.assertEquals(fldA.getBoost(), fldB.getBoost());
+    UpdateRequest updateRequest = new UpdateRequest();
+    updateRequest.deleteByQuery("*:*");
+
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.addField("id", 1);
+    doc.addField("desc", "one", 2.0f);
+    // imagine someone adding a custom Bean that implements Iterable 
+    // but is not a Collection
+    doc.addField("iter", new Iterable<String>() { 
+        public Iterator<String> iterator() { return values.iterator(); } 
+      });
+    doc.addField("desc", "1");
+    updateRequest.add(doc);
+
+    JavaBinUpdateRequestCodec codec = new JavaBinUpdateRequestCodec();
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    codec.marshal(updateRequest, baos);
+    final List<SolrInputDocument> docs = new ArrayList<SolrInputDocument>();
+    JavaBinUpdateRequestCodec.StreamingDocumentHandler handler = new JavaBinUpdateRequestCodec.StreamingDocumentHandler() {
+      public void document(SolrInputDocument document, UpdateRequest req) {
+        Assert.assertNotNull(req.getParams());
+        docs.add(document);
+      }
+    };
+
+    UpdateRequest updateUnmarshalled = codec.unmarshal(new ByteArrayInputStream(baos.toByteArray()) ,handler);
+    Assert.assertNull(updateUnmarshalled.getDocuments());
+    for (SolrInputDocument document : docs) {
+      updateUnmarshalled.add(document);
+    }
+
+    SolrInputDocument outDoc = updateUnmarshalled.getDocuments().get(0);
+    SolrInputField iter = outDoc.getField("iter");
+    Assert.assertNotNull("iter field is null", iter);
+    Object iterVal = iter.getValue();
+    Assert.assertTrue("iterVal is not a Collection", 
+                      iterVal instanceof Collection);
+    Assert.assertEquals("iterVal contents", values, iterVal);
+
+  }
+
+      
+
+  private void compareDocs(String m, 
+                           SolrInputDocument expectedDoc, 
+                           SolrInputDocument actualDoc) {
+    Assert.assertEquals(expectedDoc.getDocumentBoost(), 
+                        actualDoc.getDocumentBoost());
+
+    for (String s : expectedDoc.getFieldNames()) {
+      SolrInputField expectedField = expectedDoc.getField(s);
+      SolrInputField actualField = actualDoc.getField(s);
+      Assert.assertEquals(m + ": diff boosts for field: " + s,
+                          expectedField.getBoost(), actualField.getBoost());
+      Object expectedVal = expectedField.getValue();
+      Object actualVal = actualField.getValue();
+      if (expectedVal instanceof Set &&
+          actualVal instanceof Collection) {
+        // unmarshaled documents never contain Sets, they are just a 
+        // List in an arbitrary order based on what the iterator of 
+        // hte original Set returned, so we need a comparison that is 
+        // order agnostic.
+        actualVal = new HashSet((Collection) actualVal);
+        m += " (Set comparison)";
+      }
+
+      Assert.assertEquals(m + " diff values for field: " + s,
+                          expectedVal, actualVal);
     }
   }