Add sanity checks and support for reverse in FieldSortBuilder.parse(...)

After another round of input from @cbuescher this adds a few more sanity
checks to request parsing.

In addition adds (back) support for the reverse option.

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

@@ -35,14 +35,14 @@ import java.util.Objects;
 /**
  * A sort builder to sort based on a document field.
  */
-public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<FieldSortBuilder> {
+public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements SortBuilderParser<FieldSortBuilder> {
-    public static final String NAME = "field_sort";
     static final FieldSortBuilder PROTOTYPE = new FieldSortBuilder("");
     public static final String NAME = "field_sort";
     public static final ParseField NESTED_PATH = new ParseField("nested_path");
     public static final ParseField NESTED_FILTER = new ParseField("nested_filter");
     public static final ParseField MISSING = new ParseField("missing");
     public static final ParseField ORDER = new ParseField("order");
+    public static final ParseField REVERSE = new ParseField("reverse");
     public static final ParseField SORT_MODE = new ParseField("mode");
     public static final ParseField UNMAPPED_TYPE = new ParseField("unmapped_type");
 
@@ -304,12 +304,19 @@ public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<Fie
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if (context.parseFieldMatcher().match(currentFieldName, NESTED_FILTER)) {
                     nestedFilter = context.parseInnerQueryBuilder();
+                } else {
+                    throw new IllegalArgumentException("Expected " + NESTED_FILTER.getPreferredName() + " element.");
                 }
             } else if (token.isValue()) {
                 if (context.parseFieldMatcher().match(currentFieldName, NESTED_PATH)) {
                     nestedPath = parser.text();
                 } else if (context.parseFieldMatcher().match(currentFieldName, MISSING)) {
                     missing = parser.objectBytes();
+                } else if (context.parseFieldMatcher().match(currentFieldName, REVERSE)) {
+                    if (parser.booleanValue()) {
+                        order = SortOrder.DESC;
+                    }
+                    // else we keep the default ASC
                 } else if (context.parseFieldMatcher().match(currentFieldName, ORDER)) {
                     String sortOrder = parser.text();
                     if ("asc".equals(sortOrder)) {
@@ -323,6 +330,8 @@ public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<Fie
                     sortMode = parser.text();
                 } else if (context.parseFieldMatcher().match(currentFieldName, UNMAPPED_TYPE)) {
                     unmappedType = parser.text();
+                } else {
+                    throw new IllegalArgumentException("Option " + currentFieldName + " not supported.");
                 }
             }
         }

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

@@ -42,7 +42,7 @@ import java.util.Objects;
 /**
  * A geo distance based sorting on a geo point like field.
  */
-public class GeoDistanceSortBuilder extends SortBuilder implements SortBuilderTemp<GeoDistanceSortBuilder> {
+public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> implements SortBuilderParser<GeoDistanceSortBuilder> {
     public static final String NAME = "_geo_distance";
     public static final boolean DEFAULT_COERCE = false;
     public static final boolean DEFAULT_IGNORE_MALFORMED = false;

core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.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.xcontent.XContentBuilder;
@@ -35,7 +34,7 @@ import java.util.Objects;
 /**
  * A sort builder allowing to sort by score.
  */
-public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements NamedWriteable<ScoreSortBuilder>,
+public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements SortBuilderParser<ScoreSortBuilder>,
     SortElementParserTemp<ScoreSortBuilder> {
 
     private static final String NAME = "_score";

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

@@ -25,9 +25,9 @@ import org.elasticsearch.index.query.QueryParseContext;
 
 import java.io.IOException;
 
-public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<T>, ToXContent {
+public interface SortBuilderParser<T extends ToXContent> extends NamedWriteable<T>, ToXContent {
     /**
-     * Creates a new item from the json held by the {@link SortBuilderTemp}
+     * Creates a new item from the json held by the {@link SortBuilderParser}
      * in {@link org.elasticsearch.common.xcontent.XContent} format
      *
      * @param context
@@ -36,5 +36,5 @@ public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<T>
      *            call
      * @return the new item
      */
-    SortBuilderTemp<T> fromXContent(QueryParseContext context, String elementName) throws IOException;
+    SortBuilder fromXContent(QueryParseContext context, String elementName) throws IOException;
 }

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

@@ -42,7 +42,7 @@ import java.io.IOException;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 
-public abstract class AbstractSortTestCase<T extends SortBuilderTemp<T>> extends ESTestCase {
+public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderParser<T>> extends ESTestCase {
 
     protected static NamedWriteableRegistry namedWriteableRegistry;
 
@@ -54,6 +54,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilderTemp<T>> extends
         namedWriteableRegistry = new NamedWriteableRegistry();
         namedWriteableRegistry.registerPrototype(SortBuilder.class, GeoDistanceSortBuilder.PROTOTYPE);
         namedWriteableRegistry.registerPrototype(SortBuilder.class, ScoreSortBuilder.PROTOTYPE);
+        namedWriteableRegistry.registerPrototype(SortBuilder.class, FieldSortBuilder.PROTOTYPE);
         indicesQueriesRegistry = new SearchModule(Settings.EMPTY, namedWriteableRegistry).buildQueryParserRegistry();
     }