diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 624fbd1ae66..9b10c93113d 100644 --- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -438,7 +438,7 @@ public class DocumentMapper implements ToXContent { ParseContext.InternalParseContext context = cache.get(); if (source.type() != null && !source.type().equals(this.type)) { - throw new MapperParsingException("Type mismatch, provide type [" + source.type() + "] but mapper is of type [" + this.type + "]"); + throw new MapperParsingException("Type mismatch, provide type [" + source.type() + "] but mapper is of type [" + this.type + "]", context.mappingsModified()); } source.type(this.type); @@ -456,7 +456,7 @@ public class DocumentMapper implements ToXContent { int countDownTokens = 0; XContentParser.Token token = parser.nextToken(); if (token != XContentParser.Token.START_OBJECT) { - throw new MapperParsingException("Malformed content, must start with an object"); + throw new MapperParsingException("Malformed content, must start with an object", context.mappingsModified()); } boolean emptyDoc = false; token = parser.nextToken(); @@ -464,7 +464,7 @@ public class DocumentMapper implements ToXContent { // empty doc, we can handle it... emptyDoc = true; } else if (token != XContentParser.Token.FIELD_NAME) { - throw new MapperParsingException("Malformed content, after first object, either the type field or the actual properties should exist"); + throw new MapperParsingException("Malformed content, after first object, either the type field or the actual properties should exist", context.mappingsModified()); } for (RootMapper rootMapper : rootMappersOrdered) { @@ -490,10 +490,10 @@ public class DocumentMapper implements ToXContent { // Throw a more meaningful message if the document is empty. if (source.source() != null && source.source().length() == 0) { - throw new MapperParsingException("failed to parse, document is empty"); + throw new MapperParsingException("failed to parse, document is empty", context.mappingsModified()); } - throw new MapperParsingException("failed to parse", e); + throw new MapperParsingException("failed to parse", e, context.mappingsModified()); } finally { // only close the parser when its not provided externally if (source.parser() == null && parser != null) { diff --git a/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java b/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java index 9c180b82ae5..9cac8a26ef9 100644 --- a/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java +++ b/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java @@ -28,13 +28,30 @@ public class MapperParsingException extends MapperException { public MapperParsingException(String message) { super(message); + mappingsModified = false; + } + + public boolean isMappingsModified() { + return mappingsModified; + } + + private boolean mappingsModified = false; + + public MapperParsingException(String message, boolean mappingsModified) { + super(message); + this.mappingsModified = mappingsModified; + } + + public MapperParsingException(String message, Throwable cause, boolean mappingsModified) { + super(message, cause); + this.mappingsModified = mappingsModified; } public MapperParsingException(String message, Throwable cause) { super(message, cause); + this.mappingsModified = false; } - @Override public RestStatus status() { return RestStatus.BAD_REQUEST; diff --git a/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java b/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java index f675396369b..098d5abf086 100644 --- a/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java +++ b/src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java @@ -24,8 +24,8 @@ import org.elasticsearch.rest.RestStatus; */ public class StrictDynamicMappingException extends MapperParsingException { - public StrictDynamicMappingException(String path, String fieldName) { - super("mapping set to strict, dynamic introduction of [" + fieldName + "] within [" + path + "] is not allowed"); + public StrictDynamicMappingException(String path, String fieldName, boolean mappingsModified) { + super("mapping set to strict, dynamic introduction of [" + fieldName + "] within [" + path + "] is not allowed", mappingsModified); } @Override diff --git a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index ee0c66e9ccb..806074914d0 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -429,7 +429,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { } } } catch (Exception e) { - throw new MapperParsingException("failed to parse [" + names.fullName() + "]", e); + throw new MapperParsingException("failed to parse [" + names.fullName() + "]", e, context.mappingsModified()); } multiFields.parse(this, context); if (copyTo != null) { @@ -1077,7 +1077,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { ObjectMapper mapper = context.docMapper().objectMappers().get(objectPath); if (mapper == null) { //TODO: Create an object dynamically? - throw new MapperParsingException("attempt to copy value to non-existing object [" + field + "]"); + throw new MapperParsingException("attempt to copy value to non-existing object [" + field + "]", context.mappingsModified()); } context.path().add(objectPath); diff --git a/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java index 32e25a23caf..3d2d4c1c9ba 100644 --- a/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java @@ -307,7 +307,7 @@ public class IdFieldMapper extends AbstractFieldMapper implements Intern @Override public void postParse(ParseContext context) throws IOException { if (context.id() == null && !context.sourceToParse().flyweight()) { - throw new MapperParsingException("No id found while parsing the content source"); + throw new MapperParsingException("No id found while parsing the content source", context.mappingsModified()); } // it either get built in the preParse phase, or get parsed... } @@ -329,7 +329,7 @@ public class IdFieldMapper extends AbstractFieldMapper implements Intern // we are in the parse Phase String id = parser.text(); if (context.id() != null && !context.id().equals(id)) { - throw new MapperParsingException("Provided id [" + context.id() + "] does not match the content one [" + id + "]"); + throw new MapperParsingException("Provided id [" + context.id() + "] does not match the content one [" + id + "]", context.mappingsModified()); } context.id(id); } // else we are in the pre/post parse phase diff --git a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java index 7f349e40479..653f9f0804a 100644 --- a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java @@ -499,7 +499,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { if (token.isValue() && !allowValue()) { // if we are parsing an object but it is just a value, its only allowed on root level parsers with there // is a field name with the same name as the type - throw new MapperParsingException("object mapping for [" + name + "] tried to parse field [" + currentFieldName + "] as object, but found a concrete value"); + throw new MapperParsingException("object mapping for [" + name + "] tried to parse field [" + currentFieldName + "] as object, but found a concrete value", context.mappingsModified()); } if (nested.isNested()) { @@ -543,7 +543,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { } else if (token == XContentParser.Token.VALUE_NULL) { serializeNullValue(context, currentFieldName); } else if (token == null) { - throw new MapperParsingException("object mapping for [" + name + "] tried to parse field [" + currentFieldName + "] as object, but got EOF, has a concrete value been provided to it?"); + throw new MapperParsingException("object mapping for [" + name + "] tried to parse field [" + currentFieldName + "] as object, but got EOF, has a concrete value been provided to it?", context.mappingsModified()); } else if (token.isValue()) { serializeValue(context, currentFieldName, token); } @@ -585,18 +585,18 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { if (mapper != null) { if (mapper instanceof FieldMapper) { if (!((FieldMapper) mapper).supportsNullValue()) { - throw new MapperParsingException("no object mapping found for null value in [" + lastFieldName + "]"); + throw new MapperParsingException("no object mapping found for null value in [" + lastFieldName + "]", context.mappingsModified()); } } mapper.parse(context); } else if (dynamic == Dynamic.STRICT) { - throw new StrictDynamicMappingException(fullPath, lastFieldName); + throw new StrictDynamicMappingException(fullPath, lastFieldName, context.mappingsModified()); } } private void serializeObject(final ParseContext context, String currentFieldName) throws IOException { if (currentFieldName == null) { - throw new MapperParsingException("object mapping [" + name + "] trying to serialize an object with no field associated with it, current value [" + context.parser().textOrNull() + "]"); + throw new MapperParsingException("object mapping [" + name + "] trying to serialize an object with no field associated with it, current value [" + context.parser().textOrNull() + "]", context.mappingsModified()); } context.path().add(currentFieldName); @@ -609,7 +609,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { dynamic = context.root().dynamic(); } if (dynamic == Dynamic.STRICT) { - throw new StrictDynamicMappingException(fullPath, currentFieldName); + throw new StrictDynamicMappingException(fullPath, currentFieldName, context.mappingsModified()); } else if (dynamic == Dynamic.TRUE) { // we sync here just so we won't add it twice. Its not the end of the world // to sync here since next operations will get it before @@ -661,7 +661,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { dynamic = context.root().dynamic(); } if (dynamic == Dynamic.STRICT) { - throw new StrictDynamicMappingException(fullPath, arrayFieldName); + throw new StrictDynamicMappingException(fullPath, arrayFieldName, context.mappingsModified()); } else if (dynamic == Dynamic.TRUE) { // we sync here just so we won't add it twice. Its not the end of the world // to sync here since next operations will get it before @@ -741,7 +741,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { } else if (token == XContentParser.Token.VALUE_NULL) { serializeNullValue(context, lastFieldName); } else if (token == null) { - throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?"); + throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?", context.mappingsModified()); } else { serializeValue(context, lastFieldName, token); } @@ -750,7 +750,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { private void serializeValue(final ParseContext context, String currentFieldName, XContentParser.Token token) throws IOException { if (currentFieldName == null) { - throw new MapperParsingException("object mapping [" + name + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]"); + throw new MapperParsingException("object mapping [" + name + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]", context.mappingsModified()); } Mapper mapper = mappers.get(currentFieldName); if (mapper != null) { @@ -766,7 +766,7 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll { dynamic = context.root().dynamic(); } if (dynamic == Dynamic.STRICT) { - throw new StrictDynamicMappingException(fullPath, currentFieldName); + throw new StrictDynamicMappingException(fullPath, currentFieldName, context.mappingsModified()); } if (dynamic == Dynamic.FALSE) { return; diff --git a/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java b/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java index 08938c72c31..fb188ff0f8f 100644 --- a/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java @@ -231,7 +231,7 @@ public class RootObjectMapper extends ObjectMapper { String mappingType = dynamicTemplate.mappingType(dynamicType); Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); if (typeParser == null) { - throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]"); + throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]", context.mappingsModified()); } return typeParser.parse(name, dynamicTemplate.mappingForName(name, dynamicType), parserContext); } diff --git a/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 92c7896b68c..16014260f9e 100644 --- a/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -448,7 +448,7 @@ public class IndexShard extends AbstractIndexShardComponent { ParsedDocument doc = docMapper.v1().parse(source).setMappingsModified(docMapper); return new Engine.Create(docMapper.v1(), docMapper.v1().uidMapper().term(doc.uid().stringValue()), doc, version, versionType, origin, startTime, state != IndexShardState.STARTED || canHaveDuplicates, autoGeneratedId); } catch (Throwable t) { - if (docMapper.v2()) { + if (docMapper.v2() || (t instanceof MapperParsingException && ((MapperParsingException)t).isMappingsModified())) { throw new WriteFailureException(t, docMapper.v1().type()); } else { throw t; @@ -481,7 +481,7 @@ public class IndexShard extends AbstractIndexShardComponent { ParsedDocument doc = docMapper.v1().parse(source).setMappingsModified(docMapper); return new Engine.Index(docMapper.v1(), docMapper.v1().uidMapper().term(doc.uid().stringValue()), doc, version, versionType, origin, startTime, state != IndexShardState.STARTED || canHaveDuplicates); } catch (Throwable t) { - if (docMapper.v2()) { + if (docMapper.v2() || (t instanceof MapperParsingException && ((MapperParsingException)t).isMappingsModified())) { throw new WriteFailureException(t, docMapper.v1().type()); } else { throw t; diff --git a/src/test/java/org/elasticsearch/index/mapper/dynamic/DynamicMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/dynamic/DynamicMappingTests.java index a1fa2c62e17..47c739d4a12 100644 --- a/src/test/java/org/elasticsearch/index/mapper/dynamic/DynamicMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/dynamic/DynamicMappingTests.java @@ -20,18 +20,18 @@ package org.elasticsearch.index.mapper.dynamic; import com.google.common.base.Predicate; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.mapper.DocumentMapper; -import org.elasticsearch.index.mapper.FieldMappers; -import org.elasticsearch.index.mapper.ParsedDocument; -import org.elasticsearch.index.mapper.StrictDynamicMappingException; +import org.elasticsearch.index.mapper.*; import org.elasticsearch.index.IndexService; import org.elasticsearch.test.ElasticsearchSingleNodeTest; import org.junit.Test; import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.equalTo; @@ -242,4 +242,49 @@ public class DynamicMappingTests extends ElasticsearchSingleNodeTest { getMappingsResponse = client().admin().indices().prepareGetMappings("test").get(); assertNotNull(getMappingsResponse.getMappings().get("test").get("type")); } + + @Test + public void testFieldsCreatedWithPartialParsing() throws IOException, InterruptedException { + XContentBuilder mapping = jsonBuilder().startObject().startObject("doc") + .startObject("properties") + .startObject("z") + .field("type", "long") + .endObject() + .endObject() + .endObject().endObject(); + + IndexService indexService = createIndex("test", ImmutableSettings.EMPTY, "doc", mapping); + boolean create = randomBoolean(); + if (create == false) { + // we want to test sometimes create and sometimes index so sometimes add the document before and sometimes not + client().prepareIndex().setIndex("test").setType("doc").setId("1").setSource(jsonBuilder().startObject().field("z", 0).endObject()).get(); + } + try { + IndexRequestBuilder indexRequest = client().prepareIndex().setIndex("test").setType("doc").setId("1").setSource(jsonBuilder().startObject().field("a", "string").field("z", "string").endObject()); + indexRequest.setCreate(create); + indexRequest.get(); + fail(); + } catch (MapperParsingException e) { + // this should fail because the field z is of type long + } + //type should be in mapping + GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get(); + assertNotNull(getMappingsResponse.getMappings().get("test").get("doc")); + + client().prepareIndex().setIndex("test").setType("doc").setId("1").setSource(jsonBuilder().startObject().field("a", "string").field("z", 0).endObject()).get(); + client().admin().indices().prepareRefresh("test").get(); + assertThat(client().prepareSearch("test").get().getHits().getTotalHits(), equalTo(1l)); + + // both fields should be in local mapper + DocumentMapper mapper = indexService.mapperService().documentMapper("doc"); + assertNotNull(mapper.mappers().name("a")); + assertNotNull(mapper.mappers().name("z")); + + // both fields should be in the cluster state + getMappingsResponse = client().admin().indices().prepareGetMappings("test").get(); + assertNotNull(getMappingsResponse.getMappings().get("test").get("doc")); + Map mappings = getMappingsResponse.getMappings().get("test").get("doc").getSourceAsMap(); + assertNotNull(((LinkedHashMap) mappings.get("properties")).get("a")); + assertNotNull(((LinkedHashMap) mappings.get("properties")).get("z")); + } } \ No newline at end of file