From c5aac4705d7e5afff701830f1a9e1e82765db6eb Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 17:56:40 -0800 Subject: [PATCH] Revert "Stop automatically nesting mappings in index creation requests. (#36924)" This reverts commit ac1c6940d20b9ac993422b279d8f3a8bd73a3527. --- .../indices/create/CreateIndexRequest.java | 7 +- .../mapping/put/PutMappingRequest.java | 12 -- .../admin/indices/create/CreateIndexIT.java | 16 +- .../create/CreateIndexRequestTests.java | 149 ++++++++++-------- .../index/RandomCreateIndexGenerator.java | 20 +-- 5 files changed, 93 insertions(+), 111 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java index bf82d705216..1bbce19ee8d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; @@ -255,6 +256,10 @@ public class CreateIndexRequest extends AcknowledgedRequest if (mappings.containsKey(type)) { throw new IllegalStateException("mappings for type \"" + type + "\" were already defined"); } + // wrap it in a type map if its not + if (source.size() != 1 || !source.containsKey(type)) { + source = MapBuilder.newMapBuilder().put(type, source).map(); + } try { XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON); builder.map(source); @@ -269,7 +274,7 @@ public class CreateIndexRequest extends AcknowledgedRequest * ("field1", "type=string,store=true"). */ public CreateIndexRequest mapping(String type, Object... source) { - mapping(type, PutMappingRequest.buildFromSimplifiedDef(source)); + mapping(type, PutMappingRequest.buildFromSimplifiedDef(type, source)); return this; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java index 237e720ae2a..926ae175d65 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java @@ -184,18 +184,6 @@ public class PutMappingRequest extends AcknowledgedRequest im return source(buildFromSimplifiedDef(type, source)); } - /** - * @param source - * consisting of field/properties pairs (e.g. "field1", - * "type=string,store=true") - * @throws IllegalArgumentException - * if the number of the source arguments is not divisible by two - * @return the mappings definition - */ - public static XContentBuilder buildFromSimplifiedDef(Object... source) { - return buildFromSimplifiedDef(null, source); - } - /** * @param type * the mapping type diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index ada78767ca5..05da57cc5da 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -143,7 +143,7 @@ public class CreateIndexIT extends ESIntegTestCase { assertFalse(metadata.sourceAsMap().isEmpty()); } - public void testNonNestedEmptyMappings() throws Exception { + public void testEmptyNestedMappings() throws Exception { assertAcked(prepareCreate("test") .addMapping("_doc", XContentFactory.jsonBuilder().startObject().endObject())); @@ -173,20 +173,6 @@ public class CreateIndexIT extends ESIntegTestCase { assertTrue(metadata.sourceAsMap().isEmpty()); } - public void testFlatMappingFormat() throws Exception { - assertAcked(prepareCreate("test") - .addMapping("_doc", "field", "type=keyword")); - - GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); - - ImmutableOpenMap mappings = response.mappings().get("test"); - assertNotNull(mappings); - - MappingMetaData metadata = mappings.get("_doc"); - assertNotNull(metadata); - assertFalse(metadata.sourceAsMap().isEmpty()); - } - public void testInvalidShardCountSettings() throws Exception { int value = randomIntBetween(-10, 0); try { diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java index 64b77d0dc34..1c279349274 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequestTests.java @@ -22,86 +22,31 @@ package org.elasticsearch.action.admin.indices.create; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.common.Strings; +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.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.RandomCreateIndexGenerator; -import org.elasticsearch.test.AbstractXContentTestCase; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; import java.io.IOException; import java.util.Map; import java.util.Set; -public class CreateIndexRequestTests extends AbstractXContentTestCase { +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS; - @Override - protected CreateIndexRequest createTestInstance() { - try { - return RandomCreateIndexGenerator.randomCreateIndexRequest(); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - @Override - protected CreateIndexRequest doParseInstance(XContentParser parser) throws IOException { - CreateIndexRequest request = new CreateIndexRequest(); - request.source(parser.map(), LoggingDeprecationHandler.INSTANCE); - return request; - } - - @Override - protected void assertEqualInstances(CreateIndexRequest expectedInstance, CreateIndexRequest newInstance) { - assertEquals(expectedInstance.settings(), newInstance.settings()); - assertAliasesEqual(expectedInstance.aliases(), newInstance.aliases()); - assertMappingsEqual(expectedInstance.mappings(), newInstance.mappings()); - } - - @Override - protected boolean supportsUnknownFields() { - return false; - } - - public static void assertMappingsEqual(Map expected, Map actual) { - assertEquals(expected.keySet(), actual.keySet()); - - for (Map.Entry expectedEntry : expected.entrySet()) { - String expectedValue = expectedEntry.getValue(); - String actualValue = actual.get(expectedEntry.getKey()); - try (XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, expectedValue); - XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, actualValue)) { - assertEquals(expectedJson.map(), actualJson.map()); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - } - - public static void assertAliasesEqual(Set expected, Set actual) { - assertEquals(expected, actual); - - for (Alias expectedAlias : expected) { - for (Alias actualAlias : actual) { - if (expectedAlias.equals(actualAlias)) { - // As Alias#equals only looks at name, we check the equality of the other Alias parameters here. - assertEquals(expectedAlias.filter(), actualAlias.filter()); - assertEquals(expectedAlias.indexRouting(), actualAlias.indexRouting()); - assertEquals(expectedAlias.searchRouting(), actualAlias.searchRouting()); - } - } - } - } +public class CreateIndexRequestTests extends ESTestCase { public void testSerialization() throws IOException { CreateIndexRequest request = new CreateIndexRequest("foo"); - String mapping = Strings.toString(JsonXContent.contentBuilder().startObject() - .startObject("type").endObject().endObject()); + String mapping = Strings.toString(JsonXContent.contentBuilder().startObject().startObject("type").endObject().endObject()); request.mapping("my_type", mapping, XContentType.JSON); try (BytesStreamOutput output = new BytesStreamOutput()) { @@ -118,7 +63,7 @@ public class CreateIndexRequestTests extends AbstractXContentTestCase {request.source(createIndex, XContentType.JSON);}); + () -> {request.source(createIndex, XContentType.JSON);}); assertEquals("unknown key [FOO_SHOULD_BE_ILLEGAL_HERE] for create index", e.getMessage()); } + + public void testToXContent() throws IOException { + CreateIndexRequest request = new CreateIndexRequest("foo"); + + String mapping = Strings.toString(JsonXContent.contentBuilder().startObject().startObject("type").endObject().endObject()); + request.mapping("my_type", mapping, XContentType.JSON); + + Alias alias = new Alias("test_alias"); + alias.routing("1"); + alias.filter("{\"term\":{\"year\":2016}}"); + alias.writeIndex(true); + request.alias(alias); + + Settings.Builder settings = Settings.builder(); + settings.put(SETTING_NUMBER_OF_SHARDS, 10); + request.settings(settings); + + String actualRequestBody = Strings.toString(request); + + String expectedRequestBody = "{\"settings\":{\"index\":{\"number_of_shards\":\"10\"}}," + + "\"mappings\":{\"my_type\":{\"type\":{}}}," + + "\"aliases\":{\"test_alias\":{\"filter\":{\"term\":{\"year\":2016}},\"routing\":\"1\",\"is_write_index\":true}}}"; + + assertEquals(expectedRequestBody, actualRequestBody); + } + + public void testToAndFromXContent() throws IOException { + + final CreateIndexRequest createIndexRequest = RandomCreateIndexGenerator.randomCreateIndexRequest(); + + boolean humanReadable = randomBoolean(); + final XContentType xContentType = randomFrom(XContentType.values()); + BytesReference originalBytes = toShuffledXContent(createIndexRequest, xContentType, EMPTY_PARAMS, humanReadable); + + CreateIndexRequest parsedCreateIndexRequest = new CreateIndexRequest(); + parsedCreateIndexRequest.source(originalBytes, xContentType); + + assertMappingsEqual(createIndexRequest.mappings(), parsedCreateIndexRequest.mappings()); + assertAliasesEqual(createIndexRequest.aliases(), parsedCreateIndexRequest.aliases()); + assertEquals(createIndexRequest.settings(), parsedCreateIndexRequest.settings()); + + BytesReference finalBytes = toShuffledXContent(parsedCreateIndexRequest, xContentType, EMPTY_PARAMS, humanReadable); + ElasticsearchAssertions.assertToXContentEquivalent(originalBytes, finalBytes, xContentType); + } + + public static void assertMappingsEqual(Map expected, Map actual) throws IOException { + assertEquals(expected.keySet(), actual.keySet()); + + for (Map.Entry expectedEntry : expected.entrySet()) { + String expectedValue = expectedEntry.getValue(); + String actualValue = actual.get(expectedEntry.getKey()); + try (XContentParser expectedJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, expectedValue); + XContentParser actualJson = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, actualValue)){ + assertEquals(expectedJson.map(), actualJson.map()); + } + } + } + + public static void assertAliasesEqual(Set expected, Set actual) throws IOException { + assertEquals(expected, actual); + + for (Alias expectedAlias : expected) { + for (Alias actualAlias : actual) { + if (expectedAlias.equals(actualAlias)) { + // As Alias#equals only looks at name, we check the equality of the other Alias parameters here. + assertEquals(expectedAlias.filter(), actualAlias.filter()); + assertEquals(expectedAlias.indexRouting(), actualAlias.indexRouting()); + assertEquals(expectedAlias.searchRouting(), actualAlias.searchRouting()); + } + } + } + } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java b/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java index 27b7db3d36a..e88a9f0a38d 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java +++ b/test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java @@ -29,7 +29,6 @@ import java.io.IOException; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; -import static org.elasticsearch.test.ESTestCase.frequently; import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; import static org.elasticsearch.test.ESTestCase.randomBoolean; import static org.elasticsearch.test.ESTestCase.randomIntBetween; @@ -46,14 +45,9 @@ public final class RandomCreateIndexGenerator { String index = randomAlphaOfLength(5); CreateIndexRequest request = new CreateIndexRequest(index); randomAliases(request); - if (frequently()) { + if (randomBoolean()) { String type = randomAlphaOfLength(5); - if (randomBoolean()) { - request.mapping(type, randomMapping()); - } else { - request.mapping(type, randomMapping(type)); - - } + request.mapping(type, randomMapping(type)); } if (randomBoolean()) { request.settings(randomIndexSettings()); @@ -82,16 +76,6 @@ public final class RandomCreateIndexGenerator { return builder.build(); } - public static XContentBuilder randomMapping() throws IOException { - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject(); - - randomMappingFields(builder, true); - - builder.endObject(); - return builder; - } - public static XContentBuilder randomMapping(String type) throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject().startObject(type);