Skip to content

Commit

Permalink
[7.x] Merge V2 index/component template mappings in specific manner (#…
Browse files Browse the repository at this point in the history 
…55607) (#55619)

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 25ea6a7 commit 86129fb
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 25 deletions.
100 changes: 88 additions & 12 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 @@ -457,7 +457,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, Map<String, Object>> mappings = Collections.unmodifiableMap(parseMappings(request.mappings(),
final Map<String, Map<String, Object>> mappings = Collections.unmodifiableMap(parseV1Mappings(request.mappings(),
templates.stream().map(IndexTemplateMetadata::getMappings)
// Converts the ImmutableOpenMap into a non-terrible HashMap
.map(iom -> {
Expand Down Expand Up @@ -494,11 +494,15 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu
throws Exception {
logger.info("applying create index request using v2 template [{}]", templateName);

final Map<String, Map<String, Object>> 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<String, Map<String, Object>> mappings = resolveV2Mappings(sourceMappings,
currentState, templateName, xContentRegistry);

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

static Map<String, Map<String, Object>> resolveV2Mappings(final String requestMappings,
final ClusterState currentState,
final String templateName,
final NamedXContentRegistry xContentRegistry) throws Exception {
final Map<String, 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 Down Expand Up @@ -551,14 +564,77 @@ 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, 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, Map<String, Object>> parseMappings(Map<String, String> requestMappings,
List<Map<String, CompressedXContent>> templateMappings,
NamedXContentRegistry xContentRegistry) throws Exception {
static Map<String, Map<String, Object>> parseV1Mappings(Map<String, String> requestMappings,
List<Map<String, CompressedXContent>> templateMappings,
NamedXContentRegistry xContentRegistry) throws Exception {
Map<String, Map<String, Object>> mappings = new HashMap<>();
for (Map.Entry<String, String> entry : requestMappings.entrySet()) {
Map<String, Object> mapping = MapperService.parseMapping(xContentRegistry, entry.getValue());
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
75 changes: 69 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 @@ -104,7 +104,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 @@ -647,7 +647,7 @@ public void testParseMappingsAppliesDataFromTemplateAndRequest() throws Exceptio
});
request.mappings(singletonMap("type", createMapping("mapping_from_request", "text").string()));

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

assertThat(parsedMappings, hasKey("type"));
Expand Down Expand Up @@ -705,7 +705,7 @@ public void testRequestDataHavePriorityOverTemplateData() throws Exception {
request.aliases(singleton(new Alias("alias").searchRouting("fromRequest")));
request.settings(Settings.builder().put("key1", "requestValue").build());

Map<String, Map<String, Object>> parsedMappings = MetadataCreateIndexService.parseMappings(request.mappings(),
Map<String, Map<String, Object>> parsedMappings = MetadataCreateIndexService.parseV1Mappings(request.mappings(),
Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry());
List<AliasMetadata> resolvedAliases = resolveAndValidateAliases(request.index(), request.aliases(),
MetadataIndexTemplateService.resolveAliases(Collections.singletonList(templateMetadata)),
Expand Down Expand Up @@ -878,7 +878,7 @@ public void testParseMappingsWithTypedTemplateAndTypelessIndexMapping() throws E
}
});

Map<String, Map<String, Object>> mappings = parseMappings(singletonMap(MapperService.SINGLE_MAPPING_NAME, "{\"_doc\":{}}"),
Map<String, Map<String, Object>> mappings = parseV1Mappings(singletonMap(MapperService.SINGLE_MAPPING_NAME, "{\"_doc\":{}}"),
Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry());
assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME));
}
Expand All @@ -892,7 +892,7 @@ public void testParseMappingsWithTypedTemplate() throws Exception {
ExceptionsHelper.reThrowIfNotNull(e);
}
});
Map<String, Map<String, Object>> mappings = parseMappings(emptyMap(),
Map<String, Map<String, Object>> mappings = parseV1Mappings(emptyMap(),
Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry());
assertThat(mappings, Matchers.hasKey("type"));
}
Expand All @@ -905,7 +905,7 @@ public void testParseMappingsWithTypelessTemplate() throws Exception {
ExceptionsHelper.reThrowIfNotNull(e);
}
});
Map<String, Map<String, Object>> mappings = parseMappings(emptyMap(),
Map<String, Map<String, Object>> mappings = parseV1Mappings(emptyMap(),
Collections.singletonList(convertMappings(templateMetadata.mappings())), xContentRegistry());
assertThat(mappings, Matchers.hasKey(MapperService.SINGLE_MAPPING_NAME));
}
Expand Down Expand Up @@ -1001,6 +1001,69 @@ public void testValidateTranslogRetentionSettings() {
+ "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, 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 @@ -699,19 +699,19 @@ 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(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 {
Expand Down

0 comments on commit 86129fb

Please sign in to comment.