Skip to content

Commit

Permalink
Do not exclude empty arrays or empty objects in source filtering with…
Browse files Browse the repository at this point in the history
… Jackson streaming (elastic#112250)

(cherry picked from commit 6be3036)
  • Loading branch information
mccheah committed Oct 21, 2024
1 parent 50e63ed commit 32e713b
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/112250.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 112250
summary: Do not exclude empty arrays or empty objects in source filtering
area: Search
type: bug
issues: [109668]
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <p>
* 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.
* <p>
* 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:
* <ul>
* <li>Document: <pre>{ "myArray": [ { "myField": "myValue" } ]}</pre></li>
* <li>Filter: <pre>{ "exclude": "myArray.myField" }</pre></li>
* </ul>
* From the user's perspective, this could reasonably yield either of:
* <ol>
* <li><pre>{ "myArray": []}</pre></li>
* <li>Removing {@code myArray} entirely.</li>
* </ol>
*/
@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.
* <p>
* 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.<String, Object>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.<String, Object>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")));
}
}

0 comments on commit 32e713b

Please sign in to comment.