Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop automatically nesting mappings in index creation requests. #36924

Merged
merged 9 commits into from
Jan 4, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
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;
Expand Down Expand Up @@ -256,10 +255,6 @@ public CreateIndexRequest mapping(String type, Map<String, ?> source) {
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.<String, Object>newMapBuilder().put(type, source).map();
}
try {
XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON);
builder.map(source);
Expand All @@ -274,7 +269,7 @@ public CreateIndexRequest mapping(String type, Map<String, ?> source) {
* ("field1", "type=string,store=true").
*/
public CreateIndexRequest mapping(String type, Object... source) {
mapping(type, PutMappingRequest.buildFromSimplifiedDef(type, source));
mapping(type, PutMappingRequest.buildFromSimplifiedDef(source));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ public PutMappingRequest source(Object... source) {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void testNonNestedMappings() throws Exception {
assertFalse(metadata.sourceAsMap().isEmpty());
}

public void testEmptyNestedMappings() throws Exception {
public void testNonNestedEmptyMappings() throws Exception {
assertAcked(prepareCreate("test")
.addMapping("_doc", XContentFactory.jsonBuilder().startObject().endObject()));

Expand Down Expand Up @@ -173,6 +173,20 @@ public void testEmptyMappings() throws Exception {
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<String, MappingMetaData> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,86 @@
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.ESTestCase;
import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
import org.elasticsearch.test.AbstractXContentTestCase;

import java.io.IOException;
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS;
public class CreateIndexRequestTests extends AbstractXContentTestCase<CreateIndexRequest> {

public class CreateIndexRequestTests extends ESTestCase {
@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<String, String> expected, Map<String, String> actual) {
assertEquals(expected.keySet(), actual.keySet());

for (Map.Entry<String, String> 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<Alias> expected, Set<Alias> 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 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()) {
Expand All @@ -63,7 +118,7 @@ public void testSerialization() throws IOException {

public void testTopLevelKeys() {
String createIndex =
"{\n"
"{\n"
+ " \"FOO_SHOULD_BE_ILLEGAL_HERE\": {\n"
+ " \"BAR_IS_THE_SAME\": 42\n"
+ " },\n"
Expand All @@ -80,81 +135,7 @@ public void testTopLevelKeys() {

CreateIndexRequest request = new CreateIndexRequest();
ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class,
() -> {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<String, String> expected, Map<String, String> actual) throws IOException {
assertEquals(expected.keySet(), actual.keySet());

for (Map.Entry<String, String> 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<Alias> expected, Set<Alias> 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());
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

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;
Expand All @@ -45,9 +46,14 @@ public static CreateIndexRequest randomCreateIndexRequest() throws IOException {
String index = randomAlphaOfLength(5);
CreateIndexRequest request = new CreateIndexRequest(index);
randomAliases(request);
if (randomBoolean()) {
if (frequently()) {
String type = randomAlphaOfLength(5);
request.mapping(type, randomMapping(type));
if (randomBoolean()) {
request.mapping(type, randomMapping());
} else {
request.mapping(type, randomMapping(type));

}
}
if (randomBoolean()) {
request.settings(randomIndexSettings());
Expand Down Expand Up @@ -76,6 +82,16 @@ public static Settings randomIndexSettings() {
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);
Expand Down