From 018ca58cdba9785df2f5686ca56540e89b1c6961 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Thu, 4 Jul 2013 10:22:45 +0200 Subject: [PATCH] Filtering maps had false hit is a field was a prefix (but not a match) of an include. Also, exact matching a key whose value is an object resulted in an empty value. Closes #3288 --- .../xcontent/support/XContentMapValues.java | 31 ++++- .../support/XContentMapValuesTests.java | 121 +++++++++++++++++- 2 files changed, 145 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java b/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java index 18c5b247e17..acd6472c93e 100644 --- a/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java +++ b/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java @@ -141,6 +141,10 @@ public class XContentMapValues { } private static void filter(Map map, Map into, String[] includes, String[] excludes, StringBuilder sb) { + if (includes.length == 0 && excludes.length == 0) { + into.putAll(map); + return; + } for (Map.Entry entry : map.entrySet()) { String key = entry.getKey(); int mark = sb.length(); @@ -160,12 +164,24 @@ public class XContentMapValues { sb.setLength(mark); continue; } + boolean exactIncludeMatch = false; if (includes.length > 0) { boolean atLeastOnOneIncludeMatched = false; for (String include : includes) { - // check for prefix as well, something like: obj1.arr1.* + // check for prefix as well to see if we need to zero in, something like: obj1.arr1.* // note, this does not work well with middle matches, like obj1.*.obj3 - if (include.startsWith(path) || Regex.simpleMatch(include, path)) { + if (include.startsWith(path)) { + if (include.length() == path.length()) { + atLeastOnOneIncludeMatched = true; + exactIncludeMatch = true; + break; + } else if (include.length() > path.length() && include.charAt(path.length()) == '.') { + // include might may match deeper paths. Dive deeper. + atLeastOnOneIncludeMatched = true; + break; + } + } + if (Regex.simpleMatch(include, path)) { atLeastOnOneIncludeMatched = true; break; } @@ -179,14 +195,16 @@ public class XContentMapValues { if (entry.getValue() instanceof Map) { Map innerInto = Maps.newHashMap(); - filter((Map) entry.getValue(), innerInto, includes, excludes, sb); + // if we had an exact match, we want give deeper excludes their chance + filter((Map) 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 list = (List) entry.getValue(); List innerInto = new ArrayList(list.size()); - filter(list, innerInto, includes, excludes, sb); + // if we had an exact match, we want give deeper excludes their chance + filter(list, innerInto, exactIncludeMatch ? Strings.EMPTY_ARRAY : includes, excludes, sb); into.put(entry.getKey(), innerInto); } else { into.put(entry.getKey(), entry.getValue()); @@ -196,6 +214,11 @@ public class XContentMapValues { } private static void filter(List from, List to, String[] includes, String[] excludes, StringBuilder sb) { + if (includes.length == 0 && excludes.length == 0) { + to.addAll(from); + return; + } + for (Object o : from) { if (o instanceof Map) { Map innerInto = Maps.newHashMap(); diff --git a/src/test/java/org/elasticsearch/test/unit/common/xcontent/support/XContentMapValuesTests.java b/src/test/java/org/elasticsearch/test/unit/common/xcontent/support/XContentMapValuesTests.java index 2b2bb3a30ec..715c3456ac3 100644 --- a/src/test/java/org/elasticsearch/test/unit/common/xcontent/support/XContentMapValuesTests.java +++ b/src/test/java/org/elasticsearch/test/unit/common/xcontent/support/XContentMapValuesTests.java @@ -28,11 +28,14 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.testng.annotations.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; /** */ @@ -73,7 +76,7 @@ public class XContentMapValuesTests { source = XContentFactory.xContent(XContentType.JSON).createParser(builder.string()).mapAndClose(); filter = XContentMapValues.filter(source, new String[]{"path1"}, Strings.EMPTY_ARRAY); - assertThat(filter.size(), equalTo(0)); + assertThat(filter.size(), equalTo(1)); filter = XContentMapValues.filter(source, new String[]{"path1*"}, Strings.EMPTY_ARRAY); assertThat(filter.get("path1"), equalTo(source.get("path1"))); @@ -202,13 +205,125 @@ public class XContentMapValuesTests { public void testThatFilteringWithNestedArrayAndExclusionWorks() throws Exception { XContentBuilder builder = XContentFactory.jsonBuilder().startObject() .startArray("coordinates") - .startArray().value("foo").endArray() + .startArray().value("foo").endArray() .endArray() - .endObject(); + .endObject(); Tuple> mapTuple = XContentHelper.convertToMap(builder.bytes(), true); Map filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"nonExistingField"}); assertThat(mapTuple.v2(), equalTo(filteredSource)); } + + @Test + public void prefixedNamesFilteringTest() { + Map map = new HashMap(); + map.put("obj", "value"); + map.put("obj_name", "value_name"); + Map 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")); + } + + + @Test + @SuppressWarnings("unchecked") + public void nestedFilteringTest() { + Map map = new HashMap(); + map.put("field", "value"); + map.put("array", + Arrays.asList( + 1, + new HashMap() {{ + put("nested", 2); + put("nested_2", 3); + }})); + Map falteredMap = XContentMapValues.filter(map, new String[]{"array.nested"}, Strings.EMPTY_ARRAY); + assertThat(falteredMap.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) ((List) falteredMap.get("array")).get(1)).size(), equalTo(1)); + assertThat((Integer) ((Map) ((List) falteredMap.get("array")).get(1)).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) ((List) falteredMap.get("array")).get(1)).size(), equalTo(2)); + + map.clear(); + map.put("field", "value"); + map.put("obj", + new HashMap() {{ + put("field", "value"); + put("field2", "value2"); + }}); + falteredMap = XContentMapValues.filter(map, new String[]{"obj.field"}, Strings.EMPTY_ARRAY); + assertThat(falteredMap.size(), equalTo(1)); + assertThat(((Map) falteredMap.get("obj")).size(), equalTo(1)); + assertThat((String) ((Map) falteredMap.get("obj")).get("field"), equalTo("value")); + + falteredMap = XContentMapValues.filter(map, new String[]{"obj.*"}, Strings.EMPTY_ARRAY); + assertThat(falteredMap.size(), equalTo(1)); + assertThat(((Map) falteredMap.get("obj")).size(), equalTo(2)); + assertThat((String) ((Map) falteredMap.get("obj")).get("field"), equalTo("value")); + assertThat((String) ((Map) falteredMap.get("obj")).get("field2"), equalTo("value2")); + + } + + @SuppressWarnings("unchecked") + @Test + public void completeObjectFilteringTest() { + Map map = new HashMap(); + map.put("field", "value"); + map.put("obj", + new HashMap() {{ + put("field", "value"); + put("field2", "value2"); + }}); + map.put("array", + Arrays.asList( + 1, + new HashMap() {{ + put("field", "value"); + put("field2", "value2"); + }})); + + Map filteredMap = XContentMapValues.filter(map, new String[]{"obj"}, Strings.EMPTY_ARRAY); + assertThat(filteredMap.size(), equalTo(1)); + assertThat(((Map) filteredMap.get("obj")).size(), equalTo(2)); + assertThat(((Map) filteredMap.get("obj")).get("field").toString(), equalTo("value")); + assertThat(((Map) filteredMap.get("obj")).get("field2").toString(), equalTo("value2")); + + + filteredMap = XContentMapValues.filter(map, new String[]{"obj"}, new String[]{"*.field2"}); + assertThat(filteredMap.size(), equalTo(1)); + assertThat(((Map) filteredMap.get("obj")).size(), equalTo(1)); + assertThat(((Map) filteredMap.get("obj")).get("field").toString(), equalTo("value")); + + + filteredMap = XContentMapValues.filter(map, new String[]{"array"}, new String[]{}); + assertThat(filteredMap.size(), equalTo(1)); + assertThat(((List) filteredMap.get("array")).size(), equalTo(2)); + assertThat((Integer) ((List) filteredMap.get("array")).get(0), equalTo(1)); + assertThat(((Map) ((List) filteredMap.get("array")).get(1)).size(), equalTo(2)); + + filteredMap = XContentMapValues.filter(map, new String[]{"array"}, new String[]{"*.field2"}); + assertThat(filteredMap.size(), equalTo(1)); + assertThat(((List) filteredMap.get("array")).size(), equalTo(2)); + assertThat((Integer) ((List) filteredMap.get("array")).get(0), equalTo(1)); + assertThat(((Map) ((List) filteredMap.get("array")).get(1)).size(), equalTo(1)); + assertThat(((Map) ((List) filteredMap.get("array")).get(1)).get("field").toString(), equalTo("value")); + } + + @Test + public void filterWithEmptyIncludesExcludes() { + Map map = new HashMap(); + map.put("field", "value"); + Map filteredMap = XContentMapValues.filter(map, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY); + assertThat(filteredMap.size(), equalTo(1)); + assertThat(filteredMap.get("field").toString(), equalTo("value")); + + } }