Fix assertIngestDocument wrongfully passing (#31913)

* Fix assertIngestDocument wrongfully passing

* Previously docA being subset of docB passed because iteration was over docA's keys only
* Scalars in nested fields were not compared in all cases
* Assertion errors were hard to interpret (message wasn't correct since it only mentioned the class type)
* In cases where two paths contained different types a ClassCastException was thrown instead of an AssertionError
* Fixes #28492

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java

@@ -129,7 +129,7 @@ public class GrokProcessorTests extends ESTestCase {
     public void testMissingFieldWithIgnoreMissing() throws Exception {
         String fieldName = "foo.bar";
         IngestDocument originalIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
-        IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
+        IngestDocument ingestDocument = new IngestDocument(originalIngestDocument);
         GrokProcessor processor = new GrokProcessor(randomAlphaOfLength(10), Collections.singletonMap("ONE", "1"),
             Collections.singletonList("%{ONE:one}"), fieldName, false, true, ThreadWatchdog.noop());
         processor.execute(ingestDocument);

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java

@@ -33,7 +33,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -55,7 +54,7 @@ public class JsonProcessorTests extends ESTestCase {
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
         jsonProcessor.execute(ingestDocument);
         Map<String, Object> jsonified = ingestDocument.getFieldValue(randomTargetField, Map.class);
-        assertIngestDocument(ingestDocument.getFieldValue(randomTargetField, Object.class), jsonified);
+        assertEquals(ingestDocument.getFieldValue(randomTargetField, Object.class), jsonified);
     }
 
     public void testInvalidValue() {
@@ -161,13 +160,10 @@ public class JsonProcessorTests extends ESTestCase {
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
         jsonProcessor.execute(ingestDocument);
 
-        Map<String, Object> expected = new HashMap<>();
+        Map<String, Object> sourceAndMetadata = ingestDocument.getSourceAndMetadata();
-        expected.put("a", 1);
+        assertEquals(1, sourceAndMetadata.get("a"));
-        expected.put("b", 2);
+        assertEquals(2, sourceAndMetadata.get("b"));
-        expected.put("c", "see");
+        assertEquals("see", sourceAndMetadata.get("c"));
-        IngestDocument expectedIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), expected);
-
-        assertIngestDocument(ingestDocument, expectedIngestDocument);
     }
 
     public void testAddBoolToRoot() {

test/framework/src/main/java/org/elasticsearch/ingest/IngestDocumentMatcher.java

@@ -20,48 +20,61 @@
 package org.elasticsearch.ingest;
 
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
-
+import java.util.Objects;
-import static org.hamcrest.Matchers.equalTo;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertThat;
 
 public class IngestDocumentMatcher {
     /**
      * Helper method to assert the equivalence between two IngestDocuments.
      *
-     * @param a first object to compare
+     * @param docA first document to compare
-     * @param b second object to compare
+     * @param docB second document to compare
      */
-    public static void assertIngestDocument(Object a, Object b) {
+    public static void assertIngestDocument(IngestDocument docA, IngestDocument docB) {
+        if ((deepEquals(docA.getIngestMetadata(), docB.getIngestMetadata(), true) &&
+            deepEquals(docA.getSourceAndMetadata(), docB.getSourceAndMetadata(), false)) == false) {
+            throw new AssertionError("Expected [" + docA + "] but received [" + docB + "].");
+        }
+    }
+
+    private static boolean deepEquals(Object a, Object b, boolean isIngestMeta) {
         if (a instanceof Map) {
             Map<?, ?> mapA = (Map<?, ?>) a;
+            if (b instanceof Map == false) {
+                return false;
+            }
             Map<?, ?> mapB = (Map<?, ?>) b;
+            if (mapA.size() != mapB.size()) {
+                return false;
+            }
             for (Map.Entry<?, ?> entry : mapA.entrySet()) {
-                if (entry.getValue() instanceof List || entry.getValue() instanceof Map) {
+                Object key = entry.getKey();
-                    assertIngestDocument(entry.getValue(), mapB.get(entry.getKey()));
+                // Don't compare the timestamp of ingest metadata since it will differ between executions
+                if ((isIngestMeta && "timestamp".equals(key)) == false
+                    && deepEquals(entry.getValue(), mapB.get(key), false) == false) {
+                    return false;
                 }
             }
+            return true;
         } else if (a instanceof List) {
             List<?> listA = (List<?>) a;
+            if (b instanceof List == false) {
+                return false;
+            }
             List<?> listB = (List<?>) b;
-            for (int i = 0; i < listA.size(); i++) {
+            int countA = listA.size();
+            if (countA != listB.size()) {
+                return false;
+            }
+            for (int i = 0; i < countA; i++) {
                 Object value = listA.get(i);
-                if (value instanceof List || value instanceof Map) {
+                if (deepEquals(value, listB.get(i), false) == false) {
-                    assertIngestDocument(value, listB.get(i));
+                    return false;
                 }
             }
-        } else if (a instanceof byte[]) {
+            return true;
-            assertArrayEquals((byte[]) a, (byte[])b);
-        } else if (a instanceof IngestDocument) {
-            IngestDocument docA = (IngestDocument) a;
-            IngestDocument docB = (IngestDocument) b;
-            assertIngestDocument(docA.getSourceAndMetadata(), docB.getSourceAndMetadata());
-            assertIngestDocument(docA.getIngestMetadata(), docB.getIngestMetadata());
         } else {
-            String msg = String.format(Locale.ROOT, "Expected %s class to be equal to %s", a.getClass().getName(), b.getClass().getName());
+            return Objects.deepEquals(a, b);
-            assertThat(msg, a, equalTo(b));
         }
     }
 }

test/framework/src/test/java/org/elasticsearch/ingest/IngestDocumentMatcherTests.java

@@ -0,0 +1,82 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.ingest;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument;
+
+public class IngestDocumentMatcherTests extends ESTestCase {
+
+    public void testDifferentMapData() {
+        Map<String, Object> sourceAndMetadata1 = new HashMap<>();
+        sourceAndMetadata1.put("foo", "bar");
+        IngestDocument document1 = new IngestDocument(sourceAndMetadata1, new HashMap<>());
+        IngestDocument document2 = new IngestDocument(new HashMap<>(), new HashMap<>());
+        assertThrowsOnComparision(document1, document2);
+    }
+
+    public void testDifferentLengthListData() {
+        String rootKey = "foo";
+        IngestDocument document1 =
+            new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz")), new HashMap<>());
+        IngestDocument document2 =
+            new IngestDocument(Collections.singletonMap(rootKey, Collections.emptyList()), new HashMap<>());
+        assertThrowsOnComparision(document1, document2);
+    }
+
+    public void testDifferentNestedListFieldData() {
+        String rootKey = "foo";
+        IngestDocument document1 =
+            new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz")), new HashMap<>());
+        IngestDocument document2 =
+            new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "blub")), new HashMap<>());
+        assertThrowsOnComparision(document1, document2);
+    }
+
+    public void testDifferentNestedMapFieldData() {
+        String rootKey = "foo";
+        IngestDocument document1 =
+            new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonMap("bar", "baz")), new HashMap<>());
+        IngestDocument document2 =
+            new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonMap("bar", "blub")), new HashMap<>());
+        assertThrowsOnComparision(document1, document2);
+    }
+
+    public void testOnTypeConflict() {
+        String rootKey = "foo";
+        IngestDocument document1 =
+            new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonList("baz")), new HashMap<>());
+        IngestDocument document2 = new IngestDocument(
+            Collections.singletonMap(rootKey, Collections.singletonMap("blub", "blab")), new HashMap<>()
+        );
+        assertThrowsOnComparision(document1, document2);
+    }
+
+    private static void assertThrowsOnComparision(IngestDocument document1, IngestDocument document2) {
+        expectThrows(AssertionError.class, () -> assertIngestDocument(document1, document2));
+        expectThrows(AssertionError.class, () -> assertIngestDocument(document2, document1));
+    }
+}