From fbe637048854d6e7ca70629268f45944aa2991a9 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 6 Feb 2019 08:21:30 +0100 Subject: [PATCH] Update IndexTemplateMetaData to allow unknown fields (#38448) Updated IndexTemplateMetaData to use ObjectParser. The IndexTemplateMetaData class used old parsing logic and was not resiliant to new fields. This commit updates it to use the ConstructingObjectParser and allow unknown fields. Relates #36938 Co-authored-by: Michael Basnight Co-authored-by: Martijn van Groningen --- .../client/indices/IndexTemplateMetaData.java | 142 ++++++--------- .../GetIndexTemplatesResponseTests.java | 162 ++++++++++++++++-- 2 files changed, 197 insertions(+), 107 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/IndexTemplateMetaData.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/IndexTemplateMetaData.java index 12fc747ab3473..51e5e3297f823 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/IndexTemplateMetaData.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/indices/IndexTemplateMetaData.java @@ -18,27 +18,66 @@ */ package org.elasticsearch.client.indices; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MapperService; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; +import java.util.AbstractMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; public class IndexTemplateMetaData { + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "IndexTemplateMetaData", true, (a, name) -> { + List> alias = (List>) a[5]; + ImmutableOpenMap aliasMap = + new ImmutableOpenMap.Builder() + .putAll(alias.stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))) + .build(); + return new IndexTemplateMetaData( + name, + (Integer) a[0], + (Integer) a[1], + (List) a[2], + (Settings) a[3], + (MappingMetaData) a[4], + aliasMap); + }); + + static { + PARSER.declareInt(optionalConstructorArg(), new ParseField("order")); + PARSER.declareInt(optionalConstructorArg(), new ParseField("version")); + PARSER.declareStringArray(optionalConstructorArg(), new ParseField("index_patterns")); + PARSER.declareObject(optionalConstructorArg(), (p, c) -> { + Settings.Builder templateSettingsBuilder = Settings.builder(); + templateSettingsBuilder.put(Settings.fromXContent(p)); + templateSettingsBuilder.normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX); + return templateSettingsBuilder.build(); + }, new ParseField("settings")); + PARSER.declareObject(optionalConstructorArg(), (p, c) -> { + Map mapping = p.map(); + if (mapping.isEmpty()) { + return null; + } + return new MappingMetaData(MapperService.SINGLE_MAPPING_NAME, mapping); + }, new ParseField("mappings")); + PARSER.declareNamedObjects(optionalConstructorArg(), + (p, c, name) -> new AbstractMap.SimpleEntry<>(name, AliasMetaData.Builder.fromXContent(p)), new ParseField("aliases")); + } private final String name; @@ -125,28 +164,23 @@ public static Builder builder(String name) { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - IndexTemplateMetaData that = (IndexTemplateMetaData) o; - - if (order != that.order) return false; - if (!Objects.equals(mappings, that.mappings)) return false; - if (!name.equals(that.name)) return false; - if (!settings.equals(that.settings)) return false; - if (!patterns.equals(that.patterns)) return false; - - return Objects.equals(version, that.version); + return order == that.order && + Objects.equals(name, that.name) && + Objects.equals(version, that.version) && + Objects.equals(patterns, that.patterns) && + Objects.equals(settings, that.settings) && + Objects.equals(mappings, that.mappings) && + Objects.equals(aliases, that.aliases); } @Override public int hashCode() { - return Objects.hash(name, order, version, patterns, settings, mappings); + return Objects.hash(name, order, version, patterns, settings, mappings, aliases); } public static class Builder { - private static final Set VALID_FIELDS = Sets.newHashSet( - "template", "order", "mappings", "settings", "index_patterns", "aliases", "version"); - private String name; private int order; @@ -193,7 +227,6 @@ public Builder patterns(List indexPatterns) { return this; } - public Builder settings(Settings.Builder settings) { this.settings = settings.build(); return this; @@ -225,76 +258,7 @@ public IndexTemplateMetaData build() { public static IndexTemplateMetaData fromXContent(XContentParser parser, String templateName) throws IOException { - Builder builder = new Builder(templateName); - - String currentFieldName = skipTemplateName(parser); - XContentParser.Token token; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - currentFieldName = parser.currentName(); - } else if (token == XContentParser.Token.START_OBJECT) { - if ("settings".equals(currentFieldName)) { - Settings.Builder templateSettingsBuilder = Settings.builder(); - templateSettingsBuilder.put(Settings.fromXContent(parser)); - templateSettingsBuilder.normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX); - builder.settings(templateSettingsBuilder.build()); - } else if ("mappings".equals(currentFieldName)) { - Map mapping = parser.map(); - if (mapping.isEmpty() == false) { - MappingMetaData md = new MappingMetaData(MapperService.SINGLE_MAPPING_NAME, mapping); - builder.mapping(md); - } - } else if ("aliases".equals(currentFieldName)) { - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - builder.putAlias(AliasMetaData.Builder.fromXContent(parser)); - } - } else { - throw new ElasticsearchParseException("unknown key [{}] for index template", currentFieldName); - } - } else if (token == XContentParser.Token.START_ARRAY) { - if ("mappings".equals(currentFieldName)) { - // The server-side IndexTemplateMetaData has toXContent impl that can return mappings - // in an array but also a comment saying this never happens with typeless APIs. - throw new ElasticsearchParseException("Invalid response format - " - + "mappings are not expected to be returned in an array", currentFieldName); - } else if ("index_patterns".equals(currentFieldName)) { - List index_patterns = new ArrayList<>(); - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - index_patterns.add(parser.text()); - } - builder.patterns(index_patterns); - } - } else if (token.isValue()) { - // Prior to 5.1.0, elasticsearch only supported a single index pattern called `template` (#21009) - if("template".equals(currentFieldName)) { - builder.patterns(Collections.singletonList(parser.text())); - } else if ("order".equals(currentFieldName)) { - builder.order(parser.intValue()); - } else if ("version".equals(currentFieldName)) { - builder.version(parser.intValue()); - } - } - } - return builder.build(); - } - - private static String skipTemplateName(XContentParser parser) throws IOException { - XContentParser.Token token = parser.nextToken(); - if (token == XContentParser.Token.START_OBJECT) { - token = parser.nextToken(); - if (token == XContentParser.Token.FIELD_NAME) { - String currentFieldName = parser.currentName(); - if (VALID_FIELDS.contains(currentFieldName)) { - return currentFieldName; - } else { - // we just hit the template name, which should be ignored and we move on - parser.nextToken(); - } - } - } - - return null; + return PARSER.parse(parser, templateName); } } - } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/GetIndexTemplatesResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/GetIndexTemplatesResponseTests.java index d6bca30085165..6a1a2e89f59de 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/GetIndexTemplatesResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indices/GetIndexTemplatesResponseTests.java @@ -22,9 +22,16 @@ import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.ToXContent.MapParams; import org.elasticsearch.index.mapper.MapperService; @@ -34,45 +41,129 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static org.elasticsearch.index.RandomCreateIndexGenerator.randomIndexSettings; +import static org.elasticsearch.index.RandomCreateIndexGenerator.randomMappingFields; import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester; +import static org.hamcrest.Matchers.equalTo; public class GetIndexTemplatesResponseTests extends ESTestCase { - + static final String mappingString = "{\"properties\":{" + "\"f1\": {\"type\":\"text\"}," + "\"f2\": {\"type\":\"keyword\"}" + "}}"; - + public void testFromXContent() throws IOException { - xContentTester(this::createParser, GetIndexTemplatesResponseTests::createTestInstance, GetIndexTemplatesResponseTests::toXContent, - GetIndexTemplatesResponse::fromXContent).supportsUnknownFields(false) - .assertEqualsConsumer(GetIndexTemplatesResponseTests::assertEqualInstances) - .shuffleFieldsExceptions(new String[] {"aliases", "mappings", "patterns", "settings"}) - .test(); + xContentTester(this::createParser, + GetIndexTemplatesResponseTests::createTestInstance, + GetIndexTemplatesResponseTests::toXContent, + GetIndexTemplatesResponse::fromXContent) + .assertEqualsConsumer(GetIndexTemplatesResponseTests::assertEqualInstances) + .supportsUnknownFields(true) + .randomFieldsExcludeFilter(randomFieldsExcludeFilter()) + .shuffleFieldsExceptions(new String[] {"aliases", "mappings", "patterns", "settings"}) + .test(); + } + + public void testParsingFromEsResponse() throws IOException { + for (int runs = 0; runs < 20; runs++) { + org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse esResponse = + new org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse(new ArrayList<>()); + + XContentType xContentType = randomFrom(XContentType.values()); + int numTemplates = randomIntBetween(0, 32); + for (int i = 0; i < numTemplates; i++) { + org.elasticsearch.cluster.metadata.IndexTemplateMetaData.Builder esIMD = + new org.elasticsearch.cluster.metadata.IndexTemplateMetaData.Builder(String.format(Locale.ROOT, "%02d ", i) + + randomAlphaOfLength(4)); + esIMD.patterns(Arrays.asList(generateRandomStringArray(32, 4, false, false))); + esIMD.settings(randomIndexSettings()); + esIMD.putMapping("_doc", new CompressedXContent(BytesReference.bytes(randomMapping("_doc", xContentType)))); + int numAliases = randomIntBetween(0, 8); + for (int j = 0; j < numAliases; j++) { + esIMD.putAlias(randomAliasMetaData(String.format(Locale.ROOT, "%02d ", j) + randomAlphaOfLength(4))); + } + esIMD.order(randomIntBetween(0, Integer.MAX_VALUE)); + esIMD.version(randomIntBetween(0, Integer.MAX_VALUE)); + esResponse.getIndexTemplates().add(esIMD.build()); + } + + XContentBuilder xContentBuilder = XContentBuilder.builder(xContentType.xContent()); + esResponse.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS); + + try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, BytesReference.bytes(xContentBuilder), xContentType)) { + GetIndexTemplatesResponse response = GetIndexTemplatesResponse.fromXContent(parser); + assertThat(response.getIndexTemplates().size(), equalTo(numTemplates)); + + response.getIndexTemplates().sort(Comparator.comparing(IndexTemplateMetaData::name)); + for (int i = 0; i < numTemplates; i++) { + org.elasticsearch.cluster.metadata.IndexTemplateMetaData esIMD = esResponse.getIndexTemplates().get(i); + IndexTemplateMetaData result = response.getIndexTemplates().get(i); + + assertThat(result.patterns(), equalTo(esIMD.patterns())); + assertThat(result.settings(), equalTo(esIMD.settings())); + assertThat(result.order(), equalTo(esIMD.order())); + assertThat(result.version(), equalTo(esIMD.version())); + + assertThat(esIMD.mappings().size(), equalTo(1)); + BytesArray mappingSource = new BytesArray(esIMD.mappings().valuesIt().next().uncompressed()); + Map expectedMapping = + XContentHelper.convertToMap(mappingSource, true, xContentBuilder.contentType()).v2(); + assertThat(result.mappings().sourceAsMap(), equalTo(expectedMapping.get("_doc"))); + + assertThat(result.aliases().size(), equalTo(esIMD.aliases().size())); + List expectedAliases = Arrays.stream(esIMD.aliases().values().toArray(AliasMetaData.class)) + .sorted(Comparator.comparing(AliasMetaData::alias)) + .collect(Collectors.toList()); + List actualAliases = Arrays.stream(result.aliases().values().toArray(AliasMetaData.class)) + .sorted(Comparator.comparing(AliasMetaData::alias)) + .collect(Collectors.toList()); + for (int j = 0; j < result.aliases().size(); j++) { + assertThat(actualAliases.get(j), equalTo(expectedAliases.get(j))); + } + } + } + } + } + + private Predicate randomFieldsExcludeFilter() { + return (field) -> + field.isEmpty() + || field.endsWith("aliases") + || field.endsWith("settings") + || field.endsWith("settings.index") + || field.endsWith("mappings") // uses parser.map() + || field.contains("mappings.properties") // cannot have extra properties + ; } - private static void assertEqualInstances(GetIndexTemplatesResponse expectedInstance, GetIndexTemplatesResponse newInstance) { + private static void assertEqualInstances(GetIndexTemplatesResponse expectedInstance, GetIndexTemplatesResponse newInstance) { assertEquals(expectedInstance, newInstance); // Check there's no doc types at the root of the mapping Map expectedMap = XContentHelper.convertToMap( new BytesArray(mappingString), true, XContentType.JSON).v2(); for (IndexTemplateMetaData template : newInstance.getIndexTemplates()) { MappingMetaData mappingMD = template.mappings(); - if(mappingMD!=null) { + if(mappingMD != null) { Map mappingAsMap = mappingMD.sourceAsMap(); assertEquals(expectedMap, mappingAsMap); - } + } } - } - + } + static GetIndexTemplatesResponse createTestInstance() { List templates = new ArrayList<>(); int numTemplates = between(0, 10); @@ -108,20 +199,20 @@ static GetIndexTemplatesResponse createTestInstance() { // As the client class GetIndexTemplatesResponse doesn't have toXContent method, adding this method here only for the test static void toXContent(GetIndexTemplatesResponse response, XContentBuilder builder) throws IOException { - + //Create a server-side counterpart for the client-side class and call toXContent on it - + List serverIndexTemplates = new ArrayList<>(); List clientIndexTemplates = response.getIndexTemplates(); for (IndexTemplateMetaData clientITMD : clientIndexTemplates) { - org.elasticsearch.cluster.metadata.IndexTemplateMetaData.Builder serverTemplateBuilder = + org.elasticsearch.cluster.metadata.IndexTemplateMetaData.Builder serverTemplateBuilder = org.elasticsearch.cluster.metadata.IndexTemplateMetaData.builder(clientITMD.name()); serverTemplateBuilder.patterns(clientITMD.patterns()); Iterator aliases = clientITMD.aliases().valuesIt(); aliases.forEachRemaining((a)->serverTemplateBuilder.putAlias(a)); - + serverTemplateBuilder.settings(clientITMD.settings()); serverTemplateBuilder.order(clientITMD.order()); serverTemplateBuilder.version(clientITMD.version()); @@ -131,10 +222,45 @@ static void toXContent(GetIndexTemplatesResponse response, XContentBuilder build serverIndexTemplates.add(serverTemplateBuilder.build()); } - org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse serverResponse = new + org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse serverResponse = new org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse(serverIndexTemplates); MapParams params = - new MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "false")); + new MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "false")); serverResponse.toXContent(builder, params); } + + private static AliasMetaData randomAliasMetaData(String name) { + AliasMetaData.Builder alias = AliasMetaData.builder(name); + if (randomBoolean()) { + if (randomBoolean()) { + alias.routing(randomAlphaOfLength(5)); + } else { + if (randomBoolean()) { + alias.indexRouting(randomAlphaOfLength(5)); + } + if (randomBoolean()) { + alias.searchRouting(randomAlphaOfLength(5)); + } + } + } + + if (randomBoolean()) { + alias.filter("{\"term\":{\"year\":2016}}"); + } + + if (randomBoolean()) { + alias.writeIndex(randomBoolean()); + } + return alias.build(); + } + + static XContentBuilder randomMapping(String type, XContentType xContentType) throws IOException { + XContentBuilder builder = XContentFactory.contentBuilder(xContentType); + builder.startObject().startObject(type); + + randomMappingFields(builder, true); + + builder.endObject().endObject(); + return builder; + } }