Source filtering: only accept array items if the previous include pattern matches (#22593)

Source filtering was always accepting array items even if the include pattern did not match.

Closes #22557

Author: Michael McCandless

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

@@ -266,7 +266,7 @@ private static Map<String, Object> filter(Map<String, ?> map,
 
                 List<Object> filteredValue = filter((Iterable<?>) value,
                         subIncludeAutomaton, subIncludeState, excludeAutomaton, excludeState, matchAllAutomaton);
-                if (includeAutomaton.isAccept(includeState) || filteredValue.isEmpty() == false) {
+                if (filteredValue.isEmpty() == false) {
                     filtered.put(key, filteredValue);
                 }
 
@@ -289,6 +289,7 @@ private static List<Object> filter(Iterable<?> iterable,
             CharacterRunAutomaton excludeAutomaton, int initialExcludeState,
             CharacterRunAutomaton matchAllAutomaton) {
         List<Object> filtered = new ArrayList<>();
+        boolean isInclude = includeAutomaton.isAccept(initialIncludeState);
         for (Object value : iterable) {
             if (value instanceof Map) {
                 int includeState = includeAutomaton.step(initialIncludeState, '.');
@@ -307,9 +308,8 @@ private static List<Object> filter(Iterable<?> iterable,
                 if (filteredValue.isEmpty() == false) {
                     filtered.add(filteredValue);
                 }
-            } else {
-                // TODO: we have tests relying on this behavior on arrays even
-                // if the path does not match, but this looks like a bug?
+            } else if (isInclude) {
+                // #22557: only accept this array value if the key we are on is accepted:
                 filtered.add(value);
             }
         }

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

@@ -41,6 +41,7 @@
 import static java.util.Collections.singletonMap;
 import static org.hamcrest.Matchers.hasEntry;
 import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.core.IsEqual.equalTo;
@@ -150,8 +151,8 @@ public void testExtractValue() throws Exception {
         extValue = XContentMapValues.extractValue("path1.test", map);
         assertThat(extValue, instanceOf(List.class));
 
-        List extListValue = (List) extValue;
-        assertThat(extListValue.size(), equalTo(2));
+        List<?> extListValue = (List) extValue;
+        assertThat(extListValue, hasSize(2));
 
         builder = XContentFactory.jsonBuilder().startObject()
                 .startObject("path1")
@@ -170,7 +171,7 @@ public void testExtractValue() throws Exception {
         assertThat(extValue, instanceOf(List.class));
 
         extListValue = (List) extValue;
-        assertThat(extListValue.size(), equalTo(2));
+        assertThat(extListValue, hasSize(2));
         assertThat(extListValue.get(0).toString(), equalTo("value1"));
         assertThat(extListValue.get(1).toString(), equalTo("value2"));
 
@@ -236,9 +237,9 @@ public void testPrefixedNamesFilteringTest() {
         Map<String, Object> map = new HashMap<>();
         map.put("obj", "value");
         map.put("obj_name", "value_name");
-        Map<String, Object> filterdMap = XContentMapValues.filter(map, new String[]{"obj_name"}, Strings.EMPTY_ARRAY);
-        assertThat(filterdMap.size(), equalTo(1));
-        assertThat((String) filterdMap.get("obj_name"), equalTo("value_name"));
+        Map<String, Object> filteredMap = XContentMapValues.filter(map, new String[]{"obj_name"}, Strings.EMPTY_ARRAY);
+        assertThat(filteredMap.size(), equalTo(1));
+        assertThat((String) filteredMap.get("obj_name"), equalTo("value_name"));
     }
 
 
@@ -253,19 +254,17 @@ public void testNestedFiltering() {
                             put("nested", 2);
                             put("nested_2", 3);
                         }}));
-        Map<String, Object> falteredMap = XContentMapValues.filter(map, new String[]{"array.nested"}, Strings.EMPTY_ARRAY);
-        assertThat(falteredMap.size(), equalTo(1));
+        Map<String, Object> filteredMap = XContentMapValues.filter(map, new String[]{"array.nested"}, Strings.EMPTY_ARRAY);
+        assertThat(filteredMap.size(), equalTo(1));
 
-        // Selecting members of objects within arrays (ex. [ 1, { nested: "value"} ])  always returns all values in the array (1 in the ex)
-        // this is expected behavior as this types of objects are not supported in ES
-        assertThat((Integer) ((List) falteredMap.get("array")).get(0), equalTo(1));
-        assertThat(((Map<String, Object>) ((List) falteredMap.get("array")).get(1)).size(), equalTo(1));
-        assertThat((Integer) ((Map<String, Object>) ((List) falteredMap.get("array")).get(1)).get("nested"), equalTo(2));
+        assertThat(((List<?>) filteredMap.get("array")), hasSize(1));
+        assertThat(((Map<String, Object>) ((List) filteredMap.get("array")).get(0)).size(), equalTo(1));
+        assertThat((Integer) ((Map<String, Object>) ((List) filteredMap.get("array")).get(0)).get("nested"), equalTo(2));
 
-        falteredMap = XContentMapValues.filter(map, new String[]{"array.*"}, Strings.EMPTY_ARRAY);
-        assertThat(falteredMap.size(), equalTo(1));
-        assertThat((Integer) ((List) falteredMap.get("array")).get(0), equalTo(1));
-        assertThat(((Map<String, Object>) ((List) falteredMap.get("array")).get(1)).size(), equalTo(2));
+        filteredMap = XContentMapValues.filter(map, new String[]{"array.*"}, Strings.EMPTY_ARRAY);
+        assertThat(filteredMap.size(), equalTo(1));
+        assertThat(((List<?>) filteredMap.get("array")), hasSize(1));
+        assertThat(((Map<String, Object>) ((List) filteredMap.get("array")).get(0)).size(), equalTo(2));
 
         map.clear();
         map.put("field", "value");
@@ -274,16 +273,16 @@ public void testNestedFiltering() {
                     put("field", "value");
                     put("field2", "value2");
                 }});
-        falteredMap = XContentMapValues.filter(map, new String[]{"obj.field"}, Strings.EMPTY_ARRAY);
-        assertThat(falteredMap.size(), equalTo(1));
-        assertThat(((Map<String, Object>) falteredMap.get("obj")).size(), equalTo(1));
-        assertThat((String) ((Map<String, Object>) falteredMap.get("obj")).get("field"), equalTo("value"));
+        filteredMap = XContentMapValues.filter(map, new String[]{"obj.field"}, Strings.EMPTY_ARRAY);
+        assertThat(filteredMap.size(), equalTo(1));
+        assertThat(((Map<String, Object>) filteredMap.get("obj")).size(), equalTo(1));
+        assertThat((String) ((Map<String, Object>) filteredMap.get("obj")).get("field"), equalTo("value"));
 
-        falteredMap = XContentMapValues.filter(map, new String[]{"obj.*"}, Strings.EMPTY_ARRAY);
-        assertThat(falteredMap.size(), equalTo(1));
-        assertThat(((Map<String, Object>) falteredMap.get("obj")).size(), equalTo(2));
-        assertThat((String) ((Map<String, Object>) falteredMap.get("obj")).get("field"), equalTo("value"));
-        assertThat((String) ((Map<String, Object>) falteredMap.get("obj")).get("field2"), equalTo("value2"));
+        filteredMap = XContentMapValues.filter(map, new String[]{"obj.*"}, Strings.EMPTY_ARRAY);
+        assertThat(filteredMap.size(), equalTo(1));
+        assertThat(((Map<String, Object>) filteredMap.get("obj")).size(), equalTo(2));
+        assertThat((String) ((Map<String, Object>) filteredMap.get("obj")).get("field"), equalTo("value"));
+        assertThat((String) ((Map<String, Object>) filteredMap.get("obj")).get("field2"), equalTo("value2"));
 
     }
 
@@ -325,7 +324,7 @@ public void testCompleteObjectFiltering() {
 
         filteredMap = XContentMapValues.filter(map, new String[]{"array"}, new String[]{"*.field2"});
         assertThat(filteredMap.size(), equalTo(1));
-        assertThat(((List) filteredMap.get("array")).size(), equalTo(2));
+        assertThat(((List<?>) filteredMap.get("array")), hasSize(2));
         assertThat((Integer) ((List) filteredMap.get("array")).get(0), equalTo(1));
         assertThat(((Map<String, Object>) ((List) filteredMap.get("array")).get(1)).size(), equalTo(1));
         assertThat(((Map<String, Object>) ((List) filteredMap.get("array")).get(1)).get("field").toString(), equalTo("value"));
@@ -438,20 +437,20 @@ public void testNotOmittingObjectWithNestedExcludedObject() throws Exception {
 
         assertThat(filteredSource.size(), equalTo(1));
         assertThat(filteredSource, hasKey("obj1"));
-        assertThat(((Map) filteredSource.get("obj1")).size(), Matchers.equalTo(0));
+        assertThat(((Map) filteredSource.get("obj1")).size(), 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));
+        assertThat(((Map) filteredSource.get("obj1")).size(), 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));
+        assertThat(((Map) ((Map) filteredSource.get("obj1")).get("obj2")).size(), equalTo(0));
     }
 
     @SuppressWarnings({"unchecked"})
@@ -591,4 +590,15 @@ public void testSharedPrefixes() {
         assertEquals(Collections.singletonMap("foobar", 2), XContentMapValues.filter(map, new String[] {"foobar"}, new String[0]));
         assertEquals(Collections.singletonMap("foobaz", 3), XContentMapValues.filter(map, new String[0], new String[] {"foobar"}));
     }
+
+    public void testPrefix() {
+        Map<String, Object> map = new HashMap<>();
+        map.put("photos", Arrays.asList(new String[] {"foo", "bar"}));
+        map.put("photosCount", 2);
+
+        Map<String, Object> filtered = XContentMapValues.filter(map, new String[] {"photosCount"}, new String[0]);
+        Map<String, Object> expected = new HashMap<>();
+        expected.put("photosCount", 2);
+        assertEquals(expected, filtered);
+    }
 }