Switch AggregationSpec to ContextParser (#50871) (#50980)

We seem to have settled on the `ContextParser` interface for parsing
stuff, mostly because `ObjectParser` implements it. We don't really
*need* the old `Aggregator.Parser` interface any more because it
duplicates `ContextParser` but with the arguments reversed.

This adds support to `AggregationSpec` to declare aggregation parsers
using `ContextParser`. This should integrate cleanly with
`ObjectParser`. It doesn't drop support for `Aggregator.Parser` or
change the plugin intrface at all so it *should* be safe to backport to
7.x. And we can remove `Aggregator.Parser` in a follow up which is only
targeted to 8.0.
This commit is contained in:
Nik Everett 2020-01-14 16:50:52 -05:00 committed by GitHub
parent 9a3d4db840
commit a8aca6b2a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 6 deletions

View File

@ -26,6 +26,7 @@ import org.elasticsearch.common.io.stream.NamedWriteable;
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;
@ -256,7 +257,7 @@ public interface SearchPlugin {
/**
* 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<>();
/**
@ -269,7 +270,8 @@ public interface SearchPlugin {
* {@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);
}
@ -282,10 +284,41 @@ public interface SearchPlugin {
* {@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()}.

View File

@ -452,9 +452,9 @@ public class SearchModule {
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);
}
@ -462,7 +462,7 @@ public class SearchModule {
if (false == transportClient) {
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(