Skip to content

Commit 26e8a76

Browse files
Do not exclude empty arrays or empty objects in source filtering with Jackson streaming (#112250) (#115223)
(cherry picked from commit 6be3036) Co-authored-by: mccheah <[email protected]>
1 parent a312404 commit 26e8a76

File tree

3 files changed

+84
-0
lines changed

3 files changed

+84
-0
lines changed

docs/changelog/112250.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 112250
2+
summary: Do not exclude empty arrays or empty objects in source filtering
3+
area: Search
4+
type: bug
5+
issues: [109668]

libs/x-content/impl/src/main/java/org/elasticsearch/xcontent/provider/filtering/FilterPathBasedFilter.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,41 @@ public TokenFilter includeProperty(String name) {
9696
return filter;
9797
}
9898

99+
/**
100+
* This is overridden in order to keep empty arrays in nested exclusions - see #109668.
101+
* <p>
102+
* If we are excluding contents, we only want to exclude based on property name - but empty arrays in themselves do not have a property
103+
* name. If the empty array were to be excluded, it should be done by excluding the parent.
104+
* <p>
105+
* Note though that the expected behavior seems to be ambiguous if contentsFiltered is true - that is, that the filter has pruned all
106+
* 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
107+
* writing, not doing so would cause assertions in JsonXContentFilteringTests to fail, which expect this behavior. Yet it is not obvious
108+
* if dropping the empty array in this case is correct. For example, one could expect this sort of behavior:
109+
* <ul>
110+
* <li>Document: <pre>{ "myArray": [ { "myField": "myValue" } ]}</pre></li>
111+
* <li>Filter: <pre>{ "exclude": "myArray.myField" }</pre></li>
112+
* </ul>
113+
* From the user's perspective, this could reasonably yield either of:
114+
* <ol>
115+
* <li><pre>{ "myArray": []}</pre></li>
116+
* <li>Removing {@code myArray} entirely.</li>
117+
* </ol>
118+
*/
119+
@Override
120+
public boolean includeEmptyArray(boolean contentsFiltered) {
121+
return inclusive == false && contentsFiltered == false;
122+
}
123+
124+
/**
125+
* This is overridden in order to keep empty objects in nested exclusions - see #109668.
126+
* <p>
127+
* The same logic applies to this as to {@link #includeEmptyArray(boolean)}, only for nested objects instead of nested arrays.
128+
*/
129+
@Override
130+
public boolean includeEmptyObject(boolean contentsFiltered) {
131+
return inclusive == false && contentsFiltered == false;
132+
}
133+
99134
@Override
100135
protected boolean _includeScalar() {
101136
return inclusive == false;

server/src/test/java/org/elasticsearch/search/lookup/SourceFilterTests.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,48 @@ public Source filter(SourceFilter sourceFilter) {
112112

113113
}
114114

115+
// Verification for issue #109668
116+
public void testIncludeParentAndExcludeChildEmptyArray() {
117+
Source fromMap = Source.fromMap(Map.of("myArray", List.of()), XContentType.JSON);
118+
Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" }));
119+
assertEquals(filteredMap.source(), Map.of("myArray", List.of()));
120+
Source fromBytes = Source.fromBytes(new BytesArray("{\"myArray\": []}"), XContentType.JSON);
121+
Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" }));
122+
assertEquals(filteredBytes.source(), Map.of("myArray", List.of()));
123+
}
124+
125+
public void testIncludeParentAndExcludeChildEmptyObject() {
126+
Source fromMap = Source.fromMap(Map.of("myObject", Map.of()), XContentType.JSON);
127+
Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" }));
128+
assertEquals(filteredMap.source(), Map.of("myObject", Map.of()));
129+
Source fromBytes = Source.fromBytes(new BytesArray("{\"myObject\": {}}"), XContentType.JSON);
130+
Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" }));
131+
assertEquals(filteredBytes.source(), Map.of("myObject", Map.of()));
132+
}
133+
134+
public void testIncludeParentAndExcludeChildSubFieldsArrays() {
135+
Source fromMap = Source.fromMap(
136+
Map.of("myArray", List.of(Map.<String, Object>of("myField", "myValue", "other", "otherValue"))),
137+
XContentType.JSON
138+
);
139+
Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" }));
140+
assertEquals(filteredMap.source(), Map.of("myArray", List.of(Map.of("other", "otherValue"))));
141+
Source fromBytes = Source.fromBytes(new BytesArray("""
142+
{ "myArray": [ { "myField": "myValue", "other": "otherValue" } ] }"""), XContentType.JSON);
143+
Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myArray" }, new String[] { "myArray.myField" }));
144+
assertEquals(filteredBytes.source(), Map.of("myArray", List.of(Map.of("other", "otherValue"))));
145+
}
146+
147+
public void testIncludeParentAndExcludeChildSubFieldsObjects() {
148+
Source fromMap = Source.fromMap(
149+
Map.of("myObject", Map.<String, Object>of("myField", "myValue", "other", "otherValue")),
150+
XContentType.JSON
151+
);
152+
Source filteredMap = fromMap.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" }));
153+
assertEquals(filteredMap.source(), Map.of("myObject", Map.of("other", "otherValue")));
154+
Source fromBytes = Source.fromBytes(new BytesArray("""
155+
{ "myObject": { "myField": "myValue", "other": "otherValue" } }"""), XContentType.JSON);
156+
Source filteredBytes = fromBytes.filter(new SourceFilter(new String[] { "myObject" }, new String[] { "myObject.myField" }));
157+
assertEquals(filteredBytes.source(), Map.of("myObject", Map.of("other", "otherValue")));
158+
}
115159
}

0 commit comments

Comments
 (0)