From e926c60f8dc9f55ab26f1d0736bd664035a00477 Mon Sep 17 00:00:00 2001 From: weizijun Date: Sat, 4 Sep 2021 04:22:56 +0800 Subject: [PATCH] Fix template equals when mappings are wrapped (#77008) When create template v2. mappings will add _doc. This will cause the created template to be inconsistent with the queried template. In template class, add mappingsEquals method, to deal with this case. reproduced: MetadataIndexTemplateServiceTests.testAddComponentTemplate when mappings are not null, the case will failed. ``` Template template = new Template( Settings.builder().build(), new CompressedXContent("{\"properties\":{\"@timestamp\":{\"type\":\"date\"}}}"), ComponentTemplateTests.randomAliases() ); ComponentTemplate componentTemplate = new ComponentTemplate(template, 1L, new HashMap<>()); ``` Co-authored-by: Elastic Machine # Conflicts: # server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java --- .../cluster/metadata/Template.java | 27 ++++++++- .../metadata/ComponentTemplateTests.java | 56 ++++++++++++++++++ .../MetadataIndexTemplateServiceTests.java | 59 ++----------------- 3 files changed, 87 insertions(+), 55 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java index ca9b6b291a2b2..f5854bb97fafc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Template.java @@ -9,6 +9,7 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.cluster.AbstractDiffable; +import org.elasticsearch.common.util.Maps; import org.elasticsearch.core.Nullable; import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.Strings; @@ -141,7 +142,7 @@ public boolean equals(Object obj) { } Template other = (Template) obj; return Objects.equals(settings, other.settings) && - Objects.equals(mappings, other.mappings) && + mappingsEquals(this.mappings, other.mappings) && Objects.equals(aliases, other.aliases); } @@ -178,11 +179,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } @SuppressWarnings("unchecked") - private static Map reduceMapping(Map mapping) { + static Map reduceMapping(Map mapping) { if (mapping.size() == 1 && MapperService.SINGLE_MAPPING_NAME.equals(mapping.keySet().iterator().next())) { return (Map) mapping.values().iterator().next(); } else { return mapping; } } + + static boolean mappingsEquals(CompressedXContent m1, CompressedXContent m2) { + if (m1 == m2) { + return true; + } + + if (m1 == null || m2 == null) { + return false; + } + + if (m1.equals(m2)) { + return true; + } + + Map thisUncompressedMapping = reduceMapping( + XContentHelper.convertToMap(m1.uncompressed(), true, XContentType.JSON).v2() + ); + Map otherUncompressedMapping = reduceMapping( + XContentHelper.convertToMap(m2.uncompressed(), true, XContentType.JSON).v2() + ); + return Maps.deepEquals(thisUncompressedMapping, otherUncompressedMapping); + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ComponentTemplateTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ComponentTemplateTests.java index 6a610e394e789..0895233befac0 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ComponentTemplateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ComponentTemplateTests.java @@ -9,10 +9,16 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.cluster.Diff; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.test.AbstractDiffableSerializationTestCase; import org.elasticsearch.test.ESTestCase; @@ -20,6 +26,8 @@ import java.util.Collections; import java.util.Map; +import static org.hamcrest.Matchers.equalTo; + public class ComponentTemplateTests extends AbstractDiffableSerializationTestCase { @Override protected ComponentTemplate makeTestChanges(ComponentTemplate testInstance) { @@ -154,4 +162,52 @@ public static ComponentTemplate mutateTemplate(ComponentTemplate orig) { throw new IllegalStateException("illegal randomization branch"); } } + + public void testMappingsEquals() throws IOException { + { + CompressedXContent mappings = randomMappings(); + assertThat(Template.mappingsEquals(mappings, mappings), equalTo(true)); + } + + { + assertThat(Template.mappingsEquals(null, null), equalTo(true)); + } + + { + CompressedXContent mappings = randomMappings(); + assertThat(Template.mappingsEquals(mappings, null), equalTo(false)); + assertThat(Template.mappingsEquals(null, mappings), equalTo(false)); + } + + { + String randomString = randomAlphaOfLength(10); + CompressedXContent m1 = new CompressedXContent("{\"properties\":{\"" + randomString + "\":{\"type\":\"keyword\"}}}"); + CompressedXContent m2 = new CompressedXContent("{\"properties\":{\"" + randomString + "\":{\"type\":\"keyword\"}}}"); + assertThat(Template.mappingsEquals(m1, m2), equalTo(true)); + } + + { + CompressedXContent m1 = randomMappings(); + CompressedXContent m2 = new CompressedXContent("{\"properties\":{\"" + randomAlphaOfLength(10) + "\":{\"type\":\"keyword\"}}}"); + assertThat(Template.mappingsEquals(m1, m2), equalTo(false)); + } + + { + Map map = XContentHelper.convertToMap( + new BytesArray( + "{\"" + + MapperService.SINGLE_MAPPING_NAME + + "\":{\"properties\":{\"" + + randomAlphaOfLength(10) + + "\":{\"type\":\"keyword\"}}}}" + ), + true, + XContentType.JSON + ).v2(); + Map reduceMap = Template.reduceMapping(map); + CompressedXContent m1 = new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(map))); + CompressedXContent m2 = new CompressedXContent(Strings.toString(XContentFactory.jsonBuilder().map(reduceMap))); + assertThat(Template.mappingsEquals(m1, m2), equalTo(true)); + } + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index a3f60f86a6914..9a060c0ee89ba 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -22,11 +22,8 @@ import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.index.Index; import org.elasticsearch.index.mapper.DataStreamTimestampFieldMapper; @@ -41,7 +38,6 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESSingleNodeTestCase; -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -340,7 +336,11 @@ public void testPutGlobalTemplateWithIndexHiddenSetting() throws Exception { public void testAddComponentTemplate() throws Exception{ MetadataIndexTemplateService metadataIndexTemplateService = getMetadataIndexTemplateService(); ClusterState state = ClusterState.EMPTY_STATE; - Template template = new Template(Settings.builder().build(), null, ComponentTemplateTests.randomAliases()); + Template template = new Template( + Settings.builder().build(), + new CompressedXContent("{\"properties\":{\"@timestamp\":{\"type\":\"date\"}}}"), + ComponentTemplateTests.randomAliases() + ); ComponentTemplate componentTemplate = new ComponentTemplate(template, 1L, new HashMap<>()); state = metadataIndexTemplateService.addComponentTemplate(state, false, "foo", componentTemplate); @@ -1652,55 +1652,8 @@ clusterService, createIndexService, new AliasValidator(), indicesService, new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS), xContentRegistry()); } - @SuppressWarnings("unchecked") public static void assertTemplatesEqual(ComposableIndexTemplate actual, ComposableIndexTemplate expected) { - ComposableIndexTemplate actualNoTemplate = new ComposableIndexTemplate(actual.indexPatterns(), null, - actual.composedOf(), actual.priority(), actual.version(), actual.metadata(), actual.getDataStreamTemplate(), null); - ComposableIndexTemplate expectedNoTemplate = new ComposableIndexTemplate(expected.indexPatterns(), null, - expected.composedOf(), expected.priority(), expected.version(), expected.metadata(), expected.getDataStreamTemplate(), null); - - assertThat(actualNoTemplate, equalTo(expectedNoTemplate)); - Template actualTemplate = actual.template(); - Template expectedTemplate = expected.template(); - - assertThat("expected both templates to have either a template or no template", - Objects.nonNull(actualTemplate), equalTo(Objects.nonNull(expectedTemplate))); - - if (actualTemplate != null) { - assertThat(actualTemplate.settings(), equalTo(expectedTemplate.settings())); - assertThat(actualTemplate.aliases(), equalTo(expectedTemplate.aliases())); - assertThat("expected both templates to have either mappings or no mappings", - Objects.nonNull(actualTemplate.mappings()), equalTo(Objects.nonNull(expectedTemplate.mappings()))); - - if (actualTemplate.mappings() != null) { - Map actualMappings; - Map expectedMappings; - try (XContentParser parser = XContentType.JSON.xContent() - .createParser(new NamedXContentRegistry(Collections.emptyList()), LoggingDeprecationHandler.INSTANCE, - actualTemplate.mappings().string())) { - actualMappings = parser.map(); - } catch (IOException e) { - throw new AssertionError(e); - } - try (XContentParser parser = XContentType.JSON.xContent() - .createParser(new NamedXContentRegistry(Collections.emptyList()), LoggingDeprecationHandler.INSTANCE, - expectedTemplate.mappings().string())) { - expectedMappings = parser.map(); - } catch (IOException e) { - throw new AssertionError(e); - } - - if (actualMappings.size() == 1 && actualMappings.containsKey(MapperService.SINGLE_MAPPING_NAME)) { - actualMappings = (Map) actualMappings.get(MapperService.SINGLE_MAPPING_NAME); - } - - if (expectedMappings.size() == 1 && expectedMappings.containsKey(MapperService.SINGLE_MAPPING_NAME)) { - expectedMappings = (Map) expectedMappings.get(MapperService.SINGLE_MAPPING_NAME); - } - - assertThat(actualMappings, equalTo(expectedMappings)); - } - } + assertTrue(Objects.equals(actual, expected)); } // Composable index template with data_stream definition need _timestamp meta field mapper,