Skip to content

Commit

Permalink
Replace bespoke parser for significance heuristics (#50623) (#50659)
Browse files Browse the repository at this point in the history
This replaces the hand written xcontent parsers for significance
heristics with `ObjectParser` and parsing named xcontent.

As a happy accident, this was the last user of `ParseFieldRegistry` so
this PR entirely removes that class.

Closes #25519
  • Loading branch information
nik9000 committed Jan 6, 2020
1 parent fa57813 commit f576aef
Show file tree
Hide file tree
Showing 16 changed files with 162 additions and 297 deletions.
17 changes: 15 additions & 2 deletions server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.elasticsearch.search.aggregations.PipelineAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.significant.SignificantTerms;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser;
import org.elasticsearch.search.aggregations.pipeline.MovAvgModel;
import org.elasticsearch.search.aggregations.pipeline.MovAvgPipelineAggregator;
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
Expand All @@ -56,6 +55,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.BiFunction;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
Expand All @@ -74,7 +74,7 @@ default List<ScoreFunctionSpec<?>> getScoreFunctions() {
* The new {@link SignificanceHeuristic}s defined by this plugin. {@linkplain SignificanceHeuristic}s are used by the
* {@link SignificantTerms} aggregation to pick which terms are significant for a given query.
*/
default List<SearchExtensionSpec<SignificanceHeuristic, SignificanceHeuristicParser>> getSignificanceHeuristics() {
default List<SignificanceHeuristicSpec<?>> getSignificanceHeuristics() {
return emptyList();
}
/**
Expand Down Expand Up @@ -146,6 +146,19 @@ public ScoreFunctionSpec(String name, Writeable.Reader<T> reader, ScoreFunctionP
}
}

/**
* Specification of custom {@link SignificanceHeuristic}.
*/
class SignificanceHeuristicSpec<T extends SignificanceHeuristic> extends SearchExtensionSpec<T, BiFunction<XContentParser, Void, T>> {
public SignificanceHeuristicSpec(ParseField name, Writeable.Reader<T> reader, BiFunction<XContentParser, Void, T> parser) {
super(name, reader, parser);
}

public SignificanceHeuristicSpec(String name, Writeable.Reader<T> reader, BiFunction<XContentParser, Void, T> parser) {
super(name, reader, parser);
}
}

/**
* Specification for a {@link Suggester}.
*/
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/script/Script.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.AbstractObjectParser;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ObjectParser;
Expand All @@ -47,6 +48,9 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.BiConsumer;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;

/**
* {@link Script} represents used-defined input that can be used to
Expand Down Expand Up @@ -268,6 +272,16 @@ private Script build(String defaultLang) {
PARSER.declareField(Builder::setParams, XContentParser::map, PARAMS_PARSE_FIELD, ValueType.OBJECT);
}

/**
* Declare a script field on an {@link ObjectParser}.
* @param <T> Whatever type the {@linkplain ObjectParser} is parsing.
* @param parser the parser itself
* @param consumer the consumer for the script
*/
public static <T> void declareScript(AbstractObjectParser<T, ?> parser, BiConsumer<T, Script> consumer) {
parser.declareField(constructorArg(), (p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ValueType.OBJECT_OR_STRING);
}

/**
* Convenience method to call {@link Script#parse(XContentParser, String)}
* using the default scripting language.
Expand Down
43 changes: 18 additions & 25 deletions server/src/main/java/org/elasticsearch/search/SearchModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.elasticsearch.index.query.SpanFirstQueryBuilder;
import org.elasticsearch.index.query.SpanMultiTermQueryBuilder;
import org.elasticsearch.index.query.SpanNearQueryBuilder;
import org.elasticsearch.index.query.SpanNearQueryBuilder.SpanGapQueryBuilder;
import org.elasticsearch.index.query.SpanNotQueryBuilder;
import org.elasticsearch.index.query.SpanOrQueryBuilder;
import org.elasticsearch.index.query.SpanTermQueryBuilder;
Expand Down Expand Up @@ -97,6 +98,7 @@
import org.elasticsearch.plugins.SearchPlugin.ScoreFunctionSpec;
import org.elasticsearch.plugins.SearchPlugin.SearchExtSpec;
import org.elasticsearch.plugins.SearchPlugin.SearchExtensionSpec;
import org.elasticsearch.plugins.SearchPlugin.SignificanceHeuristicSpec;
import org.elasticsearch.plugins.SearchPlugin.SuggesterSpec;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.AggregatorFactories;
Expand All @@ -112,8 +114,8 @@
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilters;
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.geogrid.InternalGeoHashGrid;
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.geogrid.InternalGeoHashGrid;
import org.elasticsearch.search.aggregations.bucket.geogrid.InternalGeoTileGrid;
import org.elasticsearch.search.aggregations.bucket.global.GlobalAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.global.InternalGlobal;
Expand Down Expand Up @@ -153,7 +155,6 @@
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.PercentageScore;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.ScriptHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser;
import org.elasticsearch.search.aggregations.bucket.terms.DoubleTerms;
import org.elasticsearch.search.aggregations.bucket.terms.LongRareTerms;
import org.elasticsearch.search.aggregations.bucket.terms.LongTerms;
Expand Down Expand Up @@ -283,11 +284,10 @@
import java.util.function.Consumer;
import java.util.function.Function;

import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;
import static java.util.Objects.requireNonNull;
import static org.elasticsearch.index.query.CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG;
import static org.elasticsearch.index.query.SpanNearQueryBuilder.SpanGapQueryBuilder;
import static java.util.Collections.unmodifiableList;

/**
* Sets up things that can be done at search time like queries, aggregations, and suggesters.
Expand All @@ -298,8 +298,6 @@ public class SearchModule {

private final boolean transportClient;
private final Map<String, Highlighter> highlighters;
private final ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry = new ParseFieldRegistry<>(
"significance_heuristic");
private final ParseFieldRegistry<MovAvgModel.AbstractModelParser> movingAverageModelParserRegistry = new ParseFieldRegistry<>(
"moving_avg_model");

Expand Down Expand Up @@ -354,13 +352,6 @@ public Map<String, Highlighter> getHighlighters() {
return highlighters;
}

/**
* The registry of {@link SignificanceHeuristic}s.
*/
public ParseFieldRegistry<SignificanceHeuristicParser> getSignificanceHeuristicParserRegistry() {
return significanceHeuristicParserRegistry;
}

/**
* The registry of {@link MovAvgModel}s.
*/
Expand Down Expand Up @@ -427,12 +418,12 @@ private void registerAggregations(List<SearchPlugin> plugins) {
.addResultReader(UnmappedRareTerms.NAME, UnmappedRareTerms::new)
.addResultReader(LongRareTerms.NAME, LongRareTerms::new));
registerAggregation(new AggregationSpec(SignificantTermsAggregationBuilder.NAME, SignificantTermsAggregationBuilder::new,
SignificantTermsAggregationBuilder.getParser(significanceHeuristicParserRegistry))
SignificantTermsAggregationBuilder::parse)
.addResultReader(SignificantStringTerms.NAME, SignificantStringTerms::new)
.addResultReader(SignificantLongTerms.NAME, SignificantLongTerms::new)
.addResultReader(UnmappedSignificantTerms.NAME, UnmappedSignificantTerms::new));
registerAggregation(new AggregationSpec(SignificantTextAggregationBuilder.NAME, SignificantTextAggregationBuilder::new,
SignificantTextAggregationBuilder.getParser(significanceHeuristicParserRegistry)));
SignificantTextAggregationBuilder::parse));
registerAggregation(new AggregationSpec(RangeAggregationBuilder.NAME, RangeAggregationBuilder::new,
RangeAggregationBuilder::parse).addResultReader(InternalRange::new));
registerAggregation(new AggregationSpec(DateRangeAggregationBuilder.NAME, DateRangeAggregationBuilder::new,
Expand Down Expand Up @@ -720,20 +711,22 @@ private void registerValueFormat(String name, Writeable.Reader<? extends DocValu
}

private void registerSignificanceHeuristics(List<SearchPlugin> plugins) {
registerSignificanceHeuristic(new SearchExtensionSpec<>(ChiSquare.NAME, ChiSquare::new, ChiSquare.PARSER));
registerSignificanceHeuristic(new SearchExtensionSpec<>(GND.NAME, GND::new, GND.PARSER));
registerSignificanceHeuristic(new SearchExtensionSpec<>(JLHScore.NAME, JLHScore::new, JLHScore::parse));
registerSignificanceHeuristic(new SearchExtensionSpec<>(MutualInformation.NAME, MutualInformation::new, MutualInformation.PARSER));
registerSignificanceHeuristic(new SearchExtensionSpec<>(PercentageScore.NAME, PercentageScore::new, PercentageScore::parse));
registerSignificanceHeuristic(new SearchExtensionSpec<>(ScriptHeuristic.NAME, ScriptHeuristic::new, ScriptHeuristic::parse));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(ChiSquare.NAME, ChiSquare::new, ChiSquare.PARSER));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(GND.NAME, GND::new, GND.PARSER));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(JLHScore.NAME, JLHScore::new, JLHScore.PARSER));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(
MutualInformation.NAME, MutualInformation::new, MutualInformation.PARSER));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(PercentageScore.NAME, PercentageScore::new, PercentageScore.PARSER));
registerSignificanceHeuristic(new SignificanceHeuristicSpec<>(ScriptHeuristic.NAME, ScriptHeuristic::new, ScriptHeuristic.PARSER));

registerFromPlugin(plugins, SearchPlugin::getSignificanceHeuristics, this::registerSignificanceHeuristic);
}

private void registerSignificanceHeuristic(SearchExtensionSpec<SignificanceHeuristic, SignificanceHeuristicParser> heuristic) {
significanceHeuristicParserRegistry.register(heuristic.getParser(), heuristic.getName());
namedWriteables.add(new NamedWriteableRegistry.Entry(SignificanceHeuristic.class, heuristic.getName().getPreferredName(),
heuristic.getReader()));
private <T extends SignificanceHeuristic> void registerSignificanceHeuristic(SignificanceHeuristicSpec<?> spec) {
namedXContents.add(new NamedXContentRegistry.Entry(
SignificanceHeuristic.class, spec.getName(), p -> spec.getParser().apply(p, null)));
namedWriteables.add(new NamedWriteableRegistry.Entry(
SignificanceHeuristic.class, spec.getName().getPreferredName(), spec.getReader()));
}

private void registerMovingAverageModels(List<SearchPlugin> plugins) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,27 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ParseFieldRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.bucket.MultiBucketAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.JLHScore;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser;
import org.elasticsearch.search.aggregations.bucket.terms.IncludeExclude;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator.BucketCountThresholds;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;

import java.io.IOException;
import java.util.Map;
Expand All @@ -65,48 +62,36 @@ public class SignificantTermsAggregationBuilder extends ValuesSourceAggregationB
3, 0, 10, -1);
static final SignificanceHeuristic DEFAULT_SIGNIFICANCE_HEURISTIC = new JLHScore();

public static Aggregator.Parser getParser(ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry) {
ObjectParser<SignificantTermsAggregationBuilder, Void> aggregationParser =
new ObjectParser<>(SignificantTermsAggregationBuilder.NAME);
ValuesSourceParserHelper.declareAnyFields(aggregationParser, true, true);
private static final ObjectParser<SignificantTermsAggregationBuilder, Void> PARSER = new ObjectParser<>(
SignificantTermsAggregationBuilder.NAME,
SignificanceHeuristic.class, SignificantTermsAggregationBuilder::significanceHeuristic, null);
static {
ValuesSourceParserHelper.declareAnyFields(PARSER, true, true);

aggregationParser.declareInt(SignificantTermsAggregationBuilder::shardSize, TermsAggregationBuilder.SHARD_SIZE_FIELD_NAME);
PARSER.declareInt(SignificantTermsAggregationBuilder::shardSize, TermsAggregationBuilder.SHARD_SIZE_FIELD_NAME);

aggregationParser.declareLong(SignificantTermsAggregationBuilder::minDocCount, TermsAggregationBuilder.MIN_DOC_COUNT_FIELD_NAME);
PARSER.declareLong(SignificantTermsAggregationBuilder::minDocCount, TermsAggregationBuilder.MIN_DOC_COUNT_FIELD_NAME);

aggregationParser.declareLong(SignificantTermsAggregationBuilder::shardMinDocCount,
PARSER.declareLong(SignificantTermsAggregationBuilder::shardMinDocCount,
TermsAggregationBuilder.SHARD_MIN_DOC_COUNT_FIELD_NAME);

aggregationParser.declareInt(SignificantTermsAggregationBuilder::size, TermsAggregationBuilder.REQUIRED_SIZE_FIELD_NAME);
PARSER.declareInt(SignificantTermsAggregationBuilder::size, TermsAggregationBuilder.REQUIRED_SIZE_FIELD_NAME);

aggregationParser.declareString(SignificantTermsAggregationBuilder::executionHint,
PARSER.declareString(SignificantTermsAggregationBuilder::executionHint,
TermsAggregationBuilder.EXECUTION_HINT_FIELD_NAME);

aggregationParser.declareObject(SignificantTermsAggregationBuilder::backgroundFilter,
PARSER.declareObject(SignificantTermsAggregationBuilder::backgroundFilter,
(p, context) -> parseInnerQueryBuilder(p),
SignificantTermsAggregationBuilder.BACKGROUND_FILTER);

aggregationParser.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(v, b.includeExclude())),
PARSER.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(v, b.includeExclude())),
IncludeExclude::parseInclude, IncludeExclude.INCLUDE_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);

aggregationParser.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(b.includeExclude(), v)),
PARSER.declareField((b, v) -> b.includeExclude(IncludeExclude.merge(b.includeExclude(), v)),
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.STRING_ARRAY);

for (String name : significanceHeuristicParserRegistry.getNames()) {
aggregationParser.declareObject(SignificantTermsAggregationBuilder::significanceHeuristic,
(p, context) -> {
SignificanceHeuristicParser significanceHeuristicParser = significanceHeuristicParserRegistry
.lookupReturningNullIfNotFound(name, p.getDeprecationHandler());
return significanceHeuristicParser.parse(p);
},
new ParseField(name));
}
return new Aggregator.Parser() {
@Override
public AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
return aggregationParser.parse(parser, new SignificantTermsAggregationBuilder(aggregationName, null), null);
}
};
}
public static SignificantTermsAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
return PARSER.parse(parser, new SignificantTermsAggregationBuilder(aggregationName, null), null);
}

private IncludeExclude includeExclude = null;
Expand Down

0 comments on commit f576aef

Please sign in to comment.