Fixes elastic#4047 - Empty objects are not stored in _source when an include/exclude list is present

Author: RobCherry

src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java

@@ -153,14 +153,7 @@ private static void filter(Map<String, Object> map, Map<String, Object> into, St
             }
             sb.append(key);
             String path = sb.toString();
-            boolean excluded = false;
-            for (String exclude : excludes) {
-                if (Regex.simpleMatch(exclude, path)) {
-                    excluded = true;
-                    break;
-                }
-            }
-            if (excluded) {
+            if (Regex.simpleMatch(excludes, path)) {
                 sb.setLength(mark);
                 continue;
             }
@@ -205,12 +198,16 @@ private static void filter(Map<String, Object> map, Map<String, Object> into, St
             }
 
 
-            if (entry.getValue() instanceof Map && !((Map<String, Object>) entry.getValue()).isEmpty()) {
+            if (entry.getValue() instanceof Map) {
                 Map<String, Object> innerInto = Maps.newHashMap();
-                // if we had an exact match, we want give deeper excludes their chance
-                filter((Map<String, Object>) entry.getValue(), innerInto, exactIncludeMatch ? Strings.EMPTY_ARRAY : includes, excludes, sb);
-                if (!innerInto.isEmpty()) {
+                if (exactIncludeMatch && ((Map<String, Object>) entry.getValue()).isEmpty() && !Regex.simpleMatch(excludes, path + '.')) {
                     into.put(entry.getKey(), innerInto);
+                } else {
+                    // if we had an exact match, we want give deeper excludes their chance
+                    filter((Map<String, Object>) entry.getValue(), innerInto, exactIncludeMatch ? Strings.EMPTY_ARRAY : includes, excludes, sb);
+                    if (!innerInto.isEmpty()) {
+                        into.put(entry.getKey(), innerInto);
+                    }
                 }
             } else if (entry.getValue() instanceof List) {
                 List<Object> list = (List<Object>) entry.getValue();

src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java

@@ -370,7 +370,7 @@ public void filterWithEmptyIncludesExcludes() {
     }
 
     @Test
-    public void testThatFilteringWithEmptyObjectAndExclusionWorks() throws Exception {
+    public void testThatFilterIncludesEmptyObjectWhenUsingExcludes() throws Exception {
         XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
                 .startObject("obj")
                 .endObject()
@@ -383,7 +383,7 @@ public void testThatFilteringWithEmptyObjectAndExclusionWorks() throws Exception
     }
 
     @Test
-    public void testThatFilterIncludesEmptyObjectsWithoutExcludedProperties() throws Exception {
+    public void testThatFilterOmitsEmptyObjectWithExcludedProperties() throws Exception {
         XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
                 .startObject("obj")
                 .endObject()
@@ -392,9 +392,20 @@ public void testThatFilterIncludesEmptyObjectsWithoutExcludedProperties() throws
         Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
         Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"obj.*"});
 
-        assertThat(filteredSource.size(), equalTo(1));
-        assertThat(filteredSource, hasKey("obj"));
-        assertThat(((Map) filteredSource.get("obj")).size(), equalTo(0));
+        assertThat(filteredSource.size(), equalTo(0));
+    }
+
+    @Test
+    public void testThatFilterIncludesEmptyObjectWithExcludedProperty() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj")
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"obj.f1"});
+
+        assertThat(mapTuple.v2(), equalTo(filteredSource));
     }
 
     @Test
@@ -430,7 +441,7 @@ public void testThatFilterIncludesObjectsWithSomeExcludedProperties() throws Exc
     }
 
     @Test
-    public void testThatFilteringWithEmptyObjectAndInclusionWorks() throws Exception {
+    public void testThatFilterIncludesEmptyObjectWhenUsingIncludes() throws Exception {
         XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
                 .startObject("obj")
                 .endObject()
@@ -486,4 +497,83 @@ public void testThatFilterIncludesObjectsWithSomeIncludedProperties() throws Exc
         assertThat(((Map) filteredSource.get("obj")).size(), equalTo(1));
         assertThat(((Map<String, Object>) filteredSource.get("obj")), hasKey("f2"));
     }
+
+    @Test
+    public void testFilterWithIncludesAndExcludes() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .endObject()
+                .startObject("obj2")
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"obj1.*"}, new String[]{"obj2.*"});
+
+        assertThat(filteredSource.size(), equalTo(0));
+    }
+
+    @Test
+    public void testFilterExcludesWithNestedObjects() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .startObject("obj2")
+                .endObject()
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"*.obj2.*"});
+
+        assertThat(filteredSource.size(), equalTo(0));
+    }
+
+    @Test
+     public void testFilterOmitsObjectWithNestedExcludedObjects() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .startObject("obj2")
+                .endObject()
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"*.obj2"});
+
+        assertThat(filteredSource.size(), equalTo(0));
+    }
+
+    @Test
+    public void testFilterOmitsObjectWithNestedExcludedObject() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .startObject("obj2")
+                .endObject()
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"*.obj2"});
+
+        assertThat(filteredSource.size(), equalTo(0));
+    }
+
+    @Test
+    public void testFilterIncludesObjectWithNestedIncludedObject() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .startObject("obj2")
+                .endObject()
+                .endObject()
+                .endObject();
+
+        Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
+        Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"*.obj2"}, Strings.EMPTY_ARRAY);
+
+        assertThat(filteredSource.size(), equalTo(1));
+        assertThat(filteredSource, hasKey("obj1"));
+        assertThat(((Map) filteredSource.get("obj1")).size(), equalTo(1));
+        assertThat(((Map<String, Object>) filteredSource.get("obj1")), hasKey("obj2"));
+        assertThat(((Map) ((Map) filteredSource.get("obj1")).get("obj2")).size(), equalTo(0));
+    }
 }