Browse Source

Normalize registration for SignificanceHeuristics

When I pulled on the thread that is "Remove PROTOTYPEs from
SignificanceHeuristics" I ended up removing SignificanceHeuristicStreams
and replacing it with readNamedWriteable. That seems like a lot at once
but it made sense at the time. And it is what we want in the end, I think.

Anyway, this also converts registration of SignificanceHeuristics to
use ParseFieldRegistry to make them consistent with Queries, Aggregations
and lots of other stuff.

Adds a new and wonderous hack to support serialization checking of
NamedWriteables registered by plugins!

Related to #17085
Nik Everett 9 years ago
parent
commit
65f6f6bc8d
20 changed files with 269 additions and 408 deletions
  1. 39 8
      core/src/main/java/org/elasticsearch/search/SearchModule.java
  2. 2 3
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
  3. 2 4
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
  4. 4 4
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorBuilder.java
  5. 6 5
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java
  6. 2 2
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
  7. 10 17
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ChiSquare.java
  8. 14 23
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/GND.java
  9. 32 27
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/JLHScore.java
  10. 11 17
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/MutualInformation.java
  11. 10 1
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/NXYSignificanceHeuristic.java
  12. 25 25
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/PercentageScore.java
  13. 44 59
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java
  14. 4 3
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicParser.java
  15. 0 58
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicParserMapper.java
  16. 0 93
      core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java
  17. 27 34
      core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java
  18. 2 2
      core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsTests.java
  19. 21 21
      core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificanceHeuristicTests.java
  20. 14 2
      test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java

+ 39 - 8
core/src/main/java/org/elasticsearch/search/SearchModule.java

@@ -146,8 +146,14 @@ import org.elasticsearch.search.aggregations.bucket.significant.SignificantStrin
 import org.elasticsearch.search.aggregations.bucket.significant.SignificantTermsAggregatorBuilder;
 import org.elasticsearch.search.aggregations.bucket.significant.SignificantTermsParser;
 import org.elasticsearch.search.aggregations.bucket.significant.UnmappedSignificantTerms;
+import org.elasticsearch.search.aggregations.bucket.significant.heuristics.ChiSquare;
+import org.elasticsearch.search.aggregations.bucket.significant.heuristics.GND;
+import org.elasticsearch.search.aggregations.bucket.significant.heuristics.JLHScore;
+import org.elasticsearch.search.aggregations.bucket.significant.heuristics.MutualInformation;
+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.significant.heuristics.SignificanceHeuristicParserMapper;
 import org.elasticsearch.search.aggregations.bucket.terms.DoubleTerms;
 import org.elasticsearch.search.aggregations.bucket.terms.LongTerms;
 import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
@@ -273,9 +279,10 @@ public class SearchModule extends AbstractModule {
     private final ParseFieldRegistry<PipelineAggregator.Parser> pipelineAggregationParserRegistry = new ParseFieldRegistry<>(
             "pipline_aggregation");
     private final AggregatorParsers aggregatorParsers = new AggregatorParsers(aggregationParserRegistry, pipelineAggregationParserRegistry);
+    private final ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry = new ParseFieldRegistry<>(
+            "significance_heuristic");
 
     private final Set<Class<? extends FetchSubPhase>> fetchSubPhases = new HashSet<>();
-    private final Set<SignificanceHeuristicParser> heuristicParsers = new HashSet<>();
     private final Set<MovAvgModel.AbstractModelParser> modelParsers = new HashSet<>();
 
     private final Settings settings;
@@ -294,6 +301,7 @@ public class SearchModule extends AbstractModule {
         registerBuiltinRescorers();
         registerBuiltinSorts();
         registerBuiltinValueFormats();
+        registerBuiltinSignificanceHeuristics();
     }
 
     public void registerHighlighter(String key, Class<? extends Highlighter> clazz) {
@@ -359,8 +367,25 @@ public class SearchModule extends AbstractModule {
         fetchSubPhases.add(subPhase);
     }
 
-    public void registerHeuristicParser(SignificanceHeuristicParser parser) {
-        heuristicParsers.add(parser);
+    /**
+     * Register a {@link SignificanceHeuristic}.
+     * 
+     * @param heuristicName the name(s) at which the heuristic is parsed and streamed. The {@link ParseField#getPreferredName()} is the name
+     *        under which it is streamed. All names work for the parser.
+     * @param reader reads the heuristic from a stream
+     * @param parser reads the heuristic from a XContentParser
+     */
+    public void registerSignificanceHeuristic(ParseField heuristicName, Writeable.Reader<SignificanceHeuristic> reader,
+            SignificanceHeuristicParser parser) {
+        significanceHeuristicParserRegistry.register(parser, heuristicName);
+        namedWriteableRegistry.register(SignificanceHeuristic.class, heuristicName.getPreferredName(), reader);
+    }
+
+    /**
+     * The registry of {@link SignificanceHeuristic}s.
+     */
+    public ParseFieldRegistry<SignificanceHeuristicParser> getSignificanceHeuristicParserRegistry() {
+        return significanceHeuristicParserRegistry;
     }
 
     public void registerModelParser(MovAvgModel.AbstractModelParser parser) {
@@ -432,11 +457,8 @@ public class SearchModule extends AbstractModule {
     }
 
     protected void configureAggs() {
-
         MovAvgModelParserMapper movAvgModelParserMapper = new MovAvgModelParserMapper(modelParsers);
 
-        SignificanceHeuristicParserMapper significanceHeuristicParserMapper = new SignificanceHeuristicParserMapper(heuristicParsers);
-
         registerAggregation(AvgAggregatorBuilder::new, new AvgParser(), AvgAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(SumAggregatorBuilder::new, new SumParser(), SumAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(MinAggregatorBuilder::new, new MinParser(), MinAggregatorBuilder.AGGREGATION_NAME_FIELD);
@@ -462,7 +484,7 @@ public class SearchModule extends AbstractModule {
                 DiversifiedAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(TermsAggregatorBuilder::new, new TermsParser(), TermsAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(SignificantTermsAggregatorBuilder::new,
-                new SignificantTermsParser(significanceHeuristicParserMapper, queryParserRegistry),
+                new SignificantTermsParser(significanceHeuristicParserRegistry, queryParserRegistry),
                 SignificantTermsAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(RangeAggregatorBuilder::new, new RangeParser(), RangeAggregatorBuilder.AGGREGATION_NAME_FIELD);
         registerAggregation(DateRangeAggregatorBuilder::new, new DateRangeParser(), DateRangeAggregatorBuilder.AGGREGATION_NAME_FIELD);
@@ -581,6 +603,15 @@ public class SearchModule extends AbstractModule {
         registerValueFormat(DocValueFormat.RAW.getWriteableName(), in -> DocValueFormat.RAW);
     }
 
+    private void registerBuiltinSignificanceHeuristics() {
+        registerSignificanceHeuristic(ChiSquare.NAMES_FIELD, ChiSquare::new, ChiSquare.PARSER);
+        registerSignificanceHeuristic(GND.NAMES_FIELD, GND::new, GND.PARSER);
+        registerSignificanceHeuristic(JLHScore.NAMES_FIELD, JLHScore::new, JLHScore::parse);
+        registerSignificanceHeuristic(MutualInformation.NAMES_FIELD, MutualInformation::new, MutualInformation.PARSER);
+        registerSignificanceHeuristic(PercentageScore.NAMES_FIELD, PercentageScore::new, PercentageScore::parse);
+        registerSignificanceHeuristic(ScriptHeuristic.NAMES_FIELD, ScriptHeuristic::new, ScriptHeuristic::parse);
+    }
+
     private void registerBuiltinQueryParsers() {
         registerQuery(MatchQueryBuilder::new, MatchQueryBuilder::fromXContent, MatchQueryBuilder.QUERY_NAME_FIELD);
         registerQuery(MatchPhraseQueryBuilder::new, MatchPhraseQueryBuilder::fromXContent, MatchPhraseQueryBuilder.QUERY_NAME_FIELD);

+ 2 - 3
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java

@@ -27,7 +27,6 @@ import org.elasticsearch.search.aggregations.InternalAggregations;
 import org.elasticsearch.search.aggregations.bucket.BucketStreamContext;
 import org.elasticsearch.search.aggregations.bucket.BucketStreams;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
-import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicStreams;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
@@ -207,7 +206,7 @@ public class SignificantLongTerms extends InternalSignificantTerms<SignificantLo
         this.minDocCount = in.readVLong();
         this.subsetSize = in.readVLong();
         this.supersetSize = in.readVLong();
-        significanceHeuristic = SignificanceHeuristicStreams.read(in);
+        significanceHeuristic = in.readNamedWriteable(SignificanceHeuristic.class);
 
         int size = in.readVInt();
         List<InternalSignificantTerms.Bucket> buckets = new ArrayList<>(size);
@@ -228,7 +227,7 @@ public class SignificantLongTerms extends InternalSignificantTerms<SignificantLo
         out.writeVLong(minDocCount);
         out.writeVLong(subsetSize);
         out.writeVLong(supersetSize);
-        SignificanceHeuristicStreams.writeTo(significanceHeuristic, out);
+        out.writeNamedWriteable(significanceHeuristic);
         out.writeVInt(buckets.size());
         for (InternalSignificantTerms.Bucket bucket : buckets) {
 

+ 2 - 4
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java

@@ -28,12 +28,10 @@ import org.elasticsearch.search.aggregations.InternalAggregations;
 import org.elasticsearch.search.aggregations.bucket.BucketStreamContext;
 import org.elasticsearch.search.aggregations.bucket.BucketStreams;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
-import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicStreams;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -197,7 +195,7 @@ public class SignificantStringTerms extends InternalSignificantTerms<Significant
         this.minDocCount = in.readVLong();
         this.subsetSize = in.readVLong();
         this.supersetSize = in.readVLong();
-        significanceHeuristic = SignificanceHeuristicStreams.read(in);
+        significanceHeuristic = in.readNamedWriteable(SignificanceHeuristic.class);
         int size = in.readVInt();
         List<InternalSignificantTerms.Bucket> buckets = new ArrayList<>(size);
         for (int i = 0; i < size; i++) {
@@ -215,7 +213,7 @@ public class SignificantStringTerms extends InternalSignificantTerms<Significant
         out.writeVLong(minDocCount);
         out.writeVLong(subsetSize);
         out.writeVLong(supersetSize);
-        SignificanceHeuristicStreams.writeTo(significanceHeuristic, out);
+        out.writeNamedWriteable(significanceHeuristic);
         out.writeVInt(buckets.size());
         for (InternalSignificantTerms.Bucket bucket : buckets) {
             bucket.writeTo(out);

+ 4 - 4
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorBuilder.java

@@ -27,7 +27,6 @@ import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
 import org.elasticsearch.search.aggregations.AggregatorFactory;
 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.SignificanceHeuristicStreams;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator.BucketCountThresholds;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregatorBuilder;
@@ -55,12 +54,13 @@ public class SignificantTermsAggregatorBuilder extends ValuesSourceAggregatorBui
 
     static final TermsAggregator.BucketCountThresholds DEFAULT_BUCKET_COUNT_THRESHOLDS = new TermsAggregator.BucketCountThresholds(
             3, 0, 10, -1);
+    static final SignificanceHeuristic DEFAULT_SIGNIFICANCE_HEURISTIC = new JLHScore();
 
     private IncludeExclude includeExclude = null;
     private String executionHint = null;
     private QueryBuilder<?> filterBuilder = null;
     private TermsAggregator.BucketCountThresholds bucketCountThresholds = new BucketCountThresholds(DEFAULT_BUCKET_COUNT_THRESHOLDS);
-    private SignificanceHeuristic significanceHeuristic = JLHScore.PROTOTYPE;
+    private SignificanceHeuristic significanceHeuristic = DEFAULT_SIGNIFICANCE_HEURISTIC;
 
     public SignificantTermsAggregatorBuilder(String name, ValueType valueType) {
         super(name, SignificantStringTerms.TYPE, ValuesSourceType.ANY, valueType);
@@ -79,7 +79,7 @@ public class SignificantTermsAggregatorBuilder extends ValuesSourceAggregatorBui
         if (in.readBoolean()) {
             includeExclude = IncludeExclude.readFromStream(in);
         }
-        significanceHeuristic = SignificanceHeuristicStreams.read(in);
+        significanceHeuristic = in.readNamedWriteable(SignificanceHeuristic.class);
     }
 
     @Override
@@ -96,7 +96,7 @@ public class SignificantTermsAggregatorBuilder extends ValuesSourceAggregatorBui
         if (hasIncExc) {
             includeExclude.writeTo(out);
         }
-        SignificanceHeuristicStreams.writeTo(significanceHeuristic, out);
+        out.writeNamedWriteable(significanceHeuristic);
     }
 
     @Override

+ 6 - 5
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java

@@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.bucket.significant;
 
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParseFieldMatcher;
+import org.elasticsearch.common.xcontent.ParseFieldRegistry;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
 import org.elasticsearch.index.query.QueryBuilder;
@@ -28,7 +29,6 @@ import org.elasticsearch.indices.query.IndicesQueriesRegistry;
 import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser;
-import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParserMapper;
 import org.elasticsearch.search.aggregations.bucket.terms.AbstractTermsParser;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator.BucketCountThresholds;
@@ -43,12 +43,12 @@ import java.util.Map;
  *
  */
 public class SignificantTermsParser extends AbstractTermsParser {
-    private final SignificanceHeuristicParserMapper significanceHeuristicParserMapper;
+    private final ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry;
     private final IndicesQueriesRegistry queriesRegistry;
 
-    public SignificantTermsParser(SignificanceHeuristicParserMapper significanceHeuristicParserMapper,
+    public SignificantTermsParser(ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry,
             IndicesQueriesRegistry queriesRegistry) {
-        this.significanceHeuristicParserMapper = significanceHeuristicParserMapper;
+        this.significanceHeuristicParserRegistry = significanceHeuristicParserRegistry;
         this.queriesRegistry = queriesRegistry;
     }
 
@@ -81,7 +81,8 @@ public class SignificantTermsParser extends AbstractTermsParser {
     public boolean parseSpecial(String aggregationName, XContentParser parser, ParseFieldMatcher parseFieldMatcher, Token token,
             String currentFieldName, Map<ParseField, Object> otherOptions) throws IOException {
         if (token == XContentParser.Token.START_OBJECT) {
-            SignificanceHeuristicParser significanceHeuristicParser = significanceHeuristicParserMapper.get(currentFieldName);
+            SignificanceHeuristicParser significanceHeuristicParser = significanceHeuristicParserRegistry
+                    .lookupReturningNullIfNotFound(currentFieldName, parseFieldMatcher);
             if (significanceHeuristicParser != null) {
                 SignificanceHeuristic significanceHeuristic = significanceHeuristicParser.parse(parser, parseFieldMatcher);
                 otherOptions.put(SignificantTermsAggregatorBuilder.HEURISTIC, significanceHeuristic);

+ 2 - 2
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java

@@ -24,7 +24,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.aggregations.AggregationStreams;
 import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.InternalAggregations;
-import org.elasticsearch.search.aggregations.bucket.significant.heuristics.JLHScore;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
@@ -60,7 +59,8 @@ public class UnmappedSignificantTerms extends InternalSignificantTerms<UnmappedS
     public UnmappedSignificantTerms(String name, int requiredSize, long minDocCount, List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) {
         //We pass zero for index/subset sizes because for the purpose of significant term analysis
         // we assume an unmapped index's size is irrelevant to the proceedings.
-        super(0, 0, name, requiredSize, minDocCount, JLHScore.PROTOTYPE, BUCKETS, pipelineAggregators, metaData);
+        super(0, 0, name, requiredSize, minDocCount, SignificantTermsAggregatorBuilder.DEFAULT_SIGNIFICANCE_HEURISTIC, BUCKETS,
+                pipelineAggregators, metaData);
     }
 
     @Override

+ 10 - 17
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ChiSquare.java

@@ -28,15 +28,19 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import java.io.IOException;
 
 public class ChiSquare extends NXYSignificanceHeuristic {
-
-    static final ChiSquare PROTOTYPE = new ChiSquare(false, false);
-
-    protected static final ParseField NAMES_FIELD = new ParseField("chi_square");
+    public static final ParseField NAMES_FIELD = new ParseField("chi_square");
 
     public ChiSquare(boolean includeNegatives, boolean backgroundIsSuperset) {
         super(includeNegatives, backgroundIsSuperset);
     }
 
+    /**
+     * Read from a stream.
+     */
+    public ChiSquare(StreamInput in) throws IOException {
+        super(in);
+    }
+
     @Override
     public boolean equals(Object other) {
         if (!(other instanceof ChiSquare)) {
@@ -73,11 +77,6 @@ public class ChiSquare extends NXYSignificanceHeuristic {
         return NAMES_FIELD.getPreferredName();
     }
 
-    @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        return new ChiSquare(in.readBoolean(), in.readBoolean());
-    }
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAMES_FIELD.getPreferredName());
@@ -86,18 +85,12 @@ public class ChiSquare extends NXYSignificanceHeuristic {
         return builder;
     }
 
-    public static class ChiSquareParser extends NXYParser {
-
+    public static final SignificanceHeuristicParser PARSER = new NXYParser() {
         @Override
         protected SignificanceHeuristic newHeuristic(boolean includeNegatives, boolean backgroundIsSuperset) {
             return new ChiSquare(includeNegatives, backgroundIsSuperset);
         }
-
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
-        }
-    }
+    };
 
     public static class ChiSquareBuilder extends NXYSignificanceHeuristic.NXYBuilder {
 

+ 14 - 23
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/GND.java

@@ -33,15 +33,23 @@ import org.elasticsearch.index.query.QueryShardException;
 import java.io.IOException;
 
 public class GND extends NXYSignificanceHeuristic {
-
-    static final GND PROTOTYPE = new GND(false);
-
-    protected static final ParseField NAMES_FIELD = new ParseField("gnd");
+    public static final ParseField NAMES_FIELD = new ParseField("gnd");
 
     public GND(boolean backgroundIsSuperset) {
         super(true, backgroundIsSuperset);
     }
 
+    /**
+     * Read from a stream.
+     */
+    public GND(StreamInput in) throws IOException {
+        super(true, in.readBoolean());
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeBoolean(backgroundIsSuperset);
+    }
 
     @Override
     public boolean equals(Object other) {
@@ -91,16 +99,6 @@ public class GND extends NXYSignificanceHeuristic {
         return NAMES_FIELD.getPreferredName();
     }
 
-    @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        return new GND(in.readBoolean());
-    }
-
-    @Override
-    public void writeTo(StreamOutput out) throws IOException {
-        out.writeBoolean(backgroundIsSuperset);
-    }
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAMES_FIELD.getPreferredName());
@@ -109,13 +107,7 @@ public class GND extends NXYSignificanceHeuristic {
         return builder;
     }
 
-    public static class GNDParser extends NXYParser {
-
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
-        }
-
+    public static final SignificanceHeuristicParser PARSER = new NXYParser() {
         @Override
         protected SignificanceHeuristic newHeuristic(boolean includeNegatives, boolean backgroundIsSuperset) {
             return new GND(backgroundIsSuperset);
@@ -138,8 +130,7 @@ public class GND extends NXYSignificanceHeuristic {
             }
             return newHeuristic(true, backgroundIsSuperset);
         }
-
-    }
+    };
 
     public static class GNDBuilder extends NXYBuilder {
 

+ 32 - 27
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/JLHScore.java

@@ -33,22 +33,16 @@ import org.elasticsearch.index.query.QueryShardException;
 import java.io.IOException;
 
 public class JLHScore extends SignificanceHeuristic {
-
-    public static final JLHScore PROTOTYPE = new JLHScore();
-
-    protected static final ParseField NAMES_FIELD = new ParseField("jlh");
+    public static final ParseField NAMES_FIELD = new ParseField("jlh");
 
     public JLHScore() {
     }
 
-    @Override
-    public String getWriteableName() {
-        return NAMES_FIELD.getPreferredName();
-    }
-
-    @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        return PROTOTYPE;
+    /**
+     * Read from a stream.
+     */
+    public JLHScore(StreamInput in) {
+        // Nothing to read.
     }
 
     @Override
@@ -56,9 +50,8 @@ public class JLHScore extends SignificanceHeuristic {
     }
 
     @Override
-    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        builder.startObject(NAMES_FIELD.getPreferredName()).endObject();
-        return builder;
+    public String getWriteableName() {
+        return NAMES_FIELD.getPreferredName();
     }
 
     /**
@@ -106,22 +99,34 @@ public class JLHScore extends SignificanceHeuristic {
         return absoluteProbabilityChange * relativeProbabilityChange;
     }
 
-    public static class JLHScoreParser implements SignificanceHeuristicParser {
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject(NAMES_FIELD.getPreferredName()).endObject();
+        return builder;
+    }
 
-        @Override
-        public SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
-                throws IOException, QueryShardException {
-            // move to the closing bracket
-            if (!parser.nextToken().equals(XContentParser.Token.END_OBJECT)) {
-                throw new ElasticsearchParseException("failed to parse [jlh] significance heuristic. expected an empty object, but found [{}] instead", parser.currentToken());
-            }
-            return PROTOTYPE;
+    public static SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
+            throws IOException, QueryShardException {
+        // move to the closing bracket
+        if (!parser.nextToken().equals(XContentParser.Token.END_OBJECT)) {
+            throw new ElasticsearchParseException(
+                    "failed to parse [jlh] significance heuristic. expected an empty object, but found [{}] instead",
+                    parser.currentToken());
         }
+        return new JLHScore();
+    }
 
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == null || obj.getClass() != getClass()) {
+            return false;
         }
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return getClass().hashCode();
     }
 
     public static class JLHScoreBuilder implements SignificanceHeuristicBuilder {

+ 11 - 17
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/MutualInformation.java

@@ -28,10 +28,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import java.io.IOException;
 
 public class MutualInformation extends NXYSignificanceHeuristic {
-
-    static final MutualInformation PROTOTYPE = new MutualInformation(false, false);
-
-    protected static final ParseField NAMES_FIELD = new ParseField("mutual_information");
+    public static final ParseField NAMES_FIELD = new ParseField("mutual_information");
 
     private static final double log2 = Math.log(2.0);
 
@@ -39,6 +36,14 @@ public class MutualInformation extends NXYSignificanceHeuristic {
         super(includeNegatives, backgroundIsSuperset);
     }
 
+    /**
+     * Read from a stream.
+     */
+    public MutualInformation(StreamInput in) throws IOException {
+        super(in);
+    }
+
+
     @Override
     public boolean equals(Object other) {
         if (!(other instanceof MutualInformation)) {
@@ -106,11 +111,6 @@ public class MutualInformation extends NXYSignificanceHeuristic {
         return NAMES_FIELD.getPreferredName();
     }
 
-    @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        return new MutualInformation(in.readBoolean(), in.readBoolean());
-    }
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAMES_FIELD.getPreferredName());
@@ -119,18 +119,12 @@ public class MutualInformation extends NXYSignificanceHeuristic {
         return builder;
     }
 
-    public static class MutualInformationParser extends NXYParser {
-
+    public static SignificanceHeuristicParser PARSER = new NXYParser() {
         @Override
         protected SignificanceHeuristic newHeuristic(boolean includeNegatives, boolean backgroundIsSuperset) {
             return new MutualInformation(includeNegatives, backgroundIsSuperset);
         }
-
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
-        }
-    }
+    };
 
     public static class MutualInformationBuilder extends NXYBuilder {
 

+ 10 - 1
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/NXYSignificanceHeuristic.java

@@ -24,6 +24,7 @@ package org.elasticsearch.search.aggregations.bucket.significant.heuristics;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParseFieldMatcher;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -48,11 +49,19 @@ public abstract class NXYSignificanceHeuristic extends SignificanceHeuristic {
      */
     protected final boolean includeNegatives;
 
-    public NXYSignificanceHeuristic(boolean includeNegatives, boolean backgroundIsSuperset) {
+    protected NXYSignificanceHeuristic(boolean includeNegatives, boolean backgroundIsSuperset) {
         this.includeNegatives = includeNegatives;
         this.backgroundIsSuperset = backgroundIsSuperset;
     }
 
+    /**
+     * Read from a stream.
+     */
+    protected NXYSignificanceHeuristic(StreamInput in) throws IOException {
+        includeNegatives = in.readBoolean();
+        backgroundIsSuperset = in.readBoolean();
+    }
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeBoolean(includeNegatives);

+ 25 - 25
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/PercentageScore.java

@@ -33,26 +33,22 @@ import org.elasticsearch.index.query.QueryShardException;
 import java.io.IOException;
 
 public class PercentageScore extends SignificanceHeuristic {
-
-    public static final PercentageScore PROTOTYPE = new PercentageScore();
-
-    protected static final ParseField NAMES_FIELD = new ParseField("percentage");
+    public static final ParseField NAMES_FIELD = new ParseField("percentage");
 
     public PercentageScore() {
     }
 
-    @Override
-    public String getWriteableName() {
-        return NAMES_FIELD.getPreferredName();
+    public PercentageScore(StreamInput in) {
+        // Nothing to read.
     }
 
     @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        return PROTOTYPE;
+    public void writeTo(StreamOutput out) throws IOException {
     }
 
     @Override
-    public void writeTo(StreamOutput out) throws IOException {
+    public String getWriteableName() {
+        return NAMES_FIELD.getPreferredName();
     }
 
     @Override
@@ -61,6 +57,15 @@ public class PercentageScore extends SignificanceHeuristic {
         return builder;
     }
 
+    public static SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
+            throws IOException, QueryShardException {
+        // move to the closing bracket
+        if (!parser.nextToken().equals(XContentParser.Token.END_OBJECT)) {
+            throw new ElasticsearchParseException("failed to parse [percentage] significance heuristic. expected an empty object, but got [{}] instead", parser.currentToken());
+        }
+        return new PercentageScore();
+    }
+
     /**
      * Indicates the significance of a term in a sample by determining what percentage
      * of all occurrences of a term are found in the sample.
@@ -73,24 +78,19 @@ public class PercentageScore extends SignificanceHeuristic {
             return 0;
         }
         return (double) subsetFreq / (double) supersetFreq;
-   }
-
-    public static class PercentageScoreParser implements SignificanceHeuristicParser {
+    }
 
-        @Override
-        public SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
-                throws IOException, QueryShardException {
-            // move to the closing bracket
-            if (!parser.nextToken().equals(XContentParser.Token.END_OBJECT)) {
-                throw new ElasticsearchParseException("failed to parse [percentage] significance heuristic. expected an empty object, but got [{}] instead", parser.currentToken());
-            }
-            return PROTOTYPE;
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == null || obj.getClass() != getClass()) {
+            return false;
         }
+        return true;
+    }
 
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
-        }
+    @Override
+    public int hashCode() {
+        return getClass().hashCode();
     }
 
     public static class PercentageScoreBuilder implements SignificanceHeuristicBuilder {

+ 44 - 59
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java

@@ -47,16 +47,14 @@ import java.util.Map;
 import java.util.Objects;
 
 public class ScriptHeuristic extends SignificanceHeuristic {
+    public static final ParseField NAMES_FIELD = new ParseField("script_heuristic");
 
-    static final ScriptHeuristic PROTOTYPE = new ScriptHeuristic(null);
-
-    protected static final ParseField NAMES_FIELD = new ParseField("script_heuristic");
     private final LongAccessor subsetSizeHolder;
     private final LongAccessor supersetSizeHolder;
     private final LongAccessor subsetDfHolder;
     private final LongAccessor supersetDfHolder;
+    private final Script script;
     ExecutableScript searchScript = null;
-    Script script;
 
     public ScriptHeuristic(Script script) {
         subsetSizeHolder = new LongAccessor();
@@ -64,8 +62,18 @@ public class ScriptHeuristic extends SignificanceHeuristic {
         subsetDfHolder = new LongAccessor();
         supersetDfHolder = new LongAccessor();
         this.script = script;
+    }
 
+    /**
+     * Read from a stream.
+     */
+    public ScriptHeuristic(StreamInput in) throws IOException {
+        this(Script.readScript(in));
+    }
 
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        script.writeTo(out);
     }
 
     @Override
@@ -117,17 +125,6 @@ public class ScriptHeuristic extends SignificanceHeuristic {
         return NAMES_FIELD.getPreferredName();
     }
 
-    @Override
-    public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-        Script script = Script.readScript(in);
-        return new ScriptHeuristic(script);
-    }
-
-    @Override
-    public void writeTo(StreamOutput out) throws IOException {
-        script.writeTo(out);
-    }
-
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params builderParams) throws IOException {
         builder.startObject(NAMES_FIELD.getPreferredName());
@@ -154,58 +151,46 @@ public class ScriptHeuristic extends SignificanceHeuristic {
         return Objects.equals(script, other.script);
     }
 
-    public static class ScriptHeuristicParser implements SignificanceHeuristicParser {
-
-        public ScriptHeuristicParser() {
-        }
-
-        @Override
-        public SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
-                throws IOException, QueryShardException {
-            String heuristicName = parser.currentName();
-            Script script = null;
-            XContentParser.Token token;
-            Map<String, Object> params = null;
-            String currentFieldName = null;
-            ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
-            while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-                if (token.equals(XContentParser.Token.FIELD_NAME)) {
-                    currentFieldName = parser.currentName();
-                } else if (token == XContentParser.Token.START_OBJECT) {
-                    if (parseFieldMatcher.match(currentFieldName, ScriptField.SCRIPT)) {
-                        script = Script.parse(parser, parseFieldMatcher);
-                    } else if ("params".equals(currentFieldName)) { // TODO remove in 3.0 (here to support old script APIs)
-                        params = parser.map();
-                    } else {
-                        throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. unknown object [{}]", heuristicName, currentFieldName);
-                    }
-                } else if (!scriptParameterParser.token(currentFieldName, token, parser, parseFieldMatcher)) {
-                    throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. unknown field [{}]", heuristicName, currentFieldName);
+    public static SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
+            throws IOException, QueryShardException {
+        String heuristicName = parser.currentName();
+        Script script = null;
+        XContentParser.Token token;
+        Map<String, Object> params = null;
+        String currentFieldName = null;
+        ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token.equals(XContentParser.Token.FIELD_NAME)) {
+                currentFieldName = parser.currentName();
+            } else if (token == XContentParser.Token.START_OBJECT) {
+                if (parseFieldMatcher.match(currentFieldName, ScriptField.SCRIPT)) {
+                    script = Script.parse(parser, parseFieldMatcher);
+                } else if ("params".equals(currentFieldName)) { // TODO remove in 3.0 (here to support old script APIs)
+                    params = parser.map();
+                } else {
+                    throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. unknown object [{}]", heuristicName, currentFieldName);
                 }
+            } else if (!scriptParameterParser.token(currentFieldName, token, parser, parseFieldMatcher)) {
+                throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. unknown field [{}]", heuristicName, currentFieldName);
             }
+        }
 
-            if (script == null) { // Didn't find anything using the new API so try using the old one instead
-                ScriptParameterValue scriptValue = scriptParameterParser.getDefaultScriptParameterValue();
-                if (scriptValue != null) {
-                    if (params == null) {
-                        params = new HashMap<>();
-                    }
-                    script = new Script(scriptValue.script(), scriptValue.scriptType(), scriptParameterParser.lang(), params);
+        if (script == null) { // Didn't find anything using the new API so try using the old one instead
+            ScriptParameterValue scriptValue = scriptParameterParser.getDefaultScriptParameterValue();
+            if (scriptValue != null) {
+                if (params == null) {
+                    params = new HashMap<>();
                 }
-            } else if (params != null) {
-                throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. script params must be specified inside script object", heuristicName);
-            }
-
-            if (script == null) {
-                throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. no script found in script_heuristic", heuristicName);
+                script = new Script(scriptValue.script(), scriptValue.scriptType(), scriptParameterParser.lang(), params);
             }
-            return new ScriptHeuristic(script);
+        } else if (params != null) {
+            throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. script params must be specified inside script object", heuristicName);
         }
 
-        @Override
-        public String[] getNames() {
-            return NAMES_FIELD.getAllNamesIncludedDeprecated();
+        if (script == null) {
+            throw new ElasticsearchParseException("failed to parse [{}] significance heuristic. no script found in script_heuristic", heuristicName);
         }
+        return new ScriptHeuristic(script);
     }
 
     public static class ScriptHeuristicBuilder implements SignificanceHeuristicBuilder {

+ 4 - 3
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicParser.java

@@ -26,10 +26,11 @@ import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.io.IOException;
 
+/**
+ * Parses {@link SignificanceHeuristic}s from an {@link XContentParser}.
+ */
+@FunctionalInterface
 public interface SignificanceHeuristicParser {
-
     SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher) throws IOException,
             ParsingException;
-
-    String[] getNames();
 }

+ 0 - 58
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicParserMapper.java

@@ -1,58 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-
-package org.elasticsearch.search.aggregations.bucket.significant.heuristics;
-
-import org.elasticsearch.common.inject.Inject;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-
-public class SignificanceHeuristicParserMapper {
-
-    protected final Map<String, SignificanceHeuristicParser> significanceHeuristicParsers;
-
-    @Inject
-    public SignificanceHeuristicParserMapper(Set<SignificanceHeuristicParser> parsers) {
-        Map<String, SignificanceHeuristicParser> map = new HashMap<>();
-        add(map, new JLHScore.JLHScoreParser());
-        add(map, new PercentageScore.PercentageScoreParser());
-        add(map, new MutualInformation.MutualInformationParser());
-        add(map, new ChiSquare.ChiSquareParser());
-        add(map, new GND.GNDParser());
-        add(map, new ScriptHeuristic.ScriptHeuristicParser());
-        for (SignificanceHeuristicParser parser : parsers) {
-            add(map, parser);
-        }
-        significanceHeuristicParsers = Collections.unmodifiableMap(map);
-    }
-
-    public SignificanceHeuristicParser get(String parserName) {
-        return significanceHeuristicParsers.get(parserName);
-    }
-
-
-    private void add(Map<String, SignificanceHeuristicParser> map, SignificanceHeuristicParser parser) {
-        for (String type : parser.getNames()) {
-            map.put(type, parser);
-        }
-    }
-}

+ 0 - 93
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java

@@ -1,93 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.elasticsearch.search.aggregations.bucket.significant.heuristics;
-
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.io.stream.StreamOutput;
-
-import java.io.IOException;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- * A registry for all significance heuristics. This is needed for reading them from a stream without knowing which
- * one it is.
- */
-public class SignificanceHeuristicStreams {
-
-    private static Map<String, SignificanceHeuristic> STREAMS = Collections.emptyMap();
-
-    static {
-        HashMap<String, SignificanceHeuristic> map = new HashMap<>();
-        map.put(JLHScore.NAMES_FIELD.getPreferredName(), JLHScore.PROTOTYPE);
-        map.put(PercentageScore.NAMES_FIELD.getPreferredName(), PercentageScore.PROTOTYPE);
-        map.put(MutualInformation.NAMES_FIELD.getPreferredName(), MutualInformation.PROTOTYPE);
-        map.put(GND.NAMES_FIELD.getPreferredName(), GND.PROTOTYPE);
-        map.put(ChiSquare.NAMES_FIELD.getPreferredName(), ChiSquare.PROTOTYPE);
-        map.put(ScriptHeuristic.NAMES_FIELD.getPreferredName(), ScriptHeuristic.PROTOTYPE);
-        STREAMS = Collections.unmodifiableMap(map);
-    }
-
-    public static SignificanceHeuristic read(StreamInput in) throws IOException {
-        return stream(in.readString()).readFrom(in);
-    }
-
-    public static void writeTo(SignificanceHeuristic significanceHeuristic, StreamOutput out) throws IOException {
-        out.writeString(significanceHeuristic.getWriteableName());
-        significanceHeuristic.writeTo(out);
-    }
-
-    /**
-     * A stream that knows how to read an heuristic from the input.
-     */
-    public static interface Stream {
-
-        SignificanceHeuristic readResult(StreamInput in) throws IOException;
-
-        String getName();
-    }
-
-    /**
-     * Registers the given prototype.
-     *
-     * @param prototype
-     *            The prototype to register
-     */
-    public static synchronized void registerPrototype(SignificanceHeuristic prototype) {
-        if (STREAMS.containsKey(prototype.getWriteableName())) {
-            throw new IllegalArgumentException("Can't register stream with name [" + prototype.getWriteableName() + "] more than once");
-        }
-        HashMap<String, SignificanceHeuristic> map = new HashMap<>();
-        map.putAll(STREAMS);
-        map.put(prototype.getWriteableName(), prototype);
-        STREAMS = Collections.unmodifiableMap(map);
-    }
-
-    /**
-     * Returns the stream that is registered for the given name
-     *
-     * @param name The given name
-     * @return The associated stream
-     */
-    private static synchronized SignificanceHeuristic stream(String name) {
-        return STREAMS.get(name);
-    }
-
-}

+ 27 - 34
core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java

@@ -45,8 +45,6 @@ import org.elasticsearch.search.aggregations.bucket.significant.heuristics.GND;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.MutualInformation;
 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.significant.heuristics.SignificanceHeuristicStreams;
 import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
 import org.elasticsearch.search.aggregations.bucket.terms.Terms;
 import org.elasticsearch.test.ESIntegTestCase;
@@ -63,11 +61,11 @@ import java.util.concurrent.ExecutionException;
 
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.elasticsearch.search.aggregations.AggregationBuilders.filter;
-import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
 import static org.elasticsearch.search.aggregations.AggregationBuilders.significantTerms;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
@@ -89,6 +87,11 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
         return pluginList(CustomSignificanceHeuristicPlugin.class);
     }
 
+    @Override
+    protected Collection<Class<? extends Plugin>> transportClientPlugins() {
+        return pluginList(CustomSignificanceHeuristicPlugin.class);
+    }
+
     public String randomExecutionHint() {
         return randomBoolean() ? null : randomFrom(SignificantTermsAggregatorFactory.ExecutionMode.values()).toString();
     }
@@ -162,11 +165,6 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
     }
 
     public static class CustomSignificanceHeuristicPlugin extends Plugin {
-
-        static {
-            SignificanceHeuristicStreams.registerPrototype(SimpleHeuristic.PROTOTYPE);
-        }
-
         @Override
         public String name() {
             return "test-plugin-significance-heuristic";
@@ -177,9 +175,10 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
             return "Significance heuristic plugin";
         }
 
-        public void onModule(SearchModule significanceModule) {
-            significanceModule.registerHeuristicParser(new SimpleHeuristic.SimpleHeuristicParser());
+        public void onModule(SearchModule searchModule) {
+            searchModule.registerSignificanceHeuristic(SimpleHeuristic.NAMES_FIELD, SimpleHeuristic::new, SimpleHeuristic::parse);
         }
+
         public void onModule(ScriptModule module) {
             module.registerScript(NativeSignificanceScoreScriptNoParams.NATIVE_SIGNIFICANCE_SCORE_SCRIPT_NO_PARAMS, NativeSignificanceScoreScriptNoParams.Factory.class);
             module.registerScript(NativeSignificanceScoreScriptWithParams.NATIVE_SIGNIFICANCE_SCORE_SCRIPT_WITH_PARAMS, NativeSignificanceScoreScriptWithParams.Factory.class);
@@ -187,23 +186,26 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
     }
 
     public static class SimpleHeuristic extends SignificanceHeuristic {
+        public static final ParseField NAMES_FIELD = new ParseField("simple");
 
-        static final SimpleHeuristic PROTOTYPE = new SimpleHeuristic();
-
-        protected static final ParseField NAMES_FIELD = new ParseField("simple");
+        public SimpleHeuristic() {
+        }
 
-        @Override
-        public String getWriteableName() {
-            return NAMES_FIELD.getPreferredName();
+        /**
+         * Read from a stream.
+         */
+        public SimpleHeuristic(StreamInput in) throws IOException {
+            // Nothing to read
         }
 
         @Override
-        public SignificanceHeuristic readFrom(StreamInput in) throws IOException {
-            return new SimpleHeuristic();
+        public void writeTo(StreamOutput out) throws IOException {
+            // Nothing to write
         }
 
         @Override
-        public void writeTo(StreamOutput out) throws IOException {
+        public String getWriteableName() {
+            return NAMES_FIELD.getPreferredName();
         }
 
         @Override
@@ -240,19 +242,10 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
             return subsetFreq / subsetSize > supersetFreq / supersetSize ? 2.0 : 1.0;
         }
 
-        public static class SimpleHeuristicParser implements SignificanceHeuristicParser {
-
-            @Override
-            public SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
-                    throws IOException, QueryShardException {
-                parser.nextToken();
-                return new SimpleHeuristic();
-            }
-
-            @Override
-            public String[] getNames() {
-                return NAMES_FIELD.getAllNamesIncludedDeprecated();
-            }
+        public static SignificanceHeuristic parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher)
+                throws IOException, QueryShardException {
+            parser.nextToken();
+            return new SimpleHeuristic();
         }
     }
 

+ 2 - 2
core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsTests.java

@@ -193,7 +193,7 @@ public class SignificantTermsTests extends BaseAggregationTestCase<SignificantTe
             SignificanceHeuristic significanceHeuristic = null;
             switch (randomInt(5)) {
             case 0:
-                significanceHeuristic = PercentageScore.PROTOTYPE;
+                significanceHeuristic = new PercentageScore();
                 break;
             case 1:
                 significanceHeuristic = new ChiSquare(randomBoolean(), randomBoolean());
@@ -208,7 +208,7 @@ public class SignificantTermsTests extends BaseAggregationTestCase<SignificantTe
                 significanceHeuristic = new ScriptHeuristic(new Script("foo"));
                 break;
             case 5:
-                significanceHeuristic = JLHScore.PROTOTYPE;
+                significanceHeuristic = new JLHScore();
                 break;
             default:
                 fail();

+ 21 - 21
core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificanceHeuristicTests.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.ParseFieldRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -47,7 +48,6 @@ import org.elasticsearch.search.aggregations.bucket.significant.heuristics.Mutua
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.PercentageScore;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
 import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser;
-import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParserMapper;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.ESTestCase;
@@ -60,9 +60,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import static org.elasticsearch.search.aggregations.AggregationBuilders.significantTerms;
 import static org.elasticsearch.test.VersionUtils.randomVersion;
@@ -143,7 +141,7 @@ public class SignificanceHeuristicTests extends ESTestCase {
 
     SignificanceHeuristic getRandomSignificanceheuristic() {
         List<SignificanceHeuristic> heuristics = new ArrayList<>();
-        heuristics.add(JLHScore.PROTOTYPE);
+        heuristics.add(new JLHScore());
         heuristics.add(new MutualInformation(randomBoolean(), randomBoolean()));
         heuristics.add(new GND(randomBoolean()));
         heuristics.add(new ChiSquare(randomBoolean(), randomBoolean()));
@@ -204,9 +202,8 @@ public class SignificanceHeuristicTests extends ESTestCase {
     // 1. The output of the builders can actually be parsed
     // 2. The parser does not swallow parameters after a significance heuristic was defined
     public void testBuilderAndParser() throws Exception {
-
-        Set<SignificanceHeuristicParser> parsers = new HashSet<>();
-        SignificanceHeuristicParserMapper heuristicParserMapper = new SignificanceHeuristicParserMapper(parsers);
+        SearchModule searchModule = new SearchModule(Settings.EMPTY, new NamedWriteableRegistry());
+        ParseFieldRegistry<SignificanceHeuristicParser> heuristicParserMapper = searchModule.getSignificanceHeuristicParserRegistry();
         SearchContext searchContext = new SignificantTermsTestSearchContext();
 
         // test jlh with string
@@ -243,37 +240,39 @@ public class SignificanceHeuristicTests extends ESTestCase {
         checkParseException(heuristicParserMapper, searchContext, faultyHeuristicdefinition, expectedError);
     }
 
-    protected void checkParseException(SignificanceHeuristicParserMapper heuristicParserMapper, SearchContext searchContext,
-            String faultyHeuristicDefinition, String expectedError) throws IOException {
+    protected void checkParseException(ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry,
+            SearchContext searchContext, String faultyHeuristicDefinition, String expectedError) throws IOException {
 
         IndicesQueriesRegistry registry = new IndicesQueriesRegistry();
         try {
             XContentParser stParser = JsonXContent.jsonXContent.createParser("{\"field\":\"text\", " + faultyHeuristicDefinition + ",\"min_doc_count\":200}");
             QueryParseContext parseContext = new QueryParseContext(registry, stParser, ParseFieldMatcher.STRICT);
             stParser.nextToken();
-            new SignificantTermsParser(heuristicParserMapper, registry).parse("testagg", parseContext);
+            new SignificantTermsParser(significanceHeuristicParserRegistry, registry).parse("testagg", parseContext);
             fail();
         } catch (ElasticsearchParseException e) {
             assertTrue(e.getMessage().contains(expectedError));
         }
     }
 
-    protected SignificanceHeuristic parseFromBuilder(SignificanceHeuristicParserMapper heuristicParserMapper, SearchContext searchContext, SignificanceHeuristic significanceHeuristic) throws IOException {
+    protected SignificanceHeuristic parseFromBuilder(ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry,
+            SearchContext searchContext, SignificanceHeuristic significanceHeuristic) throws IOException {
         SignificantTermsAggregatorBuilder stBuilder = significantTerms("testagg");
         stBuilder.significanceHeuristic(significanceHeuristic).field("text").minDocCount(200);
         XContentBuilder stXContentBuilder = XContentFactory.jsonBuilder();
         stBuilder.internalXContent(stXContentBuilder, null);
         XContentParser stParser = JsonXContent.jsonXContent.createParser(stXContentBuilder.string());
-        return parseSignificanceHeuristic(heuristicParserMapper, searchContext, stParser);
+        return parseSignificanceHeuristic(significanceHeuristicParserRegistry, searchContext, stParser);
     }
 
-    private SignificanceHeuristic parseSignificanceHeuristic(SignificanceHeuristicParserMapper heuristicParserMapper,
-            SearchContext searchContext, XContentParser stParser) throws IOException {
+    private SignificanceHeuristic parseSignificanceHeuristic(
+            ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry, SearchContext searchContext,
+            XContentParser stParser) throws IOException {
         IndicesQueriesRegistry registry = new IndicesQueriesRegistry();
         QueryParseContext parseContext = new QueryParseContext(registry, stParser, ParseFieldMatcher.STRICT);
         stParser.nextToken();
         SignificantTermsAggregatorBuilder aggregatorFactory = (SignificantTermsAggregatorBuilder) new SignificantTermsParser(
-                heuristicParserMapper, registry).parse("testagg", parseContext);
+                significanceHeuristicParserRegistry, registry).parse("testagg", parseContext);
         stParser.nextToken();
         assertThat(aggregatorFactory.getBucketCountThresholds().getMinDocCount(), equalTo(200L));
         assertThat(stParser.currentToken(), equalTo(null));
@@ -281,9 +280,10 @@ public class SignificanceHeuristicTests extends ESTestCase {
         return aggregatorFactory.significanceHeuristic();
     }
 
-    protected SignificanceHeuristic parseFromString(SignificanceHeuristicParserMapper heuristicParserMapper, SearchContext searchContext, String heuristicString) throws IOException {
+    protected SignificanceHeuristic parseFromString(ParseFieldRegistry<SignificanceHeuristicParser> significanceHeuristicParserRegistry,
+            SearchContext searchContext, String heuristicString) throws IOException {
         XContentParser stParser = JsonXContent.jsonXContent.createParser("{\"field\":\"text\", " + heuristicString + ", \"min_doc_count\":200}");
-        return parseSignificanceHeuristic(heuristicParserMapper, searchContext, stParser);
+        return parseSignificanceHeuristic(significanceHeuristicParserRegistry, searchContext, stParser);
     }
 
     void testBackgroundAssertions(SignificanceHeuristic heuristicIsSuperset, SignificanceHeuristic heuristicNotSuperset) {
@@ -389,14 +389,14 @@ public class SignificanceHeuristicTests extends ESTestCase {
         testBackgroundAssertions(new MutualInformation(true, true), new MutualInformation(true, false));
         testBackgroundAssertions(new ChiSquare(true, true), new ChiSquare(true, false));
         testBackgroundAssertions(new GND(true), new GND(false));
-        testAssertions(PercentageScore.PROTOTYPE);
-        testAssertions(JLHScore.PROTOTYPE);
+        testAssertions(new PercentageScore());
+        testAssertions(new JLHScore());
     }
 
     public void testBasicScoreProperties() {
-        basicScoreProperties(JLHScore.PROTOTYPE, true);
+        basicScoreProperties(new JLHScore(), true);
         basicScoreProperties(new GND(true), true);
-        basicScoreProperties(PercentageScore.PROTOTYPE, true);
+        basicScoreProperties(new PercentageScore(), true);
         basicScoreProperties(new MutualInformation(true, true), false);
         basicScoreProperties(new ChiSquare(true, true), false);
     }

+ 14 - 2
test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java

@@ -62,6 +62,7 @@ import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.suggest.Suggest;
+import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.test.rest.client.http.HttpResponse;
 import org.hamcrest.CoreMatchers;
@@ -647,8 +648,19 @@ public class ElasticsearchAssertions {
     }
 
     public static void assertVersionSerializable(Version version, Streamable streamable) {
-        NamedWriteableRegistry registry = new NamedWriteableRegistry();
-        new SearchModule(Settings.EMPTY, registry); // populates the registry through side effects
+        /*
+         * If possible we fetch the NamedWriteableRegistry from the test cluster. That is the only way to make sure that we properly handle
+         * when plugins register names. If not possible we'll try and set up a registry based on whatever SearchModule registers. But that
+         * is a hack at best - it only covers some things. If you end up with errors below and get to this comment I'm sorry. Please find
+         * a way that sucks less.
+         */
+        NamedWriteableRegistry registry;
+        if (ESIntegTestCase.isInternalCluster()) {
+            registry = ESIntegTestCase.internalCluster().getInstance(NamedWriteableRegistry.class);
+        } else {
+            registry = new NamedWriteableRegistry();
+            new SearchModule(Settings.EMPTY, registry);
+        }
         assertVersionSerializable(version, streamable, registry);
     }