-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Fix and optimize "Add XContentFieldFilter (#81970)" #83152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
romseygeek
merged 13 commits into
elastic:master
from
mushao999:fix_Add_XContentFieldFilter
Feb 2, 2022
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
cbcfe0e
Add XContentFieldFilter (#81970)
mushao999 58f5615
fix XContentFieldFilter bug when source is filtered to be an empty ob…
mushao999 56983ff
FilterPathBasedFilter support dot in field name
mushao999 e5a322a
XContentFieldFilter support source to be null or empty
mushao999 37f19a5
revert <FilterPathBasedFilter support dot in field name> and move it …
mushao999 fd054d3
fix XContentFieldFilterTests typo
mushao999 3b244ed
Merge branch 'master' into fix_Add_XContentFieldFilter
mushao999 3789d76
update AwaitsFix bugUrl
mushao999 2cf2266
Merge branch 'master' into fix_Add_XContentFieldFilter
mushao999 77c0c9a
update after #83178 is merged
mushao999 5edf685
update readability
mushao999 df9cbc4
update
mushao999 6a8f3a3
Merge branch 'master' into fix_Add_XContentFieldFilter
mushao999 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
101 changes: 101 additions & 0 deletions
101
server/src/main/java/org/elasticsearch/common/xcontent/XContentFieldFilter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
package org.elasticsearch.common.xcontent; | ||
|
||
import org.elasticsearch.common.bytes.BytesReference; | ||
import org.elasticsearch.common.io.stream.BytesStreamOutput; | ||
import org.elasticsearch.common.util.CollectionUtils; | ||
import org.elasticsearch.common.xcontent.support.XContentMapValues; | ||
import org.elasticsearch.core.CheckedFunction; | ||
import org.elasticsearch.core.Nullable; | ||
import org.elasticsearch.core.Tuple; | ||
import org.elasticsearch.xcontent.XContentBuilder; | ||
import org.elasticsearch.xcontent.XContentFactory; | ||
import org.elasticsearch.xcontent.XContentParser; | ||
import org.elasticsearch.xcontent.XContentParserConfiguration; | ||
import org.elasticsearch.xcontent.XContentType; | ||
|
||
import java.io.IOException; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.function.Function; | ||
|
||
/** | ||
* A filter that filter fields away from source | ||
*/ | ||
public interface XContentFieldFilter { | ||
/** | ||
* filter source in {@link BytesReference} format and in {@link XContentType} content type | ||
* note that xContentType may be null in some case, we should guess xContentType from sourceBytes in such cases | ||
*/ | ||
BytesReference apply(BytesReference sourceBytes, @Nullable XContentType xContentType) throws IOException; | ||
|
||
/** | ||
* Construct {@link XContentFieldFilter} using given includes and excludes | ||
* | ||
* @param includes fields to keep, wildcard supported | ||
* @param excludes fields to remove, wildcard supported | ||
* @return filter that filter {@link org.elasticsearch.xcontent.XContent} with given includes and excludes | ||
*/ | ||
static XContentFieldFilter newFieldFilter(String[] includes, String[] excludes) { | ||
final CheckedFunction<XContentType, BytesReference, IOException> emptyValueSupplier = xContentType -> { | ||
BytesStreamOutput bStream = new BytesStreamOutput(); | ||
XContentBuilder builder = XContentFactory.contentBuilder(xContentType, bStream).map(Collections.emptyMap()); | ||
builder.close(); | ||
return bStream.bytes(); | ||
}; | ||
// Use the old map-based filtering mechanism if there are wildcards in the excludes. | ||
// TODO: Remove this if block once: https://github.com/elastic/elasticsearch/pull/80160 is merged | ||
if ((CollectionUtils.isEmpty(excludes) == false) && Arrays.stream(excludes).filter(field -> field.contains("*")).count() > 0) { | ||
return (originalSource, contentType) -> { | ||
if (originalSource == null || originalSource.length() <= 0) { | ||
if (contentType == null) { | ||
throw new IllegalStateException("originalSource and contentType can not be null at the same time"); | ||
} | ||
return emptyValueSupplier.apply(contentType); | ||
} | ||
Function<Map<String, ?>, Map<String, Object>> mapFilter = XContentMapValues.filter(includes, excludes); | ||
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(originalSource, true, contentType); | ||
Map<String, Object> filteredSource = mapFilter.apply(mapTuple.v2()); | ||
BytesStreamOutput bStream = new BytesStreamOutput(); | ||
XContentType actualContentType = mapTuple.v1(); | ||
XContentBuilder builder = XContentFactory.contentBuilder(actualContentType, bStream).map(filteredSource); | ||
builder.close(); | ||
return bStream.bytes(); | ||
}; | ||
} else { | ||
final XContentParserConfiguration parserConfig = XContentParserConfiguration.EMPTY.withFiltering( | ||
Set.of(includes), | ||
Set.of(excludes), | ||
true | ||
); | ||
return (originalSource, contentType) -> { | ||
if (originalSource == null || originalSource.length() <= 0) { | ||
if (contentType == null) { | ||
throw new IllegalStateException("originalSource and contentType can not be null at the same time"); | ||
} | ||
return emptyValueSupplier.apply(contentType); | ||
} | ||
if (contentType == null) { | ||
contentType = XContentHelper.xContentTypeMayCompressed(originalSource); | ||
} | ||
BytesStreamOutput streamOutput = new BytesStreamOutput(Math.min(1024, originalSource.length())); | ||
XContentBuilder builder = new XContentBuilder(contentType.xContent(), streamOutput); | ||
XContentParser parser = contentType.xContent().createParser(parserConfig, originalSource.streamInput()); | ||
if ((parser.currentToken() == null) && (parser.nextToken() == null)) { | ||
return emptyValueSupplier.apply(contentType); | ||
} | ||
builder.copyCurrentStructure(parser); | ||
return BytesReference.bytes(builder); | ||
}; | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,8 @@ | |
import org.elasticsearch.common.metrics.CounterMetric; | ||
import org.elasticsearch.common.metrics.MeanMetric; | ||
import org.elasticsearch.common.util.set.Sets; | ||
import org.elasticsearch.common.xcontent.XContentHelper; | ||
import org.elasticsearch.common.xcontent.support.XContentMapValues; | ||
import org.elasticsearch.common.xcontent.XContentFieldFilter; | ||
import org.elasticsearch.core.Nullable; | ||
import org.elasticsearch.core.Tuple; | ||
import org.elasticsearch.index.IndexSettings; | ||
import org.elasticsearch.index.VersionType; | ||
import org.elasticsearch.index.engine.Engine; | ||
|
@@ -33,8 +31,6 @@ | |
import org.elasticsearch.index.shard.AbstractIndexShardComponent; | ||
import org.elasticsearch.index.shard.IndexShard; | ||
import org.elasticsearch.search.fetch.subphase.FetchSourceContext; | ||
import org.elasticsearch.xcontent.XContentFactory; | ||
import org.elasticsearch.xcontent.XContentType; | ||
|
||
import java.io.IOException; | ||
import java.util.HashMap; | ||
|
@@ -253,15 +249,11 @@ private GetResult innerGetLoadFromStoredFields( | |
if (fetchSourceContext.fetchSource() == false) { | ||
source = null; | ||
} else if (fetchSourceContext.includes().length > 0 || fetchSourceContext.excludes().length > 0) { | ||
Map<String, Object> sourceAsMap; | ||
// TODO: The source might be parsed and available in the sourceLookup but that one uses unordered maps so different. | ||
// Do we care? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nik9000 @romseygeek is this TODO still valid here? It seems like the code around it changed quite a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Tuple<XContentType, Map<String, Object>> typeMapTuple = XContentHelper.convertToMap(source, true); | ||
XContentType sourceContentType = typeMapTuple.v1(); | ||
sourceAsMap = typeMapTuple.v2(); | ||
sourceAsMap = XContentMapValues.filter(sourceAsMap, fetchSourceContext.includes(), fetchSourceContext.excludes()); | ||
try { | ||
source = BytesReference.bytes(XContentFactory.contentBuilder(sourceContentType).map(sourceAsMap)); | ||
source = XContentFieldFilter.newFieldFilter(fetchSourceContext.includes(), fetchSourceContext.excludes()) | ||
.apply(source, null); | ||
} catch (IOException e) { | ||
throw new ElasticsearchException("Failed to get id [" + id + "] with includes/excludes set", e); | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.