From 99a42953307b6e64fed3f2cac7cb7273fd9d3994 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 30 Nov 2015 14:48:54 +0100 Subject: [PATCH] If a list or map value gets set on ingest document a deep copy needs to be made. If this is not done this can lead to processor configuration being changed by an bulk or index request. --- .../elasticsearch/ingest/IngestDocument.java | 30 +++++++++++ .../ingest/IngestDocumentTests.java | 11 ++++ .../elasticsearch/ingest/PipelineTests.java | 52 +++++++++++++++++++ .../ingest/RandomDocumentPicks.java | 2 +- 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 plugins/ingest/src/test/java/org/elasticsearch/ingest/PipelineTests.java diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 5629e25e4b8..fbb68ab812b 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -20,6 +20,8 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.collect.HppcMaps; +import org.elasticsearch.search.aggregations.support.format.ValueParser; import java.util.*; @@ -258,6 +260,8 @@ public final class IngestDocument { String[] pathElements = Strings.splitStringToArray(path, '.'); assert pathElements.length > 0; + value = deepCopy(value); + Object context = source; for (int i = 0; i < pathElements.length - 1; i++) { String pathElement = pathElements[i]; @@ -354,6 +358,32 @@ public final class IngestDocument { return sourceModified; } + static Object deepCopy(Object value) { + if (value instanceof Map) { + @SuppressWarnings("unchecked") + Map mapValue = (Map) value; + Map copy = new HashMap<>(mapValue.size()); + for (Map.Entry entry : mapValue.entrySet()) { + copy.put(entry.getKey(), deepCopy(entry.getValue())); + } + return copy; + } else if (value instanceof List) { + @SuppressWarnings("unchecked") + List listValue = (List) value; + List copy = new ArrayList<>(listValue.size()); + for (Object itemValue : listValue) { + copy.add(deepCopy(itemValue)); + } + return copy; + } else if (value == null || value instanceof String || value instanceof Integer || + value instanceof Long || value instanceof Float || + value instanceof Double || value instanceof Boolean) { + return value; + } else { + throw new IllegalArgumentException("unexpected value type [" + value.getClass() + "]"); + } + } + @Override public boolean equals(Object obj) { if (obj == this) { return true; } diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java index fb5966e76db..b3bd8aaf827 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest; +import com.carrotsearch.randomizedtesting.generators.RandomPicks; import org.elasticsearch.test.ESTestCase; import org.junit.Before; @@ -540,4 +541,14 @@ public class IngestDocumentTests extends ESTestCase { assertThat(ingestDocument.hashCode(), equalTo(thirdIngestDocument.hashCode())); } } + + public void testDeepCopy() { + int iterations = scaledRandomIntBetween(8, 64); + for (int i = 0; i < iterations; i++) { + Map map = RandomDocumentPicks.randomDocument(random()); + Object copy = IngestDocument.deepCopy(map); + assertThat("iteration: " + i, copy, equalTo(map)); + assertThat("iteration: " + i, copy, not(sameInstance(map))); + } + } } diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/PipelineTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/PipelineTests.java new file mode 100644 index 00000000000..ae23df8e71d --- /dev/null +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/PipelineTests.java @@ -0,0 +1,52 @@ +package org.elasticsearch.ingest; + +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.ingest.processor.Processor; +import org.elasticsearch.ingest.processor.set.SetProcessor; +import org.elasticsearch.ingest.processor.remove.RemoveProcessor; +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class PipelineTests extends ESTestCase { + + public void testProcessorSettingsRemainUntouched() throws Exception { + Map subField = new HashMap<>(); + subField.put("_subfield", "value"); + Map fieldSettings = new HashMap<>(); + fieldSettings.put("_field", subField); + Map addSettings = new HashMap<>(); + addSettings.put("fields", fieldSettings); + Map removeSettings = new HashMap<>(); + removeSettings.put("fields", Collections.singletonList("_field._subfield")); + Pipeline pipeline = createPipeline(processorConfig(SetProcessor.TYPE, addSettings), processorConfig(RemoveProcessor.TYPE, removeSettings)); + + IngestDocument ingestDocument = new IngestDocument("_index", "_type", "_id", new HashMap<>()); + pipeline.execute(ingestDocument); + + assertThat(ingestDocument.getSource().get("_field"), Matchers.notNullValue()); + assertThat(((Map) ingestDocument.getSource().get("_field")).get("_subfield"), Matchers.nullValue()); + assertThat(((Map) fieldSettings.get("_field")).get("_subfield"), Matchers.equalTo("value")); + } + + private Pipeline createPipeline(Map... processorConfigs) throws Exception { + Map config = new HashMap<>(); + config.put("processors", Arrays.asList(processorConfigs)); + Map factoryRegistry = new HashMap<>(); + factoryRegistry.put(SetProcessor.TYPE, new SetProcessor.Factory()); + factoryRegistry.put(RemoveProcessor.TYPE, new RemoveProcessor.Factory()); + Pipeline.Factory factory = new Pipeline.Factory(); + return factory.create("_id", config, factoryRegistry); + } + + private Map processorConfig(String type, Map settings) { + Map processorConfig = new HashMap<>(); + processorConfig.put(type, settings); + return processorConfig; + } + +} diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/RandomDocumentPicks.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/RandomDocumentPicks.java index 0443dd0b68f..e83a66a38d6 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/RandomDocumentPicks.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/RandomDocumentPicks.java @@ -135,7 +135,7 @@ public final class RandomDocumentPicks { return new IngestDocument(index, type, id, document); } - private static Map randomDocument(Random random) { + public static Map randomDocument(Random random) { Map document = new HashMap<>(); addRandomFields(random, document, 0); return document;