From commits-return-105927-archive-asf-public=cust-asf.ponee.io@lucene.apache.org Thu Jan 3 16:59:07 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id DA7F6180608 for ; Thu, 3 Jan 2019 16:59:06 +0100 (CET) Received: (qmail 79651 invoked by uid 500); 3 Jan 2019 15:59:05 -0000 Mailing-List: contact commits-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@lucene.apache.org Delivered-To: mailing list commits@lucene.apache.org Received: (qmail 79642 invoked by uid 99); 3 Jan 2019 15:59:05 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 03 Jan 2019 15:59:05 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id A0586E0968; Thu, 3 Jan 2019 15:59:05 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: dsmiley@apache.org To: commits@lucene.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: lucene-solr:master: SOLR-12633: remove anonChildDocs update parameter used in nested docs in JSON. Date: Thu, 3 Jan 2019 15:59:05 +0000 (UTC) Repository: lucene-solr Updated Branches: refs/heads/master 0a7a478c1 -> 6342ec699 SOLR-12633: remove anonChildDocs update parameter used in nested docs in JSON. Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6342ec69 Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6342ec69 Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6342ec69 Branch: refs/heads/master Commit: 6342ec699e4b5e4d1636fdf20e9b69d0a5099eab Parents: 0a7a478 Author: David Smiley Authored: Thu Jan 3 10:58:59 2019 -0500 Committer: David Smiley Committed: Thu Jan 3 10:58:59 2019 -0500 ---------------------------------------------------------------------- solr/CHANGES.txt | 8 +- .../apache/solr/handler/loader/JsonLoader.java | 27 ++--- .../org/apache/solr/handler/JsonLoaderTest.java | 116 ++++++------------- .../apache/solr/common/params/CommonParams.java | 5 - 4 files changed, 52 insertions(+), 104 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6342ec69/solr/CHANGES.txt ---------------------------------------------------------------------- diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 4ba275c..00c0081 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -65,11 +65,17 @@ Upgrade Notes SchemaSimilarityFactory, then LegacyBM25Similarity is automatically selected for 'luceneMatchVersion' < 8.0.0. See also explanation in Reference Guide chapter "Other Schema Elements". -* SOLR-12535: Solr no longer accepts index time boosts in JSON provided to Solr. This used to be provided like so: +* SOLR-12535: Solr no longer accepts index time boosts in JSON provided to Solr. This used to be provided like so: {'id':'1', 'val_s':{'value':'foo', 'boost':2.0}} but will now produce an error. A object/map structure will now only be interpreted as a child document or an atomic update; nothing else. A uniqueKey is currently required on all child documents to be interpreted as such, though this may change in the future. (David Smiley) +* SOLR-12633: When JSON data is sent to Solr with nested child documents split using the "split" parameter, the child + docs will now be associated to their parents by the field/label string used in the JSON instead of anonymously. Most + users probably won't notice the distinction since the label is lost any way unless special fields are in the schema. + This choice used to be toggleable with an internal/expert "anonChildDocs" parameter flag which is now gone. + (David Smiley) + New Features ---------------------- http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6342ec69/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java ---------------------------------------------------------------------- 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 5b484a4..5e11ff1 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 @@ -93,16 +93,11 @@ public class JsonLoader extends ContentStreamLoader { protected JSONParser parser; protected final int commitWithin; protected final boolean overwrite; - protected final boolean anonChildDoc; - /** - * {@link CommonParams#ANONYMOUS_CHILD_DOCS} Defaults to true. - */ - public SingleThreadedJsonLoader(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor processor) { + SingleThreadedJsonLoader(SolrQueryRequest req, SolrQueryResponse rsp, UpdateRequestProcessor processor) { this.processor = processor; this.req = req; this.rsp = rsp; - this.anonChildDoc = req.getParams().getBool(CommonParams.ANONYMOUS_CHILD_DOCS, true); commitWithin = req.getParams().getInt(UpdateParams.COMMIT_WITHIN, -1); overwrite = req.getParams().getBool(UpdateParams.OVERWRITE, true); @@ -258,23 +253,15 @@ public class JsonLoader extends ContentStreamLoader { List value = (List) e.getValue(); for (Object o : value) { if (o instanceof Map) { - if (anonChildDoc) { - result.addChildDocument(buildDoc((Map) o)); - } else { - // retain the value as a list, even if the list contains a single value. - if(!result.containsKey(e.getKey())) { - result.setField(e.getKey(), new ArrayList<>(1)); - } - result.addField(e.getKey(), buildDoc((Map) o)); + // retain the value as a list, even if the list contains a single value. + if(!result.containsKey(e.getKey())) { + result.setField(e.getKey(), new ArrayList<>(1)); } + result.addField(e.getKey(), buildDoc((Map) o)); } } } else if (e.getValue() instanceof Map) { - if (anonChildDoc) { - result.addChildDocument(buildDoc((Map) e.getValue())); - } else { - result.addField(e.getKey(), buildDoc((Map) e.getValue())); - } + result.addField(e.getKey(), buildDoc((Map) e.getValue())); } } else { result.setField(e.getKey(), e.getValue()); @@ -549,7 +536,7 @@ public class JsonLoader extends ContentStreamLoader { } String fieldName = parser.getString(); - if (anonChildDoc && fieldName.equals(JsonLoader.CHILD_DOC_KEY)) { + if (fieldName.equals(JsonLoader.CHILD_DOC_KEY)) { // somewhat legacy ev = parser.nextEvent(); assertEvent(ev, JSONParser.ARRAY_START); while ((ev = parser.nextEvent()) != JSONParser.ARRAY_END) { http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6342ec69/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java ---------------------------------------------------------------------- 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 15330c6..e076e31 100644 --- a/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java +++ b/solr/core/src/test/org/apache/solr/handler/JsonLoaderTest.java @@ -37,8 +37,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.noggit.ObjectBuilder; -import static org.apache.solr.common.params.CommonParams.ANONYMOUS_CHILD_DOCS; - public class JsonLoaderTest extends SolrTestCaseJ4 { @BeforeClass @@ -248,27 +246,12 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { req.close(); } - public void testJsonDocFormat() throws Exception{ + public void testMultipleDocsWithoutArray() throws Exception { String doc = "\n" + "\n" + - "{\"bool\": true,\n" + - " \"f0\": \"v0\",\n" + - " \"f2\": {\n" + - " \t \"boost\": 2.3,\n" + - " \t \"value\": \"test\"\n" + - " \t },\n" + - "\"array\": [ \"aaa\", \"bbb\" ],\n" + - "\"boosted\": {\n" + - " \t \"boost\": 6.7,\n" + - " \t \"value\": [ \"aaa\", \"bbb\" ]\n" + - " \t }\n" + - " }\n" + - "\n" + + "{\"f1\": 1111 }\n" + "\n" + - " {\"f1\": \"v1\",\n" + - " \"f1\": \"v2\",\n" + - " \"f2\": null\n" + - " }\n"; + "{\"f1\": 2222 }\n"; SolrQueryRequest req = req("srcField","_src_"); req.getContext().put("path","/update/json/docs"); SolrQueryResponse rsp = new SolrQueryResponse(); @@ -276,7 +259,16 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { JsonLoader loader = new JsonLoader(); loader.load(req, rsp, new ContentStreamBase.StringStream(doc), p); assertEquals( 2, p.addCommands.size() ); - doc = "\n" + + } + + public void testJsonDocFormat() throws Exception{ + String doc; + SolrQueryRequest req; + SolrQueryResponse rsp; + BufferingRequestProcessor p; + JsonLoader loader; + + doc = "\n" + "\n" + "{\"bool\": true,\n" + " \"f0\": \"v0\",\n" + @@ -321,6 +313,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { assertEquals("v2", obj.get("f2")); assertTrue(obj.containsKey("f3")); + //TODO new test method doc = "[{'id':'1'},{'id':'2'}]".replace('\'', '"'); req = req("srcField","_src_"); req.getContext().put("path","/update/json/docs"); @@ -339,6 +332,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { obj = (Map) ObjectBuilder.fromJSON(content); assertEquals("2", obj.get("id")); + //TODO new test method String json = "{a:{" + "b:[{c:c1, e:e1},{c:c2, e :e2, d:{p:q}}]," + "x:y" + @@ -349,30 +343,15 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { p = new BufferingRequestProcessor(null); loader = new JsonLoader(); loader.load(req, rsp, new ContentStreamBase.StringStream(json), p); - assertEquals( 1, p.addCommands.size() ); - assertEquals("y", p.addCommands.get(0).solrDoc.getFieldValue("a.x")); - List children = p.addCommands.get(0).solrDoc.getChildDocuments(); - assertEquals(2, children.size()); - SolrInputDocument d = children.get(0); - assertEquals(d.getFieldValue("c"), "c1"); - assertEquals(d.getFieldValue("e"), "e1"); - d = children.get(1); - assertEquals(d.getFieldValue("c"), "c2"); - assertEquals(d.getFieldValue("e"), "e2"); - assertEquals(d.getFieldValue("d.p"), "q"); - - req = req(PARENT_TWO_CHILDREN_PARAMS); - req.getContext().put("path", "/update/json/docs"); - rsp = new SolrQueryResponse(); - p = new BufferingRequestProcessor(null); - loader = new JsonLoader(); - loader.load(req, rsp, new ContentStreamBase.StringStream(PARENT_TWO_CHILDREN_JSON), p); - assertEquals(2, p.addCommands.get(0).solrDoc.getChildDocuments().size()); - assertEquals(1, p.addCommands.get(0).solrDoc.getChildDocuments().get(1).getChildDocuments().size()); - + assertEquals("SolrInputDocument(fields: [" + + "b=[" + + "SolrInputDocument(fields: [c=c1, e=e1]), " + + "SolrInputDocument(fields: [c=c2, e=e2, d.p=q])], " + + "a.x=y" + + "])", p.addCommands.get(0).solrDoc.toString()); } - + private static final String PARENT_TWO_CHILDREN_JSON = "{\n" + " \"id\": \"1\",\n" + " \"name\": \"i am the parent\",\n" + @@ -410,18 +389,9 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { "f", "cat:/children/grandchildren/cat"}; @Test - public void testFewParentsAnonymousJsonDoc() throws Exception { - String json = PARENT_TWO_CHILDREN_JSON; - assertFewParents(json, true); - } - - @Test public void testFewParentsJsonDoc() throws Exception { String json = PARENT_TWO_CHILDREN_JSON; - assertFewParents(json, false); - } - private void assertFewParents(String json, boolean anonChildDocsFlag) throws Exception { SolrQueryRequest req; SolrQueryResponse rsp; BufferingRequestProcessor p; @@ -439,7 +409,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { } } - req = req(PARENT_TWO_CHILDREN_PARAMS, ANONYMOUS_CHILD_DOCS, Boolean.toString(anonChildDocsFlag)); + req = req(PARENT_TWO_CHILDREN_PARAMS); req.getContext().put("path", "/update/json/docs"); rsp = new SolrQueryResponse(); p = new BufferingRequestProcessor(null); @@ -453,12 +423,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { assertOnlyValue("i am the parent", parent, "name"); assertOnlyValue("parent", parent, "cat"); - List childDocs1; - if(anonChildDocsFlag) { - childDocs1 = parent.getChildDocuments(); - } else { - childDocs1 = (List) ((parent.getField("children")).getValue()); - } + List childDocs1 = (List) ((parent.getField("children")).getValue()); assertEquals(2, childDocs1.size()); { @@ -474,12 +439,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { assertOnlyValue("test-new-label", child2, "test_s"); assertOnlyValue("child", child2, "cat"); - List childDocs2; - if(anonChildDocsFlag) { - childDocs2 = child2.getChildDocuments(); - } else { - childDocs2 = (List) ((child2.getField("grandchildren")).getValue()); - } + List childDocs2 = (List) ((child2.getField("grandchildren")).getValue()); assertEquals(1, childDocs2.size()); final SolrInputDocument grandChild = childDocs2.get(0); @@ -784,7 +744,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { req.close(); } - private static final String SIMPLE_JSON_CHILD_DOCS = "{\n" + + private static final String SIMPLE_ANON_CHILD_DOCS_JSON = "{\n" + " \"add\": {\n" + " \"doc\": {\n" + " \"id\": \"1\",\n" + @@ -803,15 +763,15 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { @Test public void testSimpleAnonymousChildDocs() throws Exception { - checkTwoAnonymousChildDocs(SIMPLE_JSON_CHILD_DOCS); + checkTwoAnonymousChildDocs(SIMPLE_ANON_CHILD_DOCS_JSON, true); } @Test public void testSimpleChildDocs() throws Exception { - checkTwoAnonymousChildDocs(SIMPLE_JSON_CHILD_DOCS, false); + checkTwoAnonymousChildDocs(SIMPLE_ANON_CHILD_DOCS_JSON, false); } - private static final String DUP_KEYS_JSON_CHILD_DOCS = "{\n" + + private static final String DUP_KEYS_ANON_CHILD_DOCS_JSON = "{\n" + " \"add\": {\n" + " \"doc\": {\n" + " \"_childDocuments_\": [\n" + @@ -833,20 +793,20 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { @Test public void testDupKeysAnonymousChildDocs() throws Exception { - checkTwoAnonymousChildDocs(DUP_KEYS_JSON_CHILD_DOCS); + checkTwoAnonymousChildDocs(DUP_KEYS_ANON_CHILD_DOCS_JSON, true); } @Test public void testDupKeysChildDocs() throws Exception { - checkTwoAnonymousChildDocs(DUP_KEYS_JSON_CHILD_DOCS, false); - } - - private void checkTwoAnonymousChildDocs(String rawJsonStr) throws Exception { - checkTwoAnonymousChildDocs(rawJsonStr, true); + checkTwoAnonymousChildDocs(DUP_KEYS_ANON_CHILD_DOCS_JSON, false); } + // rawJsonStr has "_childDocuments_" key. if anonChildDocs then we want to test with something else. private void checkTwoAnonymousChildDocs(String rawJsonStr, boolean anonChildDocs) throws Exception { - SolrQueryRequest req = req("commit","true", ANONYMOUS_CHILD_DOCS, Boolean.toString(anonChildDocs)); + if (!anonChildDocs) { + rawJsonStr = rawJsonStr.replaceAll("_childDocuments_", "childLabel"); + } + SolrQueryRequest req = req("commit","true"); SolrQueryResponse rsp = new SolrQueryResponse(); BufferingRequestProcessor p = new BufferingRequestProcessor(null); JsonLoader loader = new JsonLoader(); @@ -863,7 +823,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { if (anonChildDocs) { cd = d.getChildDocuments().get(0); } else { - cd = (SolrInputDocument) (d.getField("_childDocuments_")).getFirstValue(); + cd = (SolrInputDocument) (d.getField("childLabel")).getFirstValue(); } SolrInputField cf = cd.getField( "id" ); assertEquals("2", cf.getValue()); @@ -871,7 +831,7 @@ public class JsonLoaderTest extends SolrTestCaseJ4 { if (anonChildDocs) { cd = d.getChildDocuments().get(1); } else { - cd = (SolrInputDocument)((List)(d.getField("_childDocuments_")).getValue()).get(1); + cd = (SolrInputDocument)((List)(d.getField("childLabel")).getValue()).get(1); } cf = cd.getField( "id" ); assertEquals("3", cf.getValue()); http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6342ec69/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java ---------------------------------------------------------------------- diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java index 798410d..ac00bc0 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java @@ -294,10 +294,5 @@ public interface CommonParams { String JAVABIN_MIME = "application/javabin"; - /** - * If set to true, child documents will be added as anonymous children into the _childDocuments list, - * else, child documents will be added to SolrInputDocument as field values according to their key name. - */ - String ANONYMOUS_CHILD_DOCS = "anonChildDocs"; }