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
This commit is contained in:
Michael McCandless 2017-01-12 19:00:39 -05:00 committed by GitHub
parent baed02bbe2
commit 568e655fdb
2 changed files with 44 additions and 34 deletions

View File

@ -263,7 +263,7 @@ public class XContentMapValues {
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);
}
@ -286,6 +286,7 @@ public class XContentMapValues {
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, '.');
@ -304,9 +305,8 @@ public class XContentMapValues {
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);
}
}

View File

@ -41,6 +41,7 @@ import static java.util.Collections.emptyMap;
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;
@ -148,8 +149,8 @@ public class XContentMapValuesTests extends ESTestCase {
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")
@ -168,7 +169,7 @@ public class XContentMapValuesTests extends ESTestCase {
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"));
@ -234,9 +235,9 @@ public class XContentMapValuesTests extends ESTestCase {
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"));
}
@ -251,19 +252,17 @@ public class XContentMapValuesTests extends ESTestCase {
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");
@ -272,16 +271,16 @@ public class XContentMapValuesTests extends ESTestCase {
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"));
}
@ -323,7 +322,7 @@ public class XContentMapValuesTests extends ESTestCase {
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"));
@ -436,20 +435,20 @@ public class XContentMapValuesTests extends ESTestCase {
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"})
@ -589,4 +588,15 @@ public class XContentMapValuesTests extends ESTestCase {
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);
}
}