diff --git a/core/src/main/java/org/elasticsearch/search/sort/NestedSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/NestedSortBuilder.java index bcf5ee681e1..941ac2909ab 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/NestedSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/NestedSortBuilder.java @@ -19,8 +19,6 @@ package org.elasticsearch.search.sort; -import static org.elasticsearch.search.sort.SortBuilder.parseNestedFilter; - import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -33,12 +31,14 @@ import org.elasticsearch.index.query.QueryBuilder; import java.io.IOException; import java.util.Objects; -public class NestedSortBuilder implements Writeable, Writeable.Reader, ToXContentObject { +import static org.elasticsearch.search.sort.SortBuilder.parseNestedFilter; + +public class NestedSortBuilder implements Writeable, ToXContentObject { public static final ParseField NESTED_FIELD = new ParseField("nested"); public static final ParseField PATH_FIELD = new ParseField("path"); public static final ParseField FILTER_FIELD = new ParseField("filter"); - private String path; + private final String path; private QueryBuilder filter; private NestedSortBuilder nestedSort; @@ -47,26 +47,15 @@ public class NestedSortBuilder implements Writeable, Writeable.Reader> implements NamedWrit protected static Nested resolveNested(QueryShardContext context, String nestedPath, QueryBuilder nestedFilter) throws IOException { NestedSortBuilder nestedSortBuilder = new NestedSortBuilder(nestedPath); nestedSortBuilder.setFilter(nestedFilter); - return resolveNested(context, nestedSortBuilder); } @@ -190,7 +189,7 @@ public abstract class SortBuilder> implements NamedWrit return resolveNested(context, nestedSort, null); } - protected static Nested resolveNested(QueryShardContext context, NestedSortBuilder nestedSort, Nested nested) throws IOException { + private static Nested resolveNested(QueryShardContext context, NestedSortBuilder nestedSort, Nested nested) throws IOException { if (nestedSort == null || nestedSort.getPath() == null) { return null; } diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortBuilders.java b/core/src/main/java/org/elasticsearch/search/sort/SortBuilders.java index 9172d6b7bb7..3eae9b8d019 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/SortBuilders.java +++ b/core/src/main/java/org/elasticsearch/search/sort/SortBuilders.java @@ -56,14 +56,6 @@ public class SortBuilders { return new ScriptSortBuilder(script, type); } - /** - * Constructs a new nested sort builder. - * - */ - public static NestedSortBuilder nestedSort(String path) { - return new NestedSortBuilder(path); - } - /** * A geo distance based sort. * 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 6fa298118eb..56bb837b4e0 100644 --- a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java +++ b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java @@ -35,6 +35,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.search.sort.SortBuilders; import org.elasticsearch.search.sort.SortMode; import org.elasticsearch.search.sort.SortOrder; @@ -629,11 +630,11 @@ public class SimpleNestedIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort( SortBuilders.fieldSort("acl.operation.user.username") - .setNestedSort(SortBuilders.nestedSort("acl") + .setNestedSort(new NestedSortBuilder("acl") .setFilter(QueryBuilders.termQuery("acl.access_id", "1")) - .setNestedSort(SortBuilders.nestedSort("acl.operation") + .setNestedSort(new NestedSortBuilder("acl.operation") .setFilter(QueryBuilders.termQuery("acl.operation.name", "read")) - .setNestedSort(SortBuilders.nestedSort("acl.operation.user")))) + .setNestedSort(new NestedSortBuilder("acl.operation.user")))) .sortMode(SortMode.MAX) .order(SortOrder.ASC) ) @@ -652,11 +653,11 @@ public class SimpleNestedIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort( SortBuilders.fieldSort("acl.operation.user.username") - .setNestedSort(SortBuilders.nestedSort("acl") + .setNestedSort(new NestedSortBuilder("acl") .setFilter(QueryBuilders.termQuery("acl.access_id", "1")) - .setNestedSort(SortBuilders.nestedSort("acl.operation") + .setNestedSort(new NestedSortBuilder("acl.operation") .setFilter(QueryBuilders.termQuery("acl.operation.name", "read")) - .setNestedSort(SortBuilders.nestedSort("acl.operation.user")))) + .setNestedSort(new NestedSortBuilder("acl.operation.user")))) .sortMode(SortMode.MIN) .order(SortOrder.ASC) ) @@ -674,10 +675,10 @@ public class SimpleNestedIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort( SortBuilders.fieldSort("acl.operation.user.id") - .setNestedSort(SortBuilders.nestedSort("acl") - .setNestedSort(SortBuilders.nestedSort("acl.operation") + .setNestedSort(new NestedSortBuilder("acl") + .setNestedSort(new NestedSortBuilder("acl.operation") .setFilter(QueryBuilders.termQuery("acl.operation.name", "execute")) - .setNestedSort(SortBuilders.nestedSort("acl.operation.user") + .setNestedSort(new NestedSortBuilder("acl.operation.user") .setFilter(QueryBuilders.termsQuery("acl.operation.user.username", "matt", "luca"))))) .missing("_first") .sortMode(SortMode.MIN) @@ -696,10 +697,10 @@ public class SimpleNestedIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort( SortBuilders.fieldSort("acl.operation.user.username") - .setNestedSort(SortBuilders.nestedSort("acl") - .setNestedSort(SortBuilders.nestedSort("acl.operation") + .setNestedSort(new NestedSortBuilder("acl") + .setNestedSort(new NestedSortBuilder("acl.operation") .setFilter(QueryBuilders.termQuery("acl.operation.name", "execute")) - .setNestedSort(SortBuilders.nestedSort("acl.operation.user") + .setNestedSort(new NestedSortBuilder("acl.operation.user") .setFilter(QueryBuilders.termsQuery("acl.operation.user.username", "matt", "luca"))))) .sortMode(SortMode.MIN) .order(SortOrder.DESC) @@ -942,9 +943,9 @@ public class SimpleNestedIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort( SortBuilders.fieldSort("parent.child.child_values") - .setNestedSort(SortBuilders.nestedSort("parent") + .setNestedSort(new NestedSortBuilder("parent") .setFilter(QueryBuilders.termQuery("parent.filter", false)) - .setNestedSort(SortBuilders.nestedSort("parent.child"))) + .setNestedSort(new NestedSortBuilder("parent.child"))) .sortMode(SortMode.MAX) .order(SortOrder.ASC) ) 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 2a38b9dceb3..b368ed651ce 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java @@ -39,6 +39,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.List; +import static org.elasticsearch.search.sort.NestedSortBuilderTests.createRandomNestedSort; import static org.hamcrest.Matchers.instanceOf; public class FieldSortBuilderTests extends AbstractSortTestCase { @@ -80,11 +81,7 @@ public class FieldSortBuilderTests extends AbstractSortTestCase randomAlphaOfLengthBetween(1, 10))); - mutated.setNestedSort(nestedSort); + case 0: + mutated.setNestedSort(randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3))); break; - } - case 1: { - NestedSortBuilder nestedSort = new NestedSortBuilder(mutated.getNestedSort()); - nestedSort.setFilter(randomValueOtherThan(nestedSort.getFilter(), AbstractSortTestCase::randomNestedFilter)); - mutated.setNestedSort(nestedSort); - break; - } - case 2: + case 1: mutated.sortMode(randomValueOtherThan(original.sortMode(), () -> randomFrom(SortMode.values()))); break; - case 3: + case 2: mutated.unmappedType(randomValueOtherThan( original.unmappedType(), () -> randomAlphaOfLengthBetween(1, 10))); break; - case 4: + case 3: mutated.missing(randomValueOtherThan(original.missing(), () -> randomFrom(missingContent))); break; - case 5: + case 4: mutated.order(randomValueOtherThan(original.order(), () -> randomFrom(SortOrder.values()))); break; default: 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 7a0c91ba324..dc2e4ec17ea 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -87,15 +87,9 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase randomFrom(SortMode.values()))); } if (randomBoolean()) { - NestedSortBuilder nestedSort = SortBuilders.nestedSort( - randomValueOtherThan( - result.getNestedPath(), - () -> randomAlphaOfLengthBetween(1, 10))); - - if (randomBoolean()) { - nestedSort.setFilter(new MatchAllQueryBuilder()); - } - + // don't fully randomize here, GeoDistanceSort is picky about the filters that are allowed + NestedSortBuilder nestedSort = new NestedSortBuilder(randomAlphaOfLengthBetween(3, 10)); + nestedSort.setFilter(new MatchAllQueryBuilder()); result.setNestedSort(nestedSort); } if (randomBoolean()) { @@ -135,7 +129,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase randomFrom(SortMode.values()))); break; - case 6: { - NestedSortBuilder nestedSort = new NestedSortBuilder(result.getNestedSort()); - nestedSort.setFilter(randomValueOtherThan(nestedSort.getFilter(), AbstractSortTestCase::randomNestedFilter)); - result.setNestedSort(nestedSort); + case 6: + result.setNestedSort(randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3))); break; - } - case 7: { - NestedSortBuilder nestedSort = new NestedSortBuilder(result.getNestedSort()); - nestedSort.setPath(randomValueOtherThan(nestedSort.getPath(), () -> randomAlphaOfLengthBetween(1, 10))); - result.setNestedSort(nestedSort); - break; - } - case 8: + case 7: result.validation(randomValueOtherThan(result.validation(), () -> randomFrom(GeoValidationMethod.values()))); break; } diff --git a/core/src/test/java/org/elasticsearch/search/sort/NestedSortBuilderTests.java b/core/src/test/java/org/elasticsearch/search/sort/NestedSortBuilderTests.java new file mode 100644 index 00000000000..5e94cd1c7fd --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/sort/NestedSortBuilderTests.java @@ -0,0 +1,139 @@ +/* + * 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.NamedWriteableRegistry; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +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.common.xcontent.XContentType; +import org.elasticsearch.search.SearchModule; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EqualsHashCodeTestUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; + +import java.io.IOException; + +import static java.util.Collections.emptyList; + +public class NestedSortBuilderTests extends ESTestCase { + + private static final int NUMBER_OF_TESTBUILDERS = 20; + private static NamedWriteableRegistry namedWriteableRegistry; + private static NamedXContentRegistry xContentRegistry; + + @BeforeClass + public static void init() { + SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList()); + namedWriteableRegistry = new NamedWriteableRegistry(searchModule.getNamedWriteables()); + xContentRegistry = new NamedXContentRegistry(searchModule.getNamedXContents()); + } + + @AfterClass + public static void afterClass() throws Exception { + namedWriteableRegistry = null; + xContentRegistry = null; + } + + @Override + protected NamedXContentRegistry xContentRegistry() { + return xContentRegistry; + } + + public void testFromXContent() throws IOException { + for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { + NestedSortBuilder testItem = createRandomNestedSort(3); + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); + testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); + XContentBuilder shuffled = shuffleXContent(builder); + XContentParser parser = createParser(shuffled); + parser.nextToken(); + NestedSortBuilder parsedItem = NestedSortBuilder.fromXContent(parser); + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); + } + } + + /** + * Create a {@link NestedSortBuilder} with random path and filter of the given depth. + */ + public static NestedSortBuilder createRandomNestedSort(int depth) { + NestedSortBuilder nestedSort = new NestedSortBuilder(randomAlphaOfLengthBetween(3, 10)); + if (randomBoolean()) { + nestedSort.setFilter(AbstractSortTestCase.randomNestedFilter()); + } + if (depth > 0) { + nestedSort.setNestedSort(createRandomNestedSort(depth - 1)); + } + return nestedSort; + } + + /** + * Test serialization of the test nested sort. + */ + public void testSerialization() throws IOException { + for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { + NestedSortBuilder testsort = createRandomNestedSort(3); + NestedSortBuilder deserializedsort = copy(testsort); + assertEquals(testsort, deserializedsort); + assertEquals(testsort.hashCode(), deserializedsort.hashCode()); + assertNotSame(testsort, deserializedsort); + } + } + + private static NestedSortBuilder copy(NestedSortBuilder nestedSort) throws IOException { + return copyWriteable(nestedSort, namedWriteableRegistry, NestedSortBuilder::new); + } + + private static NestedSortBuilder mutate(NestedSortBuilder original) throws IOException { + NestedSortBuilder mutated = original.getNestedSort(); + int parameter = randomIntBetween(0, 2); + switch (parameter) { + case 0: + mutated = new NestedSortBuilder(original.getPath()+"_suffix"); + mutated.setFilter(original.getFilter()); + mutated.setNestedSort(original.getNestedSort()); + break; + case 1: + mutated.setFilter(randomValueOtherThan(original.getFilter(), AbstractSortTestCase::randomNestedFilter)); + break; + case 2: + default: + mutated.setNestedSort(randomValueOtherThan(original.getNestedSort(), () -> NestedSortBuilderTests.createRandomNestedSort(3))); + break; + } + return mutated; + } + + /** + * Test equality and hashCode properties + */ + public void testEqualsAndHashcode() { + for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { + EqualsHashCodeTestUtils.checkEqualsAndHashCode(createRandomNestedSort(3), NestedSortBuilderTests::copy, + NestedSortBuilderTests::mutate); + } + } +} diff --git a/core/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java b/core/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java index 1625a882e27..a38fbe77cf6 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/ScriptSortBuilderTests.java @@ -33,6 +33,8 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import static org.elasticsearch.search.sort.NestedSortBuilderTests.createRandomNestedSort; + public class ScriptSortBuilderTests extends AbstractSortTestCase { @Override @@ -59,12 +61,7 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase NestedSortBuilderTests.createRandomNestedSort(3))); break; - } - case 3: { - NestedSortBuilder nestedSort = new NestedSortBuilder(original.getNestedSort()); - nestedSort.setPath(nestedSort.getPath() + "_some_suffix"); - result.setNestedSort(nestedSort); - break; - } } return result; }