Skip to content

Commit a071672

Browse files
original-brownbearoriginal-brownbear
authored
Fix assertIngestDocument wrongfully passing (#31913) (#31951)
* 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
1 parent 4214375 commit a071672

File tree

4 files changed

+123
-32
lines changed

4 files changed

+123
-32
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void testMissingField() {
129129
public void testMissingFieldWithIgnoreMissing() throws Exception {
130130
String fieldName = "foo.bar";
131131
IngestDocument originalIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
132-
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
132+
IngestDocument ingestDocument = new IngestDocument(originalIngestDocument);
133133
GrokProcessor processor = new GrokProcessor(randomAlphaOfLength(10), Collections.singletonMap("ONE", "1"),
134134
Collections.singletonList("%{ONE:one}"), fieldName, false, true, ThreadWatchdog.noop());
135135
processor.execute(ingestDocument);

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

+5-9
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import java.util.List;
3434
import java.util.Map;
3535

36-
import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument;
3736
import static org.hamcrest.Matchers.containsString;
3837
import static org.hamcrest.Matchers.equalTo;
3938

@@ -55,7 +54,7 @@ public void testExecute() throws Exception {
5554
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
5655
jsonProcessor.execute(ingestDocument);
5756
Map<String, Object> jsonified = ingestDocument.getFieldValue(randomTargetField, Map.class);
58-
assertIngestDocument(ingestDocument.getFieldValue(randomTargetField, Object.class), jsonified);
57+
assertEquals(ingestDocument.getFieldValue(randomTargetField, Object.class), jsonified);
5958
}
6059

6160
public void testInvalidValue() {
@@ -161,13 +160,10 @@ public void testAddToRoot() throws Exception {
161160
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
162161
jsonProcessor.execute(ingestDocument);
163162

164-
Map<String, Object> expected = new HashMap<>();
165-
expected.put("a", 1);
166-
expected.put("b", 2);
167-
expected.put("c", "see");
168-
IngestDocument expectedIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), expected);
169-
170-
assertIngestDocument(ingestDocument, expectedIngestDocument);
163+
Map<String, Object> sourceAndMetadata = ingestDocument.getSourceAndMetadata();
164+
assertEquals(1, sourceAndMetadata.get("a"));
165+
assertEquals(2, sourceAndMetadata.get("b"));
166+
assertEquals("see", sourceAndMetadata.get("c"));
171167
}
172168

173169
public void testAddBoolToRoot() {

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

+35-22
Original file line numberDiff line numberDiff line change
@@ -20,48 +20,61 @@
2020
package org.elasticsearch.ingest;
2121

2222
import java.util.List;
23-
import java.util.Locale;
2423
import java.util.Map;
25-
26-
import static org.hamcrest.Matchers.equalTo;
27-
import static org.junit.Assert.assertArrayEquals;
28-
import static org.junit.Assert.assertThat;
24+
import java.util.Objects;
2925

3026
public class IngestDocumentMatcher {
3127
/**
3228
* Helper method to assert the equivalence between two IngestDocuments.
3329
*
34-
* @param a first object to compare
35-
* @param b second object to compare
30+
* @param docA first document to compare
31+
* @param docB second document to compare
3632
*/
37-
public static void assertIngestDocument(Object a, Object b) {
33+
public static void assertIngestDocument(IngestDocument docA, IngestDocument docB) {
34+
if ((deepEquals(docA.getIngestMetadata(), docB.getIngestMetadata(), true) &&
35+
deepEquals(docA.getSourceAndMetadata(), docB.getSourceAndMetadata(), false)) == false) {
36+
throw new AssertionError("Expected [" + docA + "] but received [" + docB + "].");
37+
}
38+
}
39+
40+
private static boolean deepEquals(Object a, Object b, boolean isIngestMeta) {
3841
if (a instanceof Map) {
3942
Map<?, ?> mapA = (Map<?, ?>) a;
43+
if (b instanceof Map == false) {
44+
return false;
45+
}
4046
Map<?, ?> mapB = (Map<?, ?>) b;
47+
if (mapA.size() != mapB.size()) {
48+
return false;
49+
}
4150
for (Map.Entry<?, ?> entry : mapA.entrySet()) {
42-
if (entry.getValue() instanceof List || entry.getValue() instanceof Map) {
43-
assertIngestDocument(entry.getValue(), mapB.get(entry.getKey()));
51+
Object key = entry.getKey();
52+
// Don't compare the timestamp of ingest metadata since it will differ between executions
53+
if ((isIngestMeta && "timestamp".equals(key)) == false
54+
&& deepEquals(entry.getValue(), mapB.get(key), false) == false) {
55+
return false;
4456
}
4557
}
58+
return true;
4659
} else if (a instanceof List) {
4760
List<?> listA = (List<?>) a;
61+
if (b instanceof List == false) {
62+
return false;
63+
}
4864
List<?> listB = (List<?>) b;
49-
for (int i = 0; i < listA.size(); i++) {
65+
int countA = listA.size();
66+
if (countA != listB.size()) {
67+
return false;
68+
}
69+
for (int i = 0; i < countA; i++) {
5070
Object value = listA.get(i);
51-
if (value instanceof List || value instanceof Map) {
52-
assertIngestDocument(value, listB.get(i));
71+
if (deepEquals(value, listB.get(i), false) == false) {
72+
return false;
5373
}
5474
}
55-
} else if (a instanceof byte[]) {
56-
assertArrayEquals((byte[]) a, (byte[])b);
57-
} else if (a instanceof IngestDocument) {
58-
IngestDocument docA = (IngestDocument) a;
59-
IngestDocument docB = (IngestDocument) b;
60-
assertIngestDocument(docA.getSourceAndMetadata(), docB.getSourceAndMetadata());
61-
assertIngestDocument(docA.getIngestMetadata(), docB.getIngestMetadata());
75+
return true;
6276
} else {
63-
String msg = String.format(Locale.ROOT, "Expected %s class to be equal to %s", a.getClass().getName(), b.getClass().getName());
64-
assertThat(msg, a, equalTo(b));
77+
return Objects.deepEquals(a, b);
6578
}
6679
}
6780
}

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

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.ingest;
21+
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.HashMap;
27+
import java.util.Map;
28+
29+
import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument;
30+
31+
public class IngestDocumentMatcherTests extends ESTestCase {
32+
33+
public void testDifferentMapData() {
34+
Map<String, Object> sourceAndMetadata1 = new HashMap<>();
35+
sourceAndMetadata1.put("foo", "bar");
36+
IngestDocument document1 = new IngestDocument(sourceAndMetadata1, new HashMap<>());
37+
IngestDocument document2 = new IngestDocument(new HashMap<>(), new HashMap<>());
38+
assertThrowsOnComparision(document1, document2);
39+
}
40+
41+
public void testDifferentLengthListData() {
42+
String rootKey = "foo";
43+
IngestDocument document1 =
44+
new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz")), new HashMap<>());
45+
IngestDocument document2 =
46+
new IngestDocument(Collections.singletonMap(rootKey, Collections.emptyList()), new HashMap<>());
47+
assertThrowsOnComparision(document1, document2);
48+
}
49+
50+
public void testDifferentNestedListFieldData() {
51+
String rootKey = "foo";
52+
IngestDocument document1 =
53+
new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz")), new HashMap<>());
54+
IngestDocument document2 =
55+
new IngestDocument(Collections.singletonMap(rootKey, Arrays.asList("bar", "blub")), new HashMap<>());
56+
assertThrowsOnComparision(document1, document2);
57+
}
58+
59+
public void testDifferentNestedMapFieldData() {
60+
String rootKey = "foo";
61+
IngestDocument document1 =
62+
new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonMap("bar", "baz")), new HashMap<>());
63+
IngestDocument document2 =
64+
new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonMap("bar", "blub")), new HashMap<>());
65+
assertThrowsOnComparision(document1, document2);
66+
}
67+
68+
public void testOnTypeConflict() {
69+
String rootKey = "foo";
70+
IngestDocument document1 =
71+
new IngestDocument(Collections.singletonMap(rootKey, Collections.singletonList("baz")), new HashMap<>());
72+
IngestDocument document2 = new IngestDocument(
73+
Collections.singletonMap(rootKey, Collections.singletonMap("blub", "blab")), new HashMap<>()
74+
);
75+
assertThrowsOnComparision(document1, document2);
76+
}
77+
78+
private static void assertThrowsOnComparision(IngestDocument document1, IngestDocument document2) {
79+
expectThrows(AssertionError.class, () -> assertIngestDocument(document1, document2));
80+
expectThrows(AssertionError.class, () -> assertIngestDocument(document2, document1));
81+
}
82+
}

0 commit comments

Comments
 (0)