From 89b538f857b14b109c1daeec93619e3855f9df54 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 30 Apr 2020 09:12:24 -0600 Subject: [PATCH] Handle merging dotted object names when merging V2 template mappings (#55982) When merging component template, index template, and request mappings, we now treat any declaration of a top-level field the same as replacing all sub-objects. For example, assuming two component templates with mappings and template B taking precedence: ``` A: {foo: {...}} B: {foo.bar: {...}} Result: {foo.bar: {...}} A: {foo.bar: {...}} B: {foo: {...}} Result: {foo: {...}} A: {foo.bar: {...}} B: {foo.baz: {...}} Result: {foo.baz: {...}} ``` Relates to #53101 --- .../metadata/MetadataCreateIndexService.java | 26 +++++++- .../MetadataCreateIndexServiceTests.java | 64 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java index 1a587facda6a1..c2c9b9b8640f2 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -85,6 +85,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -579,7 +580,7 @@ static Map parseV2Mappings(String mappingsJson, List parseV2Mappings(String mappingsJson, List parseV2Mappings(String mappingsJson, List mergeIgnoringDots(Map first, Map second) { + Objects.requireNonNull(first, "merging requires two non-null maps but the first map was null"); + Objects.requireNonNull(second, "merging requires two non-null maps but the second map was null"); + Map results = new HashMap<>(first); + Set prefixes = second.keySet().stream().map(MetadataCreateIndexService::prefix).collect(Collectors.toSet()); + results.keySet().removeIf(k -> prefixes.contains(prefix(k))); + results.putAll(second); + return results; + } + + private static String prefix(String s) { + return s.split("\\.", 2)[0]; + } + /** * Parses the provided mappings json and the inheritable mappings from the templates (if any) * into a map. diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 3306ca95599e1..a3e17f412297e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1047,6 +1047,70 @@ public void testMappingsMergingIsSmart() throws Exception { assertThat(innerInnerResolved.get("foo"), equalTo(fooMappings)); } + @SuppressWarnings("unchecked") + public void testMappingsMergingHandlesDots() throws Exception { + Template ctt1 = new Template(null, + new CompressedXContent("{\"_doc\":{\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"type\": \"long\"}}}}}}"), null); + Template ctt2 = new Template(null, + new CompressedXContent("{\"_doc\":{\"properties\":{\"foo.bar\":{\"type\": \"text\",\"analyzer\":\"english\"}}}}"), null); + + ComponentTemplate ct1 = new ComponentTemplate(ctt1, null, null); + ComponentTemplate ct2 = new ComponentTemplate(ctt2, null, null); + + IndexTemplateV2 template = new IndexTemplateV2(Collections.singletonList("index"), null, Arrays.asList("ct2", "ct1"), + null, null, null); + + ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE) + .metadata(Metadata.builder(Metadata.EMPTY_METADATA) + .put("ct1", ct1) + .put("ct2", ct2) + .put("index-template", template) + .build()) + .build(); + + Map resolved = + MetadataCreateIndexService.resolveV2Mappings("{}", state, + "index-template", new NamedXContentRegistry(Collections.emptyList())); + + assertThat("expected exactly one type but was: " + resolved, resolved.size(), equalTo(1)); + Map innerResolved = (Map) resolved.get(MapperService.SINGLE_MAPPING_NAME); + assertThat("was: " + innerResolved, innerResolved.size(), equalTo(1)); + + Map innerInnerResolved = (Map) innerResolved.get("properties"); + assertThat(innerInnerResolved.size(), equalTo(1)); + assertThat(innerInnerResolved.get("foo"), + equalTo(Collections.singletonMap("properties", Collections.singletonMap("bar", Collections.singletonMap("type", "long"))))); + } + + public void testMergeIgnoringDots() throws Exception { + Map first = new HashMap<>(); + first.put("foo", Collections.singletonMap("type", "long")); + Map second = new HashMap<>(); + second.put("foo.bar", Collections.singletonMap("type", "long")); + Map results = MetadataCreateIndexService.mergeIgnoringDots(first, second); + assertThat(results, equalTo(second)); + + results = MetadataCreateIndexService.mergeIgnoringDots(second, first); + assertThat(results, equalTo(first)); + + second.clear(); + Map inner = new HashMap<>(); + inner.put("type", "text"); + inner.put("analyzer", "english"); + second.put("foo", inner); + + results = MetadataCreateIndexService.mergeIgnoringDots(first, second); + assertThat(results, equalTo(second)); + + first.put("baz", 3); + second.put("egg", 7); + + results = MetadataCreateIndexService.mergeIgnoringDots(first, second); + Map expected = new HashMap<>(second); + expected.put("baz", 3); + assertThat(results, equalTo(expected)); + } + private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder);