Skip to content

Commit

Permalink
Fix assertIngestDocument wrongfully passing
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
original-brownbear committed Jul 10, 2018
1 parent 3189ef4 commit bcd739d
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void testMissingField() {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
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;

Expand All @@ -55,7 +54,7 @@ public void testExecute() throws Exception {
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() {
Expand Down Expand Up @@ -161,13 +160,10 @@ public void testAddToRoot() throws Exception {
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
jsonProcessor.execute(ingestDocument);

Map<String, Object> expected = new HashMap<>();
expected.put("a", 1);
expected.put("b", 2);
expected.put("c", "see");
IngestDocument expectedIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), expected);

assertIngestDocument(ingestDocument, expectedIngestDocument);
Map<String, Object> sourceAndMetadata = ingestDocument.getSourceAndMetadata();
assertEquals(1, sourceAndMetadata.get("a"));
assertEquals(2, sourceAndMetadata.get("b"));
assertEquals("see", sourceAndMetadata.get("c"));
}

public void testAddBoolToRoot() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,48 +20,61 @@
package org.elasticsearch.ingest;

import java.util.List;
import java.util.Locale;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThat;
import java.util.Objects;

public class IngestDocumentMatcher {
/**
* Helper method to assert the equivalence between two IngestDocuments.
*
* @param a first object to compare
* @param b second object to compare
* @param docA first document 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) {
assertIngestDocument(entry.getValue(), mapB.get(entry.getKey()));
Object key = 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) {
assertIngestDocument(value, listB.get(i));
if (deepEquals(value, listB.get(i), false) == false) {
return false;
}
}
} else if (a instanceof byte[]) {
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());
return true;
} else {
String msg = String.format(Locale.ROOT, "Expected %s class to be equal to %s", a.getClass().getName(), b.getClass().getName());
assertThat(msg, a, equalTo(b));
return Objects.deepEquals(a, b);
}
}
}
Original file line number Diff line number Diff line change
@@ -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 testAssertIngestDocumentFailsOnDifferentMapData() {
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 testAssertIngestDocumentFailsOnDifferentLengthListData() {
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 testAssertIngestDocumentFailsOnDifferentNestedListFieldData() {
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 testAssertIngestDocumentFailsOnDifferentNestedMapFieldData() {
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 testThrowsAssertionErrorOnTypeConflict() {
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));
}
}

0 comments on commit bcd739d

Please sign in to comment.