From 34410814b9e955b8461c72fb798003adc89dfb84 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 13 May 2020 23:24:21 +0200 Subject: [PATCH] Don't omit empty arrays when filtering _source (#56527) When using source filtering exclusions, empty arrays are not preserved in documents, and no empty arrays are returned if arrays are empty after applying exclusions. We have special treatment to make sure that we preserve empty objects, but the behaviour for arrays is different. It looks like this regression was introduced by #22593, shortly after we refactored source filtering to use automata (#20736). Note that this change affects what the search API returns when using source exclusions, as well as what gets indexed when using source exclusions for the _source field. Closes #23796 --- .../xcontent/support/XContentMapValues.java | 2 +- .../support/AbstractFilteringTestCase.java | 122 +++++++++--------- .../support/XContentMapValuesTests.java | 76 +++++++++++ 3 files changed, 138 insertions(+), 62 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java index 91d8ecadce1..05f8cb3a2fc 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java @@ -267,7 +267,7 @@ public class XContentMapValues { List filteredValue = filter((Iterable) value, subIncludeAutomaton, subIncludeState, excludeAutomaton, excludeState, matchAllAutomaton); - if (filteredValue.isEmpty() == false) { + if (includeAutomaton.isAccept(includeState) || filteredValue.isEmpty() == false) { filtered.put(key, filteredValue); } diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java b/server/src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java index c12376bd551..0f04bcd2b11 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/support/AbstractFilteringTestCase.java @@ -44,7 +44,7 @@ public abstract class AbstractFilteringTestCase extends ESTestCase { protected abstract void testFilter(Builder expected, Builder actual, Set includes, Set excludes) throws IOException; /** Sample test case **/ - private static final Builder SAMPLE = builder -> builder.startObject() + protected static final Builder SAMPLE = builder -> builder.startObject() .field("title", "My awesome book") .field("pages", 456) .field("price", 27.99) @@ -484,70 +484,70 @@ public abstract class AbstractFilteringTestCase extends ESTestCase { testFilter(expected, SAMPLE, singleton("authors.*name"), emptySet()); } - public void testSimpleArrayOfObjectsExclusive() throws Exception { - final Builder expected = builder -> builder.startObject() - .field("title", "My awesome book") - .field("pages", 456) - .field("price", 27.99) - .field("timestamp", 1428582942867L) - .nullField("default") - .startArray("tags") - .value("elasticsearch") - .value("java") - .endArray() - .startObject("properties") - .field("weight", 0.8d) - .startObject("language") - .startObject("en") - .field("lang", "English") - .field("available", true) - .startArray("distributors") - .startObject() - .field("name", "The Book Shop") - .startArray("addresses") - .startObject() - .field("name", "address #1") - .field("street", "Hampton St") - .field("city", "London") - .endObject() - .startObject() - .field("name", "address #2") - .field("street", "Queen St") - .field("city", "Stornoway") - .endObject() - .endArray() - .endObject() - .startObject() - .field("name", "Sussex Books House") - .endObject() - .endArray() + protected static final Builder SIMPLE_ARRAY_OF_OBJECTS_EXCLUSIVE = builder -> builder.startObject() + .field("title", "My awesome book") + .field("pages", 456) + .field("price", 27.99) + .field("timestamp", 1428582942867L) + .nullField("default") + .startArray("tags") + .value("elasticsearch") + .value("java") + .endArray() + .startObject("properties") + .field("weight", 0.8d) + .startObject("language") + .startObject("en") + .field("lang", "English") + .field("available", true) + .startArray("distributors") + .startObject() + .field("name", "The Book Shop") + .startArray("addresses") + .startObject() + .field("name", "address #1") + .field("street", "Hampton St") + .field("city", "London") .endObject() - .startObject("fr") - .field("lang", "French") - .field("available", false) - .startArray("distributors") - .startObject() - .field("name", "La Maison du Livre") - .startArray("addresses") - .startObject() - .field("name", "address #1") - .field("street", "Rue Mouffetard") - .field("city", "Paris") - .endObject() - .endArray() - .endObject() - .startObject() - .field("name", "Thetra") - .endObject() - .endArray() + .startObject() + .field("name", "address #2") + .field("street", "Queen St") + .field("city", "Stornoway") .endObject() - .endObject() + .endArray() .endObject() - .endObject(); + .startObject() + .field("name", "Sussex Books House") + .endObject() + .endArray() + .endObject() + .startObject("fr") + .field("lang", "French") + .field("available", false) + .startArray("distributors") + .startObject() + .field("name", "La Maison du Livre") + .startArray("addresses") + .startObject() + .field("name", "address #1") + .field("street", "Rue Mouffetard") + .field("city", "Paris") + .endObject() + .endArray() + .endObject() + .startObject() + .field("name", "Thetra") + .endObject() + .endArray() + .endObject() + .endObject() + .endObject() + .endObject(); - testFilter(expected, SAMPLE, emptySet(), singleton("authors")); - testFilter(expected, SAMPLE, emptySet(), singleton("authors.*")); - testFilter(expected, SAMPLE, emptySet(), singleton("authors.*name")); + public void testSimpleArrayOfObjectsExclusive() throws Exception { + testFilter(SIMPLE_ARRAY_OF_OBJECTS_EXCLUSIVE, SAMPLE, emptySet(), singleton("authors")); + testFilter(SIMPLE_ARRAY_OF_OBJECTS_EXCLUSIVE, SAMPLE, emptySet(), singleton("authors.*")); + testFilter(SIMPLE_ARRAY_OF_OBJECTS_EXCLUSIVE, SAMPLE, emptySet(), singleton("authors.*name")); } public void testSimpleArrayOfObjectsPropertyInclusive() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java index 534da561501..d83000bd669 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -37,6 +38,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; import static org.elasticsearch.common.xcontent.XContentHelper.convertToMap; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.hamcrest.Matchers.hasEntry; @@ -479,6 +482,79 @@ public class XContentMapValuesTests extends AbstractFilteringTestCase { assertEquals(Collections.singletonMap("foobaz", 3), XContentMapValues.filter(map, new String[0], new String[] {"foobar"})); } + @Override + public void testSimpleArrayOfObjectsExclusive() throws Exception { + //Empty arrays are preserved by XContentMapValues, they get removed only if explicitly excluded. + //See following tests around this specific behaviour + testFilter(SIMPLE_ARRAY_OF_OBJECTS_EXCLUSIVE, SAMPLE, emptySet(), singleton("authors")); + } + + public void testArraySubFieldExclusion() { + Map map = new HashMap<>(); + map.put("field", "value"); + List> array = new ArrayList<>(); + Map object = new HashMap<>(); + object.put("exclude", "bar"); + array.add(object); + map.put("array", array); + Map filtered = XContentMapValues.filter(map, new String[0], new String[]{"array.exclude"}); + assertTrue(filtered.containsKey("field")); + assertTrue(filtered.containsKey("array")); + List filteredArray = (List)filtered.get("array"); + assertThat(filteredArray, hasSize(0)); + } + + public void testEmptyArraySubFieldsExclusion() { + Map map = new HashMap<>(); + map.put("field", "value"); + List> array = new ArrayList<>(); + map.put("array", array); + Map filtered = XContentMapValues.filter(map, new String[0], new String[]{"array.exclude"}); + assertTrue(filtered.containsKey("field")); + assertTrue(filtered.containsKey("array")); + List filteredArray = (List)filtered.get("array"); + assertEquals(0, filteredArray.size()); + } + + public void testEmptyArraySubFieldsInclusion() { + Map map = new HashMap<>(); + map.put("field", "value"); + List> array = new ArrayList<>(); + map.put("array", array); + { + Map filtered = XContentMapValues.filter(map, new String[]{"array.include"}, new String[0]); + assertFalse(filtered.containsKey("field")); + assertFalse(filtered.containsKey("array")); + } + { + Map filtered = XContentMapValues.filter(map, new String[]{"array", "array.include"}, + new String[0]); + assertFalse(filtered.containsKey("field")); + assertTrue(filtered.containsKey("array")); + List filteredArray = (List)filtered.get("array"); + assertEquals(0, filteredArray.size()); + } + } + + public void testEmptyObjectsSubFieldsInclusion() { + Map map = new HashMap<>(); + map.put("field", "value"); + map.put("object", new HashMap<>()); + { + Map filtered = XContentMapValues.filter(map, new String[]{"object.include"}, new String[0]); + assertFalse(filtered.containsKey("field")); + assertFalse(filtered.containsKey("object")); + } + { + Map filtered = XContentMapValues.filter(map, new String[]{"object", "object.include"}, + new String[0]); + assertFalse(filtered.containsKey("field")); + assertTrue(filtered.containsKey("object")); + Map filteredMap = (Map)filtered.get("object"); + assertEquals(0, filteredMap.size()); + } + } + public void testPrefix() { Map map = new HashMap<>(); map.put("photos", Arrays.asList(new String[] {"foo", "bar"}));