[mappings] update dynamic fields in mapping on master even if parsing…

… fails for the rest of doc

The local DocumentMapper is updated while parsing and dynamic fields are added before
parsing has finished. If parsing fails after a dynamic field has been added already
then the field was not added to the cluster state but was present in the local mapper of this
node. New documents with the same field would not necessarily cause an update either and
after restarting the node the mapping for these fields were lost. Instead the new fields
should always be updated.

closes #9851
closes #9874

src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -438,7 +438,7 @@ public ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
         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,15 +456,15 @@ public ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
             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();
             if (token == XContentParser.Token.END_OBJECT) {
                 // 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 ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
 
             // 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) {

src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java

@@ -28,12 +28,29 @@ public class MapperParsingException extends MapperException {
 
     public MapperParsingException(String message) {
         super(message);
+        mappingsModified = false;
     }
 
-    public MapperParsingException(String message, Throwable cause) {
+    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() {

src/main/java/org/elasticsearch/index/mapper/StrictDynamicMappingException.java

@@ -24,8 +24,8 @@
  */
 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

src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java

@@ -429,7 +429,7 @@ public void parse(ParseContext context) throws IOException {
                 }
             }
         } 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 void parse(String field, ParseContext context) throws IOException {
                     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);

src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java

@@ -307,7 +307,7 @@ public void preParse(ParseContext context) throws IOException {
     @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 @@ protected void parseCreateField(ParseContext context, List<Field> fields) throws
             // 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

src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java

@@ -499,7 +499,7 @@ public void parse(ParseContext context) throws IOException {
         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 void parse(ParseContext context) throws IOException {
             } 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 @@ private void serializeNullValue(ParseContext context, String lastFieldName) thro
         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 @@ private void serializeObject(final ParseContext context, String currentFieldName
                 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 @@ private void serializeArray(ParseContext context, String lastFieldName) throws I
                 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 @@ private void serializeNonDynamicArray(ParseContext context, String lastFieldName
             } 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 @@ private void serializeNonDynamicArray(ParseContext context, String lastFieldName
 
     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 void parseDynamicValue(final ParseContext context, String currentFieldNam
             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;

src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java

@@ -231,7 +231,7 @@ public Mapper.Builder findTemplateBuilder(ParseContext context, String name, Str
         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);
     }

src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -448,7 +448,7 @@ public Engine.Create prepareCreate(SourceToParse source, long version, VersionTy
             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 Engine.Index prepareIndex(SourceToParse source, long version, VersionType
             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;

src/test/java/org/elasticsearch/index/mapper/dynamic/DynamicMappingTests.java

@@ -20,18 +20,18 @@
 
 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 boolean apply(java.lang.Object input) {
         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<String, Object> mappings = getMappingsResponse.getMappings().get("test").get("doc").getSourceAsMap();
+        assertNotNull(((LinkedHashMap) mappings.get("properties")).get("a"));
+        assertNotNull(((LinkedHashMap) mappings.get("properties")).get("z"));
+    }
 }