From 568254887f4da94767b969082c62e1c5b378a9c5 Mon Sep 17 00:00:00 2001 From: kimchy Date: Sun, 28 Feb 2010 12:55:05 +0200 Subject: [PATCH] better merge mapping logic, better failure reporting --- .../index/mapper/FieldMapper.java | 2 - .../index/mapper/MergeMappingException.java | 13 ++-- .../mapper/json/JsonBoostFieldMapper.java | 4 +- .../index/mapper/json/JsonDocumentMapper.java | 6 +- .../index/mapper/json/JsonFieldMapper.java | 7 +-- .../index/mapper/json/JsonIdFieldMapper.java | 7 ++- .../index/mapper/json/JsonMapper.java | 3 + .../index/mapper/json/JsonMergeContext.java | 62 +++++++++++++++++++ .../index/mapper/json/JsonObjectMapper.java | 45 ++++++-------- .../mapper/json/JsonSourceFieldMapper.java | 4 +- .../mapper/json/JsonTypeFieldMapper.java | 4 +- .../index/mapper/json/JsonUidFieldMapper.java | 7 ++- 12 files changed, 114 insertions(+), 50 deletions(-) create mode 100644 modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMergeContext.java diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 0fdb4a17760..3d0e3366658 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -147,8 +147,6 @@ public interface FieldMapper { Filter fieldFilter(String value); - void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException; - /** * Constructs a range query based on the mapper. */ diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MergeMappingException.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MergeMappingException.java index 75c0ee60bee..98acdb1d329 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/MergeMappingException.java +++ b/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) { - super(message); + private final String[] failures; + + public MergeMappingException(String[] failures) { + super("Merge failed with failures [" + Arrays.toString(failures) + "]"); + this.failures = failures; } - public MergeMappingException(String message, Throwable cause) { - super(message, cause); + public String[] failures() { + return failures; } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonBoostFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonBoostFieldMapper.java index 2aeff716ea1..cbd26a1c01c 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonBoostFieldMapper.java +++ b/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 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 } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonDocumentMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonDocumentMapper.java index d398844a002..8761096a238 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonDocumentMapper.java +++ b/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(); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java index 76c41ce2728..840873eeea6 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonFieldMapper.java +++ b/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 implements FieldMapper, JsonMapper { includeLower, includeUpper); } - @Override public void merge(FieldMapper mergeWith, DocumentMapper.MergeFlags mergeFlags) throws MergeMappingException { - if (mergeFlags.ignoreDuplicates()) { + @Override public void merge(JsonMapper mergeWith, JsonMergeContext mergeContext) throws MergeMappingException { + 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() { diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonIdFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonIdFieldMapper.java index 28a82f1e69b..4272755c571 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonIdFieldMapper.java +++ b/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 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 } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMapper.java index f1be5b194e2..538242978d5 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMapper.java +++ b/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); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMergeContext.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonMergeContext.java new file mode 100644 index 00000000000..4d1a790b015 --- /dev/null +++ b/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 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()]); + } +} diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonObjectMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonObjectMapper.java index 59425e2251f..dc596b53ea3 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonObjectMapper.java +++ b/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()) { - if (mapper instanceof JsonObjectMapper) { - JsonObjectMapper mergeWithMapper = (JsonObjectMapper) mapper; - JsonMapper mergeIntoMapper = mappers.get(mergeWithMapper.name()); - if (mergeIntoMapper != null) { - if (!(mergeIntoMapper instanceof JsonObjectMapper)) { - throw new MergeMappingException("Can't merge an object mapping [" + mergeWithMapper.name() + "] into a non object mapper"); - } - ((JsonObjectMapper) mergeIntoMapper).mergeMapping(docMapper, mergeWithMapper, mergeFlags); - } else { - if (!mergeFlags.simulate()) { - putMapper(mergeWithMapper); + for (JsonMapper mergeWithMapper : mergeWithObject.mappers.values()) { + JsonMapper mergeIntoMapper = mappers.get(mergeWithMapper.name()); + if (mergeIntoMapper == null) { + // no mapping, simply add it if not simulating + if (!mergeContext.mergeFlags().simulate()) { + putMapper(mergeWithMapper); + if (mergeWithMapper instanceof JsonFieldMapper) { + mergeContext.docMapper().addFieldMapper((FieldMapper) mergeWithMapper); } } } else { - JsonFieldMapper mergeWithMapper = (JsonFieldMapper) mapper; - 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); - } - } + mergeIntoMapper.merge(mergeWithMapper, mergeContext); } } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonSourceFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonSourceFieldMapper.java index 7e1d681d18c..de26490a281 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonSourceFieldMapper.java +++ b/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 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 } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonTypeFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonTypeFieldMapper.java index f2036e44ea9..c12fde25c9e 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonTypeFieldMapper.java +++ b/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 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 } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonUidFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonUidFieldMapper.java index 87c24789f9c..b6cc0e31258 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/json/JsonUidFieldMapper.java +++ b/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 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 } }