[Tests] Add unit tests for NestedSortBuilder (#26458)

The new NestedSortBuilder currently is only tested via its use in the other
SortBuilder implementations it can be used in. This adds its own simple unit
test class that at first checks our usual fromXContent parsing, serialization
and hashCode/equals checks. It also adds tests for cases where NestedSortBuilder
is nested in itself and reuses the code for creating randomized instances in the
other SortBuilder tests.

In addition to the tests, this changes the `path` parameter in NestedSortBuilder
to be mandatory and removes the `read` method since it is not really needed.
This commit is contained in:
Christoph Büscher 2017-09-01 10:53:51 +02:00 committed by GitHub
parent 80d0a32f8e
commit 2d342c0830
8 changed files with 185 additions and 116 deletions

View File

@ -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<NestedSortBuilder>, 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<NestedSort
}
public NestedSortBuilder(StreamInput in) throws IOException {
read(in);
}
public NestedSortBuilder(NestedSortBuilder other) {
if (other != null) {
this.path = other.path;
this.filter = other.filter;
this.nestedSort = other.nestedSort;
}
path = in.readOptionalString();
filter = in.readOptionalNamedWriteable(QueryBuilder.class);
nestedSort = in.readOptionalWriteable(NestedSortBuilder::new);
}
public String getPath() {
return path;
}
public NestedSortBuilder setPath(final String path) {
this.path = path;
return this;
}
public QueryBuilder getFilter() {
return filter;
}
@ -95,19 +84,6 @@ public class NestedSortBuilder implements Writeable, Writeable.Reader<NestedSort
out.writeOptionalWriteable(nestedSort);
}
/**
* Read {@code V}-type value from a stream.
*
* @param in Input to read the value from
*/
@Override
public NestedSortBuilder read(final StreamInput in) throws IOException {
path = in.readOptionalString();
filter = in.readOptionalNamedWriteable(QueryBuilder.class);
nestedSort = in.readOptionalWriteable(NestedSortBuilder::new);
return this;
}
@Override
public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException {
builder.startObject();

View File

@ -182,7 +182,6 @@ public abstract class SortBuilder<T extends SortBuilder<T>> 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<T extends SortBuilder<T>> 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;
}

View File

@ -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.
*

View File

@ -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)
)

View File

@ -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<FieldSortBuilder> {
@ -80,11 +81,7 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
}
if (randomBoolean()) {
NestedSortBuilder nestedSort = SortBuilders.nestedSort(randomAlphaOfLengthBetween(1, 10));
if (randomBoolean()) {
nestedSort.setFilter(randomNestedFilter());
}
builder.setNestedSort(nestedSort);
builder.setNestedSort(createRandomNestedSort(3));
}
return builder;
@ -93,32 +90,23 @@ public class FieldSortBuilderTests extends AbstractSortTestCase<FieldSortBuilder
@Override
protected FieldSortBuilder mutate(FieldSortBuilder original) throws IOException {
FieldSortBuilder mutated = new FieldSortBuilder(original);
int parameter = randomIntBetween(0, 5);
int parameter = randomIntBetween(0, 4);
switch (parameter) {
case 0: {
NestedSortBuilder nestedSort = new NestedSortBuilder(mutated.getNestedSort());
nestedSort.setPath(randomValueOtherThan(nestedSort.getPath(), () -> 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:

View File

@ -87,15 +87,9 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
result.sortMode(randomValueOtherThan(SortMode.SUM, () -> 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<GeoDistanc
@Override
protected GeoDistanceSortBuilder mutate(GeoDistanceSortBuilder original) throws IOException {
GeoDistanceSortBuilder result = new GeoDistanceSortBuilder(original);
int parameter = randomIntBetween(0, 8);
int parameter = randomIntBetween(0, 7);
switch (parameter) {
case 0:
while (Arrays.deepEquals(original.points(), result.points())) {
@ -160,19 +154,10 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
Arrays.asList(SortMode.SUM, result.sortMode())::contains,
() -> 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;
}

View File

@ -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);
}
}
}

View File

@ -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<ScriptSortBuilder> {
@Override
@ -59,12 +61,7 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
}
}
if (randomBoolean()) {
NestedSortBuilder nestedSort = SortBuilders.nestedSort(randomAlphaOfLengthBetween(1, 10));
if (randomBoolean()) {
nestedSort.setFilter(randomNestedFilter());
}
builder.setNestedSort(nestedSort);
builder.setNestedSort(createRandomNestedSort(3));
}
return builder;
}
@ -89,7 +86,7 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
return result;
}
result = new ScriptSortBuilder(original);
switch (randomIntBetween(0, 3)) {
switch (randomIntBetween(0, 2)) {
case 0:
if (original.order() == SortOrder.ASC) {
result.order(SortOrder.DESC);
@ -109,18 +106,10 @@ public class ScriptSortBuilderTests extends AbstractSortTestCase<ScriptSortBuild
}
}
break;
case 2: {
NestedSortBuilder nestedSort = new NestedSortBuilder(original.getNestedSort());
nestedSort.setFilter(randomValueOtherThan(nestedSort.getFilter(), AbstractSortTestCase::randomNestedFilter));
result.setNestedSort(nestedSort);
case 2:
result.setNestedSort(randomValueOtherThan(original.getNestedSort(),
() -> NestedSortBuilderTests.createRandomNestedSort(3)));
break;
}
case 3: {
NestedSortBuilder nestedSort = new NestedSortBuilder(original.getNestedSort());
nestedSort.setPath(nestedSort.getPath() + "_some_suffix");
result.setNestedSort(nestedSort);
break;
}
}
return result;
}