From f3b9926677d7a0f4b36dde17cfe644022de3547e Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Thu, 13 Dec 2018 14:58:17 +0100 Subject: [PATCH] Increase coverage in SearchSortValuesTests SearchSortValuesTests extends now AbstractSerializingTestCase which removes some code duplication and standardizes the way we test fromXContent, serialization and equals/hashcode. Also, we were never creating SearchSortValues through their public constructor that accept an array of DocValueFormat together with the array of raw sort values. That is covered now, which involved some conversion from BytesRef to String in the test. Also, the previos test was not using doing any equality check against the original and parsed versions in testFromXContent due to values being parsed with different types in some cases, which is now covered by converting those values using a new method added to RandomObjects. The code was already there as part of randomStoredFieldValues, but it is now exposed to be used in other scenarios. --- .../search/SearchSortValues.java | 1 - .../elasticsearch/search/SearchHitTests.java | 2 +- .../search/SearchSortValuesTests.java | 150 +++++++++--------- .../test/AbstractSerializingTestCase.java | 31 ++-- .../org/elasticsearch/test/RandomObjects.java | 100 ++++++------ 5 files changed, 152 insertions(+), 132 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchSortValues.java b/server/src/main/java/org/elasticsearch/search/SearchSortValues.java index 271b448d49670..c79b5ad74d785 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchSortValues.java +++ b/server/src/main/java/org/elasticsearch/search/SearchSortValues.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ToXContent.Params; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; diff --git a/server/src/test/java/org/elasticsearch/search/SearchHitTests.java b/server/src/test/java/org/elasticsearch/search/SearchHitTests.java index f64b98502be6e..3ad39404afefb 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchHitTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchHitTests.java @@ -92,7 +92,7 @@ public static SearchHit createTestItem(XContentType xContentType, boolean withOp hit.version(randomLong()); } if (randomBoolean()) { - hit.sortValues(SearchSortValuesTests.createTestItem()); + hit.sortValues(SearchSortValuesTests.createTestItem(xContentType, transportSerialization)); } if (randomBoolean()) { int size = randomIntBetween(0, 5); diff --git a/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java b/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java index d69039b72f56a..f6b8dc828f4e6 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java @@ -19,108 +19,114 @@ package org.elasticsearch.search; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.Version; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.lucene.LuceneTests; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.test.RandomObjects; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; -import java.util.function.Supplier; -import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; -import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; - -public class SearchSortValuesTests extends ESTestCase { - - public static SearchSortValues createTestItem() { - List> valueSuppliers = new ArrayList<>(); - // this should reflect all values that are allowed to go through the transport layer - valueSuppliers.add(() -> null); - valueSuppliers.add(ESTestCase::randomInt); - valueSuppliers.add(ESTestCase::randomLong); - valueSuppliers.add(ESTestCase::randomDouble); - valueSuppliers.add(ESTestCase::randomFloat); - valueSuppliers.add(ESTestCase::randomByte); - valueSuppliers.add(ESTestCase::randomShort); - valueSuppliers.add(ESTestCase::randomBoolean); - valueSuppliers.add(() -> frequently() ? randomAlphaOfLengthBetween(1, 30) : randomRealisticUnicodeOfCodepointLength(30)); +public class SearchSortValuesTests extends AbstractSerializingTestCase { + public static SearchSortValues createTestItem(XContentType xContentType, boolean transportSerialization) { int size = randomIntBetween(1, 20); Object[] values = new Object[size]; + DocValueFormat[] sortValueFormats = new DocValueFormat[size]; for (int i = 0; i < size; i++) { - Supplier supplier = randomFrom(valueSuppliers); - values[i] = supplier.get(); + Object sortValue = randomSortValue(xContentType, transportSerialization); + values[i] = sortValue; + //make sure that for BytesRef, we provide a specific doc value format that overrides format(BytesRef) + sortValueFormats[i] = sortValue instanceof BytesRef ? DocValueFormat.RAW : randomDocValueFormat(); } - return new SearchSortValues(values); + return new SearchSortValues(values, sortValueFormats); + } + + private static Object randomSortValue(XContentType xContentType, boolean transportSerialization) { + Object randomSortValue = LuceneTests.randomSortValue(); + //to simplify things, we directly serialize what we expect we would parse back when testing xcontent serialization + return transportSerialization ? randomSortValue : RandomObjects.getExpectedParsedValue(xContentType, randomSortValue); } - public void testFromXContent() throws IOException { - SearchSortValues sortValues = createTestItem(); - XContentType xcontentType = randomFrom(XContentType.values()); - boolean humanReadable = randomBoolean(); - BytesReference originalBytes = toShuffledXContent(sortValues, xcontentType, ToXContent.EMPTY_PARAMS, humanReadable); + private static DocValueFormat randomDocValueFormat() { + return randomFrom(DocValueFormat.BOOLEAN, DocValueFormat.RAW, DocValueFormat.IP, DocValueFormat.BINARY, DocValueFormat.GEOHASH); + } - SearchSortValues parsed; - try (XContentParser parser = createParser(xcontentType.xContent(), originalBytes)) { - parser.nextToken(); // skip to the elements start array token, fromXContent advances from there if called - parser.nextToken(); - parser.nextToken(); - parsed = SearchSortValues.fromXContent(parser); - parser.nextToken(); - assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken()); - assertNull(parser.nextToken()); - } - assertToXContentEquivalent(originalBytes, toXContent(parsed, xcontentType, humanReadable), xcontentType); + @Override + protected SearchSortValues doParseInstance(XContentParser parser) throws IOException { + parser.nextToken(); // skip to the elements start array token, fromXContent advances from there if called + parser.nextToken(); + parser.nextToken(); + SearchSortValues searchSortValues = SearchSortValues.fromXContent(parser); + parser.nextToken(); + assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken()); + assertNull(parser.nextToken()); + return searchSortValues; } - public void testToXContent() throws IOException { - SearchSortValues sortValues = new SearchSortValues(new Object[]{ 1, "foo", 3.0}); - XContentBuilder builder = JsonXContent.contentBuilder(); - builder.startObject(); - sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - assertEquals("{\"sort\":[1,\"foo\",3.0]}", Strings.toString(builder)); + @Override + protected SearchSortValues createXContextTestInstance(XContentType xContentType) { + return createTestItem(xContentType, false); } - /** - * Test equality and hashCode properties - */ - public void testEqualsAndHashcode() { - checkEqualsAndHashCode(createTestItem(), SearchSortValuesTests::copy, SearchSortValuesTests::mutate); + @Override + protected SearchSortValues createTestInstance() { + return createTestItem(randomFrom(XContentType.values()), true); } - public void testSerialization() throws IOException { - SearchSortValues sortValues = createTestItem(); - try (BytesStreamOutput output = new BytesStreamOutput()) { - sortValues.writeTo(output); - try (StreamInput in = output.bytes().streamInput()) { - SearchSortValues deserializedCopy = new SearchSortValues(in); - assertEquals(sortValues, deserializedCopy); - assertEquals(sortValues.hashCode(), deserializedCopy.hashCode()); - assertNotSame(sortValues, deserializedCopy); - } + @Override + protected Writeable.Reader instanceReader() { + return SearchSortValues::new; + } + + @Override + protected String[] getShuffleFieldsExceptions() { + return new String[]{"sort"}; + } + + public void testToXContent() throws IOException { + { + SearchSortValues sortValues = new SearchSortValues(new Object[]{ 1, "foo", 3.0}); + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + assertEquals("{\"sort\":[1,\"foo\",3.0]}", Strings.toString(builder)); + } + { + SearchSortValues sortValues = new SearchSortValues(new Object[0]); + XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + sortValues.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + assertEquals("{}", Strings.toString(builder)); } } - private static SearchSortValues mutate(SearchSortValues original) { - Object[] sortValues = original.sortValues(); + @Override + protected SearchSortValues mutateInstance(SearchSortValues instance) { + Object[] sortValues = instance.sortValues(); if (sortValues.length == 0) { - return new SearchSortValues(new Object[] { 1 }); + return createTestInstance(); } - return new SearchSortValues(Arrays.copyOf(sortValues, sortValues.length + 1)); + if (randomBoolean()) { + return new SearchSortValues(new Object[0]); + } + Object[] values = Arrays.copyOf(sortValues, sortValues.length + 1); + values[sortValues.length] = randomSortValue(randomFrom(XContentType.values()), true); + return new SearchSortValues(values); } - private static SearchSortValues copy(SearchSortValues original) { - return new SearchSortValues(Arrays.copyOf(original.sortValues(), original.sortValues().length)); + @Override + protected SearchSortValues copyInstance(SearchSortValues instance, Version version) { + return new SearchSortValues(Arrays.copyOf(instance.sortValues(), instance.sortValues().length)); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java index 5aeb30bfdbd5d..22ed586043d83 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractSerializingTestCase.java @@ -24,10 +24,13 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.util.function.Predicate; +import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester; + public abstract class AbstractSerializingTestCase extends AbstractWireSerializingTestCase { /** @@ -35,17 +38,14 @@ public abstract class AbstractSerializingTestCase, List> randomStoredFieldValues(Random random, XContentType xContentType) { int numValues = randomIntBetween(random, 1, 5); - List originalValues = new ArrayList<>(); - List expectedParsedValues = new ArrayList<>(); + List originalValues = randomStoredFieldValues(random, numValues); + List expectedParsedValues = new ArrayList<>(numValues); + for (Object originalValue : originalValues) { + expectedParsedValues.add(getExpectedParsedValue(xContentType, originalValue)); + } + return Tuple.tuple(originalValues, expectedParsedValues); + } + + private static List randomStoredFieldValues(Random random, int numValues) { + List values = new ArrayList<>(numValues); int dataType = randomIntBetween(random, 0, 8); for (int i = 0; i < numValues; i++) { switch(dataType) { case 0: - long randomLong = random.nextLong(); - originalValues.add(randomLong); - expectedParsedValues.add(randomLong); + values.add(random.nextLong()); break; case 1: - int randomInt = random.nextInt(); - originalValues.add(randomInt); - expectedParsedValues.add(randomInt); + values.add(random.nextInt()); break; case 2: - Short randomShort = (short) random.nextInt(); - originalValues.add(randomShort); - expectedParsedValues.add(randomShort.intValue()); + values.add((short) random.nextInt()); break; case 3: - Byte randomByte = (byte)random.nextInt(); - originalValues.add(randomByte); - expectedParsedValues.add(randomByte.intValue()); + values.add((byte) random.nextInt()); break; case 4: - double randomDouble = random.nextDouble(); - originalValues.add(randomDouble); - expectedParsedValues.add(randomDouble); + values.add(random.nextDouble()); break; case 5: - Float randomFloat = random.nextFloat(); - originalValues.add(randomFloat); - if (xContentType == XContentType.CBOR) { - //with CBOR we get back a float - expectedParsedValues.add(randomFloat); - } else if (xContentType == XContentType.SMILE) { - //with SMILE we get back a double (this will change in Jackson 2.9 where it will return a Float) - expectedParsedValues.add(randomFloat.doubleValue()); - } else { - //with JSON AND YAML we get back a double, but with float precision. - expectedParsedValues.add(Double.parseDouble(randomFloat.toString())); - } + values.add(random.nextFloat()); break; case 6: - boolean randomBoolean = random.nextBoolean(); - originalValues.add(randomBoolean); - expectedParsedValues.add(randomBoolean); + values.add(random.nextBoolean()); break; case 7: - String randomString = random.nextBoolean() ? RandomStrings.randomAsciiLettersOfLengthBetween(random, 3, 10) : - randomUnicodeOfLengthBetween(random, 3, 10); - originalValues.add(randomString); - expectedParsedValues.add(randomString); + values.add(random.nextBoolean() ? RandomStrings.randomAsciiLettersOfLengthBetween(random, 3, 10) : + randomUnicodeOfLengthBetween(random, 3, 10)); break; case 8: byte[] randomBytes = RandomStrings.randomUnicodeOfLengthBetween(random, 10, 50).getBytes(StandardCharsets.UTF_8); - BytesArray randomBytesArray = new BytesArray(randomBytes); - originalValues.add(randomBytesArray); - if (xContentType == XContentType.JSON || xContentType == XContentType.YAML) { - //JSON and YAML write the base64 format - expectedParsedValues.add(Base64.getEncoder().encodeToString(randomBytes)); - } else { - //SMILE and CBOR write the original bytes as they support binary format - expectedParsedValues.add(randomBytesArray); - } + values.add(new BytesArray(randomBytes)); break; default: throw new UnsupportedOperationException(); } } - return Tuple.tuple(originalValues, expectedParsedValues); + return values; + } + + /** + * Converts the provided field value to its corresponding expected value once printed out + * via {@link org.elasticsearch.common.xcontent.ToXContent#toXContent(XContentBuilder, ToXContent.Params)} and parsed back via + * {@link org.elasticsearch.common.xcontent.XContentParser#objectText()}. + * Generates values based on what can get printed out. Stored fields values are retrieved from lucene and converted via + * {@link org.elasticsearch.index.mapper.MappedFieldType#valueForDisplay(Object)} to either strings, numbers or booleans. + */ + public static Object getExpectedParsedValue(XContentType xContentType, Object value) { + if (value instanceof BytesArray) { + if (xContentType == XContentType.JSON || xContentType == XContentType.YAML) { + //JSON and YAML write the base64 format + return Base64.getEncoder().encodeToString(((BytesArray)value).toBytesRef().bytes); + } + } + if (value instanceof Float) { + if (xContentType == XContentType.SMILE) { + //with SMILE we get back a double (this will change in Jackson 2.9 where it will return a Float) + return ((Float)value).doubleValue(); + } else { + //with JSON AND YAML we get back a double, but with float precision. + return Double.parseDouble(value.toString()); + } + } + if (value instanceof Byte) { + return ((Byte)value).intValue(); + } + if (value instanceof Short) { + return ((Short)value).intValue(); + } + return value; } /**