diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java index 5f9936bd0c96d..fb580591ac8c7 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java @@ -344,7 +344,7 @@ public void testAppendingToTheSameField() { execProcessor(processor, ingestDocument, (result, e) -> {}); assertThat(testProcessor.getInvokedCounter(), equalTo(2)); ingestDocument.removeField("_ingest._value"); - assertThat(ingestDocument, equalTo(originalIngestDocument)); + assertIngestDocument(ingestDocument, originalIngestDocument); } public void testRemovingFromTheSameField() { @@ -355,7 +355,7 @@ public void testRemovingFromTheSameField() { execProcessor(processor, ingestDocument, (result, e) -> {}); assertThat(testProcessor.getInvokedCounter(), equalTo(2)); ingestDocument.removeField("_ingest._value"); - assertThat(ingestDocument, equalTo(originalIngestDocument)); + assertIngestDocument(ingestDocument, originalIngestDocument); } public void testMapIteration() { diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java index da71cb12a21f0..1e40345208a1b 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorTests.java @@ -114,7 +114,7 @@ public void testMatchWithoutCaptures() throws Exception { MatcherWatchdog.noop() ); processor.execute(doc); - assertThat(doc, equalTo(originalDoc)); + assertIngestDocument(doc, originalDoc); } public void testNullField() { diff --git a/server/src/main/java/org/elasticsearch/action/ingest/SimulateDocumentBaseResult.java b/server/src/main/java/org/elasticsearch/action/ingest/SimulateDocumentBaseResult.java index 67379821ba014..9e4984d7497a3 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/SimulateDocumentBaseResult.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/SimulateDocumentBaseResult.java @@ -50,12 +50,17 @@ public final class SimulateDocumentBaseResult implements SimulateDocumentResult } public SimulateDocumentBaseResult(IngestDocument ingestDocument) { + Exception failure = null; + WriteableIngestDocument wid = null; if (ingestDocument != null) { - this.ingestDocument = new WriteableIngestDocument(ingestDocument); - } else { - this.ingestDocument = null; + try { + wid = new WriteableIngestDocument(ingestDocument); + } catch (Exception ex) { + failure = ex; + } } - this.failure = null; + this.ingestDocument = wid; + this.failure = failure; } public SimulateDocumentBaseResult(Exception failure) { diff --git a/server/src/main/java/org/elasticsearch/action/ingest/SimulateProcessorResult.java b/server/src/main/java/org/elasticsearch/action/ingest/SimulateProcessorResult.java index 0271afb9a9b77..d09e766f3d2bb 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/SimulateProcessorResult.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/SimulateProcessorResult.java @@ -126,7 +126,19 @@ public SimulateProcessorResult( ) { this.processorTag = processorTag; this.description = description; - this.ingestDocument = (ingestDocument == null) ? null : new WriteableIngestDocument(ingestDocument); + WriteableIngestDocument wid = null; + if (ingestDocument != null) { + try { + wid = new WriteableIngestDocument(ingestDocument); + } catch (Exception ex) { + // if there was a failure already, then track it as a suppressed exception + if (failure != null) { + ex.addSuppressed(failure); + } + failure = ex; + } + } + this.ingestDocument = wid; this.failure = failure; this.conditionalWithResult = conditionalWithResult; this.type = type; diff --git a/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java b/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java index 3a7e3c11fa141..a77d0bf7e2b01 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/WriteableIngestDocument.java @@ -25,7 +25,6 @@ import java.io.IOException; import java.time.ZonedDateTime; import java.util.Map; -import java.util.Objects; import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg; import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; @@ -57,7 +56,7 @@ final class WriteableIngestDocument implements Writeable, ToXContentFragment { sourceAndMetadata.put(Metadata.VERSION_TYPE.getFieldName(), a[4]); } Map ingestMetadata = (Map) a[6]; - return new WriteableIngestDocument(new IngestDocument(sourceAndMetadata, ingestMetadata)); + return new WriteableIngestDocument(sourceAndMetadata, ingestMetadata); } ); static { @@ -83,17 +82,30 @@ final class WriteableIngestDocument implements Writeable, ToXContentFragment { PARSER.declareObject(constructorArg(), INGEST_DOC_PARSER, new ParseField(DOC_FIELD)); } + /** + * Builds a writeable ingest document that wraps a copy of the passed-in, non-null ingest document. + * + * @throws IllegalArgumentException if the passed-in ingest document references itself + */ WriteableIngestDocument(IngestDocument ingestDocument) { assert ingestDocument != null; - this.ingestDocument = ingestDocument; + this.ingestDocument = new IngestDocument(ingestDocument); // internal defensive copy } - WriteableIngestDocument(StreamInput in) throws IOException { - Map sourceAndMetadata = in.readMap(); - Map ingestMetadata = in.readMap(); + /** + * Builds a writeable ingest document by constructing the wrapped ingest document from the passed-in maps. + *

+ * This is intended for cases like deserialization, where we know the passed-in maps aren't self-referencing, + * and where a defensive copy is unnecessary. + */ + private WriteableIngestDocument(Map sourceAndMetadata, Map ingestMetadata) { this.ingestDocument = new IngestDocument(sourceAndMetadata, ingestMetadata); } + WriteableIngestDocument(StreamInput in) throws IOException { + this(in.readMap(), in.readMap()); + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeGenericMap(ingestDocument.getSourceAndMetadata()); @@ -127,23 +139,6 @@ public static WriteableIngestDocument fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - WriteableIngestDocument that = (WriteableIngestDocument) o; - return Objects.equals(ingestDocument, that.ingestDocument); - } - - @Override - public int hashCode() { - return Objects.hash(ingestDocument); - } - @Override public String toString() { return ingestDocument.toString(); diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 676a347c5890c..1b2d8056ac437 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -9,6 +9,7 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.index.VersionType; @@ -94,14 +95,26 @@ public IngestDocument(String index, String id, long version, String routing, Ver /** * Copy constructor that creates a new {@link IngestDocument} which has exactly the same properties as the one provided. + * + * @throws IllegalArgumentException if the passed-in ingest document references itself */ public IngestDocument(IngestDocument other) { this( - new IngestCtxMap(deepCopyMap(other.ctxMap.getSource()), other.ctxMap.getMetadata().clone()), + new IngestCtxMap(deepCopyMap(ensureNoSelfReferences(other.ctxMap.getSource())), other.ctxMap.getMetadata().clone()), deepCopyMap(other.ingestMetadata) ); } + /** + * Internal helper utility method to get around the issue that a {@code this(...) } constructor call must be the first statement + * in a constructor. This is only for use in the {@link IngestDocument#IngestDocument(IngestDocument)} copy constructor, it's not a + * general purpose method. + */ + private static Map ensureNoSelfReferences(Map source) { + CollectionUtils.ensureNoSelfReferences(source, null); + return source; + } + /** * Constructor to create an IngestDocument from its constituent maps. The maps are shallow copied. */ @@ -898,24 +911,6 @@ public void doNoSelfReferencesCheck(boolean doNoSelfReferencesCheck) { this.doNoSelfReferencesCheck = doNoSelfReferencesCheck; } - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (obj == null || getClass() != obj.getClass()) { - return false; - } - - IngestDocument other = (IngestDocument) obj; - return Objects.equals(ctxMap, other.ctxMap) && Objects.equals(ingestMetadata, other.ingestMetadata); - } - - @Override - public int hashCode() { - return Objects.hash(ctxMap, ingestMetadata); - } - @Override public String toString() { return "IngestDocument{" + " sourceAndMetadata=" + ctxMap + ", ingestMetadata=" + ingestMetadata + '}'; diff --git a/server/src/main/java/org/elasticsearch/ingest/TrackingResultProcessor.java b/server/src/main/java/org/elasticsearch/ingest/TrackingResultProcessor.java index f09b2c12c99dd..5bd811962a4c4 100644 --- a/server/src/main/java/org/elasticsearch/ingest/TrackingResultProcessor.java +++ b/server/src/main/java/org/elasticsearch/ingest/TrackingResultProcessor.java @@ -98,7 +98,7 @@ public void execute(IngestDocument ingestDocument, BiConsumer getRandomFieldsExcludeFilter() { } public static void assertEqualDocs(SimulateDocumentBaseResult response, SimulateDocumentBaseResult parsedResponse) { - assertEquals(response.getIngestDocument(), parsedResponse.getIngestDocument()); + assertIngestDocument(response.getIngestDocument(), parsedResponse.getIngestDocument()); if (response.getFailure() != null) { assertNotNull(parsedResponse.getFailure()); assertThat(parsedResponse.getFailure().getMessage(), containsString(response.getFailure().getMessage())); diff --git a/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java b/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java index 451ac526773da..f816d43c49240 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/SimulateExecutionServiceTests.java @@ -108,7 +108,7 @@ public void testExecuteItem() throws Exception { assertThat(processor.getInvokedCounter(), equalTo(2)); assertThat(actualItemResponse, instanceOf(SimulateDocumentBaseResult.class)); SimulateDocumentBaseResult simulateDocumentBaseResult = (SimulateDocumentBaseResult) actualItemResponse; - assertThat(simulateDocumentBaseResult.getIngestDocument(), equalTo(ingestDocument)); + assertIngestDocument(simulateDocumentBaseResult.getIngestDocument(), ingestDocument); assertThat(simulateDocumentBaseResult.getFailure(), nullValue()); } diff --git a/server/src/test/java/org/elasticsearch/action/ingest/SimulateProcessorResultTests.java b/server/src/test/java/org/elasticsearch/action/ingest/SimulateProcessorResultTests.java index d76df88e98189..e659893491d06 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/SimulateProcessorResultTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/SimulateProcessorResultTests.java @@ -127,7 +127,7 @@ protected Predicate getRandomFieldsExcludeFilter() { static void assertEqualProcessorResults(SimulateProcessorResult response, SimulateProcessorResult parsedResponse) { assertEquals(response.getProcessorTag(), parsedResponse.getProcessorTag()); - assertEquals(response.getIngestDocument(), parsedResponse.getIngestDocument()); + assertIngestDocument(response.getIngestDocument(), parsedResponse.getIngestDocument()); if (response.getFailure() != null) { assertNotNull(parsedResponse.getFailure()); assertThat(parsedResponse.getFailure().getMessage(), containsString(response.getFailure().getMessage())); diff --git a/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java b/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java index f4cbfac402a5a..654ae9cdd68b7 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/WriteableIngestDocumentTests.java @@ -26,7 +26,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -41,74 +40,18 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.sameInstance; public class WriteableIngestDocumentTests extends AbstractXContentTestCase { - public void testEqualsAndHashcode() throws Exception { - Map sourceAndMetadata = RandomDocumentPicks.randomSource(random()); - int numFields = randomIntBetween(1, IngestDocument.Metadata.values().length); - sourceAndMetadata.put(VERSION.getFieldName(), TestIngestDocument.randomVersion()); - for (int i = 0; i < numFields; i++) { - Tuple metadata = TestIngestDocument.randomMetadata(); - sourceAndMetadata.put(metadata.v1(), metadata.v2()); - } - Map ingestMetadata = new HashMap<>(); - numFields = randomIntBetween(1, 5); - for (int i = 0; i < numFields; i++) { - ingestMetadata.put(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)); - } - WriteableIngestDocument ingestDocument = new WriteableIngestDocument(new IngestDocument(sourceAndMetadata, ingestMetadata)); - - boolean changed = false; - Map otherSourceAndMetadata; - if (randomBoolean()) { - otherSourceAndMetadata = RandomDocumentPicks.randomSource(random()); - otherSourceAndMetadata.put(VERSION.getFieldName(), TestIngestDocument.randomVersion()); - changed = true; - } else { - otherSourceAndMetadata = new HashMap<>(sourceAndMetadata); - } - if (randomBoolean()) { - numFields = randomIntBetween(1, IngestDocument.Metadata.values().length); - for (int i = 0; i < numFields; i++) { - Tuple metadata = randomValueOtherThanMany( - t -> t.v2().equals(sourceAndMetadata.get(t.v1())), - TestIngestDocument::randomMetadata - ); - otherSourceAndMetadata.put(metadata.v1(), metadata.v2()); - } - changed = true; - } - - Map otherIngestMetadata; - if (randomBoolean()) { - otherIngestMetadata = new HashMap<>(); - numFields = randomIntBetween(1, 5); - for (int i = 0; i < numFields; i++) { - otherIngestMetadata.put(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)); - } - changed = true; - } else { - otherIngestMetadata = Collections.unmodifiableMap(ingestMetadata); - } + @Override + protected boolean assertToXContentEquivalence() { + return false; + } - WriteableIngestDocument otherIngestDocument = new WriteableIngestDocument( - new IngestDocument(otherSourceAndMetadata, otherIngestMetadata) - ); - if (changed) { - assertThat(ingestDocument, not(equalTo(otherIngestDocument))); - assertThat(otherIngestDocument, not(equalTo(ingestDocument))); - } else { - assertThat(ingestDocument, equalTo(otherIngestDocument)); - assertThat(otherIngestDocument, equalTo(ingestDocument)); - assertThat(ingestDocument.hashCode(), equalTo(otherIngestDocument.hashCode())); - WriteableIngestDocument thirdIngestDocument = new WriteableIngestDocument( - new IngestDocument(Collections.unmodifiableMap(sourceAndMetadata), Collections.unmodifiableMap(ingestMetadata)) - ); - assertThat(thirdIngestDocument, equalTo(ingestDocument)); - assertThat(ingestDocument, equalTo(thirdIngestDocument)); - assertThat(ingestDocument.hashCode(), equalTo(thirdIngestDocument.hashCode())); - } + @Override + protected void assertEqualInstances(WriteableIngestDocument expectedInstance, WriteableIngestDocument newInstance) { + assertIngestDocument(expectedInstance.getIngestDocument(), newInstance.getIngestDocument()); } public void testSerialization() throws IOException { @@ -164,8 +107,7 @@ public void testToXContent() throws IOException { sourceAndMetadata.putAll(toXContentSource); ingestDocument.getMetadata().keySet().forEach(k -> sourceAndMetadata.put(k, ingestDocument.getMetadata().get(k))); IngestDocument serializedIngestDocument = new IngestDocument(sourceAndMetadata, toXContentIngestMetadata); - // TODO(stu): is this test correct? Comparing against ingestDocument fails due to incorrectly failed byte array comparisons - assertThat(serializedIngestDocument, equalTo(serializedIngestDocument)); + assertIngestDocument(writeableIngestDocument.getIngestDocument(), serializedIngestDocument); } public void testXContentHashSetSerialization() throws Exception { @@ -183,6 +125,14 @@ public void testXContentHashSetSerialization() throws Exception { } } + public void testCopiesTheIngestDocument() { + IngestDocument document = createRandomIngestDoc(); + WriteableIngestDocument wid = new WriteableIngestDocument(document); + + assertIngestDocument(wid.getIngestDocument(), document); + assertThat(wid.getIngestDocument(), not(sameInstance(document))); + } + static IngestDocument createRandomIngestDoc() { XContentType xContentType = randomFrom(XContentType.values()); BytesReference sourceBytes = RandomObjects.randomSource(random(), xContentType); diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java index 38c984c3de933..166b830d14301 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.ingest; -import org.elasticsearch.core.Tuple; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; import org.junit.Before; @@ -958,70 +957,6 @@ public void testRemoveEmptyField() { } } - public void testEqualsAndHashcode() throws Exception { - Map sourceAndMetadata = RandomDocumentPicks.randomSource(random()); - int numFields = randomIntBetween(1, IngestDocument.Metadata.values().length); - for (int i = 0; i < numFields; i++) { - Tuple metadata = TestIngestDocument.randomMetadata(); - sourceAndMetadata.put(metadata.v1(), metadata.v2()); - } - sourceAndMetadata.putIfAbsent("_version", TestIngestDocument.randomVersion()); - Map ingestMetadata = new HashMap<>(); - numFields = randomIntBetween(1, 5); - for (int i = 0; i < numFields; i++) { - ingestMetadata.put(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)); - } - // this is testing equality so use the wire constructor - IngestDocument ingestDocument = new IngestDocument(sourceAndMetadata, ingestMetadata); - - boolean changed = false; - Map otherSourceAndMetadata; - if (randomBoolean()) { - otherSourceAndMetadata = RandomDocumentPicks.randomSource(random()); - otherSourceAndMetadata.putIfAbsent("_version", TestIngestDocument.randomVersion()); - changed = true; - } else { - otherSourceAndMetadata = new HashMap<>(sourceAndMetadata); - } - if (randomBoolean()) { - numFields = randomIntBetween(1, IngestDocument.Metadata.values().length); - for (int i = 0; i < numFields; i++) { - Tuple metadata; - do { - metadata = TestIngestDocument.randomMetadata(); - } while (metadata.v2().equals(sourceAndMetadata.get(metadata.v1()))); // must actually be a change - otherSourceAndMetadata.put(metadata.v1(), metadata.v2()); - } - changed = true; - } - - Map otherIngestMetadata; - if (randomBoolean()) { - otherIngestMetadata = new HashMap<>(); - numFields = randomIntBetween(1, 5); - for (int i = 0; i < numFields; i++) { - otherIngestMetadata.put(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10)); - } - changed = true; - } else { - otherIngestMetadata = Map.copyOf(ingestMetadata); - } - - IngestDocument otherIngestDocument = new IngestDocument(otherSourceAndMetadata, otherIngestMetadata); - if (changed) { - assertThat(ingestDocument, not(equalTo(otherIngestDocument))); - assertThat(otherIngestDocument, not(equalTo(ingestDocument))); - } else { - assertThat(ingestDocument, equalTo(otherIngestDocument)); - assertThat(otherIngestDocument, equalTo(ingestDocument)); - assertThat(ingestDocument.hashCode(), equalTo(otherIngestDocument.hashCode())); - IngestDocument thirdIngestDocument = new IngestDocument(Map.copyOf(sourceAndMetadata), Map.copyOf(ingestMetadata)); - assertThat(thirdIngestDocument, equalTo(ingestDocument)); - assertThat(ingestDocument, equalTo(thirdIngestDocument)); - assertThat(ingestDocument.hashCode(), equalTo(thirdIngestDocument.hashCode())); - } - } - public void testIngestMetadataTimestamp() throws Exception { long before = System.currentTimeMillis(); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random()); @@ -1069,6 +1004,17 @@ public void testCopyConstructor() { assertThat(ingestDocument.getFieldValue("_id", String.class), equalTo("bar1")); assertThat(ingestDocument.getFieldValue("hello", String.class), equalTo("world1")); } + + { + // the copy constructor rejects self-references + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random()); + List someList = new ArrayList<>(); + someList.add("some string"); + someList.add(someList); // the list contains itself + ingestDocument.setFieldValue("someList", someList); + Exception e = expectThrows(IllegalArgumentException.class, () -> new IngestDocument(ingestDocument)); + assertThat(e.getMessage(), equalTo("Iterable object is self-referencing itself")); + } } public void testCopyConstructorWithZonedDateTime() { diff --git a/server/src/test/java/org/elasticsearch/ingest/PipelineProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/PipelineProcessorTests.java index 40b1de8c09de4..ab7da6c952450 100644 --- a/server/src/test/java/org/elasticsearch/ingest/PipelineProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/PipelineProcessorTests.java @@ -20,6 +20,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.LongSupplier; +import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -60,7 +61,7 @@ public String getDescription() { Map config = new HashMap<>(); config.put("name", pipelineId); factory.create(Map.of(), null, null, config).execute(testIngestDocument, (result, e) -> {}); - assertEquals(testIngestDocument, invoked.get()); + assertIngestDocument(testIngestDocument, invoked.get()); } public void testThrowsOnMissingPipeline() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/ingest/TrackingResultProcessorTests.java b/server/src/test/java/org/elasticsearch/ingest/TrackingResultProcessorTests.java index 518543092db85..8ad8667a52770 100644 --- a/server/src/test/java/org/elasticsearch/ingest/TrackingResultProcessorTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/TrackingResultProcessorTests.java @@ -27,6 +27,7 @@ import static org.elasticsearch.ingest.CompoundProcessor.ON_FAILURE_MESSAGE_FIELD; import static org.elasticsearch.ingest.CompoundProcessor.ON_FAILURE_PROCESSOR_TAG_FIELD; import static org.elasticsearch.ingest.CompoundProcessor.ON_FAILURE_PROCESSOR_TYPE_FIELD; +import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument; import static org.elasticsearch.ingest.PipelineProcessorTests.createIngestService; import static org.elasticsearch.ingest.TrackingResultProcessor.decorate; import static org.hamcrest.Matchers.containsString; @@ -66,7 +67,7 @@ public void testActualProcessor() throws Exception { assertThat(actualProcessor.getInvokedCounter(), equalTo(1)); assertThat(resultList.size(), equalTo(1)); - assertThat(resultList.get(0).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(0).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(0).getFailure(), nullValue()); assertThat(resultList.get(0).getProcessorTag(), equalTo(expectedResult.getProcessorTag())); } @@ -222,7 +223,7 @@ public void testActualCompoundProcessorWithIgnoreFailure() throws Exception { ); assertThat(testProcessor.getInvokedCounter(), equalTo(1)); assertThat(resultList.size(), equalTo(1)); - assertThat(resultList.get(0).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(0).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(0).getFailure(), sameInstance(exception)); assertThat(resultList.get(0).getProcessorTag(), equalTo(expectedResult.getProcessorTag())); } @@ -281,7 +282,7 @@ public void testActualCompoundProcessorWithFalseConditional() throws Exception { assertFalse(resultList.get(2).getIngestDocument().hasField(key2)); assertTrue(resultList.get(2).getIngestDocument().hasField(key3)); - assertThat(resultList.get(2).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(2).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(2).getFailure(), nullValue()); assertThat(resultList.get(2).getProcessorTag(), nullValue()); } @@ -335,7 +336,7 @@ public void testActualPipelineProcessor() throws Exception { assertTrue(resultList.get(2).getIngestDocument().hasField(key2)); assertFalse(resultList.get(2).getIngestDocument().hasField(key3)); - assertThat(resultList.get(3).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(3).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(3).getFailure(), nullValue()); assertThat(resultList.get(3).getProcessorTag(), nullValue()); } @@ -425,7 +426,7 @@ public void testActualPipelineProcessorWithTrueConditional() throws Exception { assertTrue(resultList.get(3).getIngestDocument().hasField(key2)); assertFalse(resultList.get(3).getIngestDocument().hasField(key3)); - assertThat(resultList.get(4).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(4).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(4).getFailure(), nullValue()); assertThat(resultList.get(4).getProcessorTag(), nullValue()); } @@ -511,7 +512,7 @@ public void testActualPipelineProcessorWithFalseConditional() throws Exception { assertThat(resultList.get(2).getConditionalWithResult().v1(), equalTo(scriptName)); assertThat(resultList.get(2).getConditionalWithResult().v2(), is(Boolean.FALSE)); - assertThat(resultList.get(3).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(3).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(3).getFailure(), nullValue()); assertThat(resultList.get(3).getProcessorTag(), nullValue()); } @@ -577,7 +578,7 @@ public void testActualPipelineProcessorWithHandledFailure() throws Exception { assertTrue(resultList.get(3).getIngestDocument().hasField(key2)); assertFalse(resultList.get(3).getIngestDocument().hasField(key3)); - assertThat(resultList.get(4).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(4).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(4).getFailure(), nullValue()); assertThat(resultList.get(4).getProcessorTag(), nullValue()); } @@ -710,14 +711,17 @@ public void testActualPipelineProcessorRepeatedInvocation() throws Exception { assertNull(resultList.get(0).getConditionalWithResult()); assertThat(resultList.get(0).getType(), equalTo("pipeline")); - assertThat(resultList.get(1).getIngestDocument(), not(equalTo(expectedResult.getIngestDocument()))); + assertThat( + resultList.get(1).getIngestDocument().getFieldValue(key1, Integer.class), + not(equalTo(expectedResult.getIngestDocument().getFieldValue(key1, Integer.class))) + ); assertThat(resultList.get(1).getFailure(), nullValue()); assertThat(resultList.get(1).getProcessorTag(), nullValue()); assertNull(resultList.get(2).getConditionalWithResult()); assertThat(resultList.get(2).getType(), equalTo("pipeline")); - assertThat(resultList.get(3).getIngestDocument(), equalTo(expectedResult.getIngestDocument())); + assertIngestDocument(resultList.get(3).getIngestDocument(), expectedResult.getIngestDocument()); assertThat(resultList.get(3).getFailure(), nullValue()); assertThat(resultList.get(3).getProcessorTag(), nullValue()); diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/IngestDocumentMatcher.java b/test/framework/src/main/java/org/elasticsearch/ingest/IngestDocumentMatcher.java index e2bea702baff5..f4e88d6eb4fb5 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/IngestDocumentMatcher.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/IngestDocumentMatcher.java @@ -12,17 +12,32 @@ import java.util.Map; import java.util.Objects; -public class IngestDocumentMatcher { +public final class IngestDocumentMatcher { + + private IngestDocumentMatcher() { + // utility class + } + /** * Helper method to assert the equivalence between two IngestDocuments. * - * @param docA first document to compare - * @param docB second document to compare + * @param expected first document to compare + * @param actual second document to compare */ - 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 + "]."); + public static void assertIngestDocument(IngestDocument expected, IngestDocument actual) { + // trivially true: if they're both null, then all is well + if (expected == null && actual == null) { + return; + } + + // if only one is null, however, then that's not okay + if ((expected == null || actual == null)) { + throw new AssertionError("Expected [" + expected + "] but received [" + actual + "]."); + } + + if ((deepEquals(expected.getIngestMetadata(), actual.getIngestMetadata(), true) + && deepEquals(expected.getSourceAndMetadata(), actual.getSourceAndMetadata(), false)) == false) { + throw new AssertionError("Expected [" + expected + "] but received [" + actual + "]."); } } diff --git a/test/framework/src/test/java/org/elasticsearch/ingest/IngestDocumentMatcherTests.java b/test/framework/src/test/java/org/elasticsearch/ingest/IngestDocumentMatcherTests.java index 019f64636831d..a01f86d8a6b28 100644 --- a/test/framework/src/test/java/org/elasticsearch/ingest/IngestDocumentMatcherTests.java +++ b/test/framework/src/test/java/org/elasticsearch/ingest/IngestDocumentMatcherTests.java @@ -10,9 +10,8 @@ import org.elasticsearch.test.ESTestCase; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument; @@ -29,41 +28,52 @@ public void testDifferentMapData() { public void testDifferentLengthListData() { String rootKey = "foo"; - IngestDocument document1 = TestIngestDocument.withDefaultVersion(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz"))); - IngestDocument document2 = TestIngestDocument.withDefaultVersion(Collections.singletonMap(rootKey, Collections.emptyList())); + IngestDocument document1 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, List.of("bar", "baz"))); + IngestDocument document2 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, List.of())); assertThrowsOnComparision(document1, document2); } public void testDifferentNestedListFieldData() { String rootKey = "foo"; - IngestDocument document1 = TestIngestDocument.withDefaultVersion(Collections.singletonMap(rootKey, Arrays.asList("bar", "baz"))); - IngestDocument document2 = TestIngestDocument.withDefaultVersion(Collections.singletonMap(rootKey, Arrays.asList("bar", "blub"))); + IngestDocument document1 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, List.of("bar", "baz"))); + IngestDocument document2 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, List.of("bar", "blub"))); assertThrowsOnComparision(document1, document2); } public void testDifferentNestedMapFieldData() { String rootKey = "foo"; - IngestDocument document1 = TestIngestDocument.withDefaultVersion( - Collections.singletonMap(rootKey, Collections.singletonMap("bar", "baz")) - ); - IngestDocument document2 = TestIngestDocument.withDefaultVersion( - Collections.singletonMap(rootKey, Collections.singletonMap("bar", "blub")) - ); + IngestDocument document1 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, Map.of("bar", "baz"))); + IngestDocument document2 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, Map.of("bar", "blub"))); assertThrowsOnComparision(document1, document2); } public void testOnTypeConflict() { String rootKey = "foo"; - IngestDocument document1 = TestIngestDocument.withDefaultVersion( - Collections.singletonMap(rootKey, Collections.singletonList("baz")) - ); - IngestDocument document2 = TestIngestDocument.withDefaultVersion( - Collections.singletonMap(rootKey, Collections.singletonMap("blub", "blab")) - ); + IngestDocument document1 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, List.of("baz"))); + IngestDocument document2 = TestIngestDocument.withDefaultVersion(Map.of(rootKey, Map.of("blub", "blab"))); assertThrowsOnComparision(document1, document2); } + public void testNestedMapArrayEquivalence() { + IngestDocument ingestDocument = TestIngestDocument.emptyIngestDocument(); + // Test that equality still works when the ingest document uses primitive arrays, + // since normal .equals() methods would not work for Maps containing these arrays. + byte[] numbers = new byte[] { 0, 1, 2 }; + ingestDocument.setFieldValue("some.nested.array", numbers); + IngestDocument copy = new IngestDocument(ingestDocument); + byte[] copiedNumbers = copy.getFieldValue("some.nested.array", byte[].class); + assertArrayEquals(numbers, copiedNumbers); + assertNotEquals(numbers, copiedNumbers); + assertIngestDocument(ingestDocument, copy); + } + + public void testNullsAreEqual() { + assertIngestDocument(null, null); + } + private static void assertThrowsOnComparision(IngestDocument document1, IngestDocument document2) { + expectThrows(AssertionError.class, () -> assertIngestDocument(document1, null)); + expectThrows(AssertionError.class, () -> assertIngestDocument(null, document2)); expectThrows(AssertionError.class, () -> assertIngestDocument(document1, document2)); expectThrows(AssertionError.class, () -> assertIngestDocument(document2, document1)); }