Sort: Move up `order` field to SortBuilder

Currently all SortBuilder implementations have their separate order
field. This PR moves this up to SortBuilder, together with setter
and getter and makes sure the default is set to SortOrder.ASC
except for `_score` sorting where the default is SortOrder.DESC.

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

@@ -27,12 +27,10 @@ import java.io.IOException;
 /**
  * A sort builder to sort based on a document field.
  */
-public class FieldSortBuilder extends SortBuilder {
+public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     private final String fieldName;
 
-    private SortOrder order;
-
     private Object missing;
 
     private String unmappedType;
@@ -55,15 +53,6 @@ public class FieldSortBuilder extends SortBuilder {
         this.fieldName = fieldName;
     }
 
-    /**
-     * The order of sorting. Defaults to {@link SortOrder#ASC}.
-     */
-    @Override
-    public FieldSortBuilder order(SortOrder order) {
-        this.order = order;
-        return this;
-    }
-
     /**
      * Sets the value when a field is missing in a doc. Can also be set to <tt>_last</tt> or
      * <tt>_first</tt> to sort missing last or first respectively.
@@ -118,9 +107,7 @@ public class FieldSortBuilder extends SortBuilder {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(fieldName);
-        if (order != null) {
+        builder.field(ORDER_FIELD.getPreferredName(), order);
-            builder.field("order", order.toString());
-        }
         if (missing != null) {
             builder.field("missing", missing);
         }

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

@@ -44,7 +44,7 @@ import java.util.Objects;
 /**
  * A geo distance based sorting on a geo point like field.
  */
-public class GeoDistanceSortBuilder extends SortBuilder
+public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
         implements ToXContent, NamedWriteable<GeoDistanceSortBuilder>, SortElementParserTemp<GeoDistanceSortBuilder> {
     public static final String NAME = "_geo_distance";
     public static final boolean DEFAULT_COERCE = false;
@@ -57,14 +57,13 @@ public class GeoDistanceSortBuilder extends SortBuilder
 
     private GeoDistance geoDistance = GeoDistance.DEFAULT;
     private DistanceUnit unit = DistanceUnit.DEFAULT;
-    private SortOrder order = SortOrder.ASC;
+
-    
     // TODO there is an enum that covers that parameter which we should be using here
     private String sortMode = null;
     @SuppressWarnings("rawtypes")
     private QueryBuilder nestedFilter;
     private String nestedPath;
-    
+
     // TODO switch to GeoValidationMethod enum
     private boolean coerce = DEFAULT_COERCE;
     private boolean ignoreMalformed = DEFAULT_IGNORE_MALFORMED;
@@ -109,7 +108,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         }
         this.fieldName = fieldName;
     }
-    
+
     /**
      * Copy constructor.
      * */
@@ -125,7 +124,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         this.coerce = original.coerce;
         this.ignoreMalformed = original.ignoreMalformed;
     }
-    
+
     /**
      * Returns the geo point like field the distance based sort operates on.
      * */
@@ -153,7 +152,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         this.points.addAll(Arrays.asList(points));
         return this;
     }
-    
+
     /**
      * Returns the points to create the range distance facets from.
      */
@@ -163,7 +162,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
 
     /**
      * The geohash of the geo point to create the range distance facets from.
-     * 
+     *
      * Deprecated - please use points(GeoPoint... points) instead.
      */
     @Deprecated
@@ -173,7 +172,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         }
         return this;
     }
-    
+
     /**
      * The geo distance type used to compute the distance.
      */
@@ -181,7 +180,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         this.geoDistance = geoDistance;
         return this;
     }
-    
+
     /**
      * Returns the geo distance type used to compute the distance.
      */
@@ -204,20 +203,6 @@ public class GeoDistanceSortBuilder extends SortBuilder
         return this.unit;
     }
 
-    /**
-     * The order of sorting. Defaults to {@link SortOrder#ASC}.
-     */
-    @Override
-    public GeoDistanceSortBuilder order(SortOrder order) {
-        this.order = order;
-        return this;
-    }
-
-    /** Returns the order of sorting. */
-    public SortOrder order() { 
-        return this.order;
-    }
-
     /**
      * Defines which distance to use for sorting in the case a document contains multiple geo points.
      * Possible values: min and max
@@ -240,16 +225,16 @@ public class GeoDistanceSortBuilder extends SortBuilder
      * Sets the nested filter that the nested objects should match with in order to be taken into account
      * for sorting.
      */
-    public GeoDistanceSortBuilder setNestedFilter(QueryBuilder nestedFilter) {
+    public GeoDistanceSortBuilder setNestedFilter(QueryBuilder<?> nestedFilter) {
         this.nestedFilter = nestedFilter;
         return this;
     }
 
-    /** 
+    /**
      * Returns the nested filter that the nested objects should match with in order to be taken into account
-     * for sorting. 
+     * for sorting.
      **/
-    public QueryBuilder getNestedFilter() {
+    public QueryBuilder<?> getNestedFilter() {
         return this.nestedFilter;
     }
 
@@ -261,7 +246,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         this.nestedPath = nestedPath;
         return this;
     }
-    
+
     /**
      * Returns the nested path if sorting occurs on a field that is inside a nested object. By default when sorting on a
      * field inside a nested object, the nearest upper nested object is selected as nested path.
@@ -285,7 +270,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         }
         return this;
     }
-    
+
     public boolean ignoreMalformed() {
         return this.ignoreMalformed;
     }
@@ -302,11 +287,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
 
         builder.field("unit", unit);
         builder.field("distance_type", geoDistance.name().toLowerCase(Locale.ROOT));
-        if (order == SortOrder.DESC) {
+        builder.field(ORDER_FIELD.getPreferredName(), order);
-            builder.field("reverse", true);
-        } else {
-            builder.field("reverse", false);
-        }
 
         if (sortMode != null) {
             builder.field("mode", sortMode);
@@ -363,7 +344,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
         out.writeGenericValue(points);
-        
+
         geoDistance.writeTo(out);
         unit.writeTo(out);
         order.writeTo(out);
@@ -382,10 +363,10 @@ public class GeoDistanceSortBuilder extends SortBuilder
     @Override
     public GeoDistanceSortBuilder readFrom(StreamInput in) throws IOException {
         String fieldName = in.readString();
-        
+
-        ArrayList<GeoPoint> points = (ArrayList<GeoPoint>) in.readGenericValue(); 
+        ArrayList<GeoPoint> points = (ArrayList<GeoPoint>) in.readGenericValue();
         GeoDistanceSortBuilder result = new GeoDistanceSortBuilder(fieldName, points.toArray(new GeoPoint[points.size()]));
-        
+
         result.geoDistance(GeoDistance.readGeoDistanceFrom(in));
         result.unit(DistanceUnit.readDistanceUnit(in));
         result.order(SortOrder.readOrderFrom(in));
@@ -409,9 +390,9 @@ public class GeoDistanceSortBuilder extends SortBuilder
         List<GeoPoint> geoPoints = new ArrayList<>();
         DistanceUnit unit = DistanceUnit.DEFAULT;
         GeoDistance geoDistance = GeoDistance.DEFAULT;
-        boolean reverse = false;
+        SortOrder order = SortOrder.ASC;
         MultiValueMode sortMode = null;
-        QueryBuilder nestedFilter = null;
+        QueryBuilder<?> nestedFilter = null;
         String nestedPath = null;
 
         boolean coerce = GeoDistanceSortBuilder.DEFAULT_COERCE;
@@ -429,8 +410,8 @@ public class GeoDistanceSortBuilder extends SortBuilder
             } else if (token == XContentParser.Token.START_OBJECT) {
                 // the json in the format of -> field : { lat : 30, lon : 12 }
                 if ("nested_filter".equals(currentName) || "nestedFilter".equals(currentName)) {
-                    // TODO Note to remember: while this is kept as a QueryBuilder internally, 
+                    // TODO Note to remember: while this is kept as a QueryBuilder internally,
-                    // we need to make sure to call toFilter() on it once on the shard 
+                    // we need to make sure to call toFilter() on it once on the shard
                     // (e.g. in the new build() method)
                     nestedFilter = context.parseInnerQueryBuilder();
                 } else {
@@ -441,9 +422,9 @@ public class GeoDistanceSortBuilder extends SortBuilder
                 }
             } else if (token.isValue()) {
                 if ("reverse".equals(currentName)) {
-                    reverse = parser.booleanValue();
+                    order = parser.booleanValue() ? SortOrder.DESC : SortOrder.ASC;
                 } else if ("order".equals(currentName)) {
-                    reverse = "desc".equals(parser.text());
+                    order = SortOrder.fromString(parser.text());
                 } else if ("unit".equals(currentName)) {
                     unit = DistanceUnit.fromString(parser.text());
                 } else if ("distance_type".equals(currentName) || "distanceType".equals(currentName)) {
@@ -474,11 +455,7 @@ public class GeoDistanceSortBuilder extends SortBuilder
         GeoDistanceSortBuilder result = new GeoDistanceSortBuilder(fieldName, geoPoints.toArray(new GeoPoint[geoPoints.size()]));
         result.geoDistance(geoDistance);
         result.unit(unit);
-        if (reverse) {
+        result.order(order);
-            result.order(SortOrder.DESC);
-        } else {
-            result.order(SortOrder.ASC);
-        }
         if (sortMode != null) {
             result.sortMode(sortMode.name());
         }

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

@@ -35,38 +35,24 @@ import java.util.Objects;
 /**
  * A sort builder allowing to sort by score.
  */
-public class ScoreSortBuilder extends SortBuilder implements NamedWriteable<ScoreSortBuilder>,
+public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements NamedWriteable<ScoreSortBuilder>,
     SortElementParserTemp<ScoreSortBuilder> {
 
     private static final String NAME = "_score";
     static final ScoreSortBuilder PROTOTYPE = new ScoreSortBuilder();
     public static final ParseField REVERSE_FIELD = new ParseField("reverse");
     public static final ParseField ORDER_FIELD = new ParseField("order");
-    private SortOrder order = SortOrder.DESC;
 
-    /**
+    public ScoreSortBuilder() {
-     * The order of sort scoring. By default, its {@link SortOrder#DESC}.
+        // order defaults to desc when sorting on the _score
-     */
+        order(SortOrder.DESC);
-    @Override
-    public ScoreSortBuilder order(SortOrder order) {
-        Objects.requireNonNull(order, "sort order cannot be null.");
-        this.order = order;
-        return this;
     }
 
-    /**
-     * Get the order of sort scoring. By default, its {@link SortOrder#DESC}.
-     */
-    public SortOrder order() {
-        return this.order;
-    }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAME);
-        if (order == SortOrder.ASC) {
+        builder.field(ORDER_FIELD.getPreferredName(), order);
-            builder.field(REVERSE_FIELD.getPreferredName(), true);
-        }
         builder.endObject();
         return builder;
     }
@@ -124,7 +110,8 @@ public class ScoreSortBuilder extends SortBuilder implements NamedWriteable<Scor
 
     @Override
     public ScoreSortBuilder readFrom(StreamInput in) throws IOException {
-        return new ScoreSortBuilder().order(SortOrder.readOrderFrom(in));
+        ScoreSortBuilder builder = new ScoreSortBuilder().order(SortOrder.readOrderFrom(in));
+        return builder;
     }
 
     @Override

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

@@ -28,14 +28,12 @@ import java.io.IOException;
 /**
  * Script sort builder allows to sort based on a custom script expression.
  */
-public class ScriptSortBuilder extends SortBuilder {
+public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
 
     private Script script;
 
     private final String type;
 
-    private SortOrder order;
-
     private String sortMode;
 
     private QueryBuilder nestedFilter;
@@ -53,15 +51,6 @@ public class ScriptSortBuilder extends SortBuilder {
         this.type = type;
     }
 
-    /**
-     * Sets the sort order.
-     */
-    @Override
-    public ScriptSortBuilder order(SortOrder order) {
-        this.order = order;
-        return this;
-    }
-
     /**
      * Defines which distance to use for sorting in the case a document contains multiple geo points.
      * Possible values: min and max
@@ -75,7 +64,7 @@ public class ScriptSortBuilder extends SortBuilder {
      * Sets the nested filter that the nested objects should match with in order to be taken into account
      * for sorting.
      */
-    public ScriptSortBuilder setNestedFilter(QueryBuilder nestedFilter) {
+    public ScriptSortBuilder setNestedFilter(QueryBuilder<?> nestedFilter) {
         this.nestedFilter = nestedFilter;
         return this;
     }
@@ -94,9 +83,7 @@ public class ScriptSortBuilder extends SortBuilder {
         builder.startObject("_script");
         builder.field("script", script);
         builder.field("type", type);
-        if (order == SortOrder.DESC) {
+        builder.field(ORDER_FIELD.getPreferredName(), order);
-            builder.field("reverse", true);
-        }
         if (sortMode != null) {
             builder.field("mode", sortMode);
         }

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

@@ -20,14 +20,20 @@
 package org.elasticsearch.search.sort;
 
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 
+import java.util.Objects;
+
 /**
  *
  */
-public abstract class SortBuilder implements ToXContent {
+public abstract class SortBuilder<T extends SortBuilder<?>> implements ToXContent {
+
+    protected SortOrder order = SortOrder.ASC;
+    public static final ParseField ORDER_FIELD = new ParseField("order");
 
     @Override
     public String toString() {
@@ -42,7 +48,19 @@ public abstract class SortBuilder implements ToXContent {
     }
 
     /**
-     * The order of sorting. Defaults to {@link SortOrder#ASC}.
+     * Set the order of sorting.
      */
-    public abstract SortBuilder order(SortOrder order);
+    @SuppressWarnings("unchecked")
+    public T order(SortOrder order) {
+        Objects.requireNonNull(order, "sort order cannot be null.");
+        this.order = order;
+        return (T) this;
+    }
+
+    /**
+     * Return the {@link SortOrder} used for this {@link SortBuilder}.
+     */
+    public SortOrder order() {
+        return this.order;
+    }
 }

core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java

@@ -49,15 +49,9 @@ import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.EnvironmentModule;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.query.AbstractQueryTestCase;
-import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.EmptyQueryBuilder;
-import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.index.query.QueryParseContext;
-import org.elasticsearch.index.query.QueryRewriteContext;
-import org.elasticsearch.index.query.TermQueryBuilder;
-import org.elasticsearch.index.query.WrapperQueryBuilder;
-import org.elasticsearch.index.query.functionscore.ScoreFunctionParser;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
@@ -554,7 +548,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
                 SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser),
                         aggParsers);
                 assertEquals(1, searchSourceBuilder.sorts().size());
-                assertEquals("{\"foo\":{}}", searchSourceBuilder.sorts().get(0).toUtf8());
+                assertEquals("{\"foo\":{\"order\":\"asc\"}}", searchSourceBuilder.sorts().get(0).toUtf8());
             }
         }