excluding all fields of an object should not remove parent.

When excluding '*.f1' from `{ "obj": { "f1": 1, "f2": 2 } }` XContentMapValues.filter returns `{ "obj": { "f2": 2}}`. When run on `{ "obj": { "f1" : 1 }}` we should return `{ "obj": { }}` to maintain object structure. People currently need to always check whether `obj` is there or not.

Closes #4715
Closes #4047
Related to #4491

Author: RobCherry

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

@@ -154,24 +154,18 @@ 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;
             }
-            boolean exactIncludeMatch;
+
+            boolean exactIncludeMatch = false; // true if the current position was specifically mentioned
+            boolean pathIsPrefixOfAnInclude = false; // true if potentially a sub scope can be included
             if (includes.length == 0) {
                 // implied match anything
                 exactIncludeMatch = true;
             } else {
-                exactIncludeMatch = false;
-                boolean pathIsPrefixOfAnInclude = false;
                 for (String include : includes) {
                     // check for prefix matches as well to see if we need to zero in, something like: obj1.arr1.* or *.field
                     // note, this does not work well with middle matches, like obj1.*.obj3
@@ -198,19 +192,20 @@ private static void filter(Map<String, Object> map, Map<String, Object> into, St
                         break;
                     }
                 }
-                if (!pathIsPrefixOfAnInclude && !exactIncludeMatch) {
-                    // skip subkeys, not interesting.
-                    sb.setLength(mark);
-                    continue;
-                }
+            }
+
+            if (!(pathIsPrefixOfAnInclude || exactIncludeMatch)) {
+                // skip subkeys, not interesting.
+                sb.setLength(mark);
+                continue;
             }
 
 
             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 || !innerInto.isEmpty()) {
                     into.put(entry.getKey(), innerInto);
                 }
             } else if (entry.getValue() instanceof List) {

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

@@ -26,14 +26,14 @@
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.test.ElasticsearchTestCase;
+import org.hamcrest.Matchers;
 import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.core.IsEqual.equalTo;
 
@@ -46,6 +46,7 @@ public void testFilter() throws Exception {
         XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
                 .field("test1", "value1")
                 .field("test2", "value2")
+                .field("something_else", "value3")
                 .endObject();
 
         Map<String, Object> source = XContentFactory.xContent(XContentType.JSON).createParser(builder.string()).mapAndClose();
@@ -59,8 +60,9 @@ public void testFilter() throws Exception {
         assertThat(filter.get("test2").toString(), equalTo("value2"));
 
         filter = XContentMapValues.filter(source, Strings.EMPTY_ARRAY, new String[]{"test1"});
-        assertThat(filter.size(), equalTo(1));
+        assertThat(filter.size(), equalTo(2));
         assertThat(filter.get("test2").toString(), equalTo("value2"));
+        assertThat(filter.get("something_else").toString(), equalTo("value3"));
 
         // more complex object...
         builder = XContentFactory.jsonBuilder().startObject()
@@ -200,20 +202,6 @@ public void testExtractRawValue() throws Exception {
         assertThat(XContentMapValues.extractRawValues("path1.xxx.path2.yyy.test", map).get(0).toString(), equalTo("value"));
     }
 
-    @Test
-    public void testThatFilteringWithNestedArrayAndExclusionWorks() throws Exception {
-        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
-                .startArray("coordinates")
-                .startArray().value("foo").endArray()
-                .endArray()
-                .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[]{"nonExistingField"});
-
-        assertThat(mapTuple.v2(), equalTo(filteredSource));
-    }
-
     @Test
     public void prefixedNamesFilteringTest() {
         Map<String, Object> map = new HashMap<String, Object>();
@@ -368,4 +356,101 @@ public void filterWithEmptyIncludesExcludes() {
         assertThat(filteredMap.get("field").toString(), equalTo("value"));
 
     }
+
+    @SuppressWarnings({"unchecked"})
+    @Test
+    public void testThatFilterIncludesEmptyObjectWhenUsingIncludes() 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(), new String[]{"obj"}, Strings.EMPTY_ARRAY);
+
+        assertThat(mapTuple.v2(), equalTo(filteredSource));
+    }
+
+    @Test
+    public void testThatFilterIncludesEmptyObjectWhenUsingExcludes() 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[]{"nonExistingField"});
+
+        assertThat(mapTuple.v2(), equalTo(filteredSource));
+    }
+
+    @Test
+    public void testNotOmittingObjectsWithExcludedProperties() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj")
+                .field("f1", "v1")
+                .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(filteredSource.size(), equalTo(1));
+        assertThat(filteredSource, hasKey("obj"));
+        assertThat(((Map) filteredSource.get("obj")).size(), equalTo(0));
+    }
+
+    @SuppressWarnings({"unchecked"})
+    @Test
+    public void testNotOmittingObjectWithNestedExcludedObject() throws Exception {
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                .startObject("obj1")
+                .startObject("obj2")
+                .startObject("obj3")
+                .endObject()
+                .endObject()
+                .endObject()
+                .endObject();
+
+        // implicit include
+        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(1));
+        assertThat(filteredSource, hasKey("obj1"));
+        assertThat(((Map) filteredSource.get("obj1")).size(), Matchers.equalTo(0));
+
+        // explicit include
+        filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"obj1"}, new String[]{"*.obj2"});
+        assertThat(filteredSource.size(), equalTo(1));
+        assertThat(filteredSource, hasKey("obj1"));
+        assertThat(((Map) filteredSource.get("obj1")).size(), Matchers.equalTo(0));
+
+        // wild card include
+        filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"*.obj2"}, new String[]{"*.obj3"});
+        assertThat(filteredSource.size(), equalTo(1));
+        assertThat(filteredSource, hasKey("obj1"));
+        assertThat(((Map<String, Object>) filteredSource.get("obj1")), hasKey("obj2"));
+        assertThat(((Map) ((Map) filteredSource.get("obj1")).get("obj2")).size(), Matchers.equalTo(0));
+    }
+
+    @SuppressWarnings({"unchecked"})
+    @Test
+    public void testIncludingObjectWithNestedIncludedObject() 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));
+    }
 }

src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java

@@ -253,7 +253,7 @@ public void testPartialFields() throws Exception {
 
         SearchResponse response = client().prepareSearch("test")
                 .addPartialField("partial1", "obj1.arr1.*", null)
-                .addPartialField("partial2", null, "obj1.*")
+                .addPartialField("partial2", null, "obj1")
                 .execute().actionGet();
         assertThat("Failures " + Arrays.toString(response.getShardFailures()), response.getShardFailures().length, equalTo(0));