add 'remove' as update option for atomically removing a value from a multivalued field

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

solr/CHANGES.txt

@@ -116,6 +116,9 @@ Other Changes
 
 * SOLR-5987: Add "collection" to UpdateParams. (Mark Miller, Greg Solovyev)
 
+* SOLR-3862: Add remove" as update option for atomically removing a value 
+  from a multivalued field (Jim Musli, Steven Bower, Alaknantha via Erick Erickson) 
+
 ==================  4.8.0 ==================
 
 Versions of Major Components

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

@@ -905,49 +905,34 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
 
     IndexSchema schema = cmd.getReq().getSchema();
     for (SolrInputField sif : sdoc.values()) {
-      Object val = sif.getValue();
+     Object val = sif.getValue();
       if (val instanceof Map) {
         for (Entry<String,Object> entry : ((Map<String,Object>) val).entrySet()) {
           String key = entry.getKey();
           Object fieldVal = entry.getValue();
           boolean updateField = false;
-          if ("add".equals(key)) {
+          switch (key) {
-            updateField = true;
+            case "add":
-            oldDoc.addField( sif.getName(), fieldVal, sif.getBoost());
+              updateField = true;
-          } else if ("set".equals(key)) {
+              oldDoc.addField(sif.getName(), fieldVal, sif.getBoost());
-            updateField = true;
+              break;
-            oldDoc.setField(sif.getName(),  fieldVal, sif.getBoost());
+            case "set":
-          } else if ("inc".equals(key)) {
+              updateField = true;
-            updateField = true;
+              oldDoc.setField(sif.getName(), fieldVal, sif.getBoost());
-            SolrInputField numericField = oldDoc.get(sif.getName());
+              break;
-            if (numericField == null) {
+            case "remove":
-              oldDoc.setField(sif.getName(),  fieldVal, sif.getBoost());
+              updateField = true;
-            } else {
+              doRemove(oldDoc, sif, fieldVal);
-              // TODO: fieldtype needs externalToObject?
+              break;
-              String oldValS = numericField.getFirstValue().toString();
+            case "inc":
-              SchemaField sf = schema.getField(sif.getName());
+              updateField = true;
-              BytesRef term = new BytesRef();
+              doInc(oldDoc, schema, sif, fieldVal);
-              sf.getType().readableToIndexed(oldValS, term);
+              break;
-              Object oldVal = sf.getType().toObject(sf, term);
+            default:
-
+              //Perhaps throw an error here instead?
-              String fieldValS = fieldVal.toString();
+              log.warn("Unknown operation for the an atomic update, operation ignored: " + key);
-              Number result;
+              break;
-              if (oldVal instanceof Long) {
-                result = ((Long) oldVal).longValue() + Long.parseLong(fieldValS);
-              } else if (oldVal instanceof Float) {
-                result = ((Float) oldVal).floatValue() + Float.parseFloat(fieldValS);
-              } else if (oldVal instanceof Double) {
-                result = ((Double) oldVal).doubleValue() + Double.parseDouble(fieldValS);
-              } else {
-                // int, short, byte
-                result = ((Integer) oldVal).intValue() + Integer.parseInt(fieldValS);
-              }
-
-              oldDoc.setField(sif.getName(),  result, sif.getBoost());
-            }
-
           }
-
           // validate that the field being modified is not the id field.
           if (updateField && idField.getName().equals(sif.getName())) {
             throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid update of id field: " + sif);
@@ -965,7 +950,50 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
     return true;
   }
 
+  private void doInc(SolrInputDocument oldDoc, IndexSchema schema, SolrInputField sif, Object fieldVal) {
+    SolrInputField numericField = oldDoc.get(sif.getName());
+    if (numericField == null) {
+      oldDoc.setField(sif.getName(),  fieldVal, sif.getBoost());
+    } else {
+      // TODO: fieldtype needs externalToObject?
+      String oldValS = numericField.getFirstValue().toString();
+      SchemaField sf = schema.getField(sif.getName());
+      BytesRef term = new BytesRef();
+      sf.getType().readableToIndexed(oldValS, term);
+      Object oldVal = sf.getType().toObject(sf, term);
 
+      String fieldValS = fieldVal.toString();
+      Number result;
+      if (oldVal instanceof Long) {
+        result = ((Long) oldVal).longValue() + Long.parseLong(fieldValS);
+      } else if (oldVal instanceof Float) {
+        result = ((Float) oldVal).floatValue() + Float.parseFloat(fieldValS);
+      } else if (oldVal instanceof Double) {
+        result = ((Double) oldVal).doubleValue() + Double.parseDouble(fieldValS);
+      } else {
+        // int, short, byte
+        result = ((Integer) oldVal).intValue() + Integer.parseInt(fieldValS);
+      }
+
+      oldDoc.setField(sif.getName(),  result, sif.getBoost());
+    }
+  }
+
+  private void doRemove(SolrInputDocument oldDoc, SolrInputField sif, Object fieldVal) {
+    final String name = sif.getName();
+    SolrInputField existingField = oldDoc.get(name);
+    if (existingField != null) {
+      final Collection<Object> original = existingField.getValues();
+      if (fieldVal instanceof Collection) {
+        original.removeAll((Collection) fieldVal);
+      } else {
+        original.remove(fieldVal);
+      }
+
+      oldDoc.setField(name, original);
+
+    }
+  }
 
 
   @Override

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

@@ -0,0 +1,197 @@
+package org.apache.solr.update.processor;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/*
+ * 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.
+ */
+
+public class AtomicUpdatesTest extends SolrTestCaseJ4 {
+
+  @BeforeClass
+  public static void beforeTests() throws Exception {
+    System.setProperty("enable.update.log", "true");
+    initCore("solrconfig.xml", "schema.xml");
+  }
+
+  @Before
+  public void before() {
+    h.update("<delete><query>*:*</query></delete>");
+    assertU(commit());
+  }
+  @Test
+  public void testRemove() throws Exception {
+    SolrInputDocument doc;
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "1");
+    doc.setField("cat", new String[]{"aaa", "bbb", "ccc", "ccc", "ddd"});
+    assertU(adoc(doc));
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "2");
+    doc.setField("cat", new String[]{"aaa", "bbb", "bbb", "ccc", "ddd"});
+    assertU(adoc(doc));
+
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "20");
+    doc.setField("cat", new String[]{"aaa", "ccc", "ddd"});
+    assertU(adoc(doc));
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "21");
+    doc.setField("cat", new String[]{"aaa", "bbb", "ddd"});
+    assertU(adoc(doc));
+
+
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '3']");
+
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "1");
+    List<String> removeList = new ArrayList<String>();
+    removeList.add("bbb");
+    removeList.add("ccc");
+    doc.setField("cat", ImmutableMap.of("remove", removeList)); //behavior when hitting Solr through ZK
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '2']");
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "21");
+    removeList = new ArrayList<String>();
+    removeList.add("bbb");
+    removeList.add("ccc");
+    doc.setField("cat", ImmutableMap.of("remove", removeList)); //behavior when hitting Solr through ZK
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "1");
+    doc.setField("cat", ImmutableMap.of("remove", "aaa")); //behavior when hitting Solr directly
+
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']");
+    assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '3']");
+  }
+
+  @Test
+  public void testAdd() throws Exception {
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.setField("id", "3");
+    doc.setField("cat", new String[]{"aaa", "ccc"});
+    assertU(adoc(doc));
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "4");
+    doc.setField("cat", new String[]{"aaa", "ccc"});
+    assertU(adoc(doc));
+
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']");
+
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "3");
+    doc.setField("cat", ImmutableMap.of("add", "bbb"));
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
+  }
+
+  @Test
+  public void testSet() throws Exception {
+    SolrInputDocument doc;
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "5");
+    doc.setField("cat", new String[]{"aaa", "ccc"});
+    assertU(adoc(doc));
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "6");
+    doc.setField("cat", new String[]{"aaa", "ccc"});
+    assertU(adoc(doc));
+
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
+    assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '2']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']");
+    assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '2']");
+
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "5");
+    doc.setField("cat", ImmutableMap.of("set", "bbb"));
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']");
+    assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']");
+    assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
+  }
+
+  @Test
+  public void testInvalidOperation() {
+    SolrInputDocument doc;
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "7");
+    doc.setField("cat", new String[]{"aaa", "ccc"});
+    assertU(adoc(doc));
+
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']");
+
+    doc = new SolrInputDocument();
+    doc.setField("id", "7");
+    doc.setField("cat", ImmutableMap.of("whatever", "bbb"));
+    assertU(adoc(doc));
+    assertU(commit());
+
+    assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']");
+    assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']");
+    assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']");
+    assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']");
+
+  }
+}