diff options
author | Christoph Büscher <christoph@elastic.co> | 2016-03-15 11:41:53 +0100 |
---|---|---|
committer | Christoph Büscher <christoph@elastic.co> | 2016-03-15 12:43:19 +0100 |
commit | bc84cdfed1cf912b6ba486977d5ed9df024d75d9 (patch) | |
tree | 876e4baeae4e51ed018fc9b51e25b9c220be09c7 /core | |
parent | 1f1f6861b777cff0deb925ba88b623d2b3c249ed (diff) |
Using SortMode enum in all sort builders
Diffstat (limited to 'core')
12 files changed, 123 insertions, 161 deletions
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 1157457afb..a5707ea4a5 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -53,9 +53,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S private String unmappedType; - private String sortMode; + private SortMode sortMode; - private QueryBuilder nestedFilter; + private QueryBuilder<?> nestedFilter; private String nestedPath; @@ -65,7 +65,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S this.order(template.order()); this.missing(template.missing()); this.unmappedType(template.unmappedType()); - this.sortMode(template.sortMode()); + if (template.sortMode != null) { + this.sortMode(template.sortMode()); + } this.setNestedFilter(template.getNestedFilter()); this.setNestedPath(template.getNestedPath()); } @@ -134,12 +136,12 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S * 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. */ - public FieldSortBuilder sortMode(String sortMode) { + public FieldSortBuilder sortMode(SortMode sortMode) { + Objects.requireNonNull(sortMode, "sort mode cannot be null"); this.sortMode = sortMode; return this; } @@ -148,14 +150,14 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S * Returns what values to pick in the case a document contains multiple * values for the targeted sort field. */ - public String sortMode() { + public SortMode 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? */ @@ -263,7 +265,10 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S out.writeBoolean(false); } - out.writeOptionalString(this.sortMode); + out.writeBoolean(this.sortMode != null); + if (this.sortMode != null) { + this.sortMode.writeTo(out); + } out.writeOptionalString(this.unmappedType); } @@ -272,7 +277,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S String fieldName = in.readString(); FieldSortBuilder result = new FieldSortBuilder(fieldName); if (in.readBoolean()) { - QueryBuilder query = in.readQuery(); + QueryBuilder<?> query = in.readQuery(); result.setNestedFilter(query); } result.setNestedPath(in.readOptionalString()); @@ -281,7 +286,9 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S if (in.readBoolean()) { result.order(SortOrder.readOrderFrom(in)); } - result.sortMode(in.readOptionalString()); + if (in.readBoolean()) { + result.sortMode(SortMode.PROTOTYPE.readFrom(in)); + } result.unmappedType(in.readOptionalString()); return result; } @@ -290,11 +297,11 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S public FieldSortBuilder fromXContent(QueryParseContext context, String fieldName) throws IOException { XContentParser parser = context.parser(); - QueryBuilder nestedFilter = null; + QueryBuilder<?> nestedFilter = null; String nestedPath = null; Object missing = null; SortOrder order = null; - String sortMode = null; + SortMode sortMode = null; String unmappedType = null; String currentFieldName = null; @@ -328,7 +335,7 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S throw new IllegalStateException("Sort order " + sortOrder + " not supported."); } } else if (context.parseFieldMatcher().match(currentFieldName, SORT_MODE)) { - sortMode = parser.text(); + sortMode = SortMode.fromString(parser.text()); } else if (context.parseFieldMatcher().match(currentFieldName, UNMAPPED_TYPE)) { unmappedType = parser.text(); } else { 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 630ff635af..9785a0fc24 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -30,7 +30,6 @@ 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 org.elasticsearch.search.MultiValueMode; import java.io.IOException; import java.util.ArrayList; @@ -55,8 +54,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> private GeoDistance geoDistance = GeoDistance.DEFAULT; private DistanceUnit unit = DistanceUnit.DEFAULT; - // TODO there is an enum that covers that parameter which we should be using here - private String sortMode = null; + private SortMode sortMode = null; @SuppressWarnings("rawtypes") private QueryBuilder nestedFilter; private String nestedPath; @@ -204,9 +202,9 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> * Defines which distance to use for sorting in the case a document contains multiple geo points. * Possible values: min and max */ - public GeoDistanceSortBuilder sortMode(String sortMode) { - MultiValueMode temp = MultiValueMode.fromString(sortMode); - if (temp == MultiValueMode.SUM) { + public GeoDistanceSortBuilder sortMode(SortMode sortMode) { + Objects.requireNonNull(sortMode, "sort mode cannot be null"); + if (sortMode == SortMode.SUM) { throw new IllegalArgumentException("sort_mode [sum] isn't supported for sorting by geo distance"); } this.sortMode = sortMode; @@ -214,7 +212,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> } /** Returns which distance to use for sorting in the case a document contains multiple geo points. */ - public String sortMode() { + public SortMode sortMode() { return this.sortMode; } @@ -345,7 +343,10 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> geoDistance.writeTo(out); unit.writeTo(out); order.writeTo(out); - out.writeOptionalString(sortMode); + out.writeBoolean(this.sortMode != null); + if (this.sortMode != null) { + sortMode.writeTo(out); + } if (nestedFilter != null) { out.writeBoolean(true); out.writeQuery(nestedFilter); @@ -367,9 +368,8 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> result.geoDistance(GeoDistance.readGeoDistanceFrom(in)); result.unit(DistanceUnit.readDistanceUnit(in)); result.order(SortOrder.readOrderFrom(in)); - String sortMode = in.readOptionalString(); - if (sortMode != null) { - result.sortMode(sortMode); + if (in.readBoolean()) { + result.sortMode = SortMode.PROTOTYPE.readFrom(in); } if (in.readBoolean()) { result.setNestedFilter(in.readQuery()); @@ -388,7 +388,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> DistanceUnit unit = DistanceUnit.DEFAULT; GeoDistance geoDistance = GeoDistance.DEFAULT; SortOrder order = SortOrder.ASC; - MultiValueMode sortMode = null; + SortMode sortMode = null; QueryBuilder<?> nestedFilter = null; String nestedPath = null; @@ -437,7 +437,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> ignoreMalformed = ignore_malformed_value; } } else if ("sort_mode".equals(currentName) || "sortMode".equals(currentName) || "mode".equals(currentName)) { - sortMode = MultiValueMode.fromString(parser.text()); + sortMode = SortMode.fromString(parser.text()); } else if ("nested_path".equals(currentName) || "nestedPath".equals(currentName)) { nestedPath = parser.text(); } else { @@ -454,7 +454,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> result.unit(unit); result.order(order); if (sortMode != null) { - result.sortMode(sortMode.name()); + result.sortMode(sortMode); } result.setNestedFilter(nestedFilter); result.setNestedPath(nestedPath); 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 c416965f38..76ca56f0f9 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScoreSortBuilder.java @@ -34,8 +34,7 @@ import java.util.Objects; /** * A sort builder allowing to sort by score. */ -public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements SortBuilderParser<ScoreSortBuilder>, - SortElementParserTemp<ScoreSortBuilder> { +public class ScoreSortBuilder extends SortBuilder<ScoreSortBuilder> implements SortBuilderParser<ScoreSortBuilder> { private 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 9b51eeca31..e77d12ce47 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.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.io.stream.Writeable; @@ -44,8 +43,7 @@ import java.util.Objects; /** * Script sort builder allows to sort based on a custom script expression. */ -public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements NamedWriteable<ScriptSortBuilder>, - SortElementParserTemp<ScriptSortBuilder> { +public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements SortBuilderParser<ScriptSortBuilder> { private static final String NAME = "_script"; static final ScriptSortBuilder PROTOTYPE = new ScriptSortBuilder(new Script("_na_"), ScriptSortType.STRING); @@ -72,8 +70,8 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements * @param script * The script to use. * @param type - * The type of the script, can be either {@link ScriptSortParser#STRING_SORT_TYPE} or - * {@link ScriptSortParser#NUMBER_SORT_TYPE} + * The type of the script, can be either {@link ScriptSortType#STRING} or + * {@link ScriptSortType#NUMBER} */ public ScriptSortBuilder(Script script, ScriptSortType type) { Objects.requireNonNull(script, "script cannot be null"); diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortElementParserTemp.java b/core/src/main/java/org/elasticsearch/search/sort/SortElementParserTemp.java deleted file mode 100644 index 069f1380b4..0000000000 --- a/core/src/main/java/org/elasticsearch/search/sort/SortElementParserTemp.java +++ /dev/null @@ -1,39 +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.index.query.QueryParseContext; - -import java.io.IOException; - -// TODO once sort refactoring is done this needs to be merged into SortBuilder -public interface SortElementParserTemp<T extends SortBuilder> { - /** - * Creates a new SortBuilder from the json held by the {@link SortElementParserTemp} - * 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; -} diff --git a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java index 2f93260414..e38ac0ca76 100644 --- a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java +++ b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java @@ -34,6 +34,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.sort.SortBuilders; +import org.elasticsearch.search.sort.SortMode; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESIntegTestCase; @@ -748,7 +749,7 @@ public class SimpleNestedIT extends ESIntegTestCase { .addSort( SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") - .sortMode("sum") + .sortMode(SortMode.SUM) .order(SortOrder.ASC) ) .execute().actionGet(); @@ -768,7 +769,7 @@ public class SimpleNestedIT extends ESIntegTestCase { .addSort( SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") - .sortMode("sum") + .sortMode(SortMode.SUM) .order(SortOrder.DESC) ) .execute().actionGet(); @@ -789,7 +790,7 @@ public class SimpleNestedIT extends ESIntegTestCase { SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true)) - .sortMode("sum") + .sortMode(SortMode.SUM) .order(SortOrder.ASC) ) .execute().actionGet(); @@ -809,7 +810,7 @@ public class SimpleNestedIT extends ESIntegTestCase { .addSort( SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") - .sortMode("avg") + .sortMode(SortMode.AVG) .order(SortOrder.ASC) ) .execute().actionGet(); @@ -828,7 +829,7 @@ public class SimpleNestedIT extends ESIntegTestCase { .addSort( SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") - .sortMode("avg") + .sortMode(SortMode.AVG) .order(SortOrder.DESC) ) .execute().actionGet(); @@ -849,7 +850,7 @@ public class SimpleNestedIT extends ESIntegTestCase { SortBuilders.fieldSort("parent.child.child_values") .setNestedPath("parent.child") .setNestedFilter(QueryBuilders.termQuery("parent.child.filter", true)) - .sortMode("avg") + .sortMode(SortMode.AVG) .order(SortOrder.ASC) ) .execute().actionGet(); 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 1a812a166a..f7f9edbc0b 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -19,7 +19,6 @@ package org.elasticsearch.search.sort; -import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -87,8 +86,6 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderPa builder.endObject(); XContentParser itemParser = XContentHelper.createParser(builder.bytes()); - ParsingException except = new ParsingException(itemParser.getTokenLocation(), "mytext", itemParser.getTokenLocation()); - System.out.println(except.getMessage()); itemParser.nextToken(); /* @@ -158,7 +155,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilder & SortBuilderPa try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { T prototype = (T) namedWriteableRegistry.getPrototype(SortBuilder.class, original.getWriteableName()); - T copy = (T) prototype.readFrom(in); + T copy = prototype.readFrom(in); return copy; } } diff --git a/core/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java b/core/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java index 894f0fbe6b..ef313620b5 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java +++ b/core/src/test/java/org/elasticsearch/search/sort/FieldSortIT.java @@ -19,44 +19,6 @@ package org.elasticsearch.search.sort; -import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; -import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery; -import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.fieldValueFactorFunction; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertOrderedSearchHits; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; -import static org.hamcrest.Matchers.closeTo; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.Matchers.lessThanOrEqualTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.Locale; -import java.util.Random; -import java.util.Set; -import java.util.TreeMap; -import java.util.Map.Entry; -import java.util.concurrent.ExecutionException; - import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.TestUtil; @@ -80,6 +42,43 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalSettingsPlugin; import org.hamcrest.Matchers; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Map.Entry; +import java.util.Random; +import java.util.Set; +import java.util.TreeMap; +import java.util.concurrent.ExecutionException; + +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.fieldValueFactorFunction; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; +import static org.hamcrest.Matchers.closeTo; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; + public class FieldSortIT extends ESIntegTestCase { @Override protected Collection<Class<? extends Plugin>> nodePlugins() { @@ -985,7 +984,7 @@ public class FieldSortIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) .setSize(10) - .addSort(SortBuilders.fieldSort("long_values").order(SortOrder.DESC).sortMode("sum")) + .addSort(SortBuilders.fieldSort("long_values").order(SortOrder.DESC).sortMode(SortMode.SUM)) .execute().actionGet(); assertThat(searchResponse.getHits().getTotalHits(), equalTo(3L)); diff --git a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java index d40cbf9300..ed733fd4cd 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceIT.java @@ -270,7 +270,7 @@ public class GeoDistanceIT extends ESIntegTestCase { // Order: Asc, Mode: max searchResponse = client().prepareSearch("test").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max")) + .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX)) .execute().actionGet(); assertHitCount(searchResponse, 5); @@ -296,7 +296,7 @@ public class GeoDistanceIT extends ESIntegTestCase { // Order: Desc, Mode: min searchResponse = client().prepareSearch("test").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min")) + .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN)) .execute().actionGet(); assertHitCount(searchResponse, 5); @@ -308,7 +308,7 @@ public class GeoDistanceIT extends ESIntegTestCase { assertThat(((Number) searchResponse.getHits().getAt(4).sortValues()[0]).doubleValue(), closeTo(0d, 10d)); searchResponse = client().prepareSearch("test").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC)) + .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC)) .execute().actionGet(); assertHitCount(searchResponse, 5); @@ -320,7 +320,7 @@ public class GeoDistanceIT extends ESIntegTestCase { assertThat(((Number) searchResponse.getHits().getAt(4).sortValues()[0]).doubleValue(), closeTo(5301d, 10d)); searchResponse = client().prepareSearch("test").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("avg").order(SortOrder.DESC)) + .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.DESC)) .execute().actionGet(); assertHitCount(searchResponse, 5); @@ -333,7 +333,7 @@ public class GeoDistanceIT extends ESIntegTestCase { try { client().prepareSearch("test").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode("sum")); + .addSort(SortBuilders.geoDistanceSort("locations", 40.7143528, -74.0059731).sortMode(SortMode.SUM)); fail("sum should not be supported for sorting by geo distance"); } catch (IllegalArgumentException e) { // expected @@ -455,7 +455,7 @@ public class GeoDistanceIT extends ESIntegTestCase { // Order: Asc, Mode: max searchResponse = client() .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", - 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode("max").setNestedPath("branches")) + 40.7143528, -74.0059731).order(SortOrder.ASC).sortMode(SortMode.MAX).setNestedPath("branches")) .execute().actionGet(); assertHitCount(searchResponse, 4); @@ -480,7 +480,7 @@ public class GeoDistanceIT extends ESIntegTestCase { // Order: Desc, Mode: min searchResponse = client() .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", - 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode("min").setNestedPath("branches")) + 40.7143528, -74.0059731).order(SortOrder.DESC).sortMode(SortMode.MIN).setNestedPath("branches")) .execute().actionGet(); assertHitCount(searchResponse, 4); @@ -492,7 +492,7 @@ public class GeoDistanceIT extends ESIntegTestCase { searchResponse = client() .prepareSearch("companies").setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", - 40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC).setNestedPath("branches")) + 40.7143528, -74.0059731).sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches")) .execute().actionGet(); assertHitCount(searchResponse, 4); @@ -504,7 +504,7 @@ public class GeoDistanceIT extends ESIntegTestCase { searchResponse = client().prepareSearch("companies") .setQuery(matchAllQuery()).addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731) - .setNestedPath("branches").sortMode("avg").order(SortOrder.DESC).setNestedPath("branches")) + .setNestedPath("branches").sortMode(SortMode.AVG).order(SortOrder.DESC).setNestedPath("branches")) .execute().actionGet(); assertHitCount(searchResponse, 4); @@ -517,7 +517,7 @@ public class GeoDistanceIT extends ESIntegTestCase { searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery()) .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731) .setNestedFilter(termQuery("branches.name", "brooklyn")) - .sortMode("avg").order(SortOrder.ASC).setNestedPath("branches")) + .sortMode(SortMode.AVG).order(SortOrder.ASC).setNestedPath("branches")) .execute().actionGet(); assertHitCount(searchResponse, 4); assertFirstHit(searchResponse, hasId("4")); @@ -529,7 +529,7 @@ public class GeoDistanceIT extends ESIntegTestCase { try { client().prepareSearch("companies").setQuery(matchAllQuery()) - .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731).sortMode("sum") + .addSort(SortBuilders.geoDistanceSort("branches.location", 40.7143528, -74.0059731).sortMode(SortMode.SUM) .setNestedPath("branches")); fail("Sum should not be allowed as sort mode"); } catch (IllegalArgumentException e) { @@ -567,11 +567,11 @@ public class GeoDistanceIT extends ESIntegTestCase { assertHitCount(result, 1); } - private double randomLon() { + private static double randomLon() { return randomDouble() * 360 - 180; } - private double randomLat() { + private static double randomLat() { return randomDouble() * 180 - 90; } @@ -619,7 +619,7 @@ public class GeoDistanceIT extends ESIntegTestCase { } } - private long assertDuelOptimization(SearchResponse resp) { + private static long assertDuelOptimization(SearchResponse resp) { long matches = -1; assertSearchResponse(resp); if (matches < 0) { diff --git a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java index 309c4bcdaf..ac9270cbe2 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java @@ -95,7 +95,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { SearchResponse searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(new GeoDistanceSortBuilder("location", q).sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d1", "d2"); assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 3, 2, DistanceUnit.KILOMETERS), 0.01d)); @@ -103,7 +103,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(new GeoDistanceSortBuilder("location", q).sortMode("min").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MIN).order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d2", "d1"); assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 1, 5, 1, DistanceUnit.KILOMETERS), 0.01d)); @@ -111,7 +111,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(new GeoDistanceSortBuilder("location", q).sortMode("max").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MAX).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d1", "d2"); assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 4, 1, DistanceUnit.KILOMETERS), 0.01d)); @@ -119,7 +119,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(new GeoDistanceSortBuilder("location", q).sortMode("max").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(new GeoDistanceSortBuilder("location", q).sortMode(SortMode.MAX).order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d2", "d1"); assertThat((Double)searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 1, 6, 2, DistanceUnit.KILOMETERS), 0.01d)); @@ -194,7 +194,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { SearchResponse searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d1", "d2"); assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2.5, 1, 2, 1, DistanceUnit.KILOMETERS), 1.e-4)); @@ -202,7 +202,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(geoDistanceSortBuilder.sortMode("max").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(geoDistanceSortBuilder.sortMode(SortMode.MAX).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); assertOrderedSearchHits(searchResponse, "d1", "d2"); assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(3.25, 4, 2, 1, DistanceUnit.KILOMETERS), 1.e-4)); @@ -223,7 +223,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { SearchResponse searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); checkCorrectSortOrderForGeoSort(searchResponse); @@ -231,7 +231,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); checkCorrectSortOrderForGeoSort(searchResponse); @@ -239,7 +239,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { searchResponse = client().prepareSearch() .setQuery(matchAllQuery()) - .addSort(geoDistanceSortBuilder.sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) + .addSort(geoDistanceSortBuilder.sortMode(SortMode.MIN).order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS)) .execute().actionGet(); checkCorrectSortOrderForGeoSort(searchResponse); @@ -265,7 +265,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase { checkCorrectSortOrderForGeoSort(searchResponse); } - private void checkCorrectSortOrderForGeoSort(SearchResponse searchResponse) { + private static void checkCorrectSortOrderForGeoSort(SearchResponse searchResponse) { assertOrderedSearchHits(searchResponse, "d2", "d1"); assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 1, 2, DistanceUnit.KILOMETERS), 1.e-4)); assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], closeTo(GeoDistance.PLANE.calculate(2, 2, 1, 1, DistanceUnit.KILOMETERS), 1.e-4)); diff --git a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java index 611053b14d..50e4aeeb71 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; -import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.geo.RandomGeoGenerator; import java.io.IOException; @@ -90,16 +89,15 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc return result; } - private static String mode(String original) { - String[] modes = {"MIN", "MAX", "AVG"}; - String mode = ESTestCase.randomFrom(modes); - while (mode.equals(original)) { - mode = ESTestCase.randomFrom(modes); - } - return mode; + private static SortMode mode(SortMode original) { + SortMode result; + do { + result = randomFrom(SortMode.values()); + } while (result == SortMode.SUM || result == original); + return result; } - private DistanceUnit unit(DistanceUnit original) { + private static DistanceUnit unit(DistanceUnit original) { int id = -1; while (id == -1 || (original != null && original.ordinal() == id)) { id = randomIntBetween(0, DistanceUnit.values().length - 1); @@ -107,7 +105,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc return DistanceUnit.values()[id]; } - private GeoPoint[] points(GeoPoint[] original) { + private static GeoPoint[] points(GeoPoint[] original) { GeoPoint[] result = null; while (result == null || Arrays.deepEquals(original, result)) { int count = randomIntBetween(1, 10); @@ -119,7 +117,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc return result; } - private GeoDistance geoDistance(GeoDistance original) { + private static GeoDistance geoDistance(GeoDistance original) { int id = -1; while (id == -1 || (original != null && original.ordinal() == id)) { id = randomIntBetween(0, GeoDistance.values().length - 1); @@ -177,7 +175,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc GeoPoint point = RandomGeoGenerator.randomPoint(getRandom()); builder.point(point.getLat(), point.getLon()); try { - builder.sortMode("SUM"); + builder.sortMode(SortMode.SUM); fail("sort mode sum should not be supported"); } catch (IllegalArgumentException e) { // all good diff --git a/core/src/test/java/org/elasticsearch/search/sort/SortModeTest.java b/core/src/test/java/org/elasticsearch/search/sort/SortModeTests.java index ae15923144..29deb6dd76 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/SortModeTest.java +++ b/core/src/test/java/org/elasticsearch/search/sort/SortModeTests.java @@ -23,7 +23,9 @@ import org.elasticsearch.test.ESTestCase; import org.junit.Rule; import org.junit.rules.ExpectedException; -public class SortModeTest extends ESTestCase { +import java.util.Locale; + +public class SortModeTests extends ESTestCase { @Rule public ExpectedException exceptionRule = ExpectedException.none(); @@ -44,7 +46,7 @@ public class SortModeTest extends ESTestCase { for (SortMode mode : SortMode.values()) { assertEquals(mode, SortMode.fromString(mode.toString())); - assertEquals(mode, SortMode.fromString(mode.toString().toUpperCase())); + assertEquals(mode, SortMode.fromString(mode.toString().toUpperCase(Locale.ROOT))); } } |