SOLR-4016: Deduplication does not work with atomic/partial updates so disallow atomic update requests which change signature generating fields.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1433013 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -602,6 +602,10 @@ Other Changes
 * SOLR-4287: Removed "apache-" prefix from Solr distribution and artifact
   filenames. (Ryan Ernst, Robert Muir, Steve Rowe)
 
+* SOLR-4016: Deduplication does not work with atomic/partial updates so
+  disallow atomic update requests which change signature generating fields.
+  (Joel Nothman, yonik, shalin)
+
 ==================  4.0.0 ==================
 
 Versions of Major Components

solr/core/src/java/org/apache/solr/update/processor/SignatureUpdateProcessorFactory.java

@@ -134,7 +134,13 @@ public class SignatureUpdateProcessorFactory
       if (enabled) {
         SolrInputDocument doc = cmd.getSolrInputDocument();
         List<String> currDocSigFields = null;
+        boolean isPartialUpdate = DistributedUpdateProcessor.isAtomicUpdate(cmd);
         if (sigFields == null || sigFields.size() == 0) {
+          if (isPartialUpdate)  {
+            throw new SolrException
+                (ErrorCode.SERVER_ERROR,
+                    "Can't use SignatureUpdateProcessor with partial updates on signature fields");
+          }
           Collection<String> docFields = doc.getFieldNames();
           currDocSigFields = new ArrayList<String>(docFields.size());
           currDocSigFields.addAll(docFields);
@@ -149,6 +155,12 @@ public class SignatureUpdateProcessorFactory
         for (String field : currDocSigFields) {
           SolrInputField f = doc.getField(field);
           if (f != null) {
+            if (isPartialUpdate)  {
+              throw new SolrException
+                  (ErrorCode.SERVER_ERROR,
+                      "Can't use SignatureUpdateProcessor with partial update request " +
+                          "containing signature field: " + field);
+            }
             sig.add(field);
             Object o = f.getValue();
             if (o instanceof Collection) {

solr/core/src/test-files/solr/collection1/conf/solrconfig-tlog.xml

@@ -47,6 +47,29 @@
     </updateLog>
   </updateHandler>
 
+  <updateRequestProcessorChain name="dedupe">
+    <processor class="org.apache.solr.update.processor.SignatureUpdateProcessorFactory">
+      <bool name="enabled">true</bool>
+      <bool name="overwriteDupes">true</bool>
+      <str name="fields">v_t,t_field</str>
+      <str name="signatureClass">org.apache.solr.update.processor.TextProfileSignature</str>
+    </processor>
+    <processor class="solr.RunUpdateProcessorFactory" />
+  </updateRequestProcessorChain>
+  <updateRequestProcessorChain name="stored_sig">
+    <!-- this chain is valid even though the signature field is not
+         indexed, because we are not asking for dups to be overwritten
+      -->
+    <processor class="org.apache.solr.update.processor.SignatureUpdateProcessorFactory">
+      <bool name="enabled">true</bool>
+      <str name="signatureField">non_indexed_signature_sS</str>
+      <bool name="overwriteDupes">false</bool>
+      <str name="fields">v_t,t_field</str>
+      <str name="signatureClass">org.apache.solr.update.processor.TextProfileSignature</str>
+    </processor>
+    <processor class="solr.RunUpdateProcessorFactory" />
+  </updateRequestProcessorChain>
+
   <requestHandler name="/admin/" class="org.apache.solr.handler.admin.AdminHandlers" />
 
 </config>

solr/core/src/test/org/apache/solr/update/processor/SignatureUpdateProcessorFactoryTest.java

@@ -64,7 +64,7 @@ public class SignatureUpdateProcessorFactoryTest extends SolrTestCaseJ4 {
     chain = "dedupe"; // set the default that most tests expect
   }
 
-  void checkNumDocs(int n) {
+  static void checkNumDocs(int n) {
     SolrQueryRequest req = req();
     try {
       assertEquals(n, req.getSearcher().getIndexReader().numDocs());
@@ -353,7 +353,11 @@ public class SignatureUpdateProcessorFactoryTest extends SolrTestCaseJ4 {
     }
   }
 
-  private void addDoc(String doc) throws Exception {
+  private void addDoc(String doc) throws Exception  {
+    addDoc(doc, chain);
+  }
+
+  static void addDoc(String doc, String chain) throws Exception {
     Map<String, String[]> params = new HashMap<String, String[]>();
     MultiMapSolrParams mmparams = new MultiMapSolrParams(params);
     params.put(UpdateParams.UPDATE_CHAIN, new String[] { chain });

solr/core/src/test/org/apache/solr/update/processor/TestPartialUpdateDeduplication.java

@@ -0,0 +1,74 @@
+package org.apache.solr.update.processor;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import com.google.common.collect.Maps;
+import org.apache.noggit.ObjectBuilder;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.core.SolrCore;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.solr.update.processor.SignatureUpdateProcessorFactoryTest.addDoc;
+
+public class TestPartialUpdateDeduplication extends SolrTestCaseJ4 {
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    initCore("solrconfig-tlog.xml", "schema15.xml");
+  }
+
+  @Test
+  public void testPartialUpdates() throws Exception {
+    SignatureUpdateProcessorFactoryTest.checkNumDocs(0);
+    String chain = "dedupe";
+    // partial update
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.addField("id", "2a");
+    Map<String, Object> map = Maps.newHashMap();
+    map.put("set", "Hello Dude man!");
+    doc.addField("v_t", map);
+    UpdateRequest req = new UpdateRequest();
+    req.add(doc);
+    boolean exception_ok = false;
+    try {
+      addDoc(req.getXML(), chain);
+    } catch (Exception e) {
+      exception_ok = true;
+    }
+    assertTrue("Should have gotten an exception with partial update on signature generating field",
+        exception_ok);
+
+    SignatureUpdateProcessorFactoryTest.checkNumDocs(0);
+    addDoc(adoc("id", "2a", "v_t", "Hello Dude man!", "name", "ali babi'"), chain);
+    doc = new SolrInputDocument();
+    doc.addField("id", "2a");
+    map = Maps.newHashMap();
+    map.put("set", "name changed");
+    doc.addField("name", map);
+    req = new UpdateRequest();
+    req.add(doc);
+    addDoc(req.getXML(), chain);
+    addDoc(commit(), chain);
+    SignatureUpdateProcessorFactoryTest.checkNumDocs(1);
+  }
+}