Using SortMode enum in all sort builders

core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

@@ -53,9 +53,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
 
     private String unmappedType;
 
-    private String sortMode;
+    private SortMode sortMode;
 
-    private QueryBuilder nestedFilter;
+    private QueryBuilder<?> nestedFilter;
 
     private String nestedPath;
 
@@ -65,7 +65,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
         this.order(template.order());
         this.missing(template.missing());
         this.unmappedType(template.unmappedType());
-        this.sortMode(template.sortMode());
+        if (template.sortMode != null) {
+            this.sortMode(template.sortMode());
+        }
         this.setNestedFilter(template.getNestedFilter());
         this.setNestedPath(template.getNestedPath());
     }
@@ -134,12 +136,12 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
      * Defines what values to pick in the case a document contains multiple
      * values for the targeted sort field. Possible values: min, max, sum and
      * avg
-     * 
-     * TODO would love to see an enum here
+     *
      * <p>
      * The last two values are only applicable for number based fields.
      */
-    public FieldSortBuilder sortMode(String sortMode) {
+    public FieldSortBuilder sortMode(SortMode sortMode) {
+        Objects.requireNonNull(sortMode, "sort mode cannot be null");
         this.sortMode = sortMode;
         return this;
     }
@@ -148,14 +150,14 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
      * Returns what values to pick in the case a document contains multiple
      * values for the targeted sort field.
      */
-    public String sortMode() {
+    public SortMode sortMode() {
         return this.sortMode;
     }
 
     /**
      * Sets the nested filter that the nested objects should match with in order
      * to be taken into account for sorting.
-     * 
+     *
      * TODO should the above getters and setters be deprecated/ changed in
      * favour of real getters and setters?
      */
@@ -263,7 +265,10 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
             out.writeBoolean(false);
         }
 
-        out.writeOptionalString(this.sortMode);
+        out.writeBoolean(this.sortMode != null);
+        if (this.sortMode != null) {
+           this.sortMode.writeTo(out);
+        }
         out.writeOptionalString(this.unmappedType);
     }
 
@@ -272,7 +277,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
         String fieldName = in.readString();
         FieldSortBuilder result = new FieldSortBuilder(fieldName);
         if (in.readBoolean()) {
-            QueryBuilder query = in.readQuery();
+            QueryBuilder<?> query = in.readQuery();
             result.setNestedFilter(query);
         }
         result.setNestedPath(in.readOptionalString());
@@ -281,7 +286,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
         if (in.readBoolean()) {
             result.order(SortOrder.readOrderFrom(in));
         }
-        result.sortMode(in.readOptionalString());
+        if (in.readBoolean()) {
+            result.sortMode(SortMode.PROTOTYPE.readFrom(in));
+        }
         result.unmappedType(in.readOptionalString());
         return result;
     }
@@ -290,11 +297,11 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
     public FieldSortBuilder fromXContent(QueryParseContext context, String fieldName) throws IOException {
         XContentParser parser = context.parser();
 
-        QueryBuilder nestedFilter = null;
+        QueryBuilder<?> nestedFilter = null;
         String nestedPath = null;
         Object missing = null;
         SortOrder order = null;
-        String sortMode = null;
+        SortMode sortMode = null;
         String unmappedType = null;
 
         String currentFieldName = null;
@@ -328,7 +335,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
                         throw new IllegalStateException("Sort order " + sortOrder + " not supported.");
                     }
                 } else if (context.parseFieldMatcher().match(currentFieldName, SORT_MODE)) {
-                    sortMode = parser.text();
+                    sortMode = SortMode.fromString(parser.text());
                 } else if (context.parseFieldMatcher().match(currentFieldName, UNMAPPED_TYPE)) {
                     unmappedType = parser.text();
                 } else {

core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

@@ -30,7 +30,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
-import org.elasticsearch.search.MultiValueMode;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -55,8 +54,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     private GeoDistance geoDistance = GeoDistance.DEFAULT;
     private DistanceUnit unit = DistanceUnit.DEFAULT;
 
-    // TODO there is an enum that covers that parameter which we should be using here
-    private String sortMode = null;
+    private SortMode sortMode = null;
     @SuppressWarnings("rawtypes")
     private QueryBuilder nestedFilter;
     private String nestedPath;
@@ -204,9 +202,9 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
      * Defines which distance to use for sorting in the case a document contains multiple geo points.
      * Possible values: min and max
      */
-    public GeoDistanceSortBuilder sortMode(String sortMode) {
-        MultiValueMode temp = MultiValueMode.fromString(sortMode);
-        if (temp == MultiValueMode.SUM) {
+    public GeoDistanceSortBuilder sortMode(SortMode sortMode) {
+        Objects.requireNonNull(sortMode, "sort mode cannot be null");
+        if (sortMode == SortMode.SUM) {
             throw new IllegalArgumentException("sort_mode [sum] isn't supported for sorting by geo distance");
         }
         this.sortMode = sortMode;
@@ -214,7 +212,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     }
 
     /** Returns which distance to use for sorting in the case a document contains multiple geo points. */
-    public String sortMode() {
+    public SortMode sortMode() {
         return this.sortMode;
     }
 
@@ -345,7 +343,10 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         geoDistance.writeTo(out);
         unit.writeTo(out);
         order.writeTo(out);
-        out.writeOptionalString(sortMode);
+        out.writeBoolean(this.sortMode != null);
+        if (this.sortMode != null) {
+            sortMode.writeTo(out);
+        }
         if (nestedFilter != null) {
             out.writeBoolean(true);
             out.writeQuery(nestedFilter);
@@ -367,9 +368,8 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         result.geoDistance(GeoDistance.readGeoDistanceFrom(in));
         result.unit(DistanceUnit.readDistanceUnit(in));
         result.order(SortOrder.readOrderFrom(in));
-        String sortMode = in.readOptionalString();
-        if (sortMode != null) {
-            result.sortMode(sortMode);
+        if (in.readBoolean()) {
+            result.sortMode = SortMode.PROTOTYPE.readFrom(in);
         }
         if (in.readBoolean()) {
             result.setNestedFilter(in.readQuery());
@@ -388,7 +388,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         DistanceUnit unit = DistanceUnit.DEFAULT;
         GeoDistance geoDistance = GeoDistance.DEFAULT;
         SortOrder order = SortOrder.ASC;
-        MultiValueMode sortMode = null;
+        SortMode sortMode = null;
         QueryBuilder<?> nestedFilter = null;
         String nestedPath = null;
 
@@ -437,7 +437,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                         ignoreMalformed = ignore_malformed_value;
                     }
                 } else if ("sort_mode".equals(currentName) || "sortMode".equals(currentName) || "mode".equals(currentName)) {
-                    sortMode = MultiValueMode.fromString(parser.text());
+                    sortMode = SortMode.fromString(parser.text());
                 } else if ("nested_path".equals(currentName) || "nestedPath".equals(currentName)) {
                     nestedPath = parser.text();
                 } else {
@@ -454,7 +454,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         result.unit(unit);
         result.order(order);
         if (sortMode != null) {
-            result.sortMode(sortMode.name());
+            result.sortMode(sortMode);
         }
         result.setNestedFilter(nestedFilter);
         result.setNestedPath(nestedPath);

core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java

@@ -34,8 +34,7 @@ import java.util.Objects;
 /**
  * A sort builder allowing to sort by score.
  */
-public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements SortBuilderParser<ScoreSortBuilder>,
-    SortElementParserTemp<ScoreSortBuilder> {
+public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements SortBuilderParser<ScoreSortBuilder> {
 
     private static final String NAME = "_score";
     static final ScoreSortBuilder PROTOTYPE = new ScoreSortBuilder();

core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java

@@ -22,7 +22,6 @@ package org.elasticsearch.search.sort;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.ParsingException;
-import org.elasticsearch.common.io.stream.NamedWriteable;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -44,8 +43,7 @@ import java.util.Objects;
 /**
  * Script sort builder allows to sort based on a custom script expression.
  */
-public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements NamedWriteable<ScriptSortBuilder>,
-    SortElementParserTemp<ScriptSortBuilder> {
+public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements SortBuilderParser<ScriptSortBuilder> {
 
     private static final String NAME = "_script";
     static final ScriptSortBuilder PROTOTYPE = new ScriptSortBuilder(new Script("_na_"), ScriptSortType.STRING);
@@ -72,8 +70,8 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
      * @param script
      *            The script to use.
      * @param type
-     *            The type of the script, can be either {@link ScriptSortParser#STRING_SORT_TYPE} or
-     *            {@link ScriptSortParser#NUMBER_SORT_TYPE}
+     *            The type of the script, can be either {@link ScriptSortType#STRING} or
+     *            {@link ScriptSortType#NUMBER}
      */
     public ScriptSortBuilder(Script script, ScriptSortType type) {
         Objects.requireNonNull(script, "script cannot be null");

core/src/main/java/org/elasticsearch/search/sort/SortElementParserTemp.java

@@ -1,39 +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.sort;
-
-import org.elasticsearch.index.query.QueryParseContext;
-
-import java.io.IOException;
-
-// TODO once sort refactoring is done this needs to be merged into SortBuilder
-public interface SortElementParserTemp<T extends SortBuilder> {
-    /**
-     * Creates a new SortBuilder from the json held by the {@link SortElementParserTemp}
-     * in {@link org.elasticsearch.common.xcontent.XContent} format
-     *
-     * @param context
-     *            the input parse context. The state on the parser contained in
-     *            this context will be changed as a side effect of this method
-     *            call
-     * @return the new item
-     */
-    T fromXContent(QueryParseContext context, String elementName) throws IOException;
-}

core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java

@@ -34,6 +34,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.sort.SortBuilders;
+import org.elasticsearch.search.sort.SortMode;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.ESIntegTestCase;
 
@@ -748,7 +749,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
-                                .sortMode("sum")
+                                .sortMode(SortMode.SUM)
                                 .order(SortOrder.ASC)
                 )
                 .execute().actionGet();
@@ -768,7 +769,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
-                                .sortMode("sum")
+                                .sortMode(SortMode.SUM)
                                 .order(SortOrder.DESC)
                 )
                 .execute().actionGet();
@@ -789,7 +790,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
                                 .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
-                                .sortMode("sum")
+                                .sortMode(SortMode.SUM)
                                 .order(SortOrder.ASC)
                 )
                 .execute().actionGet();
@@ -809,7 +810,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
-                                .sortMode("avg")
+                                .sortMode(SortMode.AVG)
                                 .order(SortOrder.ASC)
                 )
                 .execute().actionGet();
@@ -828,7 +829,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                 .addSort(
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
-                                .sortMode("avg")
+                                .sortMode(SortMode.AVG)
                                 .order(SortOrder.DESC)
                 )
                 .execute().actionGet();
@@ -849,7 +850,7 @@ public class SimpleNestedIT extends ESIntegTestCase {
                         SortBuilders.fieldSort("parent.child.child_values")
                                 .setNestedPath("parent.child")
                                 .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true))
-                                .sortMode("avg")
+                                .sortMode(SortMode.AVG)
                                 .order(SortOrder.ASC)
                 )
                 .execute().actionGet();

core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.search.sort;
 
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -87,8 +86,6 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderPa
             builder.endObject();
 
             XContentParser itemParser = XContentHelper.createParser(builder.bytes());
-            ParsingException except = new ParsingException(itemParser.getTokenLocation(), "mytext", itemParser.getTokenLocation());
-            System.out.println(except.getMessage());
             itemParser.nextToken();
 
             /*
@@ -158,7 +155,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderPa
             try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
                 T prototype = (T) namedWriteableRegistry.getPrototype(SortBuilder.class,
                         original.getWriteableName());
-                T copy = (T) prototype.readFrom(in);
+                T copy = prototype.readFrom(in);
                 return copy;
             }
         }

core/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java

@@ -19,44 +19,6 @@
 
 package org.elasticsearch.search.sort;
 
-import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
-import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
-import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.fieldValueFactorFunction;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertOrderedSearchHits;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit;
-import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId;
-import static org.hamcrest.Matchers.closeTo;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.lessThan;
-import static org.hamcrest.Matchers.lessThanOrEqualTo;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Locale;
-import java.util.Random;
-import java.util.Set;
-import java.util.TreeMap;
-import java.util.Map.Entry;
-import java.util.concurrent.ExecutionException;
-
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.TestUtil;
@@ -80,6 +42,43 @@ import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.hamcrest.Matchers;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map.Entry;
+import java.util.Random;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.ExecutionException;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
+import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
+import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.fieldValueFactorFunction;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId;
+import static org.hamcrest.Matchers.closeTo;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+
 public class FieldSortIT extends ESIntegTestCase {
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
@@ -985,7 +984,7 @@ public class FieldSortIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
                 .setSize(10)
-                .addSort(SortBuilders.fieldSort("long_values").order(SortOrder.DESC).sortMode("sum"))
+                .addSort(SortBuilders.fieldSort("long_values").order(SortOrder.DESC).sortMode(SortMode.SUM))
                 .execute().actionGet();
 
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(3L));

core/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java

@@ -270,7 +270,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Asc, Mode: max
         searchResponse = client().prepareSearch("test").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max"))
+                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 5);
@@ -296,7 +296,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         // Order: Desc, Mode: min
         searchResponse = client().prepareSearch("test").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min"))
+                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 5);
@@ -308,7 +308,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         assertThat(((Number) searchResponse.getHits().getAt(4).sortValues()[0]).doubleValue(), closeTo(0d, 10d));
 
         searchResponse = client().prepareSearch("test").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC))
+                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 5);
@@ -320,7 +320,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         assertThat(((Number) searchResponse.getHits().getAt(4).sortValues()[0]).doubleValue(), closeTo(5301d, 10d));
 
         searchResponse = client().prepareSearch("test").setQuery(matchAllQuery())
-                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("avg").order(SortOrder.DESC))
+                .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.DESC))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 5);
@@ -333,7 +333,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         try {
                 client().prepareSearch("test").setQuery(matchAllQuery())
-                        .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("sum"));
+                        .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.SUM));
                 fail("sum should not be supported for sorting by geo distance");
         } catch (IllegalArgumentException e) {
             // expected
@@ -455,7 +455,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         // Order: Asc, Mode: max
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max").setNestedPath("branches"))
+                        40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -480,7 +480,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         // Order: Desc, Mode: min
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min").setNestedPath("branches"))
+                        40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -492,7 +492,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         searchResponse = client()
                 .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location",
-                        40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC).setNestedPath("branches"))
+                        40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -504,7 +504,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch("companies")
                 .setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731)
-                        .setNestedPath("branches").sortMode("avg").order(SortOrder.DESC).setNestedPath("branches"))
+                        .setNestedPath("branches").sortMode(SortMode.AVG).order(SortOrder.DESC).setNestedPath("branches"))
                 .execute().actionGet();
 
         assertHitCount(searchResponse, 4);
@@ -517,7 +517,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
                 .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731)
                         .setNestedFilter(termQuery("branches.name", "brooklyn"))
-                        .sortMode("avg").order(SortOrder.ASC).setNestedPath("branches"))
+                        .sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches"))
                 .execute().actionGet();
         assertHitCount(searchResponse, 4);
         assertFirstHit(searchResponse, hasId("4"));
@@ -529,7 +529,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
 
         try {
                 client().prepareSearch("companies").setQuery(matchAllQuery())
-                        .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731).sortMode("sum")
+                        .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731).sortMode(SortMode.SUM)
                                 .setNestedPath("branches"));
                 fail("Sum should not be allowed as sort mode");
         } catch (IllegalArgumentException e) {
@@ -567,11 +567,11 @@ public class GeoDistanceIT extends ESIntegTestCase {
         assertHitCount(result, 1);
     }
 
-    private double randomLon() {
+    private static double randomLon() {
         return randomDouble() * 360 - 180;
     }
 
-    private double randomLat() {
+    private static double randomLat() {
         return randomDouble() * 180 - 90;
     }
 
@@ -619,7 +619,7 @@ public class GeoDistanceIT extends ESIntegTestCase {
         }
     }
 
-    private long assertDuelOptimization(SearchResponse resp) {
+    private static long assertDuelOptimization(SearchResponse resp) {
         long matches = -1;
         assertSearchResponse(resp);
         if (matches < 0) {

core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java

@@ -95,7 +95,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(new GeoDistanceSortBuilder("location", q).sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
         assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 3, 2, DistanceUnit.KILOMETERS), 0.01d));
@@ -103,7 +103,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(new GeoDistanceSortBuilder("location", q).sortMode("min").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MIN).order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d2", "d1");
         assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 1, 5, 1, DistanceUnit.KILOMETERS), 0.01d));
@@ -111,7 +111,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(new GeoDistanceSortBuilder("location", q).sortMode("max").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MAX).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
         assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 4, 1, DistanceUnit.KILOMETERS), 0.01d));
@@ -119,7 +119,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(new GeoDistanceSortBuilder("location", q).sortMode("max").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MAX).order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d2", "d1");
         assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 1, 6, 2, DistanceUnit.KILOMETERS), 0.01d));
@@ -194,7 +194,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
         assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2.5, 1, 2, 1, DistanceUnit.KILOMETERS), 1.e-4));
@@ -202,7 +202,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(geoDistanceSortBuilder.sortMode("max").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(geoDistanceSortBuilder.sortMode(SortMode.MAX).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         assertOrderedSearchHits(searchResponse, "d1", "d2");
         assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(3.25, 4, 2, 1, DistanceUnit.KILOMETERS), 1.e-4));
@@ -223,7 +223,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         checkCorrectSortOrderForGeoSort(searchResponse);
 
@@ -231,7 +231,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         checkCorrectSortOrderForGeoSort(searchResponse);
 
@@ -239,7 +239,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
 
         searchResponse = client().prepareSearch()
                 .setQuery(matchAllQuery())
-                .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
+                .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
                 .execute().actionGet();
         checkCorrectSortOrderForGeoSort(searchResponse);
 
@@ -265,7 +265,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
         checkCorrectSortOrderForGeoSort(searchResponse);
     }
 
-    private void checkCorrectSortOrderForGeoSort(SearchResponse searchResponse) {
+    private static void checkCorrectSortOrderForGeoSort(SearchResponse searchResponse) {
         assertOrderedSearchHits(searchResponse, "d2", "d1");
         assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 1, 2, DistanceUnit.KILOMETERS), 1.e-4));
         assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 1, 1, DistanceUnit.KILOMETERS), 1.e-4));

core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java

@@ -27,7 +27,6 @@ import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryParseContext;
-import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.geo.RandomGeoGenerator;
 
 import java.io.IOException;
@@ -90,16 +89,15 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         return result;
     }
 
-    private static String mode(String original) {
-        String[] modes = {"MIN", "MAX", "AVG"};
-        String mode = ESTestCase.randomFrom(modes);
-        while (mode.equals(original)) {
-            mode = ESTestCase.randomFrom(modes);
-        }
-        return mode;
+    private static SortMode mode(SortMode original) {
+        SortMode result;
+        do {
+            result = randomFrom(SortMode.values());
+        } while (result == SortMode.SUM || result == original);
+        return result;
     }
 
-    private DistanceUnit unit(DistanceUnit original) {
+    private static DistanceUnit unit(DistanceUnit original) {
         int id = -1;
         while (id == -1 || (original != null && original.ordinal() == id)) {
             id = randomIntBetween(0, DistanceUnit.values().length - 1);
@@ -107,7 +105,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         return DistanceUnit.values()[id];
     }
 
-    private GeoPoint[] points(GeoPoint[] original) {
+    private static GeoPoint[] points(GeoPoint[] original) {
         GeoPoint[] result = null;
         while (result == null || Arrays.deepEquals(original, result)) {
             int count = randomIntBetween(1, 10);
@@ -119,7 +117,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         return result;
     }
 
-    private GeoDistance geoDistance(GeoDistance original) {
+    private static GeoDistance geoDistance(GeoDistance original) {
         int id = -1;
         while (id == -1 || (original != null && original.ordinal() == id)) {
             id = randomIntBetween(0, GeoDistance.values().length - 1);
@@ -177,7 +175,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
         GeoPoint point = RandomGeoGenerator.randomPoint(getRandom());
         builder.point(point.getLat(), point.getLon());
         try {
-            builder.sortMode("SUM");
+            builder.sortMode(SortMode.SUM);
             fail("sort mode sum should not be supported");
           } catch (IllegalArgumentException e) {
               // all good

core/src/test/java/org/elasticsearch/search/sort/SortModeTest.java → core/src/test/java/org/elasticsearch/search/sort/SortModeTests.java

@@ -23,7 +23,9 @@ import org.elasticsearch.test.ESTestCase;
 import org.junit.Rule;
 import org.junit.rules.ExpectedException;
 
-public class SortModeTest extends ESTestCase {
+import java.util.Locale;
+
+public class SortModeTests extends ESTestCase {
 
     @Rule
     public ExpectedException exceptionRule = ExpectedException.none();
@@ -44,7 +46,7 @@ public class SortModeTest extends ESTestCase {
 
         for (SortMode mode : SortMode.values()) {
             assertEquals(mode, SortMode.fromString(mode.toString()));
-            assertEquals(mode, SortMode.fromString(mode.toString().toUpperCase()));
+            assertEquals(mode, SortMode.fromString(mode.toString().toUpperCase(Locale.ROOT)));
         }
     }
 

docs/reference/migration/migrate_5_0/java.asciidoc

@@ -211,3 +211,7 @@ For simplicity, only one way of adding the ids to the existing list (empty by de
 
 The inner DirectCandidateGenerator class has been moved out to its own class called DirectCandidateGeneratorBuilder.
 
+===== SortBuilders
+
+The `sortMode` setter in `FieldSortBuilder`, `GeoDistanceSortBuilder` and `ScriptSortBuilder` now
+accept a `SortMode` enum instead of a String constant. Also the getter returns the same enum type.