Skip to content

Commit

Permalink
Merge V2 index/component template mappings in specific manner (#55607)
Browse files Browse the repository at this point in the history 
This commit changes the way that V2 index, component, and request mappings are merged. Specifically:

- Fields are merged in a "replacement" manner, meaning that the entire definition is replaced rather
than merging the interior configuration
- Mapping metadata (all fields outside of `properties`) are merged recursively.

The merging for V1 templates does not change.

Relates to #53101
  • Loading branch information
@dakrone
dakrone authored Apr 22, 2020
1 parent bd64da0 commit 4574802
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 21 deletions.
87 changes: 79 additions & 8 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> mappings = Collections.unmodifiableMap(parseMappings(request.mappings(),
final Map<String, Object> mappings = Collections.unmodifiableMap(parseV1Mappings(request.mappings(),
templates.stream().map(IndexTemplateMetadata::getMappings).collect(toList()), xContentRegistry));

final Settings aggregatedIndexSettings =
Expand All @@ -483,8 +483,7 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu
throws Exception {
logger.info("applying create index request using v2 template [{}]", templateName);

final Map<String, Object> mappings = Collections.unmodifiableMap(parseMappings(request.mappings(),
MetadataIndexTemplateService.resolveMappings(currentState, templateName), xContentRegistry));
final Map<String, Object> mappings = resolveV2Mappings(request.mappings(), currentState, templateName, xContentRegistry);

final Settings aggregatedIndexSettings =
aggregateIndexSettings(currentState, request,
Expand All @@ -503,6 +502,15 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu
Collections.singletonList(templateName), metadataTransformer);
}

static Map<String, Object> resolveV2Mappings(final String requestMappings,
final ClusterState currentState,
final String templateName,
final NamedXContentRegistry xContentRegistry) throws Exception {
final Map<String, Object> mappings = Collections.unmodifiableMap(parseV2Mappings(requestMappings,
MetadataIndexTemplateService.resolveMappings(currentState, templateName), xContentRegistry));
return mappings;
}

private ClusterState applyCreateIndexRequestWithExistingMetadata(final ClusterState currentState,
final CreateIndexClusterStateUpdateRequest request,
final boolean silent,
Expand All @@ -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<String, Object> parseV2Mappings(String mappingsJson, List<CompressedXContent> templateMappings,
NamedXContentRegistry xContentRegistry) throws Exception {
Map<String, Object> requestMappings = MapperService.parseMapping(xContentRegistry, mappingsJson);
// apply templates, merging the mappings into the request mapping if exists
Map<String, Object> properties = new HashMap<>();
Map<String, Object> nonProperties = new HashMap<>();
for (CompressedXContent mapping : templateMappings) {
if (mapping != null) {
Map<String, Object> 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<String, Object> innerTemplateMapping = (Map<String, Object>) templateMapping.get(MapperService.SINGLE_MAPPING_NAME);
Map<String, Object> innerTemplateNonProperties = new HashMap<>(innerTemplateMapping);
Map<String, Object> maybeProperties = (Map<String, Object>) innerTemplateNonProperties.remove("properties");

XContentHelper.mergeDefaults(innerTemplateNonProperties, nonProperties);
nonProperties = innerTemplateNonProperties;

if (maybeProperties != null) {
properties.putAll(maybeProperties);
}
}
}

if (requestMappings.get(MapperService.SINGLE_MAPPING_NAME) != null) {
Map<String, Object> innerRequestMappings = (Map<String, Object>) requestMappings.get(MapperService.SINGLE_MAPPING_NAME);
Map<String, Object> innerRequestNonProperties = new HashMap<>(innerRequestMappings);
Map<String, Object> maybeRequestProperties = (Map<String, Object>) innerRequestNonProperties.remove("properties");

XContentHelper.mergeDefaults(innerRequestNonProperties, nonProperties);
nonProperties = innerRequestNonProperties;

if (maybeRequestProperties != null) {
properties.putAll(maybeRequestProperties);
}
}

Map<String, Object> 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<String, Object> parseMappings(String mappingsJson, List<CompressedXContent> templateMappings,
NamedXContentRegistry xContentRegistry) throws Exception {
static Map<String, Object> parseV1Mappings(String mappingsJson, List<CompressedXContent> templateMappings,
NamedXContentRegistry xContentRegistry) throws Exception {
Map<String, Object> mappings = MapperService.parseMapping(xContentRegistry, mappingsJson);
// apply templates, merging the mappings into the request mapping if exists
for (CompressedXContent mapping : templateMappings) {
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -768,8 +768,6 @@ public static List<CompressedXContent> 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);
}

Expand Down
76 changes: 70 additions & 6 deletions server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -622,7 +623,7 @@ public void testParseMappingsAppliesDataFromTemplateAndRequest() throws Exceptio
});
request.mappings(createMapping("mapping_from_request", "text").string());

Map<String, Object> parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(),
Map<String, Object> parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(),
List.of(templateMetadata.getMappings()), NamedXContentRegistry.EMPTY);

assertThat(parsedMappings, hasKey("_doc"));
Expand Down Expand Up @@ -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<String, Object> parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(),
Map<String, Object> parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(),
List.of(templateMetadata.mappings()), xContentRegistry());
List<AliasMetadata> resolvedAliases = resolveAndValidateAliases(request.index(), request.aliases(),
MetadataIndexTemplateService.resolveAliases(List.of(templateMetadata)),
Expand Down Expand Up @@ -848,7 +849,7 @@ public void testParseMappingsWithTypedTemplateAndTypelessIndexMapping() throws E
}
});

Map<String, Object> mappings = parseMappings("{\"_doc\":{}}", List.of(templateMetadata.mappings()), xContentRegistry());
Map<String, Object> mappings = parseV1Mappings("{\"_doc\":{}}", List.of(templateMetadata.mappings()), xContentRegistry());
assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME));
}

Expand All @@ -861,7 +862,7 @@ public void testParseMappingsWithTypedTemplate() throws Exception {
ExceptionsHelper.reThrowIfNotNull(e);
}
});
Map<String, Object> mappings = parseMappings("", List.of(templateMetadata.mappings()), xContentRegistry());
Map<String, Object> mappings = parseV1Mappings("", List.of(templateMetadata.mappings()), xContentRegistry());
assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME));
}

Expand All @@ -873,7 +874,7 @@ public void testParseMappingsWithTypelessTemplate() throws Exception {
ExceptionsHelper.reThrowIfNotNull(e);
}
});
Map<String, Object> mappings = parseMappings("", List.of(templateMetadata.mappings()), xContentRegistry());
Map<String, Object> mappings = parseV1Mappings("", List.of(templateMetadata.mappings()), xContentRegistry());
assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME));
}

Expand Down Expand Up @@ -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<String> 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<String, Object> 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<String, Object> innerResolved = (Map<String, Object>) resolved.get(MapperService.SINGLE_MAPPING_NAME);
assertThat("was: " + innerResolved, innerResolved.size(), equalTo(3));

Map<String, Object> nonProperties = new HashMap<>(innerResolved);
nonProperties.remove("properties");
Map<String, Object> expectedNonProperties = new HashMap<>();
expectedNonProperties.put("_source", Collections.singletonMap("enabled", false));
Map<String, Object> 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<String, Object> innerInnerResolved = (Map<String, Object>) innerResolved.get("properties");
assertThat(innerInnerResolved.size(), equalTo(2));
assertThat(innerInnerResolved.get("bar"), equalTo(Collections.singletonMap("type", "text")));
Map<String, Object> 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<IndexTemplateMetadata.Builder> configurator) {
IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*");
configurator.accept(builder);
Expand Down
10 changes: 5 additions & 5 deletions ...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,16 +696,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 {
Expand Down

0 comments on commit 4574802

Please sign in to comment.