diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 1e827a2d901..007044444a3 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -93,6 +93,9 @@ Bug Fixes * SOLR-5423: CSV output doesn't include function field (Arun Kumar, hossman, Steve Rowe) +* SOLR-5777: Fix ordering of field values in JSON updates where + field name key is repeated (hossman) + Optimizations ---------------------- * SOLR-1880: Distributed Search skips GET_FIELDS stage if EXECUTE_QUERY diff --git a/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java b/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java index 71b6e03d92c..3c649a40dd0 100644 --- a/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java +++ b/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java @@ -418,11 +418,11 @@ public class JsonLoader extends ContentStreamLoader { for (;;) { SolrInputField sif = parseField(); if (sif == null) return sdoc; - SolrInputField prev = sdoc.put(sif.getName(), sif); - if (prev != null) { - // blech - repeated keys - sif.addValue(prev.getValue(), prev.getBoost()); - } + // pulling out hte pieces may seem weird, but it's because + // SolrInputDocument.addField will do the right thing + // if the doc already has another value for this field + // (ie: repeating fieldname keys) + sdoc.addField(sif.getName(), sif.getValue(), sif.getBoost()); } } @@ -550,4 +550,4 @@ public class JsonLoader extends ContentStreamLoader { } } -} \ No newline at end of file +} diff --git a/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java b/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java index 36ec068bddb..301ef72942e 100644 --- a/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java +++ b/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java @@ -209,6 +209,54 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { req.close(); } + public void testFieldValueOrdering() throws Exception { + final String pre = "{'add':[{'id':'1',"; + final String post = "},{'id':'2'}]}"; + + // list + checkFieldValueOrdering((pre+ "'f':[45,67,89]" +post) + .replace('\'', '"'), + 1.0F); + // dup fieldname keys + checkFieldValueOrdering((pre+ "'f':45,'f':67,'f':89" +post) + .replace('\'', '"'), + 1.0F); + // extended w/boost + checkFieldValueOrdering((pre+ "'f':{'boost':4.0,'value':[45,67,89]}" +post) + .replace('\'', '"'), + 4.0F); + // dup keys extended w/ multiplicitive boost + checkFieldValueOrdering((pre+ + "'f':{'boost':2.0,'value':[45,67]}," + + "'f':{'boost':2.0,'value':89}" + +post) + .replace('\'', '"'), + 4.0F); + + } + private void checkFieldValueOrdering(String rawJson, float fBoost) throws Exception { + SolrQueryRequest req = req(); + SolrQueryResponse rsp = new SolrQueryResponse(); + BufferingRequestProcessor p = new BufferingRequestProcessor(null); + JsonLoader loader = new JsonLoader(); + loader.load(req, rsp, new ContentStreamBase.StringStream(rawJson), p); + assertEquals( 2, p.addCommands.size() ); + + SolrInputDocument d = p.addCommands.get(0).solrDoc; + assertEquals(2, d.getFieldNames().size()); + assertEquals("1", d.getFieldValue("id")); + assertEquals(new Object[] {45L, 67L, 89L} , d.getFieldValues("f").toArray()); + assertEquals(0.0F, fBoost, d.getField("f").getBoost()); + + d = p.addCommands.get(1).solrDoc; + assertEquals(1, d.getFieldNames().size()); + assertEquals("2", d.getFieldValue("id")); + + req.close(); + } + + + public void testExtendedFieldValues() throws Exception { String str = "[{'id':'1', 'val_s':{'add':'foo'}}]".replace('\'', '"'); SolrQueryRequest req = req(); @@ -500,4 +548,4 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { } -} \ No newline at end of file +}