Skip to content

Make sure to accept empty unnested mappings in create index requests. #37089

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.

Sign up for GitHub

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

Merged
jtibshirani merged 1 commit into from
Jan 3, 2019
Merged

Make sure to accept empty unnested mappings in create index requests. #37089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,29 @@ private Tuple<String, Map<String, Object>> extractMapping(String type, String so
return extractMapping(type, root);
}

/**
* Given an optional type name and mapping definition, returns the type and a normalized form of the mappings.
*
* The provided mapping definition may or may not contain the type name as the root key in the map. This method
* attempts to unwrap the mappings, so that they no longer contain a type name at the root. If no type name can
* be found, through either the 'type' parameter or by examining the provided mappings, then an exception will be
* thrown.
*
* @param type An optional type name.
* @param root The mapping definition.
*
* @return A tuple of the form (type, normalized mappings).
*/
@SuppressWarnings({"unchecked"})
private Tuple<String, Map<String, Object>> extractMapping(String type, Map<String, Object> root) throws MapperParsingException {
if (root.size() == 0) {
// if we don't have any keys throw an exception
throw new MapperParsingException("malformed mapping no root object found");
if (type != null) {
return new Tuple<>(type, root);
} else {
throw new MapperParsingException("malformed mapping, no type name found");
}
}

String rootName = root.keySet().iterator().next();
Tuple<String, Map<String, Object>> mapping;
if (type == null || type.equals(rootName)) {
Expand Down
53 changes: 53 additions & 0 deletions server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,20 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.UnavailableShardsException;
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.gateway.MetaStateService;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.query.RangeQueryBuilder;
Expand Down Expand Up @@ -120,6 +123,56 @@ public void testDoubleAddMapping() throws Exception {
}
}

public void testNonNestedMappings() throws Exception {
assertAcked(prepareCreate("test")
.addMapping("_doc", XContentFactory.jsonBuilder().startObject()
.startObject("properties")
.startObject("date")
.field("type", "date")
.endObject()
.endObject()
.endObject()));

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 testEmptyNestedMappings() throws Exception {
assertAcked(prepareCreate("test")
.addMapping("_doc", XContentFactory.jsonBuilder().startObject().endObject()));

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);
assertTrue(metadata.sourceAsMap().isEmpty());
}

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

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);
assertTrue(metadata.sourceAsMap().isEmpty());
}

public void testInvalidShardCountSettings() throws Exception {
int value = randomIntBetween(-10, 0);
try {
Expand Down
22 changes: 0 additions & 22 deletions ...rg/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,6 @@ public void testIndexTemplateWithAliasNameEqualToTemplatePattern() {
assertThat(errors.get(0).getMessage(), equalTo("Alias [foobar] cannot be the same as any pattern in [foo, foobar]"));
}

public void testIndexTemplateWithValidateEmptyMapping() throws Exception {
Copy link
Contributor Author

@jtibshirani jtibshirani Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these tests were added in fd76291, not to guard against a particular bug, but as a means of checking whether index templates were being validated.

PutRequest request = new PutRequest("api", "validate_template");
request.patterns(Collections.singletonList("validate_template"));
request.putMapping("type1", "{}");

List<Throwable> errors = putTemplateDetail(request);
assertThat(errors.size(), equalTo(1));
assertThat(errors.get(0), instanceOf(MapperParsingException.class));
assertThat(errors.get(0).getMessage(), containsString("malformed mapping no root object found"));
}

public void testIndexTemplateWithValidateMapping() throws Exception {
PutRequest request = new PutRequest("api", "validate_template");
request.patterns(Collections.singletonList("te*"));
Expand All @@ -132,17 +121,6 @@ public void testBrokenMapping() throws Exception {
assertThat(errors.get(0).getMessage(), containsString("Failed to parse mapping "));
}

public void testBlankMapping() throws Exception {
PutRequest request = new PutRequest("api", "blank_mapping");
request.patterns(Collections.singletonList("te*"));
request.putMapping("type1", "{}");

List<Throwable> errors = putTemplateDetail(request);
assertThat(errors.size(), equalTo(1));
assertThat(errors.get(0), instanceOf(MapperParsingException.class));
assertThat(errors.get(0).getMessage(), containsString("malformed mapping no root object found"));
}

public void testAliasInvalidFilterInvalidJson() throws Exception {
//invalid json: put index template fails
PutRequest request = new PutRequest("api", "blank_mapping");
Expand Down
22 changes: 1 addition & 21 deletions server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ExecutionException;

import static org.hamcrest.CoreMatchers.containsString;
Expand Down Expand Up @@ -179,25 +177,7 @@ public void testUnmappedFieldType() {
assertWarnings("[unmapped_type:string] should be replaced with [unmapped_type:keyword]");
}

public void testMergeWithMap() throws Throwable {
IndexService indexService1 = createIndex("index1");
MapperService mapperService = indexService1.mapperService();
Map<String, Map<String, Object>> mappings = new HashMap<>();

mappings.put(MapperService.DEFAULT_MAPPING, MapperService.parseMapping(xContentRegistry(), "{}"));
MapperException e = expectThrows(MapperParsingException.class,
() -> mapperService.merge(mappings, MergeReason.MAPPING_UPDATE));
assertThat(e.getMessage(), startsWith("Failed to parse mapping [" + MapperService.DEFAULT_MAPPING + "]: "));

mappings.clear();
mappings.put("type1", MapperService.parseMapping(xContentRegistry(), "{}"));

e = expectThrows( MapperParsingException.class,
() -> mapperService.merge(mappings, MergeReason.MAPPING_UPDATE));
assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: "));
}

public void testPartitionedConstraints() {
public void testPartitionedConstraints() {
// partitioned index must have routing
IllegalArgumentException noRoutingException = expectThrows(IllegalArgumentException.class, () -> {
client().admin().indices().prepareCreate("test-index")
Expand Down