better merge mapping logic, better failure reporting

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -147,8 +147,6 @@ public interface FieldMapper<T> {
 
     Filter fieldFilter(String value);
 
-    void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException;
-
     /**
      * Constructs a range query based on the mapper.
      */

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MergeMappingException.java

@@ -19,16 +19,21 @@
 
 package org.elasticsearch.index.mapper;
 
+import java.util.Arrays;
+
 /**
  * @author kimchy (shay.banon)
  */
 public class MergeMappingException extends MapperException {
 
-    public MergeMappingException(String message) {
+    private final String[] failures;
-        super(message);
+
+    public MergeMappingException(String[] failures) {
+        super("Merge failed with failures [" + Arrays.toString(failures) + "]");
+        this.failures = failures;
     }
 
-    public MergeMappingException(String message, Throwable cause) {
+    public String[] failures() {
-        super(message, cause);
+        return failures;
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonBoostFieldMapper.java

@@ -27,8 +27,6 @@ import org.codehaus.jackson.JsonToken;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 import org.elasticsearch.index.analysis.NumericFloatAnalyzer;
 import org.elasticsearch.index.mapper.BoostFieldMapper;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.util.Numbers;
 import org.elasticsearch.util.json.JsonBuilder;
@@ -183,7 +181,7 @@ public class JsonBoostFieldMapper extends JsonNumberFieldMapper<Float> implement
         builder.endObject();
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
         // do nothing here, no merging, but also no exception
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonDocumentMapper.java

@@ -349,7 +349,11 @@ public class JsonDocumentMapper implements DocumentMapper, ToJson {
 
     @Override public synchronized void merge(DocumentMapper mergeWith, MergeFlags mergeFlags) throws MergeMappingException {
         JsonDocumentMapper jsonMergeWith = (JsonDocumentMapper) mergeWith;
-        rootObjectMapper.mergeMapping(this, jsonMergeWith.rootObjectMapper, mergeFlags);
+        JsonMergeContext mergeContext = new JsonMergeContext(this, mergeFlags);
+        rootObjectMapper.merge(jsonMergeWith.rootObjectMapper, mergeContext);
+        if (mergeContext.hasFailures()) {
+            throw new MergeMappingException(mergeContext.buildFailures());
+        }
         if (!mergeFlags.simulate()) {
             // update the source to the merged one
             mappingSource = buildSource();

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java

@@ -25,7 +25,6 @@ import org.apache.lucene.document.Fieldable;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.*;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
-import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.FieldMapperListener;
 import org.elasticsearch.index.mapper.MergeMappingException;
@@ -321,11 +320,11 @@ public abstract class JsonFieldMapper<T> implements FieldMapper<T>, JsonMapper {
                 includeLower, includeUpper);
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
-        if (mergeFlags.ignoreDuplicates()) {
+        if (mergeContext.mergeFlags().ignoreDuplicates()) {
             return;
         }
-        throw new MergeMappingException("Mapper [" + names.fullName() + "] exists, can't merge");
+        mergeContext.addFailure("Mapper [" + names.fullName() + "] exists, can't merge");
     }
 
     @Override public int sortType() {

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonIdFieldMapper.java

@@ -22,7 +22,10 @@ package org.elasticsearch.index.mapper.json;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.Fieldable;
-import org.elasticsearch.index.mapper.*;
+import org.elasticsearch.index.mapper.FieldMapperListener;
+import org.elasticsearch.index.mapper.IdFieldMapper;
+import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.util.json.JsonBuilder;
 import org.elasticsearch.util.lucene.Lucene;
 
@@ -123,7 +126,7 @@ public class JsonIdFieldMapper extends JsonFieldMapper<String> implements IdFiel
         // for now, don't output it at all
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
         // do nothing here, no merging, but also no exception
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMapper.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.index.mapper.json;
 
 import org.elasticsearch.index.mapper.FieldMapperListener;
+import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.util.concurrent.NotThreadSafe;
 import org.elasticsearch.util.concurrent.ThreadSafe;
 import org.elasticsearch.util.json.ToJson;
@@ -63,5 +64,7 @@ public interface JsonMapper extends ToJson {
 
     void parse(JsonParseContext jsonContext) throws IOException;
 
+    void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException;
+
     void traverse(FieldMapperListener fieldMapperListener);
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMergeContext.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elastic Search and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. Elastic Search licenses this
+ * file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.index.mapper.json;
+
+import com.google.common.collect.Lists;
+import org.elasticsearch.index.mapper.DocumentMapper;
+
+import java.util.List;
+
+/**
+ * @author kimchy (shay.banon)
+ */
+public class JsonMergeContext {
+
+    private final JsonDocumentMapper documentMapper;
+
+    private final DocumentMapper.MergeFlags mergeFlags;
+
+    private final List<String> mergeFailures = Lists.newArrayList();
+
+    public JsonMergeContext(JsonDocumentMapper documentMapper, DocumentMapper.MergeFlags mergeFlags) {
+        this.documentMapper = documentMapper;
+        this.mergeFlags = mergeFlags;
+    }
+
+    public JsonDocumentMapper docMapper() {
+        return documentMapper;
+    }
+
+    public DocumentMapper.MergeFlags mergeFlags() {
+        return mergeFlags;
+    }
+
+    public void addFailure(String mergeFailure) {
+        mergeFailures.add(mergeFailure);
+    }
+
+    public boolean hasFailures() {
+        return !mergeFailures.isEmpty();
+    }
+
+    public String[] buildFailures() {
+        return mergeFailures.toArray(new String[mergeFailures.size()]);
+    }
+}

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonObjectMapper.java

@@ -23,7 +23,10 @@ import com.google.common.collect.ImmutableMap;
 import org.codehaus.jackson.JsonParser;
 import org.codehaus.jackson.JsonToken;
 import org.elasticsearch.ElasticSearchIllegalStateException;
-import org.elasticsearch.index.mapper.*;
+import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.index.mapper.FieldMapperListener;
+import org.elasticsearch.index.mapper.InternalMapper;
+import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.util.concurrent.ThreadSafe;
 import org.elasticsearch.util.joda.FormatDateTimeFormatter;
 import org.elasticsearch.util.json.JsonBuilder;
@@ -337,35 +340,25 @@ public class JsonObjectMapper implements JsonMapper {
         }
     }
 
-    public void mergeMapping(JsonDocumentMapper docMapper, JsonObjectMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
+        if (!(mergeWith instanceof JsonObjectMapper)) {
+            mergeContext.addFailure("Can't merge a non object mapping [" + mergeWith.name() + "] with an object mapping [" + name() + "]");
+            return;
+        }
+        JsonObjectMapper mergeWithObject = (JsonObjectMapper) mergeWith;
         synchronized (mutex) {
-            for (JsonMapper mapper : mergeWith.mappers.values()) {
+            for (JsonMapper mergeWithMapper : mergeWithObject.mappers.values()) {
-                if (mapper instanceof JsonObjectMapper) {
+                JsonMapper mergeIntoMapper = mappers.get(mergeWithMapper.name());
-                    JsonObjectMapper mergeWithMapper = (JsonObjectMapper) mapper;
+                if (mergeIntoMapper == null) {
-                    JsonMapper mergeIntoMapper = mappers.get(mergeWithMapper.name());
+                    // no mapping, simply add it if not simulating
-                    if (mergeIntoMapper != null) {
+                    if (!mergeContext.mergeFlags().simulate()) {
-                        if (!(mergeIntoMapper instanceof JsonObjectMapper)) {
+                        putMapper(mergeWithMapper);
-                            throw new MergeMappingException("Can't merge an object mapping [" + mergeWithMapper.name() + "] into a non object mapper");
+                        if (mergeWithMapper instanceof JsonFieldMapper) {
-                        }
+                            mergeContext.docMapper().addFieldMapper((FieldMapper) mergeWithMapper);
-                        ((JsonObjectMapper) mergeIntoMapper).mergeMapping(docMapper, mergeWithMapper, mergeFlags);
-                    } else {
-                        if (!mergeFlags.simulate()) {
-                            putMapper(mergeWithMapper);
                         }
                     }
                 } else {
-                    JsonFieldMapper mergeWithMapper = (JsonFieldMapper) mapper;
+                    mergeIntoMapper.merge(mergeWithMapper, mergeContext);
-                    JsonFieldMapper mergeIntoMapper = (JsonFieldMapper) mappers.get(mergeWithMapper.name());
-                    // not an object mapper, bail if we have it, otherwise, add
-                    // we might get fancy later on and allow per field mapper merge
-                    if (mergeIntoMapper != null) {
-                        mergeIntoMapper.merge(mergeWithMapper, mergeFlags);
-                    } else {
-                        if (!mergeFlags.simulate()) {
-                            putMapper(mergeWithMapper);
-                            docMapper.addFieldMapper(mergeWithMapper);
-                        }
-                    }
                 }
             }
         }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonSourceFieldMapper.java

@@ -20,8 +20,6 @@
 package org.elasticsearch.index.mapper.json;
 
 import org.apache.lucene.document.*;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.util.json.JsonBuilder;
@@ -135,7 +133,7 @@ public class JsonSourceFieldMapper extends JsonFieldMapper<byte[]> implements So
         // for now, don't output it at all
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
         // do nothing here, no merging, but also no exception
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonTypeFieldMapper.java

@@ -23,8 +23,6 @@ import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.Fieldable;
 import org.apache.lucene.index.Term;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.index.mapper.TypeFieldMapper;
 import org.elasticsearch.util.json.JsonBuilder;
@@ -112,7 +110,7 @@ public class JsonTypeFieldMapper extends JsonFieldMapper<String> implements Type
         // for now, don't output it at all
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
         // do nothing here, no merging, but also no exception
     }
 }

modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonUidFieldMapper.java

@@ -22,7 +22,10 @@ package org.elasticsearch.index.mapper.json;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.Fieldable;
 import org.apache.lucene.index.Term;
-import org.elasticsearch.index.mapper.*;
+import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.MergeMappingException;
+import org.elasticsearch.index.mapper.Uid;
+import org.elasticsearch.index.mapper.UidFieldMapper;
 import org.elasticsearch.util.json.JsonBuilder;
 import org.elasticsearch.util.lucene.Lucene;
 
@@ -105,7 +108,7 @@ public class JsonUidFieldMapper extends JsonFieldMapper<Uid> implements UidField
         // for now, don't output it at all
     }
 
-    @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException {
+    @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException {
         // do nothing here, no merging, but also no exception
     }
 }