From fe9bc1fbaa009049d778f4fa2a713b32a9f34459 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 14 May 2018 15:29:08 -0400 Subject: [PATCH 1/2] HLRestClient: Follow-up for put index template api This commit addresses some comments given after the original PR was in. Follow-up #30400 --- .../template/put/PutIndexTemplateRequest.java | 16 ++++-- .../put/PutIndexTemplateRequestTests.java | 49 ++++++++++--------- .../put/PutIndexTemplateResponseTests.java | 45 +++++++++++++++++ .../test/AbstractXContentTestCase.java | 5 +- 4 files changed, 86 insertions(+), 29 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponseTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java index b018e24a565b8..5d4e558dbb25b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; @@ -45,6 +46,7 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.common.xcontent.support.XContentMapValues; import java.io.IOException; @@ -543,9 +545,6 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - if (customs.isEmpty() == false) { - throw new IllegalArgumentException("Custom data type is no longer supported in index template [" + customs + "]"); - } builder.field("index_patterns", indexPatterns); builder.field("order", order); if (version != null) { @@ -558,8 +557,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject("mappings"); for (Map.Entry entry : mappings.entrySet()) { - Map mapping = XContentHelper.convertToMap(new BytesArray(entry.getValue()), false).v2(); - builder.field(entry.getKey(), mapping); + builder.field(entry.getKey()); + XContentParser parser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, entry.getValue()); + builder.copyCurrentStructure(parser); } builder.endObject(); @@ -568,6 +569,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws alias.toXContent(builder, params); } builder.endObject(); + + for (Map.Entry entry : customs.entrySet()) { + builder.field(entry.getKey(), entry.getValue(), params); + } + return builder; } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java index 294213452596f..108f08a6c3bc9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java @@ -23,16 +23,15 @@ import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.ToXContent; 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.yaml.YamlXContent; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.AbstractXContentTestCase; import java.io.IOException; import java.util.Arrays; @@ -45,7 +44,7 @@ import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.Is.is; -public class PutIndexTemplateRequestTests extends ESTestCase { +public class PutIndexTemplateRequestTests extends AbstractXContentTestCase { // bwc for #21009 public void testPutIndexTemplateRequest510() throws IOException { @@ -137,13 +136,14 @@ public void testValidateErrorMessage() throws Exception { assertThat(noError, is(nullValue())); } - private PutIndexTemplateRequest randomPutIndexTemplateRequest() throws IOException { + @Override + protected PutIndexTemplateRequest createTestInstance() throws IOException { PutIndexTemplateRequest request = new PutIndexTemplateRequest(); request.name("test"); - if (randomBoolean()){ + if (randomBoolean()) { request.version(randomInt()); } - if (randomBoolean()){ + if (randomBoolean()) { request.order(randomInt()); } request.patterns(Arrays.asList(generateRandomStringArray(20, 100, false, false))); @@ -164,25 +164,30 @@ private PutIndexTemplateRequest randomPutIndexTemplateRequest() throws IOExcepti .startObject("field-" + randomInt()).field("type", randomFrom("keyword", "text")).endObject() .endObject().endObject().endObject()); } - if (randomBoolean()){ + if (randomBoolean()) { request.settings(Settings.builder().put("setting1", randomLong()).put("setting2", randomTimeValue()).build()); } return request; } - public void testFromToXContentPutTemplateRequest() throws Exception { - for (int i = 0; i < 10; i++) { - PutIndexTemplateRequest expected = randomPutIndexTemplateRequest(); - XContentType xContentType = randomFrom(XContentType.values()); - BytesReference shuffled = toShuffledXContent(expected, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean()); - PutIndexTemplateRequest parsed = new PutIndexTemplateRequest().source(shuffled, xContentType); - assertNotSame(expected, parsed); - assertThat(parsed.version(), equalTo(expected.version())); - assertThat(parsed.order(), equalTo(expected.order())); - assertThat(parsed.patterns(), equalTo(expected.patterns())); - assertThat(parsed.aliases(), equalTo(expected.aliases())); - assertThat(parsed.mappings(), equalTo(expected.mappings())); - assertThat(parsed.settings(), equalTo(expected.settings())); - } + @Override + protected PutIndexTemplateRequest doParseInstance(XContentParser parser) throws IOException { + return new PutIndexTemplateRequest().source(parser.map()); + } + + @Override + protected void assertEqualInstances(PutIndexTemplateRequest expected, PutIndexTemplateRequest actual) { + assertNotSame(expected, actual); + assertThat(actual.version(), equalTo(expected.version())); + assertThat(actual.order(), equalTo(expected.order())); + assertThat(actual.patterns(), equalTo(expected.patterns())); + assertThat(actual.aliases(), equalTo(expected.aliases())); + assertThat(actual.mappings(), equalTo(expected.mappings())); + assertThat(actual.settings(), equalTo(expected.settings())); + } + + @Override + protected boolean supportsUnknownFields() { + return false; } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponseTests.java new file mode 100644 index 0000000000000..096d62bf2bb5b --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponseTests.java @@ -0,0 +1,45 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin.indices.template.put; + +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; + +public class PutIndexTemplateResponseTests extends AbstractStreamableXContentTestCase { + @Override + protected PutIndexTemplateResponse doParseInstance(XContentParser parser) { + return PutIndexTemplateResponse.fromXContent(parser); + } + + @Override + protected PutIndexTemplateResponse createTestInstance() { + return new PutIndexTemplateResponse(randomBoolean()); + } + + @Override + protected PutIndexTemplateResponse createBlankInstance() { + return new PutIndexTemplateResponse(); + } + + @Override + protected PutIndexTemplateResponse mutateInstance(PutIndexTemplateResponse response) { + return new PutIndexTemplateResponse(response.isAcknowledged() == false); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java index 983897049c767..e521b9f673975 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.ToXContent; @@ -41,7 +42,7 @@ public abstract class AbstractXContentTestCase extends EST protected static final int NUMBER_OF_TEST_RUNS = 20; - public static void testFromXContent(int numberOfTestRuns, Supplier instanceSupplier, + public static void testFromXContent(int numberOfTestRuns, CheckedSupplier instanceSupplier, boolean supportsUnknownFields, String[] shuffleFieldsExceptions, Predicate randomFieldsExcludeFilter, CheckedBiFunction @@ -85,7 +86,7 @@ public final void testFromXContent() throws IOException { * called multiple times during test execution and should return a different * random instance each time it is called. */ - protected abstract T createTestInstance(); + protected abstract T createTestInstance() throws IOException; private T parseInstance(XContentParser parser) throws IOException { T parsedInstance = doParseInstance(parser); From 74dd62a2b3228a0a1ae2711dff7f5851a57fd077 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 14 May 2018 17:23:45 -0400 Subject: [PATCH 2/2] Keep AbstractXContentTestCase --- .../put/PutIndexTemplateRequestTests.java | 15 ++++++++++----- .../test/AbstractXContentTestCase.java | 5 ++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java index 108f08a6c3bc9..577a8b55e61a3 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequestTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.test.AbstractXContentTestCase; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.Arrays; import java.util.Base64; import java.util.Collections; @@ -137,7 +138,7 @@ public void testValidateErrorMessage() throws Exception { } @Override - protected PutIndexTemplateRequest createTestInstance() throws IOException { + protected PutIndexTemplateRequest createTestInstance() { PutIndexTemplateRequest request = new PutIndexTemplateRequest(); request.name("test"); if (randomBoolean()) { @@ -159,10 +160,14 @@ protected PutIndexTemplateRequest createTestInstance() throws IOException { request.alias(alias); } if (randomBoolean()) { - request.mapping("doc", XContentFactory.jsonBuilder().startObject() - .startObject("doc").startObject("properties") - .startObject("field-" + randomInt()).field("type", randomFrom("keyword", "text")).endObject() - .endObject().endObject().endObject()); + try { + request.mapping("doc", XContentFactory.jsonBuilder().startObject() + .startObject("doc").startObject("properties") + .startObject("field-" + randomInt()).field("type", randomFrom("keyword", "text")).endObject() + .endObject().endObject().endObject()); + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } } if (randomBoolean()) { request.settings(Settings.builder().put("setting1", randomLong()).put("setting2", randomTimeValue()).build()); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java index e521b9f673975..983897049c767 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractXContentTestCase.java @@ -21,7 +21,6 @@ import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.CheckedFunction; -import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.ToXContent; @@ -42,7 +41,7 @@ public abstract class AbstractXContentTestCase extends EST protected static final int NUMBER_OF_TEST_RUNS = 20; - public static void testFromXContent(int numberOfTestRuns, CheckedSupplier instanceSupplier, + public static void testFromXContent(int numberOfTestRuns, Supplier instanceSupplier, boolean supportsUnknownFields, String[] shuffleFieldsExceptions, Predicate randomFieldsExcludeFilter, CheckedBiFunction @@ -86,7 +85,7 @@ public final void testFromXContent() throws IOException { * called multiple times during test execution and should return a different * random instance each time it is called. */ - protected abstract T createTestInstance() throws IOException; + protected abstract T createTestInstance(); private T parseInstance(XContentParser parser) throws IOException { T parsedInstance = doParseInstance(parser);