From 086c244a343f1588b110fc26a887d0e22ac8dd50 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Mon, 4 Feb 2019 14:21:30 -0600 Subject: [PATCH 1/3] Update 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 --- .../client/indices/IndexTemplateMetaData.java | 142 +++++++----------- .../GetIndexTemplatesResponseTests.java | 28 +++- 2 files changed, 75 insertions(+), 95 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 d2f0c3d7eba88..0935e33e63c35 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 @@ -36,6 +36,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -50,11 +51,26 @@ public class GetIndexTemplatesResponseTests extends ESTestCase { 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(); + } + + 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) { @@ -64,7 +80,7 @@ private static void assertEqualInstances(GetIndexTemplatesResponse expectedInsta 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); } From 0cdf33454f246240d1303b8348b3918107bbab25 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 5 Feb 2019 13:57:12 +0100 Subject: [PATCH 2/3] added test duel, checking the parsing response from es server yields same results in hlrc response class. --- .../GetIndexTemplatesResponseTests.java | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) 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 0935e33e63c35..3cfec568400cd 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,10 +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.index.mapper.MapperService; import org.elasticsearch.test.ESTestCase; @@ -33,6 +39,8 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -40,7 +48,10 @@ 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 { @@ -62,6 +73,66 @@ public void testFromXContent() throws IOException { .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("%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("%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() @@ -149,4 +220,39 @@ static void toXContent(GetIndexTemplatesResponse response, XContentBuilder build org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse(serverIndexTemplates); serverResponse.toXContent(builder, ToXContent.EMPTY_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; + } } From 1d15210559af18a7da30481eb20a35e23d03103c Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 5 Feb 2019 18:01:04 +0100 Subject: [PATCH 3/3] fixed checkstyle --- .../client/indices/GetIndexTemplatesResponseTests.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 3cfec568400cd..d94d8572f3de9 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 @@ -43,6 +43,7 @@ import java.util.Comparator; 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; @@ -82,13 +83,14 @@ public void testParsingFromEsResponse() throws IOException { 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("%02d ", i) + randomAlphaOfLength(4)); + 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("%02d ", j) + randomAlphaOfLength(4))); + 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)); @@ -115,7 +117,8 @@ public void testParsingFromEsResponse() throws IOException { assertThat(esIMD.mappings().size(), equalTo(1)); BytesArray mappingSource = new BytesArray(esIMD.mappings().valuesIt().next().uncompressed()); - Map expectedMapping = XContentHelper.convertToMap(mappingSource, true, xContentBuilder.contentType()).v2(); + 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()));