Refactor FieldSortBuilder

* adds json parsing,
* refactors json serialisation,
* adds writable parsing and serialisation,
* adds json and writable roundtrip test

core/src/main/java/org/elasticsearch/common/unit/Fuzziness.java

@@ -67,7 +67,6 @@ public final class Fuzziness implements ToXContent, Writeable<Fuzziness> {
 
     /**
      * Creates a {@link Fuzziness} instance from an edit distance. The value must be one of <tt>[0, 1, 2]</tt>
-     *
      * Note: Using this method only makes sense if the field you are applying Fuzziness to is some sort of string.
      */
     public static Fuzziness fromEdits(int edits) {

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

@@ -19,15 +19,35 @@
 
 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.NamedWriteable;
+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.ToXContent;
 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> {
+    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 SORT_MODE = new ParseField("mode");    
+    public static final ParseField UNMAPPED_TYPE = new ParseField("unmapped_type");
+
  
     private final String fieldName;
 
@@ -41,6 +61,16 @@ 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.
      *
@@ -53,15 +83,29 @@ 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) {
+        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() {
+        return this.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
@@ -74,9 +118,16 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         return this;
     }
 
+    /** Returns the type to use in case the current field is not mapped in an 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.
      */
@@ -85,15 +136,26 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         return this;
     }
 
+    /** Returns what values to pick in the case a document contains multiple 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
@@ -104,26 +166,181 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         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 (! (other instanceof FieldSortBuilder)) {
+            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);
+        if (this.missing != null) {
+            out.writeBoolean(true);
+            out.writeGenericValue(this.missing);
+        } else {
+            out.writeBoolean(false);
+        }
+        
+        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());
+        if (in.readBoolean()) {
+            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 elementName) throws IOException {
+        XContentParser parser = context.parser();
+
+        String fieldName = null;
+        QueryBuilder nestedFilter = null;
+        String nestedPath = null;
+        Object missing = null;
+        SortOrder order = null;
+        String sortMode = null;
+        String unmappedType = null;
+        
+        String currentFieldName = null;
+        XContentParser.Token token;
+        fieldName = elementName;
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                currentFieldName = parser.currentName();
+                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 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, 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();
+                        }
+                    }
+                }
+            }
+        }
+
+        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/ParameterParser.java

@@ -0,0 +1,39 @@
+/*
+ * 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.xcontent.ToXContent;
+import org.elasticsearch.index.query.QueryParseContext;
+
+import java.io.IOException;
+
+public interface ParameterParser<T extends ToXContent> {
+    /**
+     * Creates a new item from the json held by the {@link ParameterParser}
+     * 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/sort/AbstractSearchSourceItemTestCase.java

@@ -0,0 +1,162 @@
+/*
+ * 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.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;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentHelper;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.index.query.QueryParseContext;
+import org.elasticsearch.indices.query.IndicesQueriesRegistry;
+import org.elasticsearch.search.SearchModule;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+import java.io.IOException;
+
+import static org.hamcrest.Matchers.*;
+
+//TODO maybe merge with AbstractsortBuilderTestCase once #14933 is in?
+public abstract class AbstractSearchSourceItemTestCase<T extends NamedWriteable<T> & ToXContent & ParameterParser<T>> extends ESTestCase {
+
+    protected static NamedWriteableRegistry namedWriteableRegistry;
+
+    private static final int NUMBER_OF_TESTBUILDERS = 20;
+    static IndicesQueriesRegistry indicesQueriesRegistry;
+
+    @BeforeClass
+    public static void init() {
+        namedWriteableRegistry = new NamedWriteableRegistry();
+        namedWriteableRegistry.registerPrototype(FieldSortBuilder.class, FieldSortBuilder.PROTOTYPE);
+        indicesQueriesRegistry = new SearchModule(Settings.EMPTY, namedWriteableRegistry).buildQueryParserRegistry();
+    }
+
+    @AfterClass
+    public static void afterClass() throws Exception {
+        namedWriteableRegistry = null;
+    }
+
+    /** Returns random sort that is put under test */
+    protected abstract T createTestItem();
+
+    /** Returns mutated version of original so the returned sort is different in terms of equals/hashcode */
+    protected abstract T mutate(T original) throws IOException;
+
+    /**
+     * Test that creates new sort from a random test sort and checks both for equality
+     */
+    public void testFromXContent() throws IOException {
+        for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
+            T testItem = createTestItem();
+
+            XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
+            if (randomBoolean()) {
+                builder.prettyPrint();
+            }
+            builder.startObject();
+            testItem.toXContent(builder, ToXContent.EMPTY_PARAMS);
+            builder.endObject();
+
+            XContentParser itemParser = XContentHelper.createParser(builder.bytes());
+            itemParser.nextToken();
+            
+            /*
+             * filter out name of sort, or field name to sort on for element fieldSort 
+             */
+            itemParser.nextToken();
+            String elementName = itemParser.currentName();
+            itemParser.nextToken();
+
+            QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
+            context.reset(itemParser);
+            NamedWriteable<T> parsedItem = testItem.fromXContent(context, elementName);
+            assertNotSame(testItem, parsedItem);
+            assertEquals(testItem, parsedItem);
+            assertEquals(testItem.hashCode(), parsedItem.hashCode());
+        }
+    }
+
+    /**
+     * Test serialization and deserialization of the test sort.
+     */
+    public void testSerialization() throws IOException {
+        for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
+            T testsort = createTestItem();
+            T deserializedsort = copyItem(testsort);
+            assertEquals(testsort, deserializedsort);
+            assertEquals(testsort.hashCode(), deserializedsort.hashCode());
+            assertNotSame(testsort, deserializedsort);
+        }
+    }
+
+    /**
+     * Test equality and hashCode properties
+     */
+    public void testEqualsAndHashcode() throws IOException {
+        for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
+            T firstsort = createTestItem();
+            assertFalse("sort is equal to null", firstsort.equals(null));
+            assertFalse("sort is equal to incompatible type", firstsort.equals(""));
+            assertTrue("sort is not equal to self", firstsort.equals(firstsort));
+            assertThat("same sort's hashcode returns different values if called multiple times", firstsort.hashCode(),
+                    equalTo(firstsort.hashCode()));
+            assertThat("different sorts should not be equal", mutate(firstsort), not(equalTo(firstsort)));
+            assertThat("different sorts should have different hashcode", mutate(firstsort).hashCode(), not(equalTo(firstsort.hashCode())));
+
+            T secondsort = copyItem(firstsort);
+            assertTrue("sort is not equal to self", secondsort.equals(secondsort));
+            assertTrue("sort is not equal to its copy", firstsort.equals(secondsort));
+            assertTrue("equals is not symmetric", secondsort.equals(firstsort));
+            assertThat("sort copy's hashcode is different from original hashcode", secondsort.hashCode(), equalTo(firstsort.hashCode()));
+
+            T thirdsort = copyItem(secondsort);
+            assertTrue("sort is not equal to self", thirdsort.equals(thirdsort));
+            assertTrue("sort is not equal to its copy", secondsort.equals(thirdsort));
+            assertThat("sort copy's hashcode is different from original hashcode", secondsort.hashCode(), equalTo(thirdsort.hashCode()));
+            assertTrue("equals is not transitive", firstsort.equals(thirdsort));
+            assertThat("sort copy's hashcode is different from original hashcode", firstsort.hashCode(), equalTo(thirdsort.hashCode()));
+            assertTrue("equals is not symmetric", thirdsort.equals(secondsort));
+            assertTrue("equals is not symmetric", thirdsort.equals(firstsort));
+        }
+    }
+
+    protected T copyItem(T original) throws IOException {
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            original.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
+                @SuppressWarnings("unchecked")
+                T prototype = (T) namedWriteableRegistry.getPrototype(getPrototype(), original.getWriteableName());
+                T copy = (T) prototype.readFrom(in);
+                return copy;
+            }
+        }
+    }
+    
+    protected abstract Class<T> getPrototype();
+}

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

@@ -0,0 +1,90 @@
+/*
+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 AbstractSearchSourceItemTestCase<FieldSortBuilder> {
+
+    @SuppressWarnings("unchecked")
+    public Class<FieldSortBuilder> getPrototype() {
+        return (Class<FieldSortBuilder>) FieldSortBuilder.PROTOTYPE.getClass();
+    }
+
+    @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;
+    }
+}