detect_noop now understands null as a valid value

If the source contrains a null value for a field then detect_noop should consider setting it to null again to be a noop. Closes #11208
elastic · May 18, 2015 · 400abfc · 400abfc
1 parent 13e5c19
commit 400abfc
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 7 deletions.
diff --git a/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java b/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java
@@ -20,7 +20,9 @@
 package org.elasticsearch.common.xcontent;
 
 import com.google.common.base.Charsets;
+import com.google.common.base.Objects;
 import com.google.common.collect.Maps;
+
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -260,11 +262,11 @@ public static boolean update(Map<String, Object> source, Map<String, Object> cha
             if (modified) {
                 continue;
             }
-            if (!checkUpdatesAreUnequal || old == null) {
+            if (!checkUpdatesAreUnequal) {
                 modified = true;
                 continue;
             }
-            modified = !old.equals(changesEntry.getValue());
+            modified = !Objects.equal(old, changesEntry.getValue());
         }
         return modified;
     }

diff --git a/src/test/java/org/elasticsearch/update/UpdateNoopTests.java b/src/test/java/org/elasticsearch/update/UpdateNoopTests.java
@@ -19,7 +19,6 @@
 
 package org.elasticsearch.update;
 
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.update.UpdateResponse;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -42,22 +41,34 @@ public void singleField() throws Exception {
         updateAndCheckSource(2, fields("bar", "bir"));
         updateAndCheckSource(2, fields("bar", "bir"));
         updateAndCheckSource(3, fields("bar", "foo"));
+        updateAndCheckSource(4, fields("bar", null));
+        updateAndCheckSource(4, fields("bar", null));
+        updateAndCheckSource(5, fields("bar", "foo"));
 
-        assertEquals(2, totalNoopUpdates());
+        assertEquals(3, totalNoopUpdates());
     }
 
     @Test
     public void twoFields() throws Exception {
         // Use random keys so we get random iteration order.
         String key1 = 1 + randomAsciiOfLength(3);
         String key2 = 2 + randomAsciiOfLength(3);
+        String key3 = 3 + randomAsciiOfLength(3);
         updateAndCheckSource(1, fields(key1, "foo", key2, "baz"));
         updateAndCheckSource(1, fields(key1, "foo", key2, "baz"));
         updateAndCheckSource(2, fields(key1, "foo", key2, "bir"));
         updateAndCheckSource(2, fields(key1, "foo", key2, "bir"));
         updateAndCheckSource(3, fields(key1, "foo", key2, "foo"));
+        updateAndCheckSource(4, fields(key1, "foo", key2, null));
+        updateAndCheckSource(4, fields(key1, "foo", key2, null));
+        updateAndCheckSource(5, fields(key1, "foo", key2, "foo"));
+        updateAndCheckSource(6, fields(key1, null, key2, "foo"));
+        updateAndCheckSource(6, fields(key1, null, key2, "foo"));
+        updateAndCheckSource(7, fields(key1, null, key2, null));
+        updateAndCheckSource(7, fields(key1, null, key2, null));
+        updateAndCheckSource(8, fields(key1, null, key2, null, key3, null));
 
-        assertEquals(2, totalNoopUpdates());
+        assertEquals(5, totalNoopUpdates());
     }
 
     @Test
@@ -83,6 +94,7 @@ public void map() throws Exception {
         // Use random keys so we get variable iteration order.
         String key1 = 1 + randomAsciiOfLength(3);
         String key2 = 2 + randomAsciiOfLength(3);
+        String key3 = 3 + randomAsciiOfLength(3);
         updateAndCheckSource(1, XContentFactory.jsonBuilder().startObject()
                 .startObject("test")
                     .field(key1, "foo")
@@ -108,8 +120,24 @@ public void map() throws Exception {
                     .field(key1, "foo")
                     .field(key2, "foo")
                 .endObject().endObject());
+        updateAndCheckSource(4, XContentFactory.jsonBuilder().startObject()
+                .startObject("test")
+                    .field(key1, "foo")
+                    .field(key2, (Object) null)
+                .endObject().endObject());
+        updateAndCheckSource(4, XContentFactory.jsonBuilder().startObject()
+                .startObject("test")
+                    .field(key1, "foo")
+                    .field(key2, (Object) null)
+                .endObject().endObject());
+        updateAndCheckSource(5, XContentFactory.jsonBuilder().startObject()
+                .startObject("test")
+                    .field(key1, "foo")
+                    .field(key2, (Object) null)
+                    .field(key3, (Object) null)
+                .endObject().endObject());
 
-        assertEquals(2, totalNoopUpdates());
+        assertEquals(3, totalNoopUpdates());
     }
 
     @Test
@@ -199,7 +227,7 @@ public void totallyEmpty() throws Exception {
 
     private XContentBuilder fields(Object... fields) throws IOException {
         assertEquals("Fields must field1, value1, field2, value2, etc", 0, fields.length % 2);
-        
+
         XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
         for (int i = 0; i < fields.length; i += 2) {
             builder.field((String) fields[i], fields[i + 1]);
@@ -229,6 +257,7 @@ private long totalNoopUpdates() {
         return client().admin().indices().prepareStats("test").setIndexing(true).get().getIndex("test").getTotal().getIndexing().getTotal()
                 .getNoopUpdateCount();
     }
+
     @Before
     public void setup() {
         createIndex("test");