Merge pull request #16127 from MaineC/enhancement/15178-sort-fieldsortbuilder

Refactor FieldSortBuilder to add serialization support

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

@@ -19,15 +19,33 @@
 
 package org.elasticsearch.search.sort;
 
+import org.apache.lucene.util.BytesRef;
+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.lucene.BytesRefs;
 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 java.io.IOException;
+import java.util.Objects;
 
 /**
  * A sort builder to sort based on a document field.
  */
-public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
+public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements SortBuilderParser<FieldSortBuilder> {
+    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");
 
     private final String fieldName;
 
@@ -41,10 +59,22 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     private String nestedPath;
 
+    /** Copy constructor. */
+    public FieldSortBuilder(FieldSortBuilder template) {
+        this(template.fieldName);
+        this.order(template.order());
+        this.missing(template.missing());
+        this.unmappedType(template.unmappedType());
+        this.sortMode(template.sortMode());
+        this.setNestedFilter(template.getNestedFilter());
+        this.setNestedPath(template.getNestedPath());
+    }
+
     /**
      * Constructs a new sort based on a document field.
      *
-     * @param fieldName The field name.
+     * @param fieldName
+     *            The field name.
      */
     public FieldSortBuilder(String fieldName) {
         if (fieldName == null) {
@@ -53,21 +83,39 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         this.fieldName = fieldName;
     }
 
+    /** Returns the document field this sort should be based on. */
+    public String getFieldName() {
+        return this.fieldName;
+    }
+
     /**
      * 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.
      */
     public FieldSortBuilder missing(Object missing) {
-        this.missing = missing;
+        if (missing instanceof String) {
+            this.missing = BytesRefs.toBytesRef(missing);
+        } else {
+            this.missing = missing;
+        }
         return this;
     }
 
+    /** Returns the value used when a field is missing in a doc. */
+    public Object missing() {
+        if (missing instanceof BytesRef) {
+            return ((BytesRef) missing).utf8ToString();
+        }
+        return missing;
+    }
+
     /**
      * Set the type to use in case the current field is not mapped in an index.
-     * Specifying a type tells Elasticsearch what type the sort values should have, which is important
+     * Specifying a type tells Elasticsearch what type the sort values should
-     * for cross-index search, if there are sort fields that exist on some indices only.
+     * have, which is important for cross-index search, if there are sort fields
-     * If the unmapped type is <tt>null</tt> then query execution will fail if one or more indices
+     * that exist on some indices only. If the unmapped type is <tt>null</tt>
-     * don't have a mapping for the current field.
+     * then query execution will fail if one or more indices don't have a
+     * mapping for the current field.
      */
     public FieldSortBuilder unmappedType(String type) {
         this.unmappedType = type;
@@ -75,8 +123,19 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
     }
 
     /**
-     * Defines what values to pick in the case a document contains multiple values for the targeted sort field.
+     * Returns the type to use in case the current field is not mapped in an
-     * Possible values: min, max, sum and avg
+     * index.
+     */
+    public String unmappedType() {
+        return this.unmappedType;
+    }
+
+    /**
+     * 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.
      */
@@ -86,44 +145,217 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
     }
 
     /**
-     * Sets the nested filter that the nested objects should match with in order to be taken into account
+     * Returns what values to pick in the case a document contains multiple
-     * for sorting.
+     * values for the targeted sort field.
+     */
+    public String 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?
      */
     public FieldSortBuilder 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.
+     */
+    public QueryBuilder getNestedFilter() {
+        return this.nestedFilter;
+    }
 
     /**
-     * Sets the nested path if sorting occurs on a field that is inside a nested object. By default when sorting on a
+     * Sets the nested path if sorting occurs on a field that is inside a nested
-     * field inside a nested object, the nearest upper nested object is selected as nested path.
+     * object. By default when sorting on a field inside a nested object, the
+     * nearest upper nested object is selected as nested path.
      */
     public FieldSortBuilder setNestedPath(String nestedPath) {
         this.nestedPath = nestedPath;
         return this;
     }
 
+    /**
+     * Returns the nested path if sorting occurs in a field that is inside a
+     * nested object.
+     */
+    public String getNestedPath() {
+        return this.nestedPath;
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(fieldName);
         builder.field(ORDER_FIELD.getPreferredName(), order);
         if (missing != null) {
-            builder.field("missing", missing);
+            if (missing instanceof BytesRef) {
+                builder.field(MISSING.getPreferredName(), ((BytesRef) missing).utf8ToString());
+            } else {
+                builder.field(MISSING.getPreferredName(), missing);
+            }
         }
         if (unmappedType != null) {
-            builder.field(SortParseElement.UNMAPPED_TYPE.getPreferredName(), unmappedType);
+            builder.field(UNMAPPED_TYPE.getPreferredName(), unmappedType);
         }
         if (sortMode != null) {
-            builder.field("mode", sortMode);
+            builder.field(SORT_MODE.getPreferredName(), sortMode);
         }
         if (nestedFilter != null) {
-            builder.field("nested_filter", nestedFilter, params);
+            builder.field(NESTED_FILTER.getPreferredName(), nestedFilter, params);
         }
         if (nestedPath != null) {
-            builder.field("nested_path", nestedPath);
+            builder.field(NESTED_PATH.getPreferredName(), nestedPath);
         }
         builder.endObject();
         return builder;
     }
+
+    @Override
+    public boolean equals(Object other) {
+        if (this == other) {
+            return true;
+        }
+
+        if (other == null || getClass() != other.getClass()) {
+            return false;
+        }
+
+        FieldSortBuilder builder = (FieldSortBuilder) other;
+        return (Objects.equals(this.fieldName, builder.fieldName) && Objects.equals(this.nestedFilter, builder.nestedFilter)
+                && Objects.equals(this.nestedPath, builder.nestedPath) && Objects.equals(this.missing, builder.missing)
+                && Objects.equals(this.order, builder.order) && Objects.equals(this.sortMode, builder.sortMode)
+                && Objects.equals(this.unmappedType, builder.unmappedType));
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(this.fieldName, this.nestedFilter, this.nestedPath, this.missing, this.order, this.sortMode, this.unmappedType);
+    }
+
+    @Override
+    public String getWriteableName() {
+        return NAME;
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeString(this.fieldName);
+        if (this.nestedFilter != null) {
+            out.writeBoolean(true);
+            out.writeQuery(this.nestedFilter);
+        } else {
+            out.writeBoolean(false);
+        }
+        out.writeOptionalString(this.nestedPath);
+        out.writeGenericValue(this.missing);
+
+        if (this.order != null) {
+            out.writeBoolean(true);
+            this.order.writeTo(out);
+        } else {
+            out.writeBoolean(false);
+        }
+
+        out.writeOptionalString(this.sortMode);
+        out.writeOptionalString(this.unmappedType);
+    }
+
+    @Override
+    public FieldSortBuilder readFrom(StreamInput in) throws IOException {
+        String fieldName = in.readString();
+        FieldSortBuilder result = new FieldSortBuilder(fieldName);
+        if (in.readBoolean()) {
+            QueryBuilder query = in.readQuery();
+            result.setNestedFilter(query);
+        }
+        result.setNestedPath(in.readOptionalString());
+        result.missing(in.readGenericValue());
+
+        if (in.readBoolean()) {
+            result.order(SortOrder.readOrderFrom(in));
+        }
+        result.sortMode(in.readOptionalString());
+        result.unmappedType(in.readOptionalString());
+        return result;
+    }
+
+    @Override
+    public FieldSortBuilder fromXContent(QueryParseContext context, String fieldName) throws IOException {
+        XContentParser parser = context.parser();
+
+        QueryBuilder nestedFilter = null;
+        String nestedPath = null;
+        Object missing = null;
+        SortOrder order = null;
+        String sortMode = null;
+        String unmappedType = null;
+
+        String currentFieldName = null;
+        XContentParser.Token token;
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                currentFieldName = parser.currentName();
+            } else if (token == XContentParser.Token.START_OBJECT) {
+                if (context.parseFieldMatcher().match(currentFieldName, NESTED_FILTER)) {
+                    nestedFilter = context.parseInnerQueryBuilder();
+                } else {
+                    throw new ParsingException(parser.getTokenLocation(), "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)) {
+                        order = SortOrder.ASC;
+                    } else if ("desc".equals(sortOrder)) {
+                        order = SortOrder.DESC;
+                    } else {
+                        throw new IllegalStateException("Sort order " + sortOrder + " not supported.");
+                    }
+                } else if (context.parseFieldMatcher().match(currentFieldName, SORT_MODE)) {
+                    sortMode = parser.text();
+                } else if (context.parseFieldMatcher().match(currentFieldName, UNMAPPED_TYPE)) {
+                    unmappedType = parser.text();
+                } else {
+                    throw new IllegalArgumentException("Option " + currentFieldName + " not supported.");
+                }
+            }
+        }
+
+        FieldSortBuilder builder = new FieldSortBuilder(fieldName);
+        if (nestedFilter != null) {
+            builder.setNestedFilter(nestedFilter);
+        }
+        if (nestedPath != null) {
+            builder.setNestedPath(nestedPath);
+        }
+        if (missing != null) {
+            builder.missing(missing);
+        }
+        if (order != null) {
+            builder.order(order);
+        }
+        if (sortMode != null) {
+            builder.sortMode(sortMode);
+        }
+        if (unmappedType != null) {
+            builder.unmappedType(unmappedType);
+        }
+        return builder;
+    }
 }

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

@@ -23,11 +23,9 @@ import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.geo.GeoDistance;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
-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.unit.DistanceUnit;
-import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryBuilder;
@@ -44,8 +42,7 @@ import java.util.Objects;
 /**
  * A geo distance based sorting on a geo point like field.
  */
-public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
+public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> implements SortBuilderParser<GeoDistanceSortBuilder> {
-        implements ToXContent, NamedWriteable<GeoDistanceSortBuilder>, SortElementParserTemp<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

@@ -0,0 +1,40 @@
+/*
+ * 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.common.io.stream.NamedWriteable;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.index.query.QueryParseContext;
+
+import java.io.IOException;
+
+public interface SortBuilderParser<T extends ToXContent> extends NamedWriteable<T>, ToXContent {
+    /**
+     * Creates a new item from the json held by the {@link SortBuilderParser}
+     * 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
+     */
+    SortBuilder fromXContent(QueryParseContext context, String elementName) throws IOException;
+}

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

@@ -19,8 +19,8 @@
 
 package org.elasticsearch.search.sort;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
-import org.elasticsearch.common.io.stream.NamedWriteable;
 import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -43,7 +43,7 @@ import java.io.IOException;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 
-public abstract class AbstractSortTestCase<T extends SortBuilder & NamedWriteable<T> & SortElementParserTemp<T>> extends ESTestCase {
+public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderParser<T>> extends ESTestCase {
 
     protected static NamedWriteableRegistry namedWriteableRegistry;
 
@@ -55,6 +55,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & NamedWriteabl
         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();
     }
 
@@ -85,6 +86,8 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & NamedWriteabl
             builder.endObject();
 
             XContentParser itemParser = XContentHelper.createParser(builder.bytes());
+            ParsingException except = new ParsingException(itemParser.getTokenLocation(), "mytext", itemParser.getTokenLocation());
+            System.out.println(except.getMessage());
             itemParser.nextToken();
 
             /*
@@ -154,7 +157,8 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & NamedWriteabl
             try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
                 T prototype = (T) namedWriteableRegistry.getPrototype(SortBuilder.class,
                         original.getWriteableName());
-                return prototype.readFrom(in);
+                T copy = (T) prototype.readFrom(in);
+                return copy;
             }
         }
     }

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

@@ -0,0 +1,85 @@
+/*
+x * 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 java.io.IOException;
+
+public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder> {
+
+    @Override
+    protected FieldSortBuilder createTestItem() {
+        String fieldName = randomAsciiOfLengthBetween(1, 10);
+        FieldSortBuilder builder = new FieldSortBuilder(fieldName);
+        if (randomBoolean()) {
+            builder.order(RandomSortDataGenerator.order(builder.order()));
+        }
+
+        if (randomBoolean()) {
+            builder.missing(RandomSortDataGenerator.missing(builder.missing()));
+        }
+
+        if (randomBoolean()) {
+            builder.unmappedType(RandomSortDataGenerator.randomAscii(builder.unmappedType()));
+        }
+
+        if (randomBoolean()) {
+            builder.sortMode(RandomSortDataGenerator.mode(builder.sortMode()));
+        }
+
+        if (randomBoolean()) {
+            builder.setNestedFilter(RandomSortDataGenerator.nestedFilter(builder.getNestedFilter()));
+        }
+
+        if (randomBoolean()) {
+            builder.setNestedPath(RandomSortDataGenerator.randomAscii(builder.getNestedPath()));
+        }
+        
+        return builder;
+    }
+
+    @Override
+    protected FieldSortBuilder mutate(FieldSortBuilder original) throws IOException {
+        FieldSortBuilder mutated = new FieldSortBuilder(original);
+        int parameter = randomIntBetween(0, 5);
+        switch (parameter) {
+        case 0:
+            mutated.setNestedPath(RandomSortDataGenerator.randomAscii(mutated.getNestedPath()));
+            break;
+        case 1:
+            mutated.setNestedFilter(RandomSortDataGenerator.nestedFilter(mutated.getNestedFilter()));
+            break;
+        case 2:
+            mutated.sortMode(RandomSortDataGenerator.mode(mutated.sortMode()));
+            break;
+        case 3:
+            mutated.unmappedType(RandomSortDataGenerator.randomAscii(mutated.unmappedType()));
+            break;
+        case 4:
+            mutated.missing(RandomSortDataGenerator.missing(mutated.missing()));
+            break;
+        case 5:
+            mutated.order(RandomSortDataGenerator.order(mutated.order()));
+            break;
+        default:
+            throw new IllegalStateException("Unsupported mutation.");
+        }
+        return mutated;
+    }
+}