Skip to content
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

Switch AggregationSpec to ContextParser #50871

Merged
merged 2 commits into from
Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.lucene.search.function.ScoreFunction;
import org.elasticsearch.common.xcontent.ContextParser;
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryBuilder;
Expand Down Expand Up @@ -248,7 +249,7 @@ public QuerySpec(String name, Writeable.Reader<T> reader, QueryParser<T> parser)
/**
* Specification for an {@link Aggregation}.
*/
class AggregationSpec extends SearchExtensionSpec<AggregationBuilder, Aggregator.Parser> {
class AggregationSpec extends SearchExtensionSpec<AggregationBuilder, ContextParser<String, ? extends AggregationBuilder>> {
private final Map<String, Writeable.Reader<? extends InternalAggregation>> resultReaders = new TreeMap<>();

/**
Expand All @@ -261,7 +262,8 @@ class AggregationSpec extends SearchExtensionSpec<AggregationBuilder, Aggregator
* {@link StreamInput}
* @param parser the parser the reads the aggregation builder from xcontent
*/
public AggregationSpec(ParseField name, Writeable.Reader<? extends AggregationBuilder> reader, Aggregator.Parser parser) {
public <T extends AggregationBuilder> AggregationSpec(ParseField name, Writeable.Reader<T> reader,
ContextParser<String, T> parser) {
super(name, reader, parser);
}

Expand All @@ -274,10 +276,41 @@ public AggregationSpec(ParseField name, Writeable.Reader<? extends AggregationBu
* {@link StreamInput}
* @param parser the parser the reads the aggregation builder from xcontent
*/
public AggregationSpec(String name, Writeable.Reader<? extends AggregationBuilder> reader, Aggregator.Parser parser) {
public <T extends AggregationBuilder> AggregationSpec(String name, Writeable.Reader<T> reader, ContextParser<String, T> parser) {
super(name, reader, parser);
}

/**
* Specification for an {@link Aggregation}.
*
* @param name holds the names by which this aggregation might be parsed. The {@link ParseField#getPreferredName()} is special as it
* is the name by under which the reader is registered. So it is the name that the {@link AggregationBuilder} should return
* from {@link NamedWriteable#getWriteableName()}.
* @param reader the reader registered for this aggregation's builder. Typically a reference to a constructor that takes a
* {@link StreamInput}
* @param parser the parser the reads the aggregation builder from xcontent
* @deprecated Use the ctor that takes a {@link ContextParser} instead
*/
@Deprecated
public AggregationSpec(ParseField name, Writeable.Reader<? extends AggregationBuilder> reader, Aggregator.Parser parser) {
super(name, reader, (p, aggName) -> parser.parse(aggName, p));
}

/**
* Specification for an {@link Aggregation}.
*
* @param name the name by which this aggregation might be parsed or deserialized. Make sure that the {@link AggregationBuilder}
* returns this from {@link NamedWriteable#getWriteableName()}.
* @param reader the reader registered for this aggregation's builder. Typically a reference to a constructor that takes a
* {@link StreamInput}
* @param parser the parser the reads the aggregation builder from xcontent
* @deprecated Use the ctor that takes a {@link ContextParser} instead
*/
@Deprecated
public AggregationSpec(String name, Writeable.Reader<? extends AggregationBuilder> reader, Aggregator.Parser parser) {
super(name, reader, (p, aggName) -> parser.parse(aggName, p));
}

/**
* Add a reader for the shard level results of the aggregation with {@linkplain #getName}'s {@link ParseField#getPreferredName()} as
* the {@link NamedWriteable#getWriteableName()}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,16 +422,16 @@ private void registerAggregations(List<SearchPlugin> plugins) {
registerAggregation(new AggregationSpec(GeoCentroidAggregationBuilder.NAME, GeoCentroidAggregationBuilder::new,
GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new));
registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new,
(name, p) -> ScriptedMetricAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalScriptedMetric::new));
ScriptedMetricAggregationBuilder.PARSER).addResultReader(InternalScriptedMetric::new));
registerAggregation((new AggregationSpec(CompositeAggregationBuilder.NAME, CompositeAggregationBuilder::new,
(name, p) -> CompositeAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalComposite::new)));
CompositeAggregationBuilder.PARSER).addResultReader(InternalComposite::new)));
registerFromPlugin(plugins, SearchPlugin::getAggregations, this::registerAggregation);
}

private void registerAggregation(AggregationSpec spec) {
namedXContents.add(new NamedXContentRegistry.Entry(BaseAggregationBuilder.class, spec.getName(), (p, c) -> {
String name = (String) c;
return spec.getParser().parse(name, p);
return spec.getParser().parse(p, name);
}));
namedWriteables.add(
new NamedWriteableRegistry.Entry(AggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
Expand Down