From 64d362ab9dafecede99d27f1bc59a4cd848bc9f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 22 Mar 2016 14:28:29 +0100 Subject: [PATCH 1/2] Add parsing of list of sort builders to SortBuilder Moving the current parsing code for the whole "sort" element in the seach source over to static "fromXContent" method in SortBuilder. --- .../search/sort/GeoDistanceSortBuilder.java | 1 + .../search/sort/ScoreSortBuilder.java | 3 +- .../search/sort/ScriptSortBuilder.java | 2 +- .../search/sort/SortBuilder.java | 167 ++++++++++-- .../search/sort/FieldSortBuilderTests.java | 6 +- .../sort/GeoDistanceSortBuilderTests.java | 6 +- .../search/sort/RandomSortDataGenerator.java | 25 +- .../search/sort/ScoreSortBuilderTests.java | 10 +- .../search/sort/ScriptSortBuilderTests.java | 6 +- .../search/sort/SortBuilderTests.java | 250 ++++++++++++++++++ 10 files changed, 430 insertions(+), 46 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/search/sort/SortBuilderTests.java diff --git a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 4263e148323..66c4fba43fb 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -62,6 +62,7 @@ import java.util.Objects; */ public class GeoDistanceSortBuilder extends SortBuilder implements SortBuilderParser { public static final String NAME = "_geo_distance"; + public static final String ALTERNATIVE_NAME = "_geoDistance"; public static final boolean DEFAULT_COERCE = false; public static final boolean DEFAULT_IGNORE_MALFORMED = false; public static final ParseField UNIT_FIELD = new ParseField("unit"); diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java index 422be339788..db2fccf6712 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java @@ -38,7 +38,7 @@ import java.util.Objects; */ public class ScoreSortBuilder extends SortBuilder implements SortBuilderParser { - private static final String NAME = "_score"; + public static final String NAME = "_score"; static final ScoreSortBuilder PROTOTYPE = new ScoreSortBuilder(); public static final ParseField REVERSE_FIELD = new ParseField("reverse"); public static final ParseField ORDER_FIELD = new ParseField("order"); @@ -88,6 +88,7 @@ public class ScoreSortBuilder extends SortBuilder implements S return result; } + @Override public SortField build(QueryShardContext context) { if (order == SortOrder.DESC) { return SORT_SCORE; diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index 6005d9354ff..1702c03bc6c 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -66,7 +66,7 @@ import java.util.Objects; */ public class ScriptSortBuilder extends SortBuilder implements SortBuilderParser { - private static final String NAME = "_script"; + public static final String NAME = "_script"; static final ScriptSortBuilder PROTOTYPE = new ScriptSortBuilder(new Script("_na_"), ScriptSortType.STRING); public static final ParseField TYPE_FIELD = new ParseField("type"); public static final ParseField SCRIPT_FIELD = new ParseField("script"); diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java index 35d59de011c..45fd4d45796 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -20,6 +20,8 @@ package org.elasticsearch.search.sort; import org.apache.lucene.search.Query; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; import org.apache.lucene.search.join.BitSetProducer; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.ParseField; @@ -27,47 +29,44 @@ import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.elasticsearch.index.mapper.object.ObjectMapper; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; +import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Optional; + +import static java.util.Collections.unmodifiableMap; /** * */ -public abstract class SortBuilder> implements ToXContent { - - protected static Nested resolveNested(QueryShardContext context, String nestedPath, QueryBuilder nestedFilter) throws IOException { - Nested nested = null; - if (nestedPath != null) { - BitSetProducer rootDocumentsFilter = context.bitsetFilter(Queries.newNonNestedFilter()); - ObjectMapper nestedObjectMapper = context.getObjectMapper(nestedPath); - if (nestedObjectMapper == null) { - throw new QueryShardException(context, "[nested] failed to find nested object under path [" + nestedPath + "]"); - } - if (!nestedObjectMapper.nested().isNested()) { - throw new QueryShardException(context, "[nested] nested object under path [" + nestedPath + "] is not of nested type"); - } - Query innerDocumentsQuery; - if (nestedFilter != null) { - context.nestedScope().nextLevel(nestedObjectMapper); - innerDocumentsQuery = QueryBuilder.rewriteQuery(nestedFilter, context).toFilter(context); - context.nestedScope().previousLevel(); - } else { - innerDocumentsQuery = nestedObjectMapper.nestedTypeFilter(); - } - nested = new Nested(rootDocumentsFilter, innerDocumentsQuery); - } - return nested; - } +public abstract class SortBuilder> implements SortBuilderParser, ToXContent { protected SortOrder order = SortOrder.ASC; public static final ParseField ORDER_FIELD = new ParseField("order"); + private static final Map> PARSERS; + + static { + Map> parsers = new HashMap<>(); + parsers.put(ScriptSortBuilder.NAME, ScriptSortBuilder.PROTOTYPE); + parsers.put(GeoDistanceSortBuilder.NAME, new GeoDistanceSortBuilder("_na_", -1, -1)); + parsers.put(GeoDistanceSortBuilder.ALTERNATIVE_NAME, new GeoDistanceSortBuilder("_na_", -1, -1)); + parsers.put(ScoreSortBuilder.NAME, ScoreSortBuilder.PROTOTYPE); + PARSERS = unmodifiableMap(parsers); + } + @Override public String toString() { try { @@ -96,4 +95,122 @@ public abstract class SortBuilder> implements ToXConten public SortOrder order() { return this.order; } + + public static List> fromXContent(QueryParseContext context) throws IOException { + List> sortFields = new ArrayList<>(2); + XContentParser parser = context.parser(); + XContentParser.Token token = parser.currentToken(); + if (token == XContentParser.Token.START_ARRAY) { + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + if (token == XContentParser.Token.START_OBJECT) { + parseCompoundSortField(parser, context, sortFields); + } else if (token == XContentParser.Token.VALUE_STRING) { + String fieldName = parser.text(); + if (fieldName.equals(ScoreSortBuilder.NAME)) { + sortFields.add(new ScoreSortBuilder()); + } else { + sortFields.add(new FieldSortBuilder(fieldName)); + } + } else { + throw new IllegalArgumentException("malformed sort format, " + + "within the sort array, an object, or an actual string are allowed"); + } + } + } else if (token == XContentParser.Token.VALUE_STRING) { + String fieldName = parser.text(); + if (fieldName.equals(ScoreSortBuilder.NAME)) { + sortFields.add(new ScoreSortBuilder()); + } else { + sortFields.add(new FieldSortBuilder(fieldName)); + } + } else if (token == XContentParser.Token.START_OBJECT) { + parseCompoundSortField(parser, context, sortFields); + } else { + throw new IllegalArgumentException("malformed sort format, either start with array, object, or an actual string"); + } + return sortFields; + } + + private static void parseCompoundSortField(XContentParser parser, QueryParseContext context, List> sortFields) + throws IOException { + XContentParser.Token token; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + String fieldName = parser.currentName(); + token = parser.nextToken(); + if (token == XContentParser.Token.VALUE_STRING) { + SortOrder order = SortOrder.fromString(parser.text()); + if (fieldName.equals(ScoreSortBuilder.NAME)) { + sortFields.add(new ScoreSortBuilder().order(order)); + } else { + sortFields.add(new FieldSortBuilder(fieldName).order(order)); + } + } else { + if (PARSERS.containsKey(fieldName)) { + sortFields.add(PARSERS.get(fieldName).fromXContent(context, fieldName)); + } else { + sortFields.add(FieldSortBuilder.PROTOTYPE.fromXContent(context, fieldName)); + } + } + } + } + } + + public static void parseSort(XContentParser parser, SearchContext context) throws IOException { + QueryParseContext parseContext = context.getQueryShardContext().parseContext(); + parseContext.reset(parser); + Optional sortOptional = buildSort(SortBuilder.fromXContent(parseContext), context.getQueryShardContext()); + if (sortOptional.isPresent()) { + context.sort(sortOptional.get()); + } + } + + public static Optional buildSort(List> sortBuilders, QueryShardContext context) throws IOException { + List sortFields = new ArrayList<>(sortBuilders.size()); + for (SortBuilder builder : sortBuilders) { + sortFields.add(builder.build(context)); + } + if (!sortFields.isEmpty()) { + // optimize if we just sort on score non reversed, we don't really need sorting + boolean sort; + if (sortFields.size() > 1) { + sort = true; + } else { + SortField sortField = sortFields.get(0); + if (sortField.getType() == SortField.Type.SCORE && !sortField.getReverse()) { + sort = false; + } else { + sort = true; + } + } + if (sort) { + return Optional.of(new Sort(sortFields.toArray(new SortField[sortFields.size()]))); + } + } + return Optional.empty(); + } + + protected static Nested resolveNested(QueryShardContext context, String nestedPath, QueryBuilder nestedFilter) throws IOException { + Nested nested = null; + if (nestedPath != null) { + BitSetProducer rootDocumentsFilter = context.bitsetFilter(Queries.newNonNestedFilter()); + ObjectMapper nestedObjectMapper = context.getObjectMapper(nestedPath); + if (nestedObjectMapper == null) { + throw new QueryShardException(context, "[nested] failed to find nested object under path [" + nestedPath + "]"); + } + if (!nestedObjectMapper.nested().isNested()) { + throw new QueryShardException(context, "[nested] nested object under path [" + nestedPath + "] is not of nested type"); + } + Query innerDocumentsQuery; + if (nestedFilter != null) { + context.nestedScope().nextLevel(nestedObjectMapper); + innerDocumentsQuery = QueryBuilder.rewriteQuery(nestedFilter, context).toFilter(context); + context.nestedScope().previousLevel(); + } else { + innerDocumentsQuery = nestedObjectMapper.nestedTypeFilter(); + } + nested = new Nested(rootDocumentsFilter, innerDocumentsQuery); + } + return nested; + } } diff --git a/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java b/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java index d00b60e0c83..74b56353a91 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java @@ -25,10 +25,14 @@ public class FieldSortBuilderTests extends AbstractSortTestCase> result = parseSort(json); + assertEquals(1, result.size()); + SortBuilder sortBuilder = result.get(0); + assertEquals(new FieldSortBuilder("field1").order(order), sortBuilder); + + json = "{ \"sort\" : \"field1\" }"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new FieldSortBuilder("field1"), sortBuilder); + + json = "{ \"sort\" : { \"_doc\" : \"" + order + "\" }}"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new FieldSortBuilder("_doc").order(order), sortBuilder); + + json = "{ \"sort\" : \"_doc\" }"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new FieldSortBuilder("_doc"), sortBuilder); + + json = "{ \"sort\" : { \"_score\" : \"" + order +"\" }}"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new ScoreSortBuilder().order(order), sortBuilder); + + json = "{ \"sort\" : \"_score\" }"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new ScoreSortBuilder(), sortBuilder); + + // test two spellings for _geo_disctance + json = "{ \"sort\" : [" + + "{\"_geoDistance\" : {" + + "\"pin.location\" : \"40,-70\" } }" + + "] }"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new GeoDistanceSortBuilder("pin.location", 40, -70), sortBuilder); + + json = "{ \"sort\" : [" + + "{\"_geo_distance\" : {" + + "\"pin.location\" : \"40,-70\" } }" + + "] }"; + result = parseSort(json); + assertEquals(1, result.size()); + sortBuilder = result.get(0); + assertEquals(new GeoDistanceSortBuilder("pin.location", 40, -70), sortBuilder); + } + + /** + * test random syntax variations + */ + public void testRandomSortBuilders() throws IOException { + for (int runs = 0; runs < NUMBER_OF_RUNS; runs++) { + List> testBuilders = randomSortBuilderList(); + XContentBuilder xContentBuilder = XContentFactory.jsonBuilder(); + xContentBuilder.startObject(); + if (testBuilders.size() > 1) { + xContentBuilder.startArray("sort"); + } else { + xContentBuilder.field("sort"); + } + for (SortBuilder builder : testBuilders) { + if (builder instanceof ScoreSortBuilder || builder instanceof FieldSortBuilder) { + switch (randomIntBetween(0, 2)) { + case 0: + if (builder instanceof ScoreSortBuilder) { + xContentBuilder.value("_score"); + } else { + xContentBuilder.value(((FieldSortBuilder) builder).getFieldName()); + } + break; + case 1: + xContentBuilder.startObject(); + if (builder instanceof ScoreSortBuilder) { + xContentBuilder.field("_score"); + } else { + xContentBuilder.field(((FieldSortBuilder) builder).getFieldName()); + } + xContentBuilder.value(builder.order()); + xContentBuilder.endObject(); + break; + case 2: + xContentBuilder.startObject(); + builder.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + break; + } + } else { + xContentBuilder.startObject(); + builder.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + xContentBuilder.endObject(); + } + } + if (testBuilders.size() > 1) { + xContentBuilder.endArray(); + } + xContentBuilder.endObject(); + List> parsedSort = parseSort(xContentBuilder.string()); + assertEquals(testBuilders.size(), parsedSort.size()); + Iterator> iterator = testBuilders.iterator(); + for (SortBuilder parsedBuilder : parsedSort) { + assertEquals(iterator.next(), parsedBuilder); + } + } + } + + public static List> randomSortBuilderList() { + int size = randomIntBetween(1, 5); + List> list = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + switch (randomIntBetween(0, 3)) { + case 0: + list.add(new ScoreSortBuilder()); + break; + case 1: + String fieldName = rarely() ? SortParseElement.DOC_FIELD_NAME : randomAsciiOfLengthBetween(1, 10); + list.add(new FieldSortBuilder(fieldName)); + break; + case 2: + list.add(GeoDistanceSortBuilderTests.randomGeoDistanceSortBuilder()); + break; + case 3: + list.add(ScriptSortBuilderTests.randomScriptSortBuilder()); + break; + default: + throw new IllegalStateException("unexpected randomization in tests"); + } + } + return list; + } + + /** + * test array syntax variations: + * - "sort" : [ "fieldname", { "fieldname2" : "asc" }, ...] + */ + public void testMultiFieldSort() throws IOException { + String json = "{ \"sort\" : [" + + "{ \"post_date\" : {\"order\" : \"asc\"}}," + + "\"user\"," + + "{ \"name\" : \"desc\" }," + + "{ \"age\" : \"desc\" }," + + "{" + + "\"_geo_distance\" : {" + + "\"pin.location\" : \"40,-70\" } }," + + "\"_score\"" + + "] }"; + List> result = parseSort(json); + assertEquals(6, result.size()); + assertEquals(new FieldSortBuilder("post_date").order(SortOrder.ASC), result.get(0)); + assertEquals(new FieldSortBuilder("user").order(SortOrder.ASC), result.get(1)); + assertEquals(new FieldSortBuilder("name").order(SortOrder.DESC), result.get(2)); + assertEquals(new FieldSortBuilder("age").order(SortOrder.DESC), result.get(3)); + assertEquals(new GeoDistanceSortBuilder("pin.location", new GeoPoint(40, -70)), result.get(4)); + assertEquals(new ScoreSortBuilder(), result.get(5)); + } + + private static List> parseSort(String jsonString) throws IOException { + XContentParser itemParser = XContentHelper.createParser(new BytesArray(jsonString)); + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); + context.reset(itemParser); + + assertEquals(XContentParser.Token.START_OBJECT, itemParser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, itemParser.nextToken()); + assertEquals("sort", itemParser.currentName()); + itemParser.nextToken(); + return SortBuilder.fromXContent(context); + } +} From 2bb57b6f2c376fe36d7ea550a22df91791faba9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 23 Mar 2016 11:55:27 +0100 Subject: [PATCH 2/2] Remove SortBuilderParser interface, using abstract methods in SortBuilder --- .../index/query/QueryParser.java | 2 +- .../search/sort/FieldSortBuilder.java | 2 +- .../search/sort/GeoDistanceSortBuilder.java | 2 +- .../search/sort/ScoreSortBuilder.java | 2 +- .../search/sort/ScriptSortBuilder.java | 2 +- .../search/sort/SortBuilder.java | 103 ++++++------------ .../search/sort/SortBuilderParser.java | 47 -------- .../search/sort/AbstractSortTestCase.java | 2 +- 8 files changed, 41 insertions(+), 121 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/search/sort/SortBuilderParser.java diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParser.java b/core/src/main/java/org/elasticsearch/index/query/QueryParser.java index 0a3f6d6147c..1226564bb3b 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParser.java @@ -33,7 +33,7 @@ public interface QueryParser> { String[] names(); /** - * Creates a new {@link QueryBuilder} from the query held by the {@link QueryShardContext} + * Creates a new {@link QueryBuilder} from the query held by the {@link QueryParseContext} * in {@link org.elasticsearch.common.xcontent.XContent} format * * @param parseContext diff --git a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 02f39cf9490..fe946521ea4 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -43,7 +43,7 @@ import java.util.Objects; /** * A sort builder to sort based on a document field. */ -public class FieldSortBuilder extends SortBuilder implements SortBuilderParser { +public class FieldSortBuilder extends SortBuilder { static final FieldSortBuilder PROTOTYPE = new FieldSortBuilder(""); public static final String NAME = "field_sort"; public static final ParseField NESTED_PATH = new ParseField("nested_path"); diff --git a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 66c4fba43fb..df8eb1f2d7c 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -60,7 +60,7 @@ import java.util.Objects; /** * A geo distance based sorting on a geo point like field. */ -public class GeoDistanceSortBuilder extends SortBuilder implements SortBuilderParser { +public class GeoDistanceSortBuilder extends SortBuilder { public static final String NAME = "_geo_distance"; public static final String ALTERNATIVE_NAME = "_geoDistance"; public static final boolean DEFAULT_COERCE = false; diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java index db2fccf6712..b35b9b94164 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java @@ -36,7 +36,7 @@ import java.util.Objects; /** * A sort builder allowing to sort by score. */ -public class ScoreSortBuilder extends SortBuilder implements SortBuilderParser { +public class ScoreSortBuilder extends SortBuilder { public static final String NAME = "_score"; static final ScoreSortBuilder PROTOTYPE = new ScoreSortBuilder(); diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index 1702c03bc6c..b905af881b3 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -64,7 +64,7 @@ import java.util.Objects; /** * Script sort builder allows to sort based on a custom script expression. */ -public class ScriptSortBuilder extends SortBuilder implements SortBuilderParser { +public class ScriptSortBuilder extends SortBuilder { public static final String NAME = "_script"; static final ScriptSortBuilder PROTOTYPE = new ScriptSortBuilder(new Script("_na_"), ScriptSortType.STRING); diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java index 45fd4d45796..c966a343751 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/SortBuilder.java @@ -20,15 +20,13 @@ package org.elasticsearch.search.sort; import org.apache.lucene.search.Query; -import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; import org.apache.lucene.search.join.BitSetProducer; -import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.support.ToXContentToBytes; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.lucene.search.Queries; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.elasticsearch.index.mapper.object.ObjectMapper; @@ -36,7 +34,6 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; -import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.ArrayList; @@ -44,14 +41,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import static java.util.Collections.unmodifiableMap; /** * */ -public abstract class SortBuilder> implements SortBuilderParser, ToXContent { +public abstract class SortBuilder> extends ToXContentToBytes implements NamedWriteable { protected SortOrder order = SortOrder.ASC; public static final ParseField ORDER_FIELD = new ParseField("order"); @@ -67,17 +63,26 @@ public abstract class SortBuilder> implements SortBuild PARSERS = unmodifiableMap(parsers); } - @Override - public String toString() { - try { - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.prettyPrint(); - toXContent(builder, EMPTY_PARAMS); - return builder.string(); - } catch (Exception e) { - throw new ElasticsearchException("Failed to build query", e); - } - } + /** + * Creates a new {@link SortBuilder} from the query held by the {@link QueryParseContext} + * in {@link org.elasticsearch.common.xcontent.XContent} format + * + * @param parseContext + * the input parse context. The state on the parser contained in + * this context will be changed as a side effect of this method call + * @param fieldName + * in some sort syntax variations the field name precedes the xContent object that + * specifies further parameters, e.g. in '{ "foo": { "order" : "asc"} }'. When + * parsing the inner object, the field name can be passed in via this argument + * + * @return the new sort builder instance + */ + protected abstract T fromXContent(QueryParseContext parseContext, @Nullable String fieldName) throws IOException; + + /** + * Create a @link {@link SortField} from this builder. + */ + protected abstract SortField build(QueryShardContext context) throws IOException; /** * Set the order of sorting. @@ -106,11 +111,7 @@ public abstract class SortBuilder> implements SortBuild parseCompoundSortField(parser, context, sortFields); } else if (token == XContentParser.Token.VALUE_STRING) { String fieldName = parser.text(); - if (fieldName.equals(ScoreSortBuilder.NAME)) { - sortFields.add(new ScoreSortBuilder()); - } else { - sortFields.add(new FieldSortBuilder(fieldName)); - } + sortFields.add(fieldOrScoreSort(fieldName)); } else { throw new IllegalArgumentException("malformed sort format, " + "within the sort array, an object, or an actual string are allowed"); @@ -118,11 +119,7 @@ public abstract class SortBuilder> implements SortBuild } } else if (token == XContentParser.Token.VALUE_STRING) { String fieldName = parser.text(); - if (fieldName.equals(ScoreSortBuilder.NAME)) { - sortFields.add(new ScoreSortBuilder()); - } else { - sortFields.add(new FieldSortBuilder(fieldName)); - } + sortFields.add(fieldOrScoreSort(fieldName)); } else if (token == XContentParser.Token.START_OBJECT) { parseCompoundSortField(parser, context, sortFields); } else { @@ -131,6 +128,14 @@ public abstract class SortBuilder> implements SortBuild return sortFields; } + private static SortBuilder fieldOrScoreSort(String fieldName) { + if (fieldName.equals(ScoreSortBuilder.NAME)) { + return new ScoreSortBuilder(); + } else { + return new FieldSortBuilder(fieldName); + } + } + private static void parseCompoundSortField(XContentParser parser, QueryParseContext context, List> sortFields) throws IOException { XContentParser.Token token; @@ -140,11 +145,7 @@ public abstract class SortBuilder> implements SortBuild token = parser.nextToken(); if (token == XContentParser.Token.VALUE_STRING) { SortOrder order = SortOrder.fromString(parser.text()); - if (fieldName.equals(ScoreSortBuilder.NAME)) { - sortFields.add(new ScoreSortBuilder().order(order)); - } else { - sortFields.add(new FieldSortBuilder(fieldName).order(order)); - } + sortFields.add(fieldOrScoreSort(fieldName).order(order)); } else { if (PARSERS.containsKey(fieldName)) { sortFields.add(PARSERS.get(fieldName).fromXContent(context, fieldName)); @@ -156,40 +157,6 @@ public abstract class SortBuilder> implements SortBuild } } - public static void parseSort(XContentParser parser, SearchContext context) throws IOException { - QueryParseContext parseContext = context.getQueryShardContext().parseContext(); - parseContext.reset(parser); - Optional sortOptional = buildSort(SortBuilder.fromXContent(parseContext), context.getQueryShardContext()); - if (sortOptional.isPresent()) { - context.sort(sortOptional.get()); - } - } - - public static Optional buildSort(List> sortBuilders, QueryShardContext context) throws IOException { - List sortFields = new ArrayList<>(sortBuilders.size()); - for (SortBuilder builder : sortBuilders) { - sortFields.add(builder.build(context)); - } - if (!sortFields.isEmpty()) { - // optimize if we just sort on score non reversed, we don't really need sorting - boolean sort; - if (sortFields.size() > 1) { - sort = true; - } else { - SortField sortField = sortFields.get(0); - if (sortField.getType() == SortField.Type.SCORE && !sortField.getReverse()) { - sort = false; - } else { - sort = true; - } - } - if (sort) { - return Optional.of(new Sort(sortFields.toArray(new SortField[sortFields.size()]))); - } - } - return Optional.empty(); - } - protected static Nested resolveNested(QueryShardContext context, String nestedPath, QueryBuilder nestedFilter) throws IOException { Nested nested = null; if (nestedPath != null) { diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortBuilderParser.java b/core/src/main/java/org/elasticsearch/search/sort/SortBuilderParser.java deleted file mode 100644 index 706fa7863f0..00000000000 --- a/core/src/main/java/org/elasticsearch/search/sort/SortBuilderParser.java +++ /dev/null @@ -1,47 +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.apache.lucene.search.SortField; -import org.elasticsearch.common.io.stream.NamedWriteable; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.index.query.QueryParseContext; -import org.elasticsearch.index.query.QueryShardContext; - -import java.io.IOException; - -public interface SortBuilderParser extends NamedWriteable, 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 - */ - T fromXContent(QueryParseContext context, String elementName) throws IOException; - - /** - * Create a @link {@link SortField} from this builder. - */ - SortField build(QueryShardContext context) throws IOException; -} diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index 84b23fb1449..c3e31bf2899 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -72,7 +72,7 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; -public abstract class AbstractSortTestCase & SortBuilderParser> extends ESTestCase { +public abstract class AbstractSortTestCase> extends ESTestCase { protected static NamedWriteableRegistry namedWriteableRegistry;