Merge pull request #17453 from cbuescher/add-xcontent-randomization

Add randomization of XContentBuilder output to query tests

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java

@@ -22,6 +22,7 @@ package org.elasticsearch.index.query;
 import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
 import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.io.JsonStringEncoder;
+
 import org.apache.lucene.search.BoostQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
@@ -381,7 +382,8 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) {
             QB testQuery = createTestQueryBuilder();
             XContentBuilder builder = toXContent(testQuery, randomFrom(XContentType.values()));
-            assertParsedQuery(builder.bytes(), testQuery);
+            XContentBuilder shuffled = shuffleXContent(builder, shuffleProtectedFields());
+            assertParsedQuery(shuffled.bytes(), testQuery);
             for (Map.Entry<String, QB> alternateVersion : getAlternateVersions().entrySet()) {
                 String queryAsString = alternateVersion.getKey();
                 assertParsedQuery(new BytesArray(queryAsString), alternateVersion.getValue(), ParseFieldMatcher.EMPTY);
@@ -389,6 +391,14 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         }
     }
 
+    /**
+     * Subclasses can override this method and return a set of fields which should be protected from
+     * recursive random shuffling in the {@link #testFromXContent()} test case
+     */
+    protected Set<String> shuffleProtectedFields() {
+        return Collections.emptySet();
+    }
+
     protected static XContentBuilder toXContent(QueryBuilder<?> query, XContentType contentType) throws IOException {
         XContentBuilder builder = XContentFactory.contentBuilder(contentType);
         if (randomBoolean()) {

core/src/test/java/org/elasticsearch/index/query/PercolatorQueryBuilderTests.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.index.query;
 
 import com.fasterxml.jackson.core.JsonParseException;
+
 import org.apache.lucene.search.Query;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.ResourceNotFoundException;
@@ -37,12 +38,15 @@ import org.hamcrest.Matchers;
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
 public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<PercolatorQueryBuilder> {
 
+    private static final Set<String> SHUFFLE_PROTECTED_FIELDS =
+            Collections.singleton(PercolatorQueryParser.DOCUMENT_FIELD.getPreferredName());
     private String indexedDocumentIndex;
     private String indexedDocumentType;
     private String indexedDocumentId;
@@ -75,6 +79,16 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<Percolato
         }
     }
 
+    /**
+     * we don't want to shuffle the "document" field internally in {@link #testFromXContent()} because even though the
+     * documents would be functionally the same, their {@link BytesReference} representation isn't and thats what we
+     * compare when check for equality of the original and the shuffled builder
+     */
+    @Override
+    protected Set<String> shuffleProtectedFields() {
+        return SHUFFLE_PROTECTED_FIELDS;
+    }
+
     @Override
     protected GetResponse executeGet(GetRequest getRequest) {
         assertThat(getRequest.index(), Matchers.equalTo(indexedDocumentIndex));
@@ -132,6 +146,7 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<Percolato
 
     // overwrite this test, because adding bogus field to the document part is valid and that would make the test fail
     // (the document part represents the document being percolated and any key value pair is allowed there)
+    @Override
     public void testUnknownObjectException() throws IOException {
         String validQuery = createTestQueryBuilder().toString();
         int endPos = validQuery.indexOf("document");

core/src/test/java/org/elasticsearch/search/SearchModuleTests.java

@@ -18,8 +18,6 @@
  */
 package org.elasticsearch.search;
 
-import java.io.IOException;
-
 import org.elasticsearch.common.inject.ModuleTestCase;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.settings.Settings;
@@ -31,9 +29,10 @@ import org.elasticsearch.search.highlight.CustomHighlighter;
 import org.elasticsearch.search.highlight.Highlighter;
 import org.elasticsearch.search.highlight.PlainHighlighter;
 import org.elasticsearch.search.suggest.CustomSuggester;
-import org.elasticsearch.search.suggest.Suggester;
 import org.elasticsearch.search.suggest.phrase.PhraseSuggester;
 
+import java.io.IOException;
+
 import static org.hamcrest.Matchers.containsString;
 /**
  */

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -18,7 +18,6 @@
  */
 package org.elasticsearch.test;
 
-import com.carrotsearch.randomizedtesting.RandomizedContext;
 import com.carrotsearch.randomizedtesting.RandomizedTest;
 import com.carrotsearch.randomizedtesting.annotations.Listeners;
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
@@ -29,6 +28,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomInts;
 import com.carrotsearch.randomizedtesting.generators.RandomPicks;
 import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter;
+
 import org.apache.lucene.uninverting.UninvertingReader;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
@@ -40,6 +40,7 @@ import org.elasticsearch.bootstrap.BootstrapForTesting;
 import org.elasticsearch.cache.recycler.MockPageCacheRecycler;
 import org.elasticsearch.client.Requests;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.io.PathUtilsForTesting;
 import org.elasticsearch.common.logging.ESLogger;
@@ -47,6 +48,9 @@ import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.common.util.MockBigArrays;
+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.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
@@ -73,7 +77,9 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Random;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -598,6 +604,38 @@ public abstract class ESTestCase extends LuceneTestCase {
         return tempList.subList(0, size);
     }
 
+    /**
+     * Randomly shuffles the fields inside objects in the {@link XContentBuilder} passed in.
+     * Recursively goes through inner objects and also shuffles them. Exceptions for this
+     * recursive shuffling behavior can be made by passing in the names of fields which
+     * internally should stay untouched.
+     */
+    public static XContentBuilder shuffleXContent(XContentBuilder builder, Set<String> exceptFieldNames) throws IOException {
+        BytesReference bytes = builder.bytes();
+        XContentParser parser = XContentFactory.xContent(bytes).createParser(bytes);
+        // use ordered maps for reproducibility
+        Map<String, Object> shuffledMap = shuffleMap(parser.mapOrdered(), exceptFieldNames);
+        XContentBuilder jsonBuilder = XContentFactory.contentBuilder(builder.contentType());
+        return jsonBuilder.map(shuffledMap);
+    }
+
+    private static Map<String, Object> shuffleMap(Map<String, Object> map, Set<String> exceptFieldNames) {
+        List<String> keys = new ArrayList<>(map.keySet());
+        // even though we shuffle later, we need this to make tests reproduce on different jvms
+        Collections.sort(keys);
+        Map<String, Object> targetMap = new TreeMap<>();
+        Collections.shuffle(keys, random());
+        for (String key : keys) {
+            Object value = map.get(key);
+            if (value instanceof Map && exceptFieldNames.contains(key) == false) {
+                targetMap.put(key, shuffleMap((Map) value, exceptFieldNames));
+            } else {
+                targetMap.put(key, value);
+            }
+        }
+        return targetMap;
+    }
+
     /**
      * Returns true iff assertions for elasticsearch packages are enabled
      */

test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java

@@ -20,9 +20,22 @@
 package org.elasticsearch.test.test;
 
 import junit.framework.AssertionFailedError;
+
+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 java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 public class ESTestCaseTests extends ESTestCase {
+
     public void testExpectThrows() {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
             throw new IllegalArgumentException("bad arg");
@@ -48,4 +61,52 @@ public class ESTestCaseTests extends ESTestCase {
             assertEquals("Expected exception IllegalArgumentException", assertFailed.getMessage());
         }
     }
+
+    public void testShuffleXContent() throws IOException {
+        Map<String, Object> randomStringObjectMap = randomStringObjectMap(5);
+        XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
+        builder.map(randomStringObjectMap);
+        XContentBuilder shuffleXContent = shuffleXContent(builder, Collections.emptySet());
+        XContentParser parser = XContentFactory.xContent(shuffleXContent.bytes()).createParser(shuffleXContent.bytes());
+        Map<String, Object> 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());
+    }
+
+    private static Map<String, Object> randomStringObjectMap(int depth) {
+        Map<String, Object> result = new HashMap<>();
+        int entries = randomInt(10);
+        for (int i = 0; i < entries; i++) {
+            String key = randomAsciiOfLengthBetween(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<String> stringList = new ArrayList<>();
+                int size = randomInt(5);
+                for (int s = 0; s < size; s++) {
+                    stringList.add(randomUnicodeOfCodepointLength(20));
+                }
+                result.put(key, stringList);
+                break;
+            default:
+                throw new IllegalArgumentException("unexpected random option: " + suprise);
+            }
+        }
+        if (depth > 0) {
+            result.put(randomAsciiOfLengthBetween(5, 15), randomStringObjectMap(depth - 1));
+        }
+        return result;
+    }
 }