Third round of comments

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

@@ -67,6 +67,7 @@ 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/indices/query/IndicesQueriesRegistry.java

@@ -24,11 +24,9 @@ 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);
@@ -41,11 +39,4 @@ 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

@@ -286,10 +286,9 @@ public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<Fie
     }
 
     @Override
-    public FieldSortBuilder fromXContent(QueryParseContext context, String elementName) throws IOException {
+    public FieldSortBuilder fromXContent(QueryParseContext context, String fieldName) throws IOException {
         XContentParser parser = context.parser();
 
-        String fieldName = null;
         QueryBuilder nestedFilter = null;
         String nestedPath = null;
         Object missing = null;
@@ -299,37 +298,31 @@ public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<Fie
 
         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) {
+            } else if (token == XContentParser.Token.START_OBJECT) {
-                    if (token == XContentParser.Token.FIELD_NAME) {
+                if (context.parseFieldMatcher().match(currentFieldName, NESTED_FILTER)) {
-                        currentFieldName = parser.currentName();
+                    nestedFilter = context.parseInnerQueryBuilder();
-                    } else if (token == XContentParser.Token.START_OBJECT) {
+                }
-                        if (context.parseFieldMatcher().match(currentFieldName, NESTED_FILTER)) {
+            } else if (token.isValue()) {
-                            nestedFilter = context.parseInnerQueryBuilder();
+                if (context.parseFieldMatcher().match(currentFieldName, NESTED_PATH)) {
-                        }
+                    nestedPath = parser.text();
-                    } else if (token.isValue()) {
+                } else if (context.parseFieldMatcher().match(currentFieldName, MISSING)) {
-                        if (context.parseFieldMatcher().match(currentFieldName, NESTED_PATH)) {
+                    missing = parser.objectBytes();
-                            nestedPath = parser.text();
+                } else if (context.parseFieldMatcher().match(currentFieldName, ORDER)) {
-                        } else if (context.parseFieldMatcher().match(currentFieldName, MISSING)) {
+                    String sortOrder = parser.text();
-                            missing = parser.objectBytes();
+                    if ("asc".equals(sortOrder)) {
-                        } else if (context.parseFieldMatcher().match(currentFieldName, ORDER)) {
+                        order = SortOrder.ASC;
-                            String sortOrder = parser.text();
+                    } else if ("desc".equals(sortOrder)) {
-                            if ("asc".equals(sortOrder)) {
+                        order = SortOrder.DESC;
-                                order = SortOrder.ASC;
+                    } else {
-                            } else if ("desc".equals(sortOrder)) {
+                        throw new IllegalStateException("Sort order " + sortOrder + " not supported.");
-                                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 if (context.parseFieldMatcher().match(currentFieldName, SORT_MODE)) {
+                    sortMode = parser.text();
+                } else if (context.parseFieldMatcher().match(currentFieldName, UNMAPPED_TYPE)) {
+                    unmappedType = parser.text();
                 }
             }
         }
@@ -355,15 +348,4 @@ public class FieldSortBuilder extends SortBuilder implements SortBuilderTemp<Fie
         }
         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/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 implements SortBuilderTemp<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/SortBuilderTemp.java

@@ -25,7 +25,7 @@ import org.elasticsearch.index.query.QueryParseContext;
 
 import java.io.IOException;
 
-public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<FieldSortBuilder>, ToXContent {
+public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<T>, ToXContent {
     /**
      * Creates a new item from the json held by the {@link SortBuilderTemp}
      * in {@link org.elasticsearch.common.xcontent.XContent} format
@@ -36,9 +36,5 @@ public interface SortBuilderTemp<T extends ToXContent> extends NamedWriteable<Fi
      *            call
      * @return the new item
      */
-    NamedWriteable<T> fromXContent(QueryParseContext context, String elementName) throws IOException;
+    SortBuilderTemp<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 = DESC;
+    private static final SortOrder PROTOTYPE = ASC;
 
     @Override
     public SortOrder readFrom(StreamInput in) throws IOException {

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

@@ -1,163 +0,0 @@
-/*
- * 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.*;
-
-public abstract class AbstractSearchSourceItemTestCase<T extends SortBuilderTemp<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(SortBuilderTemp<T> sort) throws IOException {
-        try (BytesStreamOutput output = new BytesStreamOutput()) {
-            sort.writeTo(output);
-            try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
-                SortBuilderTemp<?> prototype = sortParser(sort.getName()).getBuilderPrototype();
-                @SuppressWarnings("unchecked")
-                T secondQuery = (T) prototype.readFrom(in);
-                return secondQuery;
-            }
-        }
-    }
-
-    private SortBuilderTemp<?> sortParser(String queryId) {
-        return indicesQueriesRegistry.sortParsers().get(queryId);
-    }
-}

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

@@ -20,7 +20,6 @@
 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;
@@ -43,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 SortBuilder & NamedWriteable<T> & SortElementParserTemp<T>> extends ESTestCase {
+public abstract class AbstractSortTestCase<T extends SortBuilderTemp<T>> extends ESTestCase {
 
     protected static NamedWriteableRegistry namedWriteableRegistry;
 
@@ -154,7 +153,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

@@ -21,12 +21,7 @@ package org.elasticsearch.search.sort;
 
 import java.io.IOException;
 
-public class FieldSortBuilderTests extends AbstractSearchSourceItemTestCase<FieldSortBuilder> {
+public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder> {
-
-    @SuppressWarnings("unchecked")
-    public Class<FieldSortBuilder> getPrototype() {
-        return (Class<FieldSortBuilder>) FieldSortBuilder.PROTOTYPE.getClass();
-    }
 
     @Override
     protected FieldSortBuilder createTestItem() {