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 d059ab537066e..4661cdb43602f 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -456,7 +456,7 @@ private ClusterState applyCreateIndexRequestWithV1Templates(final ClusterState c logger.info("applying create index request using v1 templates {}", templates.stream().map(IndexTemplateMetadata::name).collect(Collectors.toList())); - final Map mappings = Collections.unmodifiableMap(parseMappings(request.mappings(), + final Map mappings = Collections.unmodifiableMap(parseV1Mappings(request.mappings(), templates.stream().map(IndexTemplateMetadata::getMappings).collect(toList()), xContentRegistry)); final Settings aggregatedIndexSettings = @@ -483,8 +483,7 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu throws Exception { logger.info("applying create index request using v2 template [{}]", templateName); - final Map mappings = Collections.unmodifiableMap(parseMappings(request.mappings(), - MetadataIndexTemplateService.resolveMappings(currentState, templateName), xContentRegistry)); + final Map mappings = resolveV2Mappings(request.mappings(), currentState, templateName, xContentRegistry); final Settings aggregatedIndexSettings = aggregateIndexSettings(currentState, request, @@ -503,6 +502,15 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu Collections.singletonList(templateName), metadataTransformer); } + static Map resolveV2Mappings(final String requestMappings, + final ClusterState currentState, + final String templateName, + final NamedXContentRegistry xContentRegistry) throws Exception { + final Map mappings = Collections.unmodifiableMap(parseV2Mappings(requestMappings, + MetadataIndexTemplateService.resolveMappings(currentState, templateName), xContentRegistry)); + return mappings; + } + private ClusterState applyCreateIndexRequestWithExistingMetadata(final ClusterState currentState, final CreateIndexClusterStateUpdateRequest request, final boolean silent, @@ -529,13 +537,76 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(final ClusterSt } /** - * Parses the provided mappings json and the inheritable mappings from the templates (if any) into a map. + * Parses the provided mappings json and the inheritable mappings from the templates (if any) + * into a map. * - * The template mappings are applied in the order they are encountered in the list (clients should make sure the lower index, closer - * to the head of the list, templates have the highest {@link IndexTemplateMetadata#order()}) + * The template mappings are applied in the order they are encountered in the list, with the + * caveat that mapping fields are only merged at the top-level, meaning that field settings are + * not merged, instead they replace any previous field definition. + */ + @SuppressWarnings("unchecked") + static Map parseV2Mappings(String mappingsJson, List templateMappings, + NamedXContentRegistry xContentRegistry) throws Exception { + Map requestMappings = MapperService.parseMapping(xContentRegistry, mappingsJson); + // apply templates, merging the mappings into the request mapping if exists + Map properties = new HashMap<>(); + Map nonProperties = new HashMap<>(); + for (CompressedXContent mapping : templateMappings) { + if (mapping != null) { + Map templateMapping = MapperService.parseMapping(xContentRegistry, mapping.string()); + if (templateMapping.isEmpty()) { + // Someone provided an empty '{}' for mappings, which is okay, but to avoid + // tripping the below assertion, we can safely ignore it + continue; + } + assert templateMapping.size() == 1 : "expected exactly one mapping value, got: " + templateMapping; + if (templateMapping.get(MapperService.SINGLE_MAPPING_NAME) instanceof Map == false) { + throw new IllegalStateException("invalid mapping definition, expected a single map underneath [" + + MapperService.SINGLE_MAPPING_NAME + "] but it was: [" + templateMapping + "]"); + } + + Map innerTemplateMapping = (Map) templateMapping.get(MapperService.SINGLE_MAPPING_NAME); + Map innerTemplateNonProperties = new HashMap<>(innerTemplateMapping); + Map maybeProperties = (Map) innerTemplateNonProperties.remove("properties"); + + XContentHelper.mergeDefaults(innerTemplateNonProperties, nonProperties); + nonProperties = innerTemplateNonProperties; + + if (maybeProperties != null) { + properties.putAll(maybeProperties); + } + } + } + + if (requestMappings.get(MapperService.SINGLE_MAPPING_NAME) != null) { + Map innerRequestMappings = (Map) requestMappings.get(MapperService.SINGLE_MAPPING_NAME); + Map innerRequestNonProperties = new HashMap<>(innerRequestMappings); + Map maybeRequestProperties = (Map) innerRequestNonProperties.remove("properties"); + + XContentHelper.mergeDefaults(innerRequestNonProperties, nonProperties); + nonProperties = innerRequestNonProperties; + + if (maybeRequestProperties != null) { + properties.putAll(maybeRequestProperties); + } + } + + Map finalMappings = new HashMap<>(nonProperties); + finalMappings.put("properties", properties); + return Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, finalMappings); + } + + /** + * Parses the provided mappings json and the inheritable mappings from the templates (if any) + * into a map. + * + * The template mappings are applied in the order they are encountered in the list (clients + * should make sure the lower index, closer to the head of the list, templates have the highest + * {@link IndexTemplateMetadata#order()}). This merging makes no distinction between field + * definitions, as may result in an invalid field definition */ - static Map parseMappings(String mappingsJson, List templateMappings, - NamedXContentRegistry xContentRegistry) throws Exception { + static Map parseV1Mappings(String mappingsJson, List templateMappings, + NamedXContentRegistry xContentRegistry) throws Exception { Map mappings = MapperService.parseMapping(xContentRegistry, mappingsJson); // apply templates, merging the mappings into the request mapping if exists for (CompressedXContent mapping : templateMappings) { diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index d8194b171b1b4..fa6efb0d5b9da 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -767,8 +767,6 @@ public static List resolveMappings(final ClusterState state, Optional.ofNullable(template.template()) .map(Template::mappings) .ifPresent(mappings::add); - // When actually merging mappings, the highest precedence ones should go first, so reverse the list - Collections.reverse(mappings); return Collections.unmodifiableList(mappings); } 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 b4b4b6009c6c6..3306ca95599e1 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -73,6 +73,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -98,7 +99,7 @@ import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.buildIndexMetadata; import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.clusterStateCreateIndex; import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.getIndexNumberOfRoutingShards; -import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.parseMappings; +import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.parseV1Mappings; import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.resolveAndValidateAliases; import static org.elasticsearch.cluster.shards.ClusterShardLimitIT.ShardCounts.forDataNodeCount; import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; @@ -622,7 +623,7 @@ public void testParseMappingsAppliesDataFromTemplateAndRequest() throws Exceptio }); request.mappings(createMapping("mapping_from_request", "text").string()); - Map parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(), + Map parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(), List.of(templateMetadata.getMappings()), NamedXContentRegistry.EMPTY); assertThat(parsedMappings, hasKey("_doc")); @@ -678,7 +679,7 @@ public void testRequestDataHavePriorityOverTemplateData() throws Exception { request.aliases(Set.of(new Alias("alias").searchRouting("fromRequest"))); request.settings(Settings.builder().put("key1", "requestValue").build()); - Map parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(), + Map parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(), List.of(templateMetadata.mappings()), xContentRegistry()); List resolvedAliases = resolveAndValidateAliases(request.index(), request.aliases(), MetadataIndexTemplateService.resolveAliases(List.of(templateMetadata)), @@ -848,7 +849,7 @@ public void testParseMappingsWithTypedTemplateAndTypelessIndexMapping() throws E } }); - Map mappings = parseMappings("{\"_doc\":{}}", List.of(templateMetadata.mappings()), xContentRegistry()); + Map mappings = parseV1Mappings("{\"_doc\":{}}", List.of(templateMetadata.mappings()), xContentRegistry()); assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME)); } @@ -861,7 +862,7 @@ public void testParseMappingsWithTypedTemplate() throws Exception { ExceptionsHelper.reThrowIfNotNull(e); } }); - Map mappings = parseMappings("", List.of(templateMetadata.mappings()), xContentRegistry()); + Map mappings = parseV1Mappings("", List.of(templateMetadata.mappings()), xContentRegistry()); assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME)); } @@ -873,7 +874,7 @@ public void testParseMappingsWithTypelessTemplate() throws Exception { ExceptionsHelper.reThrowIfNotNull(e); } }); - Map mappings = parseMappings("", List.of(templateMetadata.mappings()), xContentRegistry()); + Map mappings = parseV1Mappings("", List.of(templateMetadata.mappings()), xContentRegistry()); assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME)); } @@ -983,6 +984,69 @@ public void testDeprecateTranslogRetentionSettings() { + "and [index.translog.retention.size] are deprecated and effectively ignored. They will be removed in a future version."); } + @SuppressWarnings("unchecked") + public void testMappingsMergingIsSmart() throws Exception { + Template ctt1 = new Template(null, + new CompressedXContent("{\"_doc\":{\"_source\":{\"enabled\": false},\"_meta\":{\"ct1\":{\"ver\": \"text\"}}," + + "\"properties\":{\"foo\":{\"type\":\"text\",\"ignore_above\":7,\"analyzer\":\"english\"}}}}"), null); + Template ctt2 = new Template(null, + new CompressedXContent("{\"_doc\":{\"_meta\":{\"ct1\":{\"ver\": \"keyword\"},\"ct2\":\"potato\"}," + + "\"properties\":{\"foo\":{\"type\":\"keyword\",\"ignore_above\":13}}}}"), null); + + ComponentTemplate ct1 = new ComponentTemplate(ctt1, null, null); + ComponentTemplate ct2 = new ComponentTemplate(ctt2, null, null); + + boolean shouldBeText = randomBoolean(); + List composedOf = shouldBeText ? Arrays.asList("ct2", "ct1") : Arrays.asList("ct1", "ct2"); + logger.info("--> the {} analyzer should win ({})", shouldBeText ? "text" : "keyword", composedOf); + IndexTemplateV2 template = new IndexTemplateV2(Collections.singletonList("index"), null, composedOf, 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("{\"_doc\":{\"_meta\":{\"ct2\":\"eggplant\"}," + + "\"properties\":{\"bar\":{\"type\":\"text\"}}}}", 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(3)); + + Map nonProperties = new HashMap<>(innerResolved); + nonProperties.remove("properties"); + Map expectedNonProperties = new HashMap<>(); + expectedNonProperties.put("_source", Collections.singletonMap("enabled", false)); + Map meta = new HashMap<>(); + meta.put("ct2", "eggplant"); + if (shouldBeText) { + meta.put("ct1", Collections.singletonMap("ver", "text")); + } else { + meta.put("ct1", Collections.singletonMap("ver", "keyword")); + } + expectedNonProperties.put("_meta", meta); + assertThat(nonProperties, equalTo(expectedNonProperties)); + + Map innerInnerResolved = (Map) innerResolved.get("properties"); + assertThat(innerInnerResolved.size(), equalTo(2)); + assertThat(innerInnerResolved.get("bar"), equalTo(Collections.singletonMap("type", "text"))); + Map fooMappings = new HashMap<>(); + if (shouldBeText) { + fooMappings.put("type", "text"); + fooMappings.put("ignore_above", 7); + fooMappings.put("analyzer", "english"); + } else { + fooMappings.put("type", "keyword"); + fooMappings.put("ignore_above", 13); + } + assertThat(innerInnerResolved.get("foo"), equalTo(fooMappings)); + } + private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder); 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 00eb39546aeed..28c1912bb364d 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -677,16 +677,16 @@ public void testResolveMappings() throws Exception { .collect(Collectors.toList()); // The order of mappings should be: - // - index template - // - ct_high // - ct_low - // Because the first elements when merging mappings have the highest precedence + // - ct_high + // - index template + // Because the first elements when merging mappings have the lowest precedence assertThat(parsedMappings.get(0), - equalTo(Map.of("_doc", Map.of("properties", Map.of("field", Map.of("type", "keyword")))))); + equalTo(Map.of("_doc", Map.of("properties", Map.of("field2", Map.of("type", "text")))))); assertThat(parsedMappings.get(1), equalTo(Map.of("_doc", Map.of("properties", Map.of("field2", Map.of("type", "keyword")))))); assertThat(parsedMappings.get(2), - equalTo(Map.of("_doc", Map.of("properties", Map.of("field2", Map.of("type", "text")))))); + equalTo(Map.of("_doc", Map.of("properties", Map.of("field", Map.of("type", "keyword")))))); } public void testResolveSettings() throws Exception {