Skip to content

Commit

Permalink
[Tests] Improve testing of FieldSortBuilder
Browse files Browse the repository at this point in the history 
Currently we don't do much unit testing about the SortField that is created then
calling the
SortBuilders `build` method. Most of this is covered by integration tests
somewhere but it
would be good to have some basic checks in FieldSortBuilderTest as well.

This adds testing for the sort order, mode, missing values and checks that
`nested` gets set
in the XFieldComparatorSource when `nestedPath` and `nestedFilter` are set on
the builder.

Relates to #17286
  • Loading branch information
@cbuescher
cbuescher committed Aug 31, 2017
1 parent 78681bc commit 2140868
Show file tree
Hide file tree
Showing 8 changed files with 233 additions and 59 deletions.
18 changes: 18 additions & 0 deletions core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,24 @@ public static MemoryStorageFormat fromString(String string) {
// on another node (we don't have the custom source them...)
abstract class XFieldComparatorSource extends FieldComparatorSource {

protected final MultiValueMode sortMode;
protected final Object missingValue;
protected final Nested nested;

public XFieldComparatorSource(Object missingValue, MultiValueMode sortMode, Nested nested) {
this.sortMode = sortMode;
this.missingValue = missingValue;
this.nested = nested;
}

public MultiValueMode sortMode() {
return this.sortMode;
}

public Nested nested() {
return this.nested;
}

/**
* Simple wrapper class around a filter that matches parent documents
* and a filter that matches child documents. For every root document R,
Expand Down
7 changes: 1 addition & 6 deletions ...java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefFieldComparatorSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,10 @@
public class BytesRefFieldComparatorSource extends IndexFieldData.XFieldComparatorSource {

private final IndexFieldData<?> indexFieldData;
private final MultiValueMode sortMode;
private final Object missingValue;
private final Nested nested;

public BytesRefFieldComparatorSource(IndexFieldData<?> indexFieldData, Object missingValue, MultiValueMode sortMode, Nested nested) {
super(missingValue, sortMode, nested);
this.indexFieldData = indexFieldData;
this.sortMode = sortMode;
this.missingValue = missingValue;
this.nested = nested;
}

@Override
Expand Down
7 changes: 1 addition & 6 deletions .../java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,10 @@
public class DoubleValuesComparatorSource extends IndexFieldData.XFieldComparatorSource {

private final IndexNumericFieldData indexFieldData;
private final Object missingValue;
private final MultiValueMode sortMode;
private final Nested nested;

public DoubleValuesComparatorSource(IndexNumericFieldData indexFieldData, @Nullable Object missingValue, MultiValueMode sortMode, Nested nested) {
super(missingValue, sortMode, nested);
this.indexFieldData = indexFieldData;
this.missingValue = missingValue;
this.sortMode = sortMode;
this.nested = nested;
}

@Override
Expand Down
7 changes: 1 addition & 6 deletions ...n/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,10 @@
public class FloatValuesComparatorSource extends IndexFieldData.XFieldComparatorSource {

private final IndexNumericFieldData indexFieldData;
private final Object missingValue;
private final MultiValueMode sortMode;
private final Nested nested;

public FloatValuesComparatorSource(IndexNumericFieldData indexFieldData, @Nullable Object missingValue, MultiValueMode sortMode, Nested nested) {
super(missingValue, sortMode, nested);
this.indexFieldData = indexFieldData;
this.missingValue = missingValue;
this.sortMode = sortMode;
this.nested = nested;
}

@Override
Expand Down
7 changes: 1 addition & 6 deletions ...in/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,10 @@
public class LongValuesComparatorSource extends IndexFieldData.XFieldComparatorSource {

private final IndexNumericFieldData indexFieldData;
private final Object missingValue;
private final MultiValueMode sortMode;
private final Nested nested;

public LongValuesComparatorSource(IndexNumericFieldData indexFieldData, @Nullable Object missingValue, MultiValueMode sortMode, Nested nested) {
super(missingValue, sortMode, nested);
this.indexFieldData = indexFieldData;
this.missingValue = missingValue;
this.sortMode = sortMode;
this.nested = nested;
}

@Override
Expand Down
11 changes: 5 additions & 6 deletions core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,8 @@ public SortFieldAndFormat build(QueryShardContext context) throws IOException {
DocValueFormat.RAW);
}

IndexFieldData.XFieldComparatorSource geoDistanceComparatorSource = new IndexFieldData.XFieldComparatorSource() {
IndexFieldData.XFieldComparatorSource geoDistanceComparatorSource = new IndexFieldData.XFieldComparatorSource(null, finalSortMode,
nested) {

@Override
public SortField.Type reducedType() {
Expand All @@ -599,11 +600,10 @@ public SortField.Type reducedType() {
public FieldComparator<?> newComparator(String fieldname, int numHits, int sortPos, boolean reversed) {
return new FieldComparator.DoubleComparator(numHits, null, null) {
@Override
protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field)
throws IOException {
protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) throws IOException {
final MultiGeoPointValues geoPointValues = geoIndexFieldData.load(context).getGeoPointValues();
final SortedNumericDoubleValues distanceValues = GeoUtils.distanceValues(geoDistance, unit,
geoPointValues, localPoints);
final SortedNumericDoubleValues distanceValues = GeoUtils.distanceValues(geoDistance, unit, geoPointValues,
localPoints);
final NumericDoubleValues selectedValues;
if (nested == null) {
selectedValues = finalSortMode.select(distanceValues, Double.POSITIVE_INFINITY);
Expand All @@ -617,7 +617,6 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String
}
};
}

};

return new SortFieldAndFormat(new SortField(fieldName, geoDistanceComparatorSource, reverse),
Expand Down
39 changes: 16 additions & 23 deletions core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.search.sort;

import org.apache.lucene.search.SortField;
import org.apache.lucene.util.Accountable;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
Expand All @@ -35,7 +34,8 @@
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.IndexFieldDataCache;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper.BuilderContext;
Expand All @@ -47,8 +47,6 @@
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
import org.elasticsearch.script.MockScriptEngine;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptModule;
Expand All @@ -59,16 +57,19 @@
import org.elasticsearch.test.IndexSettingsModule;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.mockito.Mockito;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;
import java.util.function.BiFunction;
import java.util.function.Function;

import static java.util.Collections.emptyList;
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;

public abstract class AbstractSortTestCase<T extends SortBuilder<T>> extends ESTestCase {

private static final int NUMBER_OF_TESTBUILDERS = 20;

protected static NamedWriteableRegistry namedWriteableRegistry;
Expand All @@ -77,7 +78,7 @@ public abstract class AbstractSortTestCase<T extends SortBuilder<T>> extends EST
private static ScriptService scriptService;

@BeforeClass
public static void init() throws IOException {
public static void init() {
Settings baseSettings = Settings.builder()
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
.build();
Expand Down Expand Up @@ -166,7 +167,7 @@ public void testSerialization() throws IOException {
/**
* Test equality and hashCode properties
*/
public void testEqualsAndHashcode() throws IOException {
public void testEqualsAndHashcode() {
for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
checkEqualsAndHashCode(createTestItem(), this::copy, this::mutate);
}
Expand All @@ -176,22 +177,14 @@ protected QueryShardContext createMockShardContext() {
Index index = new Index(randomAlphaOfLengthBetween(1, 10), "_na_");
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index,
Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build());
IndicesFieldDataCache cache = new IndicesFieldDataCache(Settings.EMPTY, null);
IndexFieldDataService ifds = new IndexFieldDataService(IndexSettingsModule.newIndexSettings("test", Settings.EMPTY),
cache, null, null);
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(idxSettings, new BitsetFilterCache.Listener() {

@Override
public void onRemoval(ShardId shardId, Accountable accountable) {
}
BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(idxSettings, Mockito.mock(BitsetFilterCache.Listener.class));
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup = (fieldType, fieldIndexName) -> {
IndexFieldData.Builder builder = fieldType.fielddataBuilder(fieldIndexName);
return builder.build(idxSettings, fieldType, new IndexFieldDataCache.None(), null, null);
};
return new QueryShardContext(0, idxSettings, bitsetFilterCache, indexFieldDataLookup, null, null, scriptService,
xContentRegistry(), namedWriteableRegistry, null, null, () -> randomNonNegativeLong(), null) {

@Override
public void onCache(ShardId shardId, Accountable accountable) {
}
});
long nowInMillis = randomNonNegativeLong();
return new QueryShardContext(0, idxSettings, bitsetFilterCache, ifds::getForField, null, null, scriptService,
xContentRegistry(), namedWriteableRegistry, null, null, () -> nowInMillis, null) {
@Override
public MappedFieldType fieldMapper(String name) {
return provideMappedFieldType(name);
Expand All @@ -207,7 +200,7 @@ public ObjectMapper getObjectMapper(String name) {

/**
* Return a field type. We use {@link NumberFieldMapper.NumberFieldType} by default since it is compatible with all sort modes
* Tests that require other field type than double can override this.
* Tests that require other field types can override this.
*/
protected MappedFieldType provideMappedFieldType(String name) {
NumberFieldMapper.NumberFieldType doubleFieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
Expand Down Expand Up @@ -237,7 +230,7 @@ protected static QueryBuilder randomNestedFilter() {
private T copy(T original) throws IOException {
/* The cast below is required to make Java 9 happy. Java 8 infers the T in copyWriterable to be the same as AbstractSortTestCase's
* T but Java 9 infers it to be SortBuilder. */
return (T) copyWriteable(original, namedWriteableRegistry,
return copyWriteable(original, namedWriteableRegistry,
namedWriteableRegistry.getReader(SortBuilder.class, original.getWriteableName()));
}
}
Loading

0 comments on commit 2140868

Please sign in to comment.