From 32e713b5af7195a2ab412d74e640d778807e79c3 Mon Sep 17 00:00:00 2001 From: mccheah Date: Mon, 21 Oct 2024 01:28:44 -0700 Subject: [PATCH] Do not exclude empty arrays or empty objects in source filtering with Jackson streaming (#112250) (cherry picked from commit 6be3036c01caffb8f82711e32f15ffbc6b8fda2d) --- docs/changelog/112250.yaml | 5 +++ .../filtering/FilterPathBasedFilter.java | 35 +++++++++++++++ .../search/lookup/SourceFilterTests.java | 44 +++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 docs/changelog/112250.yaml diff --git a/docs/changelog/112250.yaml b/docs/changelog/112250.yaml new file mode 100644 index 0000000000000..edbb5667d4b9d --- /dev/null +++ b/docs/changelog/112250.yaml @@ -0,0 +1,5 @@ +pr: 112250 +summary: Do not exclude empty arrays or empty objects in source filtering +area: Search +type: bug +issues: [109668] diff --git a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/filtering/FilterPathBasedFilter.java b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/filtering/FilterPathBasedFilter.java index e0b5875c6c108..4562afa8af693 100644 --- a/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/filtering/FilterPathBasedFilter.java +++ b/libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/filtering/FilterPathBasedFilter.java @@ -96,6 +96,41 @@ public TokenFilter includeProperty(String name) { return filter; } + /** + * This is overridden in order to keep empty arrays in nested exclusions - see #109668. + *

+ * If we are excluding contents, we only want to exclude based on property name - but empty arrays in themselves do not have a property + * name. If the empty array were to be excluded, it should be done by excluding the parent. + *

+ * Note though that the expected behavior seems to be ambiguous if contentsFiltered is true - that is, that the filter has pruned all + * the contents of a given array, such that we are left with the empty array. The behavior below drops that array, for at the time of + * writing, not doing so would cause assertions in JsonXContentFilteringTests to fail, which expect this behavior. Yet it is not obvious + * if dropping the empty array in this case is correct. For example, one could expect this sort of behavior: + *

+ * From the user's perspective, this could reasonably yield either of: + *
    + * 
  1. { "myArray": []}
    2. + *
  2. Removing {@code myArray} entirely.
    3. + *
+ */ + @Override + public boolean includeEmptyArray(boolean contentsFiltered) { + return inclusive == false && contentsFiltered == false; + } + + /** + * This is overridden in order to keep empty objects in nested exclusions - see #109668. + *

+ * The same logic applies to this as to {@link #includeEmptyArray(boolean)}, only for nested objects instead of nested arrays. + */ + @Override + public boolean includeEmptyObject(boolean contentsFiltered) { + return inclusive == false && contentsFiltered == false; + } + @Override protected boolean _includeScalar() { return inclusive == false; diff --git a/server/src/test/java/org/elasticsearch/search/lookup/SourceFilterTests.java b/server/src/test/java/org/elasticsearch/search/lookup/SourceFilterTests.java index 370584e3f29f5..bddfd53b2b120 100644 --- a/server/src/test/java/org/elasticsearch/search/lookup/SourceFilterTests.java +++ b/server/src/test/java/org/elasticsearch/search/lookup/SourceFilterTests.java @@ -112,4 +112,48 @@ public Source filter(SourceFilter sourceFilter) { } + // Verification for issue #109668 + public void testIncludeParentAndExcludeChildEmptyArray() { + Source fromMap = Source.fromMap(Map.of("myArray", List.of()), XContentType.JSON); + Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" })); + assertEquals(filteredMap.source(), Map.of("myArray", List.of())); + Source fromBytes = Source.fromBytes(new BytesArray("{\"myArray\": []}"), XContentType.JSON); + Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" })); + assertEquals(filteredBytes.source(), Map.of("myArray", List.of())); + } + + public void testIncludeParentAndExcludeChildEmptyObject() { + Source fromMap = Source.fromMap(Map.of("myObject", Map.of()), XContentType.JSON); + Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" })); + assertEquals(filteredMap.source(), Map.of("myObject", Map.of())); + Source fromBytes = Source.fromBytes(new BytesArray("{\"myObject\": {}}"), XContentType.JSON); + Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" })); + assertEquals(filteredBytes.source(), Map.of("myObject", Map.of())); + } + + public void testIncludeParentAndExcludeChildSubFieldsArrays() { + Source fromMap = Source.fromMap( + Map.of("myArray", List.of(Map.of("myField", "myValue", "other", "otherValue"))), + XContentType.JSON + ); + Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" })); + assertEquals(filteredMap.source(), Map.of("myArray", List.of(Map.of("other", "otherValue")))); + Source fromBytes = Source.fromBytes(new BytesArray(""" + { "myArray": [ { "myField": "myValue", "other": "otherValue" } ] }"""), XContentType.JSON); + Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" })); + assertEquals(filteredBytes.source(), Map.of("myArray", List.of(Map.of("other", "otherValue")))); + } + + public void testIncludeParentAndExcludeChildSubFieldsObjects() { + Source fromMap = Source.fromMap( + Map.of("myObject", Map.of("myField", "myValue", "other", "otherValue")), + XContentType.JSON + ); + Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" })); + assertEquals(filteredMap.source(), Map.of("myObject", Map.of("other", "otherValue"))); + Source fromBytes = Source.fromBytes(new BytesArray(""" + { "myObject": { "myField": "myValue", "other": "otherValue" } }"""), XContentType.JSON); + Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" })); + assertEquals(filteredBytes.source(), Map.of("myObject", Map.of("other", "otherValue"))); + } }