Second round of comments

core/src/main/java/org/elasticsearch/indices/query/IndicesQueriesRegistry.java

@@ -24,9 +24,11 @@ import java.util.Map;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.query.QueryParser;
+import org.elasticsearch.search.sort.SortBuilderTemp;
 
 public class IndicesQueriesRegistry extends AbstractComponent {
     private Map<String, QueryParser<?>> queryParsers;
+    private Map<String, SortBuilderTemp<?>> sortParsers;
 
     public IndicesQueriesRegistry(Settings settings, Map<String, QueryParser<?>> queryParsers) {
         super(settings);
@@ -39,4 +41,11 @@ public class IndicesQueriesRegistry extends AbstractComponent {
     public Map<String, QueryParser<?>> queryParsers() {
         return queryParsers;
     }
+    
+    /**
+     * Returns all registered sort parsers
+     */
+    public Map<String, SortBuilderTemp<?>> sortParsers() {
+        return sortParsers;
+    }
 }

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

@@ -21,12 +21,9 @@ 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;
@@ -38,7 +35,8 @@ 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 implements SortBuilderTemp<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");
@@ -48,7 +46,6 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
     public static final ParseField SORT_MODE = new ParseField("mode");
     public static final ParseField UNMAPPED_TYPE = new ParseField("unmapped_type");
 
- 
     private final String fieldName;
 
     private Object missing;
@@ -71,10 +68,12 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         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) {
@@ -111,24 +110,29 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
 
     /**
      * 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;
         return this;
     }
 
-    /** Returns the type to use in case the current field is not mapped in an index. */
+    /**
+     * 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.
+     * Defines what values to pick in the case a document contains multiple
-     * Possible values: min, max, sum and avg
+     * values for the targeted sort field. Possible values: min, max, sum and
+     * avg
      * 
      * TODO would love to see an enum here
      * <p>
@@ -139,37 +143,48 @@ 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. */
+    /**
+     * 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
+     * Sets the nested filter that the nested objects should match with in order
-     * for sorting.
+     * to be taken into account for sorting.
      * 
-     * TODO should the above getters and setters be deprecated/ changed in favour of real getters and setters?
+     * 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. */
+     * 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. */
+    /**
+     * Returns the nested path if sorting occurs in a field that is inside a
+     * nested object.
+     */
     public String getNestedPath() {
         return this.nestedPath;
     }
@@ -207,24 +222,20 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
             return true;
         }
 
-        if (other== null || getClass() != other.getClass()) {
+        if (other == null || getClass() != other.getClass()) {
             return false;
         }
 
         FieldSortBuilder builder = (FieldSortBuilder) other;
-        return (Objects.equals(this.fieldName, builder.fieldName) &&
+        return (Objects.equals(this.fieldName, builder.fieldName) && Objects.equals(this.nestedFilter, builder.nestedFilter)
-                Objects.equals(this.nestedFilter, builder.nestedFilter) &&
+                && Objects.equals(this.nestedPath, builder.nestedPath) && Objects.equals(this.missing, builder.missing)
-                Objects.equals(this.nestedPath, builder.nestedPath) &&
+                && Objects.equals(this.order, builder.order) && Objects.equals(this.sortMode, builder.sortMode)
-                Objects.equals(this.missing, builder.missing) &&
+                && Objects.equals(this.unmappedType, builder.unmappedType));
-                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,
+        return Objects.hash(this.fieldName, this.nestedFilter, this.nestedPath, this.missing, this.order, this.sortMode, this.unmappedType);
-                this.missing, this.order, this.sortMode, this.unmappedType);
     }
 
     @Override
@@ -345,4 +356,14 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         return builder;
     }
 
+    @Override
+    public String getName() {
+        return "field_sort_builder";
+    }
+
+    @Override
+    public SortBuilderTemp<FieldSortBuilder> getBuilderPrototype() {
+        return PROTOTYPE;
+    }
+
 }

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

@@ -19,14 +19,15 @@
 
 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 ParameterParser<T extends ToXContent> {
+public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<FieldSortBuilder>, ToXContent {
     /**
-     * Creates a new item from the json held by the {@link ParameterParser}
+     * Creates a new item from the json held by the {@link SortBuilderTemp}
      * in {@link org.elasticsearch.common.xcontent.XContent} format
      *
      * @param context
@@ -35,5 +36,9 @@ public interface ParameterParser<T extends ToXContent> {
      *            call
      * @return the new item
      */
-    T fromXContent(QueryParseContext context, String elementName) throws IOException;
+    NamedWriteable<T> fromXContent(QueryParseContext context, String elementName) throws IOException;
+    
+    String getName();
+
+    SortBuilderTemp<T> getBuilderPrototype();
 }

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

@@ -51,7 +51,7 @@ public enum SortOrder implements Writeable<SortOrder> {
         }
     };
     
-    private static final SortOrder PROTOTYPE = ASC;
+    private static final SortOrder PROTOTYPE = DESC;
 
     @Override
     public SortOrder readFrom(StreamInput in) throws IOException {

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

@@ -42,8 +42,7 @@ import java.io.IOException;
 
 import static org.hamcrest.Matchers.*;
 
-//TODO maybe merge with AbstractsortBuilderTestCase once #14933 is in?
+public abstract class AbstractSearchSourceItemTestCase<T extends SortBuilderTemp<T>> extends ESTestCase {
-public abstract class AbstractSearchSourceItemTestCase<T extends NamedWriteable<T> & ToXContent & ParameterParser<T>> extends ESTestCase {
 
     protected static NamedWriteableRegistry namedWriteableRegistry;
 
@@ -146,17 +145,19 @@ public abstract class AbstractSearchSourceItemTestCase<T extends NamedWriteable<
         }
     }
 
-    protected T copyItem(T original) throws IOException {
+    protected T copyItem(SortBuilderTemp<T> sort) throws IOException {
         try (BytesStreamOutput output = new BytesStreamOutput()) {
-            original.writeTo(output);
+            sort.writeTo(output);
             try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
+                SortBuilderTemp<?> prototype = sortParser(sort.getName()).getBuilderPrototype();
                 @SuppressWarnings("unchecked")
-                T prototype = (T) namedWriteableRegistry.getPrototype(getPrototype(), original.getWriteableName());
+                T secondQuery = (T) prototype.readFrom(in);
-                T copy = (T) prototype.readFrom(in);
+                return secondQuery;
-                return copy;
             }
         }
     }
 
-    protected abstract Class<T> getPrototype();
+    private SortBuilderTemp<?> sortParser(String queryId) {
+        return indicesQueriesRegistry.sortParsers().get(queryId);
+    }
 }