diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cb3e40c378b..62ae214588f 100644 --- a/solr/CHANGES.txt +++ b/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 ---------------------- diff --git a/solr/src/common/org/apache/solr/common/util/JavaBinCodec.java b/solr/src/common/org/apache/solr/common/util/JavaBinCodec.java index ada509660ff..514e6ab1d16 100755 --- a/solr/src/common/org/apache/solr/common/util/JavaBinCodec.java +++ b/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) { - writeArray((List) val); + if (val instanceof Collection) { + 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++) { diff --git a/solr/src/solrj/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java b/solr/src/solrj/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java index bdb98664e84..3c7e0867737 100644 --- a/solr/src/solrj/org/apache/solr/client/solrj/request/JavaBinUpdateRequestCodec.java +++ b/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); diff --git a/solr/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java b/solr/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java index 6354c991070..55587ae32e1 100644 --- a/solr/src/test/org/apache/solr/client/solrj/request/TestUpdateRequestCodec.java +++ b/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 foobar = new HashSet(); + 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.getDeleteQuery().get(0) , updateRequest.getDeleteQuery().get(0)); + Assert.assertEquals(updateUnmarshalled.getDeleteById().get(0) , + updateRequest.getDeleteById().get(0)); + Assert.assertEquals(updateUnmarshalled.getDeleteQuery().get(0) , + updateRequest.getDeleteQuery().get(0)); } - private void compareDocs(SolrInputDocument docA, SolrInputDocument docB) { - Assert.assertEquals(docA.getDocumentBoost(), docB.getDocumentBoost()); - for (String s : docA.getFieldNames()) { - SolrInputField fldA = docA.getField(s); - SolrInputField fldB = docB.getField(s); - Assert.assertEquals(fldA.getValue(), fldB.getValue()); - Assert.assertEquals(fldA.getBoost(), fldB.getBoost()); + @Test + public void testIteratable() throws IOException { + final List values = new ArrayList(); + values.add("iterItem1"); + values.add("iterItem2"); + + 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() { + public Iterator 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 docs = new ArrayList(); + 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); } }