From 1a5fbb57d731eaac38568aa53fea50b06b42255d Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Tue, 25 Feb 2025 15:20:01 -0500 Subject: [PATCH] Use ordered maps for PipelineConfiguration xcontent deserialization (#123403) --- docs/changelog/123403.yaml | 5 +++ .../ingest/PipelineConfiguration.java | 2 +- .../ingest/PipelineConfigurationTests.java | 37 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/123403.yaml diff --git a/docs/changelog/123403.yaml b/docs/changelog/123403.yaml new file mode 100644 index 0000000000000..836c4b685d72d --- /dev/null +++ b/docs/changelog/123403.yaml @@ -0,0 +1,5 @@ +pr: 123403 +summary: Use ordered maps for `PipelineConfiguration` xcontent deserialization +area: Ingest Node +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java b/server/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java index 9f3f3aaba62fc..44cd32b910342 100644 --- a/server/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java +++ b/server/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java @@ -46,7 +46,7 @@ public final class PipelineConfiguration implements SimpleDiffable builder.setConfig(parser.map()), + (parser, builder, aVoid) -> builder.setConfig(parser.mapOrdered()), new ParseField("config"), ObjectParser.ValueType.OBJECT ); diff --git a/server/src/test/java/org/elasticsearch/ingest/PipelineConfigurationTests.java b/server/src/test/java/org/elasticsearch/ingest/PipelineConfigurationTests.java index 7be6e97762ccf..78e3213e690a8 100644 --- a/server/src/test/java/org/elasticsearch/ingest/PipelineConfigurationTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/PipelineConfigurationTests.java @@ -28,9 +28,11 @@ import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.function.Predicate; import static org.hamcrest.Matchers.anEmptyMap; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; @@ -143,6 +145,41 @@ public void testGetVersion() { } } + @SuppressWarnings("unchecked") + public void testMapKeyOrderingRoundTrip() throws IOException { + // make up two random keys + String key1 = randomAlphaOfLength(10); + String key2 = randomValueOtherThan(key1, () -> randomAlphaOfLength(10)); + // stick them as mappings onto themselves in the _meta of a pipeline configuration + // this happens to use the _meta as a convenient map to test that the ordering of the key sets is the same + String configJson = Strings.format(""" + {"description": "blah", "_meta" : {"foo": "bar", "%s": "%s", "%s": "%s"}}""", key1, key1, key2, key2); + PipelineConfiguration configuration = new PipelineConfiguration( + "1", + new BytesArray(configJson.getBytes(StandardCharsets.UTF_8)), + XContentType.JSON + ); + + // serialize it to bytes + XContentType xContentType = randomFrom(XContentType.values()); + final BytesReference bytes; + try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) { + configuration.toXContent(builder, ToXContent.EMPTY_PARAMS); + bytes = BytesReference.bytes(builder); + } + + // deserialize it back + ContextParser parser = PipelineConfiguration.getParser(); + XContentParser xContentParser = xContentType.xContent() + .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, bytes.streamInput()); + PipelineConfiguration parsed = parser.parse(xContentParser, null); + + // make sure the _meta key sets are in the same order + Set keys1 = ((Map) configuration.getConfig().get("_meta")).keySet(); + Set keys2 = ((Map) parsed.getConfig().get("_meta")).keySet(); + assertThat(keys1, contains(keys2.toArray(new String[0]))); + } + @Override protected PipelineConfiguration createTestInstance() { BytesArray config;