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 19666439dd4..9f04012ff5a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -457,7 +457,7 @@ public class MetadataCreateIndexService { 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) // Converts the ImmutableOpenMap into a non-terrible HashMap .map(iom -> { @@ -494,11 +494,15 @@ public class MetadataCreateIndexService { throws Exception { logger.info("applying create index request using v2 template [{}]", templateName); - final Map> mappings = Collections.unmodifiableMap(parseMappings(request.mappings(), - MetadataIndexTemplateService.resolveMappings(currentState, templateName).stream() - .map(compressedMapping -> Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, compressedMapping)) - .collect(toList()), - xContentRegistry)); + final String sourceMappings; + if (request.mappings().size() > 0) { + assert request.mappings().size() == 1 : "expected request metadata mappings to have 1 type but it had: " + request.mappings(); + sourceMappings = request.mappings().values().iterator().next(); + } else { + sourceMappings = "{}"; + } + final Map> mappings = resolveV2Mappings(sourceMappings, + currentState, templateName, xContentRegistry); final Settings aggregatedIndexSettings = aggregateIndexSettings(currentState, request, @@ -517,6 +521,15 @@ public class MetadataCreateIndexService { 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, @@ -551,14 +564,77 @@ public class MetadataCreateIndexService { } /** - * 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. */ - static Map> parseMappings(Map requestMappings, - List> templateMappings, - NamedXContentRegistry xContentRegistry) throws Exception { + @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> parseV1Mappings(Map requestMappings, + List> templateMappings, + NamedXContentRegistry xContentRegistry) throws Exception { Map> mappings = new HashMap<>(); for (Map.Entry entry : requestMappings.entrySet()) { Map mapping = MapperService.parseMapping(xContentRegistry, entry.getValue()); 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 c004671a8ba..d9186344a39 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -768,8 +768,6 @@ public class MetadataIndexTemplateService { 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 3229241187d..d720cdf8480 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -104,7 +104,7 @@ import static org.elasticsearch.cluster.metadata.MetadataCreateIndexService.aggr 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; @@ -647,7 +647,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { }); request.mappings(singletonMap("type", createMapping("mapping_from_request", "text").string())); - Map> parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(), + Map> parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(), Collections.singletonList(convertMappings(templateMetadata.getMappings())), NamedXContentRegistry.EMPTY); assertThat(parsedMappings, hasKey("type")); @@ -705,7 +705,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { request.aliases(singleton(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(), Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry()); List resolvedAliases = resolveAndValidateAliases(request.index(), request.aliases(), MetadataIndexTemplateService.resolveAliases(Collections.singletonList(templateMetadata)), @@ -878,7 +878,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { } }); - Map> mappings = parseMappings(singletonMap(MapperService.SINGLE_MAPPING_NAME, "{\"_doc\":{}}"), + Map> mappings = parseV1Mappings(singletonMap(MapperService.SINGLE_MAPPING_NAME, "{\"_doc\":{}}"), Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry()); assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME)); } @@ -892,7 +892,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { ExceptionsHelper.reThrowIfNotNull(e); } }); - Map> mappings = parseMappings(emptyMap(), + Map> mappings = parseV1Mappings(emptyMap(), Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry()); assertThat(mappings, Matchers.hasKey("type")); } @@ -905,7 +905,7 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { ExceptionsHelper.reThrowIfNotNull(e); } }); - Map> mappings = parseMappings(emptyMap(), + Map> mappings = parseV1Mappings(emptyMap(), Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry()); assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME)); } @@ -1001,6 +1001,69 @@ public class MetadataCreateIndexServiceTests extends ESTestCase { + "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 7ccf9f98fd4..f073d79616e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -699,19 +699,19 @@ public class MetadataIndexTemplateServiceTests extends ESSingleNodeTestCase { .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(Collections.singletonMap("_doc", Collections.singletonMap("properties", - Collections.singletonMap("field", Collections.singletonMap("type", "keyword")))))); + Collections.singletonMap("field2", Collections.singletonMap("type", "text")))))); assertThat(parsedMappings.get(1), equalTo(Collections.singletonMap("_doc", Collections.singletonMap("properties", Collections.singletonMap("field2", Collections.singletonMap("type", "keyword")))))); assertThat(parsedMappings.get(2), equalTo(Collections.singletonMap("_doc", Collections.singletonMap("properties", - Collections.singletonMap("field2", Collections.singletonMap("type", "text")))))); + Collections.singletonMap("field", Collections.singletonMap("type", "keyword")))))); } public void testResolveSettings() throws Exception {