Use ObjectParser in ScriptSortBuilder

core/src/main/java/org/elasticsearch/script/Script.java

@@ -23,6 +23,7 @@ import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParseFieldMatcher;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -32,6 +33,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.script.ScriptService.ScriptType;
 
 import java.io.IOException;
@@ -189,6 +191,14 @@ public final class Script implements ToXContent, Writeable {
         return parse(parser, parseFieldMatcher, null);
     }
 
+    public static Script parse(XContentParser parser, QueryParseContext context) {
+        try {
+            return parse(parser, context.getParseFieldMatcher(), context.getDefaultScriptLanguage());
+        } catch (IOException e) {
+            throw new ParsingException(parser.getTokenLocation(), "Error parsing [" + ScriptField.SCRIPT.getPreferredName() + "] field", e);
+        }
+    }
+
     public static Script parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher, @Nullable String lang) throws IOException {
         XContentParser.Token token = parser.currentToken();
         // If the parser hasn't yet been pushed to the first token, do it now

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

@@ -21,7 +21,6 @@ package org.elasticsearch.search.sort;
 
 import org.apache.lucene.search.SortField;
 import org.elasticsearch.common.ParseField;
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -46,10 +45,7 @@ import java.util.Objects;
  */
 public class FieldSortBuilder extends SortBuilder<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 SORT_MODE = new ParseField("mode");
     public static final ParseField UNMAPPED_TYPE = new ParseField("unmapped_type");
 
@@ -239,10 +235,10 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
             builder.field(SORT_MODE.getPreferredName(), sortMode);
         }
         if (nestedFilter != null) {
-            builder.field(NESTED_FILTER.getPreferredName(), nestedFilter, params);
+            builder.field(NESTED_FILTER_FIELD.getPreferredName(), nestedFilter, params);
         }
         if (nestedPath != null) {
-            builder.field(NESTED_PATH.getPreferredName(), nestedPath);
+            builder.field(NESTED_PATH_FIELD.getPreferredName(), nestedPath);
         }
         builder.endObject();
         builder.endObject();
@@ -334,17 +330,10 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     static {
         PARSER.declareField(FieldSortBuilder::missing, p -> p.objectText(),  MISSING, ValueType.VALUE);
-        PARSER.declareString(FieldSortBuilder::setNestedPath , NESTED_PATH);
+        PARSER.declareString(FieldSortBuilder::setNestedPath , NESTED_PATH_FIELD);
         PARSER.declareString(FieldSortBuilder::unmappedType , UNMAPPED_TYPE);
-        PARSER.declareField(FieldSortBuilder::order, p -> SortOrder.fromString(p.text()), ORDER_FIELD, ValueType.STRING);
-        PARSER.declareField(FieldSortBuilder::sortMode, p -> SortMode.fromString(p.text()), SORT_MODE, ValueType.STRING);
-        PARSER.declareObject(FieldSortBuilder::setNestedFilter,  (p, c) -> {
-            try {
-                QueryBuilder builder = c.parseInnerQueryBuilder().orElseThrow(
-                        () -> new ParsingException(p.getTokenLocation(), "Expected " + NESTED_FILTER.getPreferredName() + " element."));
-                return builder;
-            } catch (Exception e) {
-                throw new ParsingException(p.getTokenLocation(), "Expected " + NESTED_FILTER.getPreferredName() + " element.", e);
-            }}, NESTED_FILTER);
+        PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)) , ORDER_FIELD);
+        PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORT_MODE);
+        PARSER.declareObject(FieldSortBuilder::setNestedFilter, SortBuilder::parseNestedFilter, NESTED_FILTER_FIELD);
     }
 }

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

@@ -80,8 +80,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
             .withAllDeprecated("use validation_method instead");
     private static final ParseField SORTMODE_FIELD = new ParseField("mode", "sort_mode");
-    private static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
-    private static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");
 
     private final String fieldName;
     private final List<GeoPoint> points = new ArrayList<>();
@@ -511,7 +509,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
     public SortFieldAndFormat build(QueryShardContext context) throws IOException {
         final boolean indexCreatedBeforeV2_0 = context.indexVersionCreated().before(Version.V_2_0_0);
         // validation was not available prior to 2.x, so to support bwc percolation queries we only ignore_malformed on 2.x created indexes
-        List<GeoPoint> localPoints = new ArrayList<GeoPoint>();
+        List<GeoPoint> localPoints = new ArrayList<>();
         for (GeoPoint geoPoint : this.points) {
             localPoints.add(new GeoPoint(geoPoint));
         }

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

@@ -20,11 +20,9 @@
 package org.elasticsearch.search.sort;
 
 import org.apache.lucene.search.SortField;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ObjectParser;
-import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
@@ -39,7 +37,6 @@ import java.util.Objects;
 public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> {
 
     public static final String NAME = "_score";
-    public static final ParseField ORDER_FIELD = new ParseField("order");
     private static final SortFieldAndFormat SORT_SCORE = new SortFieldAndFormat(
             new SortField(null, SortField.Type.SCORE), DocValueFormat.RAW);
     private static final SortFieldAndFormat SORT_SCORE_REVERSE = new SortFieldAndFormat(
@@ -91,7 +88,7 @@ public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> {
     private static ObjectParser<ScoreSortBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME, ScoreSortBuilder::new);
 
     static {
-        PARSER.declareField(ScoreSortBuilder::order, p -> SortOrder.fromString(p.text()), ORDER_FIELD, ValueType.STRING);
+        PARSER.declareString((builder, order) -> builder.order(SortOrder.fromString(order)), ORDER_FIELD);
     }
 
     @Override

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

@@ -26,13 +26,12 @@ import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 import org.elasticsearch.common.ParseField;
-import org.elasticsearch.common.ParseFieldMatcher;
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
+import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
@@ -57,7 +56,8 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.Locale;
 import java.util.Objects;
-import java.util.Optional;
+
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
 /**
  * Script sort builder allows to sort based on a custom script expression.
@@ -68,8 +68,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
     public static final ParseField TYPE_FIELD = new ParseField("type");
     public static final ParseField SCRIPT_FIELD = new ParseField("script");
     public static final ParseField SORTMODE_FIELD = new ParseField("mode");
-    public static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
-    public static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");
 
     private final Script script;
 
@@ -216,6 +214,18 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         return builder;
     }
 
+    private static ConstructingObjectParser<ScriptSortBuilder, QueryParseContext> PARSER = new ConstructingObjectParser<>(NAME,
+            a -> new ScriptSortBuilder((Script) a[0], (ScriptSortType) a[1]));
+
+    static {
+        PARSER.declareField(constructorArg(), Script::parse, ScriptField.SCRIPT, ValueType.OBJECT_OR_STRING);
+        PARSER.declareField(constructorArg(), p -> ScriptSortType.fromString(p.text()), TYPE_FIELD, ValueType.STRING);
+        PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)), ORDER_FIELD);
+        PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORTMODE_FIELD);
+        PARSER.declareString(ScriptSortBuilder::setNestedPath , NESTED_PATH_FIELD);
+        PARSER.declareObject(ScriptSortBuilder::setNestedFilter, SortBuilder::parseNestedFilter, NESTED_FILTER_FIELD);
+    }
+
     /**
      * Creates a new {@link ScriptSortBuilder} from the query held by the {@link QueryParseContext} in
      * {@link org.elasticsearch.common.xcontent.XContent} format.
@@ -226,59 +236,7 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
      *        in '{ "foo": { "order" : "asc"} }'. When parsing the inner object, the field name can be passed in via this argument
      */
     public static ScriptSortBuilder fromXContent(QueryParseContext context, String elementName) throws IOException {
-        XContentParser parser = context.parser();
-        ParseFieldMatcher parseField = context.getParseFieldMatcher();
-        Script script = null;
-        ScriptSortType type = null;
-        SortMode sortMode = null;
-        SortOrder order = null;
-        Optional<QueryBuilder> nestedFilter = Optional.empty();
-        String nestedPath = null;
-
-        XContentParser.Token token;
-        String currentName = parser.currentName();
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                currentName = parser.currentName();
-            } else if (token == XContentParser.Token.START_OBJECT) {
-                if (parseField.match(currentName, ScriptField.SCRIPT)) {
-                    script = Script.parse(parser, parseField, context.getDefaultScriptLanguage());
-                } else if (parseField.match(currentName, NESTED_FILTER_FIELD)) {
-                    nestedFilter = context.parseInnerQueryBuilder();
-                } else {
-                    throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] failed to parse field [" + currentName + "]");
-                }
-            } else if (token.isValue()) {
-                if (parseField.match(currentName, ORDER_FIELD)) {
-                    order = SortOrder.fromString(parser.text());
-                } else if (parseField.match(currentName, TYPE_FIELD)) {
-                    type = ScriptSortType.fromString(parser.text());
-                } else if (parseField.match(currentName, SORTMODE_FIELD)) {
-                    sortMode = SortMode.fromString(parser.text());
-                } else if (parseField.match(currentName, NESTED_PATH_FIELD)) {
-                    nestedPath = parser.text();
-                } else if (parseField.match(currentName, ScriptField.SCRIPT)) {
-                    script = Script.parse(parser, parseField, context.getDefaultScriptLanguage());
-                } else {
-                    throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] failed to parse field [" + currentName + "]");
-                }
-            } else {
-                throw new ParsingException(parser.getTokenLocation(), "[" + NAME + "] unexpected token [" + token + "]");
-            }
-        }
-
-        ScriptSortBuilder result = new ScriptSortBuilder(script, type);
-        if (order != null) {
-            result.order(order);
-        }
-        if (sortMode != null) {
-            result.sortMode(sortMode);
-        }
-        nestedFilter.ifPresent(result::setNestedFilter);
-        if (nestedPath != null) {
-            result.setNestedPath(nestedPath);
-        }
-        return result;
+        return PARSER.apply(context.parser(), context);
     }
 
 

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

@@ -25,6 +25,7 @@ import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.join.BitSetProducer;
 import org.elasticsearch.action.support.ToXContentToBytes;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.NamedWriteable;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -49,7 +50,11 @@ import static java.util.Collections.unmodifiableMap;
 public abstract class SortBuilder<T extends SortBuilder<T>> extends ToXContentToBytes implements NamedWriteable {
 
     protected SortOrder order = SortOrder.ASC;
+
+    // parse fields common to more than one SortBuilder
     public static final ParseField ORDER_FIELD = new ParseField("order");
+    public static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");
+    public static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
 
     private static final Map<String, Parser<?>> PARSERS;
     static {
@@ -196,6 +201,16 @@ public abstract class SortBuilder<T extends SortBuilder<T>> extends ToXContentTo
         return nested;
     }
 
+    protected static QueryBuilder parseNestedFilter(XContentParser parser, QueryParseContext context) {
+        try {
+            QueryBuilder builder = context.parseInnerQueryBuilder().orElseThrow(() -> new ParsingException(parser.getTokenLocation(),
+                    "Expected " + NESTED_FILTER_FIELD.getPreferredName() + " element."));
+            return builder;
+        } catch (Exception e) {
+            throw new ParsingException(parser.getTokenLocation(), "Expected " + NESTED_FILTER_FIELD.getPreferredName() + " element.", e);
+        }
+    }
+
     @FunctionalInterface
     private interface Parser<T extends SortBuilder<?>> {
         T fromXContent(QueryParseContext context, String elementName) throws IOException;

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

@@ -41,10 +41,10 @@ import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
 import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.mapper.ContentPath;
-import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.index.mapper.ObjectMapper;
 import org.elasticsearch.index.mapper.LegacyDoubleFieldMapper.DoubleFieldType;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper.BuilderContext;
+import org.elasticsearch.index.mapper.ObjectMapper;
 import org.elasticsearch.index.mapper.ObjectMapper.Nested;
 import org.elasticsearch.index.query.IdsQueryBuilder;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
@@ -245,7 +245,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilder<T>> extends EST
             @Override
             public ObjectMapper getObjectMapper(String name) {
                 BuilderContext context = new BuilderContext(this.getIndexSettings().getSettings(), new ContentPath());
-                return (ObjectMapper) new ObjectMapper.Builder<>(name).nested(Nested.newNested(false, false)).build(context);
+                return new ObjectMapper.Builder<>(name).nested(Nested.newNested(false, false)).build(context);
             }
         };
     }

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

@@ -22,7 +22,6 @@ package org.elasticsearch.search.sort;
 
 import org.apache.lucene.search.SortField;
 import org.elasticsearch.common.ParseFieldMatcher;
-import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryParseContext;
@@ -30,8 +29,6 @@ import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService.ScriptType;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType;
-import org.junit.Rule;
-import org.junit.rules.ExpectedException;
 
 import java.io.IOException;
 import java.util.HashSet;
@@ -146,19 +143,14 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         assertEquals(ScriptSortType.NUMBER, ScriptSortType.fromString("NUMBER"));
     }
 
-    @Rule
-    public ExpectedException exceptionRule = ExpectedException.none();
-
     public void testScriptSortTypeNull() {
-        exceptionRule.expect(NullPointerException.class);
-        exceptionRule.expectMessage("input string is null");
-        ScriptSortType.fromString(null);
+        Exception e = expectThrows(NullPointerException.class, () -> ScriptSortType.fromString(null));
+        assertEquals("input string is null", e.getMessage());
     }
 
     public void testScriptSortTypeIllegalArgument() {
-        exceptionRule.expect(IllegalArgumentException.class);
-        exceptionRule.expectMessage("Unknown ScriptSortType [xyz]");
-        ScriptSortType.fromString("xyz");
+        Exception e = expectThrows(IllegalArgumentException.class, () -> ScriptSortType.fromString("xyz"));
+        assertEquals("Unknown ScriptSortType [xyz]", e.getMessage());
     }
 
     public void testParseJson() throws IOException {
@@ -226,9 +218,8 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         parser.nextToken();
 
         QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT);
-        exceptionRule.expect(ParsingException.class);
-        exceptionRule.expectMessage("failed to parse field [bad_field]");
-        ScriptSortBuilder.fromXContent(context, null);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> ScriptSortBuilder.fromXContent(context, null));
+        assertEquals("[_script] unknown field [bad_field], parser not found", e.getMessage());
     }
 
     public void testParseBadFieldNameExceptionsOnStartObject() throws IOException {
@@ -240,9 +231,8 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         parser.nextToken();
 
         QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT);
-        exceptionRule.expect(ParsingException.class);
-        exceptionRule.expectMessage("failed to parse field [bad_field]");
-        ScriptSortBuilder.fromXContent(context, null);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> ScriptSortBuilder.fromXContent(context, null));
+        assertEquals("[_script] unknown field [bad_field], parser not found", e.getMessage());
     }
 
     public void testParseUnexpectedToken() throws IOException {
@@ -253,9 +243,8 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
         parser.nextToken();
 
         QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT);
-        exceptionRule.expect(ParsingException.class);
-        exceptionRule.expectMessage("unexpected token [START_ARRAY]");
-        ScriptSortBuilder.fromXContent(context, null);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> ScriptSortBuilder.fromXContent(context, null));
+        assertEquals("[_script] script doesn't support values of type: START_ARRAY", e.getMessage());
     }
 
     /**
@@ -263,9 +252,9 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
      */
     public void testBadSortMode() throws IOException {
         ScriptSortBuilder builder = new ScriptSortBuilder(new Script("something"), ScriptSortType.STRING);
-        exceptionRule.expect(IllegalArgumentException.class);
-        exceptionRule.expectMessage("script sort of type [string] doesn't support mode");
-        builder.sortMode(SortMode.fromString(randomFrom(new String[]{"avg", "median", "sum"})));
+        String sortMode = randomFrom(new String[] { "avg", "median", "sum" });
+        Exception e = expectThrows(IllegalArgumentException.class, () -> builder.sortMode(SortMode.fromString(sortMode)));
+        assertEquals("script sort of type [string] doesn't support mode [" + sortMode + "]", e.getMessage());
     }
 
     @Override