Internally manipulate the terms execution hint as an enum instead of a String.

Close #5530

src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java

@@ -20,9 +20,11 @@ package org.elasticsearch.search.aggregations.bucket.terms;
 
 import org.apache.lucene.index.AtomicReaderContext;
 import org.elasticsearch.ElasticsearchIllegalArgumentException;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.Aggregator.BucketAggregationMode;
+import org.elasticsearch.search.aggregations.AggregatorFactories;
 import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude;
 import org.elasticsearch.search.aggregations.support.AggregationContext;
 import org.elasticsearch.search.aggregations.support.ValueSourceAggregatorFactory;
@@ -36,8 +38,55 @@ import org.elasticsearch.search.aggregations.support.numeric.NumericValuesSource
  */
 public class TermsAggregatorFactory extends ValueSourceAggregatorFactory {
 
-    public static final String EXECUTION_HINT_VALUE_MAP = "map";
+    public enum ExecutionMode {
-    public static final String EXECUTION_HINT_VALUE_ORDINALS = "ordinals";
+        MAP(new ParseField("map")) {
+
+            @Override
+            Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
+                    InternalOrder order, int requiredSize, int shardSize, long minDocCount, IncludeExclude includeExclude,
+                    AggregationContext aggregationContext, Aggregator parent) {
+                return new StringTermsAggregator(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
+            }
+
+        },
+        ORDINALS(new ParseField("ordinals")) {
+
+            @Override
+            Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
+                    InternalOrder order, int requiredSize, int shardSize, long minDocCount, IncludeExclude includeExclude,
+                    AggregationContext aggregationContext, Aggregator parent) {
+                if (includeExclude != null) {
+                    throw new ElasticsearchIllegalArgumentException("The `" + this + "` execution mode cannot filter terms.");
+                }
+                return new StringTermsAggregator.WithOrdinals(name, factories, (BytesValuesSource.WithOrdinals) valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, aggregationContext, parent);
+            }
+
+        };
+
+        public static ExecutionMode fromString(String value) {
+            for (ExecutionMode mode : values()) {
+                if (mode.parseField.match(value)) {
+                    return mode;
+                }
+            }
+            throw new ElasticsearchIllegalArgumentException("Unknown `execution_hint`: [" + value + "], expected any of " + values());
+        }
+
+        private final ParseField parseField;
+
+        ExecutionMode(ParseField parseField) {
+            this.parseField = parseField;
+        }
+
+        abstract Aggregator create(String name, AggregatorFactories factories, ValuesSource valuesSource, long estimatedBucketCount,
+                InternalOrder order, int requiredSize, int shardSize, long minDocCount,
+                IncludeExclude includeExclude, AggregationContext aggregationContext, Aggregator parent);
+
+        @Override
+        public String toString() {
+            return parseField.getPreferredName();
+        }
+    }
 
     private final InternalOrder order;
     private final int requiredSize;
@@ -113,31 +162,30 @@ public class TermsAggregatorFactory extends ValueSourceAggregatorFactory {
         estimatedBucketCount = Math.min(estimatedBucketCount, 512);
 
         if (valuesSource instanceof BytesValuesSource) {
-            if (executionHint != null && !executionHint.equals(EXECUTION_HINT_VALUE_MAP) && !executionHint.equals(EXECUTION_HINT_VALUE_ORDINALS)) {
+            ExecutionMode execution = null;
-                throw new ElasticsearchIllegalArgumentException("execution_hint can only be '" + EXECUTION_HINT_VALUE_MAP + "' or '" + EXECUTION_HINT_VALUE_ORDINALS + "', not " + executionHint);
+            if (executionHint != null) {
+                execution = ExecutionMode.fromString(executionHint);
             }
-            String execution = executionHint;
+
+            // In some cases, using ordinals is just not supported: override it
             if (!(valuesSource instanceof BytesValuesSource.WithOrdinals)) {
-                execution = EXECUTION_HINT_VALUE_MAP;
+                execution = ExecutionMode.MAP;
             } else if (includeExclude != null) {
-                execution = EXECUTION_HINT_VALUE_MAP;
+                execution = ExecutionMode.MAP;
             }
+
             if (execution == null) {
+                // Let's try to use a good default
                 if ((valuesSource instanceof BytesValuesSource.WithOrdinals)
                         && shouldUseOrdinals(parent, valuesSource, aggregationContext)) {
-                    execution = EXECUTION_HINT_VALUE_ORDINALS;
+                    execution = ExecutionMode.ORDINALS;
                 } else {
-                    execution = EXECUTION_HINT_VALUE_MAP;
+                    execution = ExecutionMode.MAP;
                 }
             }
             assert execution != null;
 
-            if (execution.equals(EXECUTION_HINT_VALUE_ORDINALS)) {
+            return execution.create(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
-                assert includeExclude == null;
-                return new StringTermsAggregator.WithOrdinals(name, factories, (BytesValuesSource.WithOrdinals) valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, aggregationContext, parent);
-            } else {
-                return new StringTermsAggregator(name, factories, valuesSource, estimatedBucketCount, order, requiredSize, shardSize, minDocCount, includeExclude, aggregationContext, parent);
-            }
         }
 
         if (includeExclude != null) {

src/test/java/org/elasticsearch/search/aggregations/RandomTests.java

@@ -191,8 +191,8 @@ public class RandomTests extends ElasticsearchIntegrationTest {
         SearchResponse resp = client().prepareSearch("idx")
                 .addAggregation(terms("long").field("long_values").size(maxNumTerms).subAggregation(min("min").field("num")))
                 .addAggregation(terms("double").field("double_values").size(maxNumTerms).subAggregation(max("max").field("num")))
-                .addAggregation(terms("string_map").field("string_values").executionHint(TermsAggregatorFactory.EXECUTION_HINT_VALUE_MAP).size(maxNumTerms).subAggregation(stats("stats").field("num")))
+                .addAggregation(terms("string_map").field("string_values").executionHint(TermsAggregatorFactory.ExecutionMode.MAP.toString()).size(maxNumTerms).subAggregation(stats("stats").field("num")))
-                .addAggregation(terms("string_ordinals").field("string_values").executionHint(TermsAggregatorFactory.EXECUTION_HINT_VALUE_ORDINALS).size(maxNumTerms).subAggregation(extendedStats("stats").field("num"))).execute().actionGet();
+                .addAggregation(terms("string_ordinals").field("string_values").executionHint(TermsAggregatorFactory.ExecutionMode.ORDINALS.toString()).size(maxNumTerms).subAggregation(extendedStats("stats").field("num"))).execute().actionGet();
         assertEquals(0, resp.getFailedShards());
 
         final Terms longTerms = resp.getAggregations().get("long");

src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java

@@ -38,7 +38,6 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Pattern;
@@ -61,7 +60,7 @@ public class StringTermsTests extends ElasticsearchIntegrationTest {
     private static final String MULTI_VALUED_FIELD_NAME = "s_values";
 
     public static String randomExecutionHint() {
-        return randomFrom(Arrays.asList(null, TermsAggregatorFactory.EXECUTION_HINT_VALUE_MAP, TermsAggregatorFactory.EXECUTION_HINT_VALUE_ORDINALS));
+        return randomBoolean() ? null : randomFrom(TermsAggregatorFactory.ExecutionMode.values()).toString();
     }
 
     @Before