From 573c85251efbfbbf17413fd55786099b29b79253 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 23 Jun 2015 12:34:49 -0700 Subject: [PATCH] Added better error message when field types are not the same --- .../index/mapper/FieldTypeLookup.java | 14 +++-- .../index/mapper/MappedFieldType.java | 29 +++++++--- .../mapper/core/AbstractFieldMapper.java | 20 +++---- .../index/mapper/core/BinaryFieldMapper.java | 9 ++-- .../index/mapper/core/BooleanFieldMapper.java | 9 ++-- .../index/mapper/core/ByteFieldMapper.java | 5 ++ .../mapper/core/CompletionFieldMapper.java | 9 ++-- .../index/mapper/core/DateFieldMapper.java | 5 ++ .../index/mapper/core/DoubleFieldMapper.java | 7 ++- .../index/mapper/core/FloatFieldMapper.java | 5 ++ .../index/mapper/core/IntegerFieldMapper.java | 5 ++ .../index/mapper/core/LongFieldMapper.java | 5 ++ .../index/mapper/core/NumberFieldMapper.java | 1 - .../index/mapper/core/ShortFieldMapper.java | 5 ++ .../index/mapper/core/StringFieldMapper.java | 9 ++-- .../index/mapper/geo/GeoPointFieldMapper.java | 9 ++-- .../index/mapper/geo/GeoShapeFieldMapper.java | 9 ++-- .../index/mapper/internal/AllFieldMapper.java | 9 ++-- .../internal/FieldNamesFieldMapper.java | 19 ++++--- .../index/mapper/internal/IdFieldMapper.java | 8 +-- .../mapper/internal/IndexFieldMapper.java | 9 ++-- .../mapper/internal/ParentFieldMapper.java | 9 ++-- .../mapper/internal/RoutingFieldMapper.java | 9 ++-- .../mapper/internal/SourceFieldMapper.java | 9 ++-- .../mapper/internal/TypeFieldMapper.java | 9 ++-- .../index/mapper/internal/UidFieldMapper.java | 9 ++-- .../mapper/internal/VersionFieldMapper.java | 9 ++-- .../index/mapper/ip/IpFieldMapper.java | 5 ++ .../index/mapper/FieldTypeLookupTests.java | 18 ++++++- .../index/mapper/FieldTypeTestCase.java | 53 ++++++++++++++++--- .../index/mapper/MappedFieldTypeTests.java | 27 ---------- .../mapper/externalvalues/ExternalMapper.java | 23 +++++++- .../mapper/geo/GeoPointFieldTypeTests.java | 6 ++- 33 files changed, 270 insertions(+), 116 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/index/mapper/MappedFieldTypeTests.java diff --git a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index c4eab151a56..7f4d76d0b42 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -107,9 +107,12 @@ class FieldTypeLookup implements Iterable { for (FieldMapper fieldMapper : newFieldMappers) { MappedFieldTypeReference ref = fullNameToFieldType.get(fieldMapper.fieldType().names().fullName()); if (ref != null) { - boolean strict = ref.getNumAssociatedMappers() > 1 && updateAllTypes == false; List conflicts = new ArrayList<>(); - ref.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); + ref.get().checkTypeName(fieldMapper.fieldType(), conflicts); + if (conflicts.isEmpty()) { // only check compat if they are the same type + boolean strict = ref.getNumAssociatedMappers() > 1 && updateAllTypes == false; + ref.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); + } if (conflicts.isEmpty() == false) { throw new IllegalArgumentException("Mapper for [" + fieldMapper.fieldType().names().fullName() + "] conflicts with existing mapping in other types" + conflicts.toString()); } @@ -118,9 +121,12 @@ class FieldTypeLookup implements Iterable { // field type for the index name must be compatible too MappedFieldTypeReference indexNameRef = fullNameToFieldType.get(fieldMapper.fieldType().names().indexName()); if (indexNameRef != null) { - boolean strict = indexNameRef.getNumAssociatedMappers() > 1 && updateAllTypes == false; List conflicts = new ArrayList<>(); - indexNameRef.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); + ref.get().checkTypeName(fieldMapper.fieldType(), conflicts); + if (conflicts.isEmpty()) { // only check compat if they are the same type + boolean strict = indexNameRef.getNumAssociatedMappers() > 1 && updateAllTypes == false; + indexNameRef.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); + } if (conflicts.isEmpty() == false) { throw new IllegalArgumentException("Mapper for [" + fieldMapper.fieldType().names().fullName() + "] conflicts with mapping with the same index name in other types" + conflicts.toString()); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java index 9480ae83b86..c2af1255fe4 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java @@ -50,7 +50,7 @@ import java.util.Objects; /** * This defines the core properties and functions to operate on a field. */ -public class MappedFieldType extends FieldType { +public abstract class MappedFieldType extends FieldType { public static class Names { @@ -194,12 +194,17 @@ public class MappedFieldType extends FieldType { this.nullValueAsString = ref.nullValueAsString(); } - public MappedFieldType() {} - - public MappedFieldType clone() { - return new MappedFieldType(this); + public MappedFieldType() { + setTokenized(true); + setStored(false); + setStoreTermVectors(false); + setOmitNorms(false); + setIndexOptions(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS); + setBoost(1.0f); } + public abstract MappedFieldType clone(); + @Override public boolean equals(Object o) { if (!super.equals(o)) return false; @@ -224,6 +229,18 @@ public class MappedFieldType extends FieldType { // norelease: we need to override freeze() and add safety checks that all settings are actually set + /** Returns the name of this type, as would be specified in mapping properties */ + public abstract String typeName(); + + /** Checks this type is the same type as other. Adds a conflict if they are different. */ + public final void checkTypeName(MappedFieldType other, List conflicts) { + if (typeName().equals(other.typeName()) == false) { + conflicts.add("mapper [" + names().fullName() + "] cannot be changed from type [" + typeName() + "] to [" + other.typeName() + "]"); + } else if (getClass() != other.getClass()) { + throw new IllegalStateException("Type names equal for class " + getClass().getSimpleName() + " and " + other.getClass().getSimpleName()); + } + } + /** * Checks for any conflicts between this field type and other. * If strict is true, all properties must be equal. @@ -240,7 +257,7 @@ public class MappedFieldType extends FieldType { conflicts.add("mapper [" + names().fullName() + "] has different store values"); } if (hasDocValues() == false && other.hasDocValues()) { - // don't add conflict if this mapper has doc values while the mapper to merge doesn't since doc values are implicitely set + // don't add conflict if this mapper has doc values while the mapper to merge doesn't since doc values are implicitly set // when the doc_values field data format is configured conflicts.add("mapper [" + names().fullName() + "] has different doc_values values"); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index e60cba25c87..2c6a865ccd9 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -63,18 +63,6 @@ import static org.elasticsearch.index.mapper.core.TypeParsers.DOC_VALUES; public abstract class AbstractFieldMapper implements FieldMapper { public static class Defaults { - public static final MappedFieldType FIELD_TYPE = new MappedFieldType(); - - static { - FIELD_TYPE.setTokenized(true); - FIELD_TYPE.setStored(false); - FIELD_TYPE.setStoreTermVectors(false); - FIELD_TYPE.setOmitNorms(false); - FIELD_TYPE.setIndexOptions(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS); - FIELD_TYPE.setBoost(Defaults.BOOST); - FIELD_TYPE.freeze(); - } - public static final float BOOST = 1.0f; public static final ContentPath.Type PATH_TYPE = ContentPath.Type.FULL; } @@ -409,6 +397,14 @@ public abstract class AbstractFieldMapper implements FieldMapper { } AbstractFieldMapper fieldMergeWith = (AbstractFieldMapper) mergeWith; List subConflicts = new ArrayList<>(); // TODO: just expose list from MergeResult? + fieldType().checkTypeName(fieldMergeWith.fieldType(), subConflicts); + if (subConflicts.isEmpty() == false) { + // return early if field types don't match + assert subConflicts.size() == 1; + mergeResult.addConflict(subConflicts.get(0)); + return; + } + boolean strict = this.fieldTypeRef.getNumAssociatedMappers() > 1 && mergeResult.updateAllTypes() == false; fieldType().checkCompatibility(fieldMergeWith.fieldType(), subConflicts, strict); for (String conflict : subConflicts) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java index 20127b86bab..654c190cda0 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/BinaryFieldMapper.java @@ -109,9 +109,7 @@ public class BinaryFieldMapper extends AbstractFieldMapper { static final class BinaryFieldType extends MappedFieldType { private boolean tryUncompressing = false; - public BinaryFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public BinaryFieldType() {} protected BinaryFieldType(BinaryFieldType ref) { super(ref); @@ -135,6 +133,11 @@ public class BinaryFieldMapper extends AbstractFieldMapper { return Objects.hash(super.hashCode(), tryUncompressing); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + public boolean tryUncompressing() { return tryUncompressing; } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/BooleanFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/BooleanFieldMapper.java index 878ed1b89ea..5a76195df2b 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/BooleanFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/BooleanFieldMapper.java @@ -118,9 +118,7 @@ public class BooleanFieldMapper extends AbstractFieldMapper { public static final class BooleanFieldType extends MappedFieldType { - public BooleanFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public BooleanFieldType() {} protected BooleanFieldType(BooleanFieldType ref) { super(ref); @@ -131,6 +129,11 @@ public class BooleanFieldMapper extends AbstractFieldMapper { return new BooleanFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Boolean nullValue() { return (Boolean)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java index 24e4c99e8cd..4448381296f 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/ByteFieldMapper.java @@ -134,6 +134,11 @@ public class ByteFieldMapper extends NumberFieldMapper { return new ByteFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Byte nullValue() { return (Byte)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java index ca9dbb28d96..97030ea5f46 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java @@ -226,9 +226,7 @@ public class CompletionFieldMapper extends AbstractFieldMapper { private AnalyzingCompletionLookupProvider analyzingSuggestLookupProvider; private SortedMap contextMapping = ContextMapping.EMPTY_MAPPING; - public CompletionFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public CompletionFieldType() {} protected CompletionFieldType(CompletionFieldType ref) { super(ref); @@ -242,6 +240,11 @@ public class CompletionFieldMapper extends AbstractFieldMapper { return new CompletionFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { super.checkCompatibility(fieldType, conflicts, strict); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java index 54950cfe8f9..c7a290ed3bd 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java @@ -251,6 +251,11 @@ public class DateFieldMapper extends NumberFieldMapper { return Objects.hash(super.hashCode(), dateTimeFormatter.format(), timeUnit); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { super.checkCompatibility(fieldType, conflicts, strict); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java index b11c2f4384d..d0af749156c 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/DoubleFieldMapper.java @@ -126,7 +126,7 @@ public class DoubleFieldMapper extends NumberFieldMapper { } } - static final class DoubleFieldType extends NumberFieldType { + public static final class DoubleFieldType extends NumberFieldType { public DoubleFieldType() { super(NumericType.DOUBLE); @@ -141,6 +141,11 @@ public class DoubleFieldMapper extends NumberFieldMapper { return new DoubleFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Double nullValue() { return (Double)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java index a83902fb18b..9cfaa8b8999 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/FloatFieldMapper.java @@ -142,6 +142,11 @@ public class FloatFieldMapper extends NumberFieldMapper { return new FloatFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Float nullValue() { return (Float)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java index 147fea564cc..f4958ee2ee0 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/IntegerFieldMapper.java @@ -143,6 +143,11 @@ public class IntegerFieldMapper extends NumberFieldMapper { return new IntegerFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Integer nullValue() { return (Integer)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java index 388450aa01b..18596404c86 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/LongFieldMapper.java @@ -142,6 +142,11 @@ public class LongFieldMapper extends NumberFieldMapper { return new LongFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Long nullValue() { return (Long)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java index 070d0d0a001..e9d9896a101 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java @@ -136,7 +136,6 @@ public abstract class NumberFieldMapper extends AbstractFieldMapper implements A public static abstract class NumberFieldType extends MappedFieldType { public NumberFieldType(NumericType numericType) { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); setTokenized(false); setOmitNorms(true); setIndexOptions(IndexOptions.DOCS); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java index 61cb08fc56e..8522fd026a5 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/ShortFieldMapper.java @@ -140,6 +140,11 @@ public class ShortFieldMapper extends NumberFieldMapper { return new ShortFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Short nullValue() { return (Short)super.nullValue(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java index 697d1bcd317..0e0b927a550 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java @@ -186,9 +186,7 @@ public class StringFieldMapper extends AbstractFieldMapper implements AllFieldMa public static final class StringFieldType extends MappedFieldType { - public StringFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public StringFieldType() {} protected StringFieldType(StringFieldType ref) { super(ref); @@ -198,6 +196,11 @@ public class StringFieldMapper extends AbstractFieldMapper implements AllFieldMa return new StringFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java index 85a8a9f78d2..6ca88d63d41 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java @@ -287,9 +287,7 @@ public class GeoPointFieldMapper extends AbstractFieldMapper implements ArrayVal private boolean normalizeLon = true; private boolean normalizeLat = true; - public GeoPointFieldType() { - super(StringFieldMapper.Defaults.FIELD_TYPE); - } + public GeoPointFieldType() {} protected GeoPointFieldType(GeoPointFieldType ref) { super(ref); @@ -328,6 +326,11 @@ public class GeoPointFieldMapper extends AbstractFieldMapper implements ArrayVal public int hashCode() { return java.util.Objects.hash(super.hashCode(), geohashFieldType, geohashPrecision, geohashPrefixEnabled, latFieldType, lonFieldType, validateLon, validateLat, normalizeLon, normalizeLat); } + + @Override + public String typeName() { + return CONTENT_TYPE; + } @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java index f83d0575f34..3a3d568216d 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapper.java @@ -183,9 +183,7 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { private RecursivePrefixTreeStrategy recursiveStrategy; private TermQueryPrefixTreeStrategy termStrategy; - public GeoShapeFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public GeoShapeFieldType() {} protected GeoShapeFieldType(GeoShapeFieldType ref) { super(ref); @@ -221,6 +219,11 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper { return Objects.hash(super.hashCode(), tree, strategyName, treeLevels, precisionInMeters, distanceErrorPct, defaultDistanceErrorPct, orientation); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public void freeze() { super.freeze(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java index 68539208fd3..73e495b1558 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java @@ -156,9 +156,7 @@ public class AllFieldMapper extends AbstractFieldMapper implements RootMapper { static final class AllFieldType extends MappedFieldType { - public AllFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public AllFieldType() {} protected AllFieldType(AllFieldType ref) { super(ref); @@ -169,6 +167,11 @@ public class AllFieldMapper extends AbstractFieldMapper implements RootMapper { return new AllFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java index 4a6a2b0a4ec..09f970b3e6e 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java @@ -137,15 +137,18 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implements RootMa private boolean enabled = Defaults.ENABLED; - public FieldNamesFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public FieldNamesFieldType() {} protected FieldNamesFieldType(FieldNamesFieldType ref) { super(ref); this.enabled = ref.enabled; } + @Override + public FieldNamesFieldType clone() { + return new FieldNamesFieldType(this); + } + @Override public boolean equals(Object o) { if (!super.equals(o)) return false; @@ -158,6 +161,11 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implements RootMa return Objects.hash(super.hashCode(), enabled); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { if (strict) { @@ -177,11 +185,6 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implements RootMa return enabled; } - @Override - public FieldNamesFieldType clone() { - return new FieldNamesFieldType(this); - } - @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java index 5a6c7e1c121..9a52d50c5a4 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java @@ -136,9 +136,7 @@ public class IdFieldMapper extends AbstractFieldMapper implements RootMapper { static final class IdFieldType extends MappedFieldType { - public IdFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public IdFieldType() {} protected IdFieldType(IdFieldType ref) { super(ref); @@ -149,6 +147,10 @@ public class IdFieldMapper extends AbstractFieldMapper implements RootMapper { return new IdFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } @Override public String value(Object value) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java index 5f60e1abcc7..f359f5a1ca8 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java @@ -120,9 +120,7 @@ public class IndexFieldMapper extends AbstractFieldMapper implements RootMapper static final class IndexFieldType extends MappedFieldType { - public IndexFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public IndexFieldType() {} protected IndexFieldType(IndexFieldType ref) { super(ref); @@ -133,6 +131,11 @@ public class IndexFieldMapper extends AbstractFieldMapper implements RootMapper return new IndexFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java index 29d81a5ed44..08de520247a 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java @@ -147,9 +147,7 @@ public class ParentFieldMapper extends AbstractFieldMapper implements RootMapper static final class ParentFieldType extends MappedFieldType { - public ParentFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public ParentFieldType() {} protected ParentFieldType(ParentFieldType ref) { super(ref); @@ -160,6 +158,11 @@ public class ParentFieldMapper extends AbstractFieldMapper implements RootMapper return new ParentFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Uid value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java index 29368a54d0f..f4e00f9ccf1 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/RoutingFieldMapper.java @@ -125,9 +125,7 @@ public class RoutingFieldMapper extends AbstractFieldMapper implements RootMappe static final class RoutingFieldType extends MappedFieldType { - public RoutingFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public RoutingFieldType() {} protected RoutingFieldType(RoutingFieldType ref) { super(ref); @@ -138,6 +136,11 @@ public class RoutingFieldMapper extends AbstractFieldMapper implements RootMappe return new RoutingFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java index 9a831e32cc8..d6c16d291bb 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/SourceFieldMapper.java @@ -200,9 +200,7 @@ public class SourceFieldMapper extends AbstractFieldMapper implements RootMapper static final class SourceFieldType extends MappedFieldType { - public SourceFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public SourceFieldType() {} protected SourceFieldType(SourceFieldType ref) { super(ref); @@ -213,6 +211,11 @@ public class SourceFieldMapper extends AbstractFieldMapper implements RootMapper return new SourceFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public byte[] value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/TypeFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/TypeFieldMapper.java index beae9cd0bf5..fa84e6f4cbe 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/TypeFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/TypeFieldMapper.java @@ -106,9 +106,7 @@ public class TypeFieldMapper extends AbstractFieldMapper implements RootMapper { static final class TypeFieldType extends MappedFieldType { - public TypeFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public TypeFieldType() {} protected TypeFieldType(TypeFieldType ref) { super(ref); @@ -119,6 +117,11 @@ public class TypeFieldMapper extends AbstractFieldMapper implements RootMapper { return new TypeFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public String value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/UidFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/UidFieldMapper.java index 8203622444f..d030fcc7f6c 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/UidFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/UidFieldMapper.java @@ -107,9 +107,7 @@ public class UidFieldMapper extends AbstractFieldMapper implements RootMapper { static final class UidFieldType extends MappedFieldType { - public UidFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public UidFieldType() {} protected UidFieldType(UidFieldType ref) { super(ref); @@ -120,6 +118,11 @@ public class UidFieldMapper extends AbstractFieldMapper implements RootMapper { return new UidFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Uid value(Object value) { if (value == null) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/VersionFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/VersionFieldMapper.java index 6dc793c383f..327ef7aebcd 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/VersionFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/VersionFieldMapper.java @@ -90,9 +90,7 @@ public class VersionFieldMapper extends AbstractFieldMapper implements RootMappe static final class VersionFieldType extends MappedFieldType { - public VersionFieldType() { - super(AbstractFieldMapper.Defaults.FIELD_TYPE); - } + public VersionFieldType() {} protected VersionFieldType(VersionFieldType ref) { super(ref); @@ -103,6 +101,11 @@ public class VersionFieldMapper extends AbstractFieldMapper implements RootMappe return new VersionFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Long value(Object value) { if (value == null || (value instanceof Long)) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ip/IpFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ip/IpFieldMapper.java index 54218fa1ecb..5874fcc180e 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ip/IpFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ip/IpFieldMapper.java @@ -173,6 +173,11 @@ public class IpFieldMapper extends NumberFieldMapper { return new IpFieldType(this); } + @Override + public String typeName() { + return CONTENT_TYPE; + } + @Override public Long value(Object value) { if (value == null) { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java index c5885ba019c..933eb14f7de 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -175,17 +175,31 @@ public class FieldTypeLookupTests extends ElasticsearchTestCase { return Lists.newArrayList(mapper); } - // this sucks how much must be overriden just do get a dummy field mapper... + // this sucks how much must be overridden just do get a dummy field mapper... static class FakeFieldMapper extends AbstractFieldMapper { static Settings dummySettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); public FakeFieldMapper(String fullName, String indexName) { super(makeFieldType(fullName, indexName), null, null, dummySettings, null, null); } static MappedFieldType makeFieldType(String fullName, String indexName) { - MappedFieldType fieldType = Defaults.FIELD_TYPE.clone(); + FakeFieldType fieldType = new FakeFieldType(); fieldType.setNames(new MappedFieldType.Names(fullName, indexName, indexName, fullName)); return fieldType; } + static class FakeFieldType extends MappedFieldType { + public FakeFieldType() {} + protected FakeFieldType(FakeFieldType ref) { + super(ref); + } + @Override + public MappedFieldType clone() { + return new FakeFieldType(this); + } + @Override + public String typeName() { + return "faketype"; + } + } @Override public MappedFieldType defaultFieldType() { return null; } @Override diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java index 0dba5b58818..4d7e9c7137e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java @@ -24,12 +24,21 @@ import org.elasticsearch.index.fielddata.FieldDataType; import org.elasticsearch.index.similarity.BM25SimilarityProvider; import org.elasticsearch.test.ElasticsearchTestCase; +import java.util.ArrayList; +import java.util.List; + /** Base test case for subclasses of MappedFieldType */ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { /** Create a default constructed fieldtype */ protected abstract MappedFieldType createDefaultFieldType(); + MappedFieldType createNamedDefaultFieldType(String name) { + MappedFieldType fieldType = createDefaultFieldType(); + fieldType.setNames(new MappedFieldType.Names(name)); + return fieldType; + } + /** A dummy null value to use when modifying null value */ protected Object dummyNullValue() { return "dummyvalue"; @@ -79,7 +88,7 @@ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { } public void testClone() { - MappedFieldType fieldType = createDefaultFieldType(); + MappedFieldType fieldType = createNamedDefaultFieldType("foo"); MappedFieldType clone = fieldType.clone(); assertNotSame(clone, fieldType); assertEquals(clone.getClass(), fieldType.getClass()); @@ -87,7 +96,7 @@ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { assertEquals(clone, clone.clone()); // transitivity for (int i = 0; i < numProperties(); ++i) { - fieldType = createDefaultFieldType(); + fieldType = createNamedDefaultFieldType("foo"); modifyProperty(fieldType, i); clone = fieldType.clone(); assertNotSame(clone, fieldType); @@ -96,15 +105,15 @@ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { } public void testEquals() { - MappedFieldType ft1 = createDefaultFieldType(); - MappedFieldType ft2 = createDefaultFieldType(); + MappedFieldType ft1 = createNamedDefaultFieldType("foo"); + MappedFieldType ft2 = createNamedDefaultFieldType("foo"); assertEquals(ft1, ft1); // reflexive assertEquals(ft1, ft2); // symmetric assertEquals(ft2, ft1); assertEquals(ft1.hashCode(), ft2.hashCode()); for (int i = 0; i < numProperties(); ++i) { - ft2 = createDefaultFieldType(); + ft2 = createNamedDefaultFieldType("foo"); modifyProperty(ft2, i); assertNotEquals(ft1, ft2); assertNotEquals(ft1.hashCode(), ft2.hashCode()); @@ -113,7 +122,7 @@ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { public void testFreeze() { for (int i = 0; i < numProperties(); ++i) { - MappedFieldType fieldType = createDefaultFieldType(); + MappedFieldType fieldType = createNamedDefaultFieldType("foo"); fieldType.freeze(); try { modifyProperty(fieldType, i); @@ -123,4 +132,36 @@ public abstract class FieldTypeTestCase extends ElasticsearchTestCase { } } } + + public void testCheckTypeName() { + final MappedFieldType fieldType = createNamedDefaultFieldType("foo"); + List conflicts = new ArrayList<>(); + fieldType.checkTypeName(fieldType, conflicts); + assertTrue(conflicts.toString(), conflicts.isEmpty()); + + MappedFieldType bogus = new MappedFieldType() { + @Override + public MappedFieldType clone() {return null;} + @Override + public String typeName() { return fieldType.typeName();} + }; + try { + fieldType.checkTypeName(bogus, conflicts); + fail("expected bad types exception"); + } catch (IllegalStateException e) { + assertTrue(e.getMessage().contains("Type names equal")); + } + assertTrue(conflicts.toString(), conflicts.isEmpty()); + + MappedFieldType other = new MappedFieldType() { + @Override + public MappedFieldType clone() {return null;} + @Override + public String typeName() { return "othertype";} + }; + fieldType.checkTypeName(other, conflicts); + assertFalse(conflicts.isEmpty()); + assertTrue(conflicts.get(0).contains("cannot be changed from type")); + assertEquals(1, conflicts.size()); + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/MappedFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/MappedFieldTypeTests.java deleted file mode 100644 index 6cdc809ca23..00000000000 --- a/core/src/test/java/org/elasticsearch/index/mapper/MappedFieldTypeTests.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch 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; - -public class MappedFieldTypeTests extends FieldTypeTestCase { - - @Override - public MappedFieldType createDefaultFieldType() { - return new MappedFieldType(); - } -} diff --git a/core/src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java b/core/src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java index 45c6322f359..9de34988360 100755 --- a/core/src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java @@ -80,7 +80,7 @@ public class ExternalMapper extends AbstractFieldMapper { private String mapperName; public Builder(String name, String generatedValue, String mapperName) { - super(name, Defaults.FIELD_TYPE); + super(name, new ExternalFieldType()); this.builder = this; this.stringBuilder = stringField(name).store(false); this.generatedValue = generatedValue; @@ -142,6 +142,25 @@ public class ExternalMapper extends AbstractFieldMapper { } } + static class ExternalFieldType extends MappedFieldType { + + public ExternalFieldType() {} + + protected ExternalFieldType(ExternalFieldType ref) { + super(ref); + } + + @Override + public MappedFieldType clone() { + return new ExternalFieldType(this); + } + + @Override + public String typeName() { + return "faketype"; + } + } + private final String generatedValue; private final String mapperName; @@ -168,7 +187,7 @@ public class ExternalMapper extends AbstractFieldMapper { @Override public MappedFieldType defaultFieldType() { - return Defaults.FIELD_TYPE; + return new ExternalFieldType(); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java index 71f6f88d9aa..8254e0b8bec 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldTypeTests.java @@ -20,6 +20,8 @@ package org.elasticsearch.index.mapper.geo; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.core.DoubleFieldMapper; +import org.elasticsearch.index.mapper.core.StringFieldMapper; public class GeoPointFieldTypeTests extends FieldTypeTestCase { @Override @@ -36,8 +38,8 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase { protected void modifyProperty(MappedFieldType ft, int propNum) { GeoPointFieldMapper.GeoPointFieldType gft = (GeoPointFieldMapper.GeoPointFieldType)ft; switch (propNum) { - case 0: gft.setGeohashEnabled(new MappedFieldType(), 1, true); break; - case 1: gft.setLatLonEnabled(new MappedFieldType(), new MappedFieldType()); break; + case 0: gft.setGeohashEnabled(new StringFieldMapper.StringFieldType(), 1, true); break; + case 1: gft.setLatLonEnabled(new DoubleFieldMapper.DoubleFieldType(), new DoubleFieldMapper.DoubleFieldType()); break; case 2: gft.setValidateLon(!gft.validateLon()); break; case 3: gft.setValidateLat(!gft.validateLat()); break; case 4: gft.setNormalizeLon(!gft.normalizeLon()); break;