[solr] branch main updated: Fix bug in in-place update when failOnVersionConflicts=false (#898)

[an...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

anshum pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 329fda71bf3 Fix bug in in-place update when failOnVersionConflicts=false (#898)
329fda71bf3 is described below

commit 329fda71bf3f532967eb7b6c75ab9dc0f64ecb8f
Author: Lamine <10...@users.noreply.github.com>
AuthorDate: Thu Jun 9 19:15:26 2022 -0500

    Fix bug in in-place update when failOnVersionConflicts=false (#898)
    
    Co-authored-by: Lamine Idjeraoui <li...@apple.com>
---
 solr/CHANGES.txt                                   |  2 ++
 .../processor/DistributedUpdateProcessor.java      |  4 ++++
 .../solr/update/TestInPlaceUpdatesStandalone.java  | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 51389bdc24b..99f8cfedb0f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -85,6 +85,8 @@ Bug Fixes
 
 * SOLR-16110: Using Schema/Config API breaks the File-Upload of Config Set File (Steffen Moldenhauer, Kevin Risden)
 
+* SOLR-16218: failOnVersionConflicts option not working for in-place updates (Lamine Idjeraoui)
+
 Other Changes
 ---------------------
 * SOLR-15897: Remove <jmx/> from all unit test solrconfig.xml files. (Eric Pugh)
diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
index b637b9f63b0..8466f022368 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
@@ -798,6 +798,10 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
     SolrInputDocument mergedDoc;
     if (oldRootDocWithChildren == null) {
       if (versionOnUpdate > 0 || !rootDocIdString.equals(cmd.getSelfOrNestedDocIdStr())) {
+        if (cmd.getReq().getParams().getBool(CommonParams.FAIL_ON_VERSION_CONFLICTS, true)
+            == false) {
+          return false;
+        }
         // could just let the optimistic locking throw the error
         throw new SolrException(
             ErrorCode.CONFLICT, "Document not found for update.  id=" + rootDocIdString);
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
index 413595b610b..8e6eee93f86 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
@@ -1579,6 +1579,28 @@ public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
     assertQ(req("q", "title_s:second"), "//*[@numFound='1']");
     assertQ(req("q", "title_s:first1"), "//*[@numFound='1']"); // but the old value exists
     assertQ(req("q", "title_s:first2"), "//*[@numFound='0']"); // and the new value does not reflect
+
+    // tests inplace updates when doc does not exist
+    assertFailedU(add(doc("id", "3", "title_s", "third", "_version_", "1")));
+    params.set(CommonParams.FAIL_ON_VERSION_CONFLICTS, "true");
+    params.set("_version_", "1");
+    SolrInputDocument doc1_v3 = sdoc("id", "1", "title_s", map("set", "first3"));
+    SolrInputDocument doc3 = sdoc("id", "3", "title_s", map("set", "third"));
+    ex =
+        expectThrows(
+            SolrException.class,
+            "This should have failed",
+            () -> updateJ(jsonAdd(doc1_v3, doc3), params));
+    assertTrue(ex.getMessage().contains("Document not found for update"));
+
+    params.set(CommonParams.FAIL_ON_VERSION_CONFLICTS, "false");
+    SolrInputDocument doc1_v4 = sdoc("id", "1", "title_s", map("set", "first4"));
+    updateJ(jsonAdd(doc1_v4, doc3), params); // this should not throw any error
+
+    assertU(commit());
+    assertQ(req("q", "title_s:first4"), "//*[@numFound='1']"); // the new value does reflect
+    assertQ(req("q", "title_s:first1"), "//*[@numFound='0']"); // but the old value does not exist
+    assertQ(req("q", "title_s:third"), "//*[@numFound='0']"); // doc3 does not exist
   }
   /**
    * Helper method that sets up a req/cmd to run {@link