Add randomization of XContentBuilder output to query tests

Currently our testing of parsing query builders is limited to the
default order of the parameters that each builders toXContent()
method produces. To better test real queries where the order of
parameters can be different, this change adds a helper
method to ESTestCase that takes a XContentBuilder and randomly
shuffles the order of the fields inside an object. This is
used in AbstractQueryTestCase, but it can be used in other similar
places in the future.

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;
@@ -382,7 +383,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, provideShuffleproofFields());
+            assertParsedQuery(shuffled.bytes(), testQuery);
             for (Map.Entry<String, QB> alternateVersion : getAlternateVersions().entrySet()) {
                 String queryAsString = alternateVersion.getKey();
                 assertParsedQuery(new BytesArray(queryAsString), alternateVersion.getValue(), ParseFieldMatcher.EMPTY);
@@ -390,6 +392,14 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         }
     }
 
+    /**
+     * subclasses should override this method in case some fields in xContent should be protected from random
+     * shuffling in the {@link #testFromXContent()} test case
+     */
+    protected Set<String> provideShuffleproofFields() {
+        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,6 +38,8 @@ import org.hamcrest.Matchers;
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -75,6 +78,18 @@ public class PercolatorQueryBuilderTests extends AbstractQueryTestCase<Percolato
         }
     }
 
+    /**
+     * prevent fields in the "document" field from being shuffled randomly, because it later is parsed to
+     * a {@link BytesReference} and even though the documents are the same, equals will fail when comparing
+     * BytesReference
+     */
+    @Override
+    protected Set<String> provideShuffleproofFields() {
+        Set<String> fieldNames = new HashSet<>();
+        fieldNames.add(PercolatorQueryParser.DOCUMENT_FIELD.getPreferredName());
+        return fieldNames;
+    }
+
     @Override
     protected GetResponse executeGet(GetRequest getRequest) {
         assertThat(getRequest.index(), Matchers.equalTo(indexedDocumentIndex));
@@ -132,6 +147,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");

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,10 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Random;
+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 +605,30 @@ public abstract class ESTestCase extends LuceneTestCase {
         return tempList.subList(0, size);
     }
 
+    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, random());
+        XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
+        return jsonBuilder.map(shuffledMap);
+    }
+
+    private static Map<String, Object> shuffleMap(Map<String, Object> map, Set<String> exceptFieldNames, Random r) {
+        List<String> keys = new ArrayList<>(map.keySet());
+        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, r));
+            } 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,21 @@
 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.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 +60,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.jsonBuilder();
+        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.string(), shuffleXContent.string());
+    }
+
+    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;
+    }
 }