From 5e24a010e0dee7270a2f94bf9ab46685e2479d3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20W=C3=B6ckinger?=
 <thomaswoeckinger@users.noreply.github.com>
Date: Mon, 25 Nov 2019 09:57:07 -0500
Subject: [PATCH] SOLR-13961: Allow null/empty for removal of child doc in
 atomic update

Cherry pick: b5fd6d7b22002a06bdc626999a6a527ff6f46488
---
 solr/CHANGES.txt                              |  3 +
 .../processor/AtomicUpdateDocumentMerger.java |  2 +-
 .../processor/NestedAtomicUpdateTest.java     | 67 +++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5469d4837d2..177397c7321 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -77,6 +77,9 @@ Improvements
 
 * SOLR-13905: Make findRequestType in AuditEvent more robust (janhoy)
 
+* SOLR-13961: When using partial/atomic updates to remove child documents, we now support setting to null or
+  an empty list as equivalent to the "remove" command. (Thomas Wöckinger)
+
 Optimizations
 ---------------------
 (No changes)
diff --git a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
index f0972dbae9a..0f5a3a67ca4 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
@@ -552,7 +552,7 @@ public class AtomicUpdateDocumentMerger {
   }
 
   private Object getNativeFieldValue(String fieldName, Object val) {
-    if(isChildDoc(val)) {
+    if (isChildDoc(val) || val == null || (val instanceof Collection && ((Collection) val).isEmpty())) {
       return val;
     }
     SchemaField sf = schema.getField(fieldName);
diff --git a/solr/core/src/test/org/apache/solr/update/processor/NestedAtomicUpdateTest.java b/solr/core/src/test/org/apache/solr/update/processor/NestedAtomicUpdateTest.java
index e52176f74a5..62f412aaa8c 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/NestedAtomicUpdateTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/NestedAtomicUpdateTest.java
@@ -642,6 +642,73 @@ public class NestedAtomicUpdateTest extends SolrTestCaseJ4 {
     );
   }
 
+  @Test
+  public void testBlockAtomicSetToNull() throws Exception {
+    testBlockAtomicSetToNullOrEmpty(false);
+  }
+
+  @Test
+  public void testBlockAtomicSetToEmpty() throws Exception {
+    testBlockAtomicSetToNullOrEmpty(true);
+  }
+
+  private void testBlockAtomicSetToNullOrEmpty(boolean empty) throws Exception {
+    SolrInputDocument doc = sdoc("id", "1",
+        "cat_ss", new String[] {"aaa", "ccc"},
+        "child1", sdocs(sdoc("id", "2", "cat_ss", "child"), sdoc("id", "3", "cat_ss", "child")));
+    assertU(adoc(doc));
+
+    BytesRef rootDocId = new BytesRef("1");
+    SolrCore core = h.getCore();
+    SolrInputDocument block = RealTimeGetComponent.getInputDocument(core, rootDocId,
+        RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
+    // assert block doc has child docs
+    assertTrue(block.containsKey("child1"));
+
+    assertJQ(req("q", "id:1"), "/response/numFound==0");
+
+    // commit the changes
+    assertU(commit());
+
+    SolrInputDocument committedBlock = RealTimeGetComponent.getInputDocument(core, rootDocId,
+        RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN);
+    BytesRef childDocId = new BytesRef("2");
+    // ensure the whole block is returned when resolveBlock is true and id of a child doc is provided
+    assertEquals(committedBlock.toString(), RealTimeGetComponent
+        .getInputDocument(core, childDocId, RealTimeGetComponent.Resolution.ROOT_WITH_CHILDREN).toString());
+
+    assertJQ(req("q", "id:1"), "/response/numFound==1");
+
+    assertJQ(req("qt", "/get", "id", "1", "fl", "id, cat_ss, child1, [child]"), "=={\"doc\":{'id':\"1\"" +
+        ", cat_ss:[\"aaa\",\"ccc\"], child1:[{\"id\":\"2\",\"cat_ss\":[\"child\"]}, {\"id\":\"3\",\"cat_ss\":[\"child\"]}]}}");
+
+    assertU(commit());
+
+    assertJQ(req("qt", "/get", "id", "1", "fl", "id, cat_ss, child1, [child]"), "=={\"doc\":{'id':\"1\"" +
+        ", cat_ss:[\"aaa\",\"ccc\"], child1:[{\"id\":\"2\",\"cat_ss\":[\"child\"]}, {\"id\":\"3\",\"cat_ss\":[\"child\"]}]}}");
+
+    doc = sdoc("id", "1", "child1", Collections.singletonMap("set", empty ? new ArrayList<>() : null));
+    addAndGetVersion(doc, params("wt", "json"));
+
+    assertJQ(req("qt", "/get", "id", "1", "fl", "id, cat_ss, child1, [child]"),
+        "=={\"doc\":{'id':\"1\", cat_ss:[\"aaa\",\"ccc\"]}}");
+
+    assertU(commit());
+
+    // a cut-n-paste of the first big query, but this time it will be retrieved from the index rather than the
+    // transaction log
+    // this requires ChildDocTransformer to get the whole block, since the document is retrieved using an index lookup
+    assertJQ(req("qt", "/get", "id", "1", "fl", "id, cat_ss, child1, [child]"),
+        "=={'doc':{'id':'1', cat_ss:[\"aaa\",\"ccc\"]}}");
+
+    // ensure the whole block has been committed correctly to the index.
+    assertJQ(req("q", "id:1", "fl", "*, [child]"),
+        "/response/numFound==1",
+        "/response/docs/[0]/id=='1'",
+        "/response/docs/[0]/cat_ss/[0]==\"aaa\"",
+        "/response/docs/[0]/cat_ss/[1]==\"ccc\"");
+  }
+
   private static void assertDocContainsSubset(SolrInputDocument subsetDoc, SolrInputDocument fullDoc) {
     for(SolrInputField field: subsetDoc) {
       String fieldName = field.getName();