Skip to content

Commit 47eb727

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

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
@@ -95,6 +95,41 @@ public TokenFilter includeProperty(String name) {
9595
return filter;
9696
}
9797

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

112112
}
113113

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

0 commit comments

Comments
 (0)