diff --git a/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java index 99deef9cde7..6de9cbab620 100644 --- a/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java @@ -126,11 +126,8 @@ public class InnerHitBuilderTests extends ESTestCase { InnerHitBuilder innerHit = randomInnerHits(true, false); XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); innerHit.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); - if (randomBoolean()) { - shuffled.prettyPrint(); - } - + //fields is printed out as an object but parsed into a List where order matters, we disable shuffling + XContentBuilder shuffled = shuffleXContent(builder, "fields"); XContentParser parser = createParser(shuffled); QueryParseContext context = new QueryParseContext(parser); InnerHitBuilder secondInnerHits = InnerHitBuilder.fromXContent(context); @@ -236,7 +233,7 @@ public class InnerHitBuilderTests extends ESTestCase { assertThat(innerHitBuilders.get(leafInnerHits.getName()), notNullValue()); } - public void testBuild_ingoreUnmappedNestQuery() throws Exception { + public void testBuildIgnoreUnmappedNestQuery() throws Exception { QueryShardContext queryShardContext = mock(QueryShardContext.class); when(queryShardContext.getObjectMapper("path")).thenReturn(null); SearchContext searchContext = mock(SearchContext.class); diff --git a/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java b/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java index ddd24fdee9e..2d1fb3d986c 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java @@ -125,8 +125,17 @@ public class HighlightBuilderTests extends ESTestCase { if (randomBoolean()) { builder.prettyPrint(); } - highlightBuilder.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); + + XContentBuilder shuffled; + if (randomBoolean()) { + //this way `fields` is printed out as a json array + highlightBuilder.useExplicitFieldOrder(true); + highlightBuilder.toXContent(builder, ToXContent.EMPTY_PARAMS); + shuffled = shuffleXContent(builder); + } else { + highlightBuilder.toXContent(builder, ToXContent.EMPTY_PARAMS); + shuffled = shuffleXContent(builder, "fields"); + } XContentParser parser = createParser(shuffled); QueryParseContext context = new QueryParseContext(parser); diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index bb1fd4207b6..377f73e5c79 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -117,12 +117,12 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Random; import java.util.Set; -import java.util.TreeMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -919,9 +919,11 @@ public abstract class ESTestCase extends LuceneTestCase { * recursive shuffling behavior can be made by passing in the names of fields which * internally should stay untouched. */ - protected static XContentBuilder shuffleXContent(XContentParser parser, boolean prettyPrint, String... exceptFieldNames) throws IOException { - //TODO why do we need sorted map if we later sort the keys right before shuffling them? That should be enough? - Map shuffledMap = shuffleMap(parser.mapOrdered(), new HashSet<>(Arrays.asList(exceptFieldNames))); + protected static XContentBuilder shuffleXContent(XContentParser parser, boolean prettyPrint, String... exceptFieldNames) + throws IOException { + //we need a sorted map for reproducibility, as we are going to shuffle its keys and write XContent back + Map shuffledMap = shuffleMap((LinkedHashMap)parser.mapOrdered(), + new HashSet<>(Arrays.asList(exceptFieldNames))); XContentBuilder xContentBuilder = XContentFactory.contentBuilder(parser.contentType()); if (prettyPrint) { xContentBuilder.prettyPrint(); @@ -929,17 +931,15 @@ public abstract class ESTestCase extends LuceneTestCase { return xContentBuilder.map(shuffledMap); } - private static Map shuffleMap(Map map, Set exceptFields) { + public static LinkedHashMap shuffleMap(LinkedHashMap map, Set exceptFields) { List keys = new ArrayList<>(map.keySet()); - // even though we shuffle later, we need this to make tests reproduce on different jvms - Collections.sort(keys); - Map targetMap = new TreeMap<>(); + LinkedHashMap targetMap = new LinkedHashMap<>(); Collections.shuffle(keys, random()); for (String key : keys) { Object value = map.get(key); if (value instanceof Map && exceptFields.contains(key) == false) { @SuppressWarnings("unchecked") - Map valueMap = (Map) value; + LinkedHashMap valueMap = (LinkedHashMap) value; targetMap.put(key, shuffleMap(valueMap, exceptFields)); } else { targetMap.put(key, value); diff --git a/test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java b/test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java index 1ea09ca76f8..c9631034363 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java +++ b/test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java @@ -157,13 +157,24 @@ public final class RandomObjects { /** * Returns a random source in a given XContentType containing a random number of fields, objects and array, with maximum depth 5. + * The minimum number of fields per object is 1. * * @param random Random generator */ public static BytesReference randomSource(Random random, XContentType xContentType) { + return randomSource(random, xContentType, 1); + } + + /** + * Returns a random source in a given XContentType containing a random number of fields, objects and array, with maximum depth 5. + * The minimum number of fields per object is provided as an argument. + * + * @param random Random generator + */ + public static BytesReference randomSource(Random random, XContentType xContentType, int minNumFields) { try (XContentBuilder builder = XContentFactory.contentBuilder(xContentType)) { builder.startObject(); - addFields(random, builder, 0); + addFields(random, builder, minNumFields, 0); builder.endObject(); return builder.bytes(); } catch(IOException e) { @@ -174,13 +185,13 @@ public final class RandomObjects { /** * Randomly adds fields, objects, or arrays to the provided builder. The maximum depth is 5. */ - private static void addFields(Random random, XContentBuilder builder, int currentDepth) throws IOException { - int numFields = randomIntBetween(random, 1, 5); + private static void addFields(Random random, XContentBuilder builder, int minNumFields, int currentDepth) throws IOException { + int numFields = randomIntBetween(random, minNumFields, 10); for (int i = 0; i < numFields; i++) { if (currentDepth < 5 && random.nextBoolean()) { if (random.nextBoolean()) { builder.startObject(RandomStrings.randomAsciiOfLengthBetween(random, 4, 10)); - addFields(random, builder, currentDepth + 1); + addFields(random, builder, minNumFields, currentDepth + 1); builder.endObject(); } else { builder.startArray(RandomStrings.randomAsciiOfLengthBetween(random, 4, 10)); @@ -193,7 +204,7 @@ public final class RandomObjects { for (int j = 0; j < numElements; j++) { if (object) { builder.startObject(); - addFields(random, builder, 5); + addFields(random, builder, minNumFields, 5); builder.endObject(); } else { builder.value(randomFieldValue(random, dataType)); diff --git a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java index cdcdc3c99c3..d30a6a57db5 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java @@ -20,18 +20,22 @@ package org.elasticsearch.test.test; import junit.framework.AssertionFailedError; - +import org.elasticsearch.common.bytes.BytesReference; 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.test.ESTestCase; +import org.elasticsearch.test.RandomObjects; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.hamcrest.Matchers.greaterThan; @@ -65,52 +69,82 @@ public class ESTestCaseTests extends ESTestCase { } } - public void testShuffleXContent() throws IOException { - Map randomStringObjectMap = randomStringObjectMap(5); - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); - builder.map(randomStringObjectMap); - XContentBuilder shuffleXContent = shuffleXContent(builder); - XContentParser parser = createParser(shuffleXContent); - Map resultMap = parser.map(); - assertEquals("both maps should contain the same mappings", randomStringObjectMap, resultMap); - assertNotEquals("Both builders string representations should be different", builder.bytes(), shuffleXContent.bytes()); + public void testShuffleMap() throws IOException { + XContentType xContentType = randomFrom(XContentType.values()); + BytesReference source = RandomObjects.randomSource(random(), xContentType, 5); + try (XContentParser parser = createParser(xContentType.xContent(), source)) { + LinkedHashMap initialMap = (LinkedHashMap)parser.mapOrdered(); + + Set> distinctKeys = new HashSet<>(); + for (int i = 0; i < 10; i++) { + LinkedHashMap shuffledMap = shuffleMap(initialMap, Collections.emptySet()); + assertEquals("both maps should contain the same mappings", initialMap, shuffledMap); + List shuffledKeys = new ArrayList<>(shuffledMap.keySet()); + distinctKeys.add(shuffledKeys); + } + //out of 10 shuffling runs we expect to have at least more than 1 distinct output. + //This is to make sure that we actually do the shuffling + assertThat(distinctKeys.size(), greaterThan(1)); + } } - private static Map randomStringObjectMap(int depth) { - Map result = new HashMap<>(); - int entries = randomInt(10); - for (int i = 0; i < entries; i++) { - String key = randomAlphaOfLengthBetween(5, 15); - int suprise = randomIntBetween(0, 4); - switch (suprise) { - case 0: - result.put(key, randomUnicodeOfCodepointLength(20)); - break; - case 1: - result.put(key, randomInt(100)); - break; - case 2: - result.put(key, randomDoubleBetween(-100.0, 100.0, true)); - break; - case 3: - result.put(key, randomBoolean()); - break; - case 4: - List stringList = new ArrayList<>(); - int size = randomInt(5); - for (int s = 0; s < size; s++) { - stringList.add(randomUnicodeOfCodepointLength(20)); + public void testShuffleXContentExcludeFields() throws IOException { + XContentType xContentType = randomFrom(XContentType.values()); + try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) { + builder.startObject(); + { + builder.field("field1", "value1"); + builder.field("field2", "value2"); + { + builder.startObject("object1"); + builder.field("inner1", "value1"); + builder.field("inner2", "value2"); + builder.field("inner3", "value3"); + builder.endObject(); + } + { + builder.startObject("object2"); + builder.field("inner4", "value4"); + builder.field("inner5", "value5"); + builder.field("inner6", "value6"); + builder.endObject(); } - result.put(key, stringList); - break; - default: - throw new IllegalArgumentException("unexpected random option: " + suprise); } + builder.endObject(); + BytesReference bytes = builder.bytes(); + final LinkedHashMap initialMap; + try (XContentParser parser = createParser(xContentType.xContent(), bytes)) { + initialMap = (LinkedHashMap)parser.mapOrdered(); + } + + List expectedInnerKeys1 = Arrays.asList("inner1", "inner2", "inner3"); + Set> distinctTopLevelKeys = new HashSet<>(); + Set> distinctInnerKeys2 = new HashSet<>(); + for (int i = 0; i < 10; i++) { + try (XContentParser parser = createParser(xContentType.xContent(), bytes)) { + try (XContentBuilder shuffledBuilder = shuffleXContent(parser, randomBoolean(), "object1")) { + try (XContentParser shuffledParser = createParser(shuffledBuilder)) { + Map shuffledMap = shuffledParser.mapOrdered(); + assertEquals("both maps should contain the same mappings", initialMap, shuffledMap); + List shuffledKeys = new ArrayList<>(shuffledMap.keySet()); + distinctTopLevelKeys.add(shuffledKeys); + @SuppressWarnings("unchecked") + Map innerMap1 = (Map)shuffledMap.get("object1"); + List actualInnerKeys1 = new ArrayList<>(innerMap1.keySet()); + assertEquals("object1 should have been left untouched", expectedInnerKeys1, actualInnerKeys1); + @SuppressWarnings("unchecked") + Map innerMap2 = (Map)shuffledMap.get("object2"); + List actualInnerKeys2 = new ArrayList<>(innerMap2.keySet()); + distinctInnerKeys2.add(actualInnerKeys2); + } + } + } + } + + //out of 10 shuffling runs we expect to have at least more than 1 distinct output for both top level keys and inner object2 + assertThat(distinctTopLevelKeys.size(), greaterThan(1)); + assertThat(distinctInnerKeys2.size(), greaterThan(1)); } - if (depth > 0) { - result.put(randomAlphaOfLengthBetween(5, 15), randomStringObjectMap(depth - 1)); - } - return result; } public void testRandomUniqueNotUnique() {