From c01b377ea844e66efae63d1cc017dbcb5ad79408 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 26 Aug 2015 14:32:52 -0700 Subject: [PATCH] Mappings: Fix numerous checks for equality and compatibility The field type tests for mappings had a huge hole: check compatibility was not tested directly at all! I had meant for this to happen in a follow up after #8871, and was relying on existing mapping tests. However, there were a number of issues. This change reworks the fieldtype tests to be able to check all settable properties on a field type work with checkCompatibility. It fixes a handful of small bugs in various field types. In particular, analyzer comparison was just wrong: it was comparing reference equality for search analyzer instead of the analyzer name. There was also no check for search quote analyzer. closes #13112 --- .../index/analysis/NamedAnalyzer.java | 15 + .../index/mapper/MappedFieldType.java | 52 ++-- .../index/mapper/core/BinaryFieldMapper.java | 9 + .../mapper/core/CompletionFieldMapper.java | 30 +- .../index/mapper/geo/GeoPointFieldMapper.java | 16 +- .../index/mapper/geo/GeoShapeFieldMapper.java | 17 +- .../internal/FieldNamesFieldMapper.java | 1 + .../index/mapper/FieldTypeLookupTests.java | 4 +- .../index/mapper/FieldTypeTestCase.java | 281 +++++++++++++++--- .../mapper/core/BinaryFieldTypeTests.java | 22 +- .../mapper/core/BooleanFieldTypeTests.java | 7 +- .../index/mapper/core/ByteFieldTypeTests.java | 7 +- .../mapper/core/CompletionFieldTypeTests.java | 45 ++- .../index/mapper/core/DateFieldTypeTests.java | 40 +-- .../mapper/core/DoubleFieldTypeTests.java | 7 +- .../mapper/core/FloatFieldTypeTests.java | 7 +- .../mapper/core/IntegerFieldTypeTests.java | 7 +- .../index/mapper/core/LongFieldTypeTests.java | 7 +- .../mapper/core/ShortFieldTypeTests.java | 7 +- .../mapper/geo/GeoPointFieldMapperTests.java | 5 +- .../mapper/geo/GeoPointFieldTypeTests.java | 44 ++- .../mapper/geo/GeoShapeFieldMapperTests.java | 8 +- .../mapper/geo/GeoShapeFieldTypeTests.java | 60 ++-- .../internal/FieldNamesFieldTypeTests.java | 22 +- .../merge/JavaMultiFieldMergeTests.java | 8 +- .../string/SimpleStringMappingTests.java | 2 +- .../timestamp/TimestampMappingTests.java | 13 +- .../update/UpdateMappingOnClusterIT.java | 16 +- 28 files changed, 552 insertions(+), 207 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/analysis/NamedAnalyzer.java b/core/src/main/java/org/elasticsearch/index/analysis/NamedAnalyzer.java index f3a50390667..25ff8f96834 100644 --- a/core/src/main/java/org/elasticsearch/index/analysis/NamedAnalyzer.java +++ b/core/src/main/java/org/elasticsearch/index/analysis/NamedAnalyzer.java @@ -22,6 +22,8 @@ package org.elasticsearch.index.analysis; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.DelegatingAnalyzerWrapper; +import java.util.Objects; + /** * Named analyzer is an analyzer wrapper around an actual analyzer ({@link #analyzer} that is associated * with a name ({@link #name()}. @@ -104,4 +106,17 @@ public class NamedAnalyzer extends DelegatingAnalyzerWrapper { throw new IllegalStateException("NamedAnalyzer cannot be wrapped with a wrapper, only a delegator"); } }; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof NamedAnalyzer)) return false; + NamedAnalyzer that = (NamedAnalyzer) o; + return Objects.equals(name, that.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } } 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 65113e61033..112a8f4b480 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java @@ -192,13 +192,24 @@ public abstract class MappedFieldType extends FieldType { public boolean equals(Object o) { if (!super.equals(o)) return false; MappedFieldType fieldType = (MappedFieldType) o; + // check similarity first because we need to check the name, and it might be null + // TODO: SimilarityProvider should have equals? + if (similarity == null || fieldType.similarity == null) { + if (similarity != fieldType.similarity) { + return false; + } + } else { + if (Objects.equals(similarity.name(), fieldType.similarity.name()) == false) { + return false; + } + } + return boost == fieldType.boost && docValues == fieldType.docValues && Objects.equals(names, fieldType.names) && Objects.equals(indexAnalyzer, fieldType.indexAnalyzer) && Objects.equals(searchAnalyzer, fieldType.searchAnalyzer) && Objects.equals(searchQuoteAnalyzer(), fieldType.searchQuoteAnalyzer()) && - Objects.equals(similarity, fieldType.similarity) && Objects.equals(normsLoading, fieldType.normsLoading) && Objects.equals(fieldDataType, fieldType.fieldDataType) && Objects.equals(nullValue, fieldType.nullValue) && @@ -207,10 +218,11 @@ public abstract class MappedFieldType extends FieldType { @Override public int hashCode() { - return Objects.hash(super.hashCode(), names, boost, docValues, indexAnalyzer, searchAnalyzer, searchQuoteAnalyzer, similarity, normsLoading, fieldDataType, nullValue, nullValueAsString); + return Objects.hash(super.hashCode(), names, boost, docValues, indexAnalyzer, searchAnalyzer, searchQuoteAnalyzer, + similarity == null ? null : similarity.name(), normsLoading, fieldDataType, nullValue, nullValueAsString); } -// norelease: we need to override freeze() and add safety checks that all settings are actually set + // 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(); @@ -234,51 +246,48 @@ public abstract class MappedFieldType extends FieldType { boolean mergeWithIndexed = other.indexOptions() != IndexOptions.NONE; // TODO: should be validating if index options go "up" (but "down" is ok) if (indexed != mergeWithIndexed || tokenized() != other.tokenized()) { - conflicts.add("mapper [" + names().fullName() + "] has different index values"); + conflicts.add("mapper [" + names().fullName() + "] has different [index] values"); } if (stored() != other.stored()) { - conflicts.add("mapper [" + names().fullName() + "] has different store values"); + 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 implicitly set // when the doc_values field data format is configured - conflicts.add("mapper [" + names().fullName() + "] has different doc_values values"); + conflicts.add("mapper [" + names().fullName() + "] has different [doc_values] values, cannot change from disabled to enabled"); } if (omitNorms() && !other.omitNorms()) { - conflicts.add("mapper [" + names().fullName() + "] cannot enable norms (`norms.enabled`)"); - } - if (tokenized() != other.tokenized()) { - conflicts.add("mapper [" + names().fullName() + "] has different tokenize values"); + conflicts.add("mapper [" + names().fullName() + "] has different [omit_norms] values, cannot change from disable to enabled"); } if (storeTermVectors() != other.storeTermVectors()) { - conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector values"); + conflicts.add("mapper [" + names().fullName() + "] has different [store_term_vector] values"); } if (storeTermVectorOffsets() != other.storeTermVectorOffsets()) { - conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_offsets values"); + conflicts.add("mapper [" + names().fullName() + "] has different [store_term_vector_offsets] values"); } if (storeTermVectorPositions() != other.storeTermVectorPositions()) { - conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_positions values"); + conflicts.add("mapper [" + names().fullName() + "] has different [store_term_vector_positions] values"); } if (storeTermVectorPayloads() != other.storeTermVectorPayloads()) { - conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_payloads values"); + conflicts.add("mapper [" + names().fullName() + "] has different [store_term_vector_payloads] values"); } // null and "default"-named index analyzers both mean the default is used if (indexAnalyzer() == null || "default".equals(indexAnalyzer().name())) { if (other.indexAnalyzer() != null && "default".equals(other.indexAnalyzer().name()) == false) { - conflicts.add("mapper [" + names().fullName() + "] has different analyzer"); + conflicts.add("mapper [" + names().fullName() + "] has different [analyzer]"); } } else if (other.indexAnalyzer() == null || "default".equals(other.indexAnalyzer().name())) { - conflicts.add("mapper [" + names().fullName() + "] has different analyzer"); + conflicts.add("mapper [" + names().fullName() + "] has different [analyzer]"); } else if (indexAnalyzer().name().equals(other.indexAnalyzer().name()) == false) { - conflicts.add("mapper [" + names().fullName() + "] has different analyzer"); + conflicts.add("mapper [" + names().fullName() + "] has different [analyzer]"); } if (!names().indexName().equals(other.names().indexName())) { - conflicts.add("mapper [" + names().fullName() + "] has different index_name"); + conflicts.add("mapper [" + names().fullName() + "] has different [index_name]"); } if (Objects.equals(similarity(), other.similarity()) == false) { - conflicts.add("mapper [" + names().fullName() + "] has different similarity"); + conflicts.add("mapper [" + names().fullName() + "] has different [similarity]"); } if (strict) { @@ -289,11 +298,14 @@ public abstract class MappedFieldType extends FieldType { conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [boost] across all types."); } if (normsLoading() != other.normsLoading()) { - conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [norms].loading across all types."); + conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [norms.loading] across all types."); } if (Objects.equals(searchAnalyzer(), other.searchAnalyzer()) == false) { conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types."); } + if (Objects.equals(searchQuoteAnalyzer(), other.searchQuoteAnalyzer()) == false) { + conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types."); + } if (Objects.equals(fieldDataType(), other.fieldDataType()) == false) { conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [fielddata] across all types."); } 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 d225b3f6ae6..78d038526b3 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 @@ -134,6 +134,15 @@ public class BinaryFieldMapper extends FieldMapper { return CONTENT_TYPE; } + @Override + public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { + super.checkCompatibility(fieldType, conflicts, strict); + BinaryFieldType other = (BinaryFieldType)fieldType; + if (tryUncompressing() != other.tryUncompressing()) { + conflicts.add("mapper [" + names().fullName() + "] has different [try_uncompressing] (IMPOSSIBLE)"); + } + } + public boolean tryUncompressing() { return tryUncompressing; } 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 dab41b4a78a..7eb01fd352e 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 @@ -57,6 +57,7 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedMap; @@ -237,6 +238,27 @@ public class CompletionFieldMapper extends FieldMapper { this.contextMapping = ref.contextMapping; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof CompletionFieldType)) return false; + if (!super.equals(o)) return false; + CompletionFieldType fieldType = (CompletionFieldType) o; + return analyzingSuggestLookupProvider.getPreserveSep() == fieldType.analyzingSuggestLookupProvider.getPreserveSep() && + analyzingSuggestLookupProvider.getPreservePositionsIncrements() == fieldType.analyzingSuggestLookupProvider.getPreservePositionsIncrements() && + analyzingSuggestLookupProvider.hasPayloads() == fieldType.analyzingSuggestLookupProvider.hasPayloads() && + Objects.equals(getContextMapping(), fieldType.getContextMapping()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), + analyzingSuggestLookupProvider.getPreserveSep(), + analyzingSuggestLookupProvider.getPreservePositionsIncrements(), + analyzingSuggestLookupProvider.hasPayloads(), + getContextMapping()); + } + @Override public CompletionFieldType clone() { return new CompletionFieldType(this); @@ -252,16 +274,16 @@ public class CompletionFieldMapper extends FieldMapper { super.checkCompatibility(fieldType, conflicts, strict); CompletionFieldType other = (CompletionFieldType)fieldType; if (analyzingSuggestLookupProvider.hasPayloads() != other.analyzingSuggestLookupProvider.hasPayloads()) { - conflicts.add("mapper [" + names().fullName() + "] has different payload values"); + conflicts.add("mapper [" + names().fullName() + "] has different [payload] values"); } if (analyzingSuggestLookupProvider.getPreservePositionsIncrements() != other.analyzingSuggestLookupProvider.getPreservePositionsIncrements()) { - conflicts.add("mapper [" + names().fullName() + "] has different 'preserve_position_increments' values"); + conflicts.add("mapper [" + names().fullName() + "] has different [preserve_position_increments] values"); } if (analyzingSuggestLookupProvider.getPreserveSep() != other.analyzingSuggestLookupProvider.getPreserveSep()) { - conflicts.add("mapper [" + names().fullName() + "] has different 'preserve_separators' values"); + conflicts.add("mapper [" + names().fullName() + "] has different [preserve_separators] values"); } if(!ContextMapping.mappingsAreEqual(getContextMapping(), other.getContextMapping())) { - conflicts.add("mapper [" + names().fullName() + "] has different 'context_mapping' values"); + conflicts.add("mapper [" + names().fullName() + "] has different [context_mapping] values"); } } 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 8addceff7e0..c958998fcf6 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 @@ -350,20 +350,26 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper super.checkCompatibility(fieldType, conflicts, strict); GeoPointFieldType other = (GeoPointFieldType)fieldType; if (isLatLonEnabled() != other.isLatLonEnabled()) { - conflicts.add("mapper [" + names().fullName() + "] has different lat_lon"); + conflicts.add("mapper [" + names().fullName() + "] has different [lat_lon]"); } if (isGeohashEnabled() != other.isGeohashEnabled()) { - conflicts.add("mapper [" + names().fullName() + "] has different geohash"); + conflicts.add("mapper [" + names().fullName() + "] has different [geohash]"); } if (geohashPrecision() != other.geohashPrecision()) { - conflicts.add("mapper [" + names().fullName() + "] has different geohash_precision"); + conflicts.add("mapper [" + names().fullName() + "] has different [geohash_precision]"); } if (isGeohashPrefixEnabled() != other.isGeohashPrefixEnabled()) { - conflicts.add("mapper [" + names().fullName() + "] has different geohash_prefix"); + conflicts.add("mapper [" + names().fullName() + "] has different [geohash_prefix]"); } if (isLatLonEnabled() && other.isLatLonEnabled() && latFieldType().numericPrecisionStep() != other.latFieldType().numericPrecisionStep()) { - conflicts.add("mapper [" + names().fullName() + "] has different precision_step"); + conflicts.add("mapper [" + names().fullName() + "] has different [precision_step]"); + } + if (ignoreMalformed() != other.ignoreMalformed()) { + conflicts.add("mapper [" + names().fullName() + "] has different [ignore_malformed]"); + } + if (coerce() != other.coerce()) { + conflicts.add("mapper [" + names().fullName() + "] has different [coerce]"); } } 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 3e7aa39e42e..c28285f95a5 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 @@ -280,21 +280,30 @@ public class GeoShapeFieldMapper extends FieldMapper { GeoShapeFieldType other = (GeoShapeFieldType)fieldType; // prevent user from changing strategies if (strategyName().equals(other.strategyName()) == false) { - conflicts.add("mapper [" + names().fullName() + "] has different strategy"); + conflicts.add("mapper [" + names().fullName() + "] has different [strategy]"); } // prevent user from changing trees (changes encoding) if (tree().equals(other.tree()) == false) { - conflicts.add("mapper [" + names().fullName() + "] has different tree"); + conflicts.add("mapper [" + names().fullName() + "] has different [tree]"); } // TODO we should allow this, but at the moment levels is used to build bookkeeping variables // in lucene's SpatialPrefixTree implementations, need a patch to correct that first if (treeLevels() != other.treeLevels()) { - conflicts.add("mapper [" + names().fullName() + "] has different tree_levels"); + conflicts.add("mapper [" + names().fullName() + "] has different [tree_levels]"); } if (precisionInMeters() != other.precisionInMeters()) { - conflicts.add("mapper [" + names().fullName() + "] has different precision"); + conflicts.add("mapper [" + names().fullName() + "] has different [precision]"); + } + + if (strict) { + if (orientation() != other.orientation()) { + conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [orientation] across all types."); + } + if (distanceErrorPct() != other.distanceErrorPct()) { + conflicts.add("mapper [" + names().fullName() + "] is used by multiple types. Set update_all_types to true to update [distance_error_pct] across all types."); + } } } 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 53c07c41309..ac2ef99c20b 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 @@ -167,6 +167,7 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper { @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { + super.checkCompatibility(fieldType, conflicts, strict); if (strict) { FieldNamesFieldType other = (FieldNamesFieldType)fieldType; if (isEnabled() != other.isEnabled()) { 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 5ea01ee244e..41a44d4a88c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -182,14 +182,14 @@ public class FieldTypeLookupTests extends ESTestCase { lookup.checkCompatibility(newList(f3), false); fail("expected conflict"); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("has different store values")); + assertTrue(e.getMessage().contains("has different [store] values")); } // even with updateAllTypes == true, incompatible try { lookup.checkCompatibility(newList(f3), true); fail("expected conflict"); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("has different store values")); + assertTrue(e.getMessage().contains("has different [store] values")); } } 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 a27a4344c56..a45348d530c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java @@ -18,57 +18,197 @@ */ package org.elasticsearch.index.mapper; -import org.elasticsearch.common.lucene.Lucene; +import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.fielddata.FieldDataType; import org.elasticsearch.index.similarity.BM25SimilarityProvider; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** Base test case for subclasses of MappedFieldType */ public abstract class FieldTypeTestCase extends ESTestCase { + /** Abstraction for mutating a property of a MappedFieldType */ + public static abstract class Modifier { + /** The name of the property that is being modified. Used in test failure messages. */ + public final String property; + /** true if this modifier only makes types incompatible in strict mode, false otherwise */ + public final boolean strictOnly; + /** true if reversing the order of checkCompatibility arguments should result in the same conflicts, false otherwise **/ + public final boolean symmetric; + + public Modifier(String property, boolean strictOnly, boolean symmetric) { + this.property = property; + this.strictOnly = strictOnly; + this.symmetric = symmetric; + } + + /** Modifies the property */ + public abstract void modify(MappedFieldType ft); + /** + * Optional method to implement that allows the field type that will be compared to be modified, + * so that it does not have the default value for the property being modified. + */ + public void normalizeOther(MappedFieldType other) {} + } + + private final List modifiers = new ArrayList<>(Arrays.asList( + new Modifier("boost", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setBoost(1.1f); + } + }, + new Modifier("doc_values", false, false) { + @Override + public void modify(MappedFieldType ft) { + ft.setHasDocValues(ft.hasDocValues() == false); + } + }, + new Modifier("analyzer", false, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setIndexAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + }, + new Modifier("analyzer", false, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setIndexAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + @Override + public void normalizeOther(MappedFieldType other) { + other.setIndexAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer())); + } + }, + new Modifier("search_analyzer", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSearchAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + }, + new Modifier("search_analyzer", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSearchAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + @Override + public void normalizeOther(MappedFieldType other) { + other.setSearchAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer())); + } + }, + new Modifier("search_quote_analyzer", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSearchQuoteAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + }, + new Modifier("search_quote_analyzer", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSearchQuoteAnalyzer(new NamedAnalyzer("bar", new StandardAnalyzer())); + } + @Override + public void normalizeOther(MappedFieldType other) { + other.setSearchQuoteAnalyzer(new NamedAnalyzer("foo", new StandardAnalyzer())); + } + }, + new Modifier("similarity", false, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSimilarity(new BM25SimilarityProvider("foo", Settings.EMPTY)); + } + }, + new Modifier("similarity", false, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setSimilarity(new BM25SimilarityProvider("foo", Settings.EMPTY)); + } + @Override + public void normalizeOther(MappedFieldType other) { + other.setSimilarity(new BM25SimilarityProvider("bar", Settings.EMPTY)); + } + }, + new Modifier("norms.loading", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setNormsLoading(MappedFieldType.Loading.LAZY); + } + }, + new Modifier("fielddata", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setFieldDataType(new FieldDataType("foo", Settings.builder().put("loading", "eager").build())); + } + }, + new Modifier("null_value", true, true) { + @Override + public void modify(MappedFieldType ft) { + ft.setNullValue(dummyNullValue); + } + } + )); + + /** + * Add a mutation that will be tested for all expected semantics of equality and compatibility. + * These should be added in an @Before method. + */ + protected void addModifier(Modifier modifier) { + modifiers.add(modifier); + } + + private Object dummyNullValue = "dummyvalue"; + + /** Sets the null value used by the modifier for null value testing. This should be set in an @Before method. */ + protected void setDummyNullValue(Object value) { + dummyNullValue = value; + } + /** Create a default constructed fieldtype */ protected abstract MappedFieldType createDefaultFieldType(); - MappedFieldType createNamedDefaultFieldType(String name) { + MappedFieldType createNamedDefaultFieldType() { MappedFieldType fieldType = createDefaultFieldType(); - fieldType.setNames(new MappedFieldType.Names(name)); + fieldType.setNames(new MappedFieldType.Names("foo")); return fieldType; } - /** A dummy null value to use when modifying null value */ - protected Object dummyNullValue() { - return "dummyvalue"; - } - - /** Returns the number of properties that can be modified for the fieldtype */ - protected int numProperties() { - return 10; - } - - /** Modifies a property, identified by propNum, on the given fieldtype */ - protected void modifyProperty(MappedFieldType ft, int propNum) { - switch (propNum) { - case 0: ft.setNames(new MappedFieldType.Names("dummy")); break; - case 1: ft.setBoost(1.1f); break; - case 2: ft.setHasDocValues(!ft.hasDocValues()); break; - case 3: ft.setIndexAnalyzer(Lucene.STANDARD_ANALYZER); break; - case 4: ft.setSearchAnalyzer(Lucene.STANDARD_ANALYZER); break; - case 5: ft.setSearchQuoteAnalyzer(Lucene.STANDARD_ANALYZER); break; - case 6: ft.setSimilarity(new BM25SimilarityProvider("foo", Settings.EMPTY)); break; - case 7: ft.setNormsLoading(MappedFieldType.Loading.LAZY); break; - case 8: ft.setFieldDataType(new FieldDataType("foo", Settings.builder().put("loading", "eager").build())); break; - case 9: ft.setNullValue(dummyNullValue()); break; - default: fail("unknown fieldtype property number " + propNum); + // TODO: remove this once toString is no longer final on FieldType... + protected void assertFieldTypeEquals(String property, MappedFieldType ft1, MappedFieldType ft2) { + if (ft1.equals(ft2) == false) { + fail("Expected equality, testing property " + property + "\nexpected: " + toString(ft1) + "; \nactual: " + toString(ft2) + "\n"); } } - // TODO: remove this once toString is no longer final on FieldType... - protected void assertEquals(int i, MappedFieldType ft1, MappedFieldType ft2) { - assertEquals("prop " + i + "\nexpected: " + toString(ft1) + "; \nactual: " + toString(ft2), ft1, ft2); + protected void assertFieldTypeNotEquals(String property, MappedFieldType ft1, MappedFieldType ft2) { + if (ft1.equals(ft2)) { + fail("Expected inequality, testing property " + property + "\nfirst: " + toString(ft1) + "; \nsecond: " + toString(ft2) + "\n"); + } + } + + protected void assertCompatible(String msg, MappedFieldType ft1, MappedFieldType ft2, boolean strict) { + List conflicts = new ArrayList<>(); + ft1.checkCompatibility(ft2, conflicts, strict); + assertTrue("Found conflicts for " + msg + ": " + conflicts, conflicts.isEmpty()); + } + + protected void assertNotCompatible(String msg, MappedFieldType ft1, MappedFieldType ft2, boolean strict, String... messages) { + assert messages.length != 0; + List conflicts = new ArrayList<>(); + ft1.checkCompatibility(ft2, conflicts, strict); + for (String message : messages) { + boolean found = false; + for (String conflict : conflicts) { + if (conflict.contains(message)) { + found = true; + } + } + assertTrue("Missing conflict for " + msg + ": [" + message + "] in conflicts " + conflicts, found); + } } protected String toString(MappedFieldType ft) { @@ -88,45 +228,50 @@ public abstract class FieldTypeTestCase extends ESTestCase { } public void testClone() { - MappedFieldType fieldType = createNamedDefaultFieldType("foo"); + MappedFieldType fieldType = createNamedDefaultFieldType(); MappedFieldType clone = fieldType.clone(); assertNotSame(clone, fieldType); assertEquals(clone.getClass(), fieldType.getClass()); assertEquals(clone, fieldType); assertEquals(clone, clone.clone()); // transitivity - for (int i = 0; i < numProperties(); ++i) { - fieldType = createNamedDefaultFieldType("foo"); - modifyProperty(fieldType, i); + for (Modifier modifier : modifiers) { + fieldType = createNamedDefaultFieldType(); + modifier.modify(fieldType); clone = fieldType.clone(); assertNotSame(clone, fieldType); - assertEquals(i, clone, fieldType); + assertFieldTypeEquals(modifier.property, clone, fieldType); } } public void testEquals() { - MappedFieldType ft1 = createNamedDefaultFieldType("foo"); - MappedFieldType ft2 = createNamedDefaultFieldType("foo"); + MappedFieldType ft1 = createNamedDefaultFieldType(); + MappedFieldType ft2 = createNamedDefaultFieldType(); assertEquals(ft1, ft1); // reflexive assertEquals(ft1, ft2); // symmetric assertEquals(ft2, ft1); assertEquals(ft1.hashCode(), ft2.hashCode()); - for (int i = 0; i < numProperties(); ++i) { - ft2 = createNamedDefaultFieldType("foo"); - modifyProperty(ft2, i); - assertNotEquals(ft1, ft2); - assertNotEquals(ft1.hashCode(), ft2.hashCode()); + for (Modifier modifier : modifiers) { + ft1 = createNamedDefaultFieldType(); + ft2 = createNamedDefaultFieldType(); + modifier.modify(ft2); + assertFieldTypeNotEquals(modifier.property, ft1, ft2); + assertNotEquals("hash code for modified property " + modifier.property, ft1.hashCode(), ft2.hashCode()); + // modify the same property and they are equal again + modifier.modify(ft1); + assertFieldTypeEquals(modifier.property, ft1, ft2); + assertEquals("hash code for modified property " + modifier.property, ft1.hashCode(), ft2.hashCode()); } } public void testFreeze() { - for (int i = 0; i < numProperties(); ++i) { - MappedFieldType fieldType = createNamedDefaultFieldType("foo"); + for (Modifier modifier : modifiers) { + MappedFieldType fieldType = createNamedDefaultFieldType(); fieldType.freeze(); try { - modifyProperty(fieldType, i); - fail("expected already frozen exception for property " + i); + modifier.modify(fieldType); + fail("expected already frozen exception for property " + modifier.property); } catch (IllegalStateException e) { assertTrue(e.getMessage().contains("already frozen")); } @@ -134,7 +279,7 @@ public abstract class FieldTypeTestCase extends ESTestCase { } public void testCheckTypeName() { - final MappedFieldType fieldType = createNamedDefaultFieldType("foo"); + final MappedFieldType fieldType = createNamedDefaultFieldType(); List conflicts = new ArrayList<>(); fieldType.checkTypeName(fieldType, conflicts); assertTrue(conflicts.toString(), conflicts.isEmpty()); @@ -164,4 +309,46 @@ public abstract class FieldTypeTestCase extends ESTestCase { assertTrue(conflicts.get(0).contains("cannot be changed from type")); assertEquals(1, conflicts.size()); } + + public void testCheckCompatibility() { + MappedFieldType ft1 = createNamedDefaultFieldType(); + MappedFieldType ft2 = createNamedDefaultFieldType(); + assertCompatible("default", ft1, ft2, true); + assertCompatible("default", ft1, ft2, false); + assertCompatible("default", ft2, ft1, true); + assertCompatible("default", ft2, ft1, false); + + for (Modifier modifier : modifiers) { + ft1 = createNamedDefaultFieldType(); + ft2 = createNamedDefaultFieldType(); + modifier.normalizeOther(ft1); + modifier.modify(ft2); + if (modifier.strictOnly) { + String[] conflicts = { + "mapper [foo] is used by multiple types", + "update [" + modifier.property + "]" + }; + assertCompatible(modifier.property, ft1, ft2, false); + assertNotCompatible(modifier.property, ft1, ft2, true, conflicts); + assertCompatible(modifier.property, ft2, ft1, false); // always symmetric when not strict + if (modifier.symmetric) { + assertNotCompatible(modifier.property, ft2, ft1, true, conflicts); + } else { + assertCompatible(modifier.property, ft2, ft1, true); + } + } else { + // not compatible whether strict or not + String conflict = "different [" + modifier.property + "]"; + assertNotCompatible(modifier.property, ft1, ft2, true, conflict); + assertNotCompatible(modifier.property, ft1, ft2, false, conflict); + if (modifier.symmetric) { + assertNotCompatible(modifier.property, ft2, ft1, true, conflict); + assertNotCompatible(modifier.property, ft2, ft1, false, conflict); + } else { + assertCompatible(modifier.property, ft2, ft1, true); + assertCompatible(modifier.property, ft2, ft1, false); + } + } + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java index 47403c8b1df..f241d555e12 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/BinaryFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class BinaryFieldTypeTests extends FieldTypeTestCase { @@ -28,17 +29,14 @@ public class BinaryFieldTypeTests extends FieldTypeTestCase { return new BinaryFieldMapper.BinaryFieldType(); } - @Override - protected int numProperties() { - return 1 + super.numProperties(); - } - - @Override - protected void modifyProperty(MappedFieldType ft, int propNum) { - BinaryFieldMapper.BinaryFieldType bft = (BinaryFieldMapper.BinaryFieldType)ft; - switch (propNum) { - case 0: bft.setTryUncompressing(!bft.tryUncompressing()); break; - default: super.modifyProperty(ft, propNum - 1); - } + @Before + public void setupProperties() { + addModifier(new Modifier("try_uncompressing", false, true) { + @Override + public void modify(MappedFieldType ft) { + BinaryFieldMapper.BinaryFieldType bft = (BinaryFieldMapper.BinaryFieldType)ft; + bft.setTryUncompressing(!bft.tryUncompressing()); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldTypeTests.java index 3485a383cd6..0800e4ae3d9 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class BooleanFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class BooleanFieldTypeTests extends FieldTypeTestCase { return new BooleanFieldMapper.BooleanFieldType(); } - @Override - protected Object dummyNullValue() { - return true; + @Before + public void setupProperties() { + setDummyNullValue(true); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/ByteFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/ByteFieldTypeTests.java index 33bf21e2710..08697ccd364 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/ByteFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/ByteFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class ByteFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class ByteFieldTypeTests extends FieldTypeTestCase { return new ByteFieldMapper.ByteFieldType(); } - @Override - protected Object dummyNullValue() { - return (byte)10; + @Before + public void setupProperties() { + setDummyNullValue((byte)10); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/CompletionFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/CompletionFieldTypeTests.java index b5afc1ae672..55dd7f8f7c9 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/CompletionFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/CompletionFieldTypeTests.java @@ -20,10 +20,53 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.suggest.completion.AnalyzingCompletionLookupProvider; +import org.elasticsearch.search.suggest.context.ContextBuilder; +import org.elasticsearch.search.suggest.context.ContextMapping; +import org.junit.Before; + +import java.util.SortedMap; +import java.util.TreeMap; public class CompletionFieldTypeTests extends FieldTypeTestCase { @Override protected MappedFieldType createDefaultFieldType() { - return new CompletionFieldMapper.CompletionFieldType(); + CompletionFieldMapper.CompletionFieldType ft = new CompletionFieldMapper.CompletionFieldType(); + ft.setProvider(new AnalyzingCompletionLookupProvider(true, false, true, false)); + return ft; + } + + @Before + public void setupProperties() { + addModifier(new Modifier("preserve_separators", false, true) { + @Override + public void modify(MappedFieldType ft) { + CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft; + cft.setProvider(new AnalyzingCompletionLookupProvider(false, false, true, false)); + } + }); + addModifier(new Modifier("preserve_position_increments", false, true) { + @Override + public void modify(MappedFieldType ft) { + CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft; + cft.setProvider(new AnalyzingCompletionLookupProvider(true, false, false, false)); + } + }); + addModifier(new Modifier("payload", false, true) { + @Override + public void modify(MappedFieldType ft) { + CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft; + cft.setProvider(new AnalyzingCompletionLookupProvider(true, false, true, true)); + } + }); + addModifier(new Modifier("context_mapping", false, true) { + @Override + public void modify(MappedFieldType ft) { + CompletionFieldMapper.CompletionFieldType cft = (CompletionFieldMapper.CompletionFieldType)ft; + SortedMap contextMapping = new TreeMap<>(); + contextMapping.put("foo", ContextBuilder.location("foo").build()); + cft.setContextMapping(contextMapping); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java index b6e162c8c38..3c37af6f49a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/DateFieldTypeTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.common.joda.Joda; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; import java.util.Locale; import java.util.concurrent.TimeUnit; @@ -31,23 +32,26 @@ public class DateFieldTypeTests extends FieldTypeTestCase { return new DateFieldMapper.DateFieldType(); } - @Override - protected Object dummyNullValue() { - return 10; - } - - @Override - protected int numProperties() { - return 2 + super.numProperties(); - } - - @Override - protected void modifyProperty(MappedFieldType ft, int propNum) { - DateFieldMapper.DateFieldType dft = (DateFieldMapper.DateFieldType)ft; - switch (propNum) { - case 0: dft.setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT)); break; - case 1: dft.setTimeUnit(TimeUnit.HOURS); break; - default: super.modifyProperty(ft, propNum - 2); - } + @Before + public void setupProperties() { + setDummyNullValue(10); + addModifier(new Modifier("format", true, true) { + @Override + public void modify(MappedFieldType ft) { + ((DateFieldMapper.DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT)); + } + }); + addModifier(new Modifier("locale", true, true) { + @Override + public void modify(MappedFieldType ft) { + ((DateFieldMapper.DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("date_optional_time", Locale.CANADA)); + } + }); + addModifier(new Modifier("numeric_resolution", true, true) { + @Override + public void modify(MappedFieldType ft) { + ((DateFieldMapper.DateFieldType)ft).setTimeUnit(TimeUnit.HOURS); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/DoubleFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/DoubleFieldTypeTests.java index 247f5e2364d..5f34e746ece 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/DoubleFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/DoubleFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class DoubleFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class DoubleFieldTypeTests extends FieldTypeTestCase { return new DoubleFieldMapper.DoubleFieldType(); } - @Override - protected Object dummyNullValue() { - return 10.0D; + @Before + public void setupProperties() { + setDummyNullValue(10.0D); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/FloatFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/FloatFieldTypeTests.java index 934f656ae76..73d593ac2f6 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/FloatFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/FloatFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class FloatFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class FloatFieldTypeTests extends FieldTypeTestCase { return new DoubleFieldMapper.DoubleFieldType(); } - @Override - protected Object dummyNullValue() { - return 10.0; + @Before + public void setupProperties() { + setDummyNullValue(10.0); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/IntegerFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/IntegerFieldTypeTests.java index 6e4a49e758b..b8c40af72c5 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/IntegerFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/IntegerFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class IntegerFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class IntegerFieldTypeTests extends FieldTypeTestCase { return new IntegerFieldMapper.IntegerFieldType(); } - @Override - protected Object dummyNullValue() { - return 10; + @Before + public void setupProperties() { + setDummyNullValue(10); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/LongFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/LongFieldTypeTests.java index 671483e8978..e7b41bf21d1 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/LongFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/LongFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class LongFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class LongFieldTypeTests extends FieldTypeTestCase { return new LongFieldMapper.LongFieldType(); } - @Override - protected Object dummyNullValue() { - return (long)10; + @Before + public void setupProperties() { + setDummyNullValue((long)10); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/core/ShortFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/core/ShortFieldTypeTests.java index 8a10da5cac2..12ddf827f43 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/core/ShortFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/core/ShortFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.core; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class ShortFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,8 +28,8 @@ public class ShortFieldTypeTests extends FieldTypeTestCase { return new ShortFieldMapper.ShortFieldType(); } - @Override - protected Object dummyNullValue() { - return (short)10; + @Before + public void setupProperties() { + setDummyNullValue((short)10); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java index 1ca1c3a2837..4d61ecf397e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java @@ -626,9 +626,10 @@ public class GeoPointFieldMapperTests extends ESSingleNodeTestCase { MergeResult mergeResult = stage1.merge(stage2.mapping(), false, false); assertThat(mergeResult.hasConflicts(), equalTo(true)); - assertThat(mergeResult.buildConflicts().length, equalTo(1)); + assertThat(mergeResult.buildConflicts().length, equalTo(2)); // todo better way of checking conflict? - assertThat("mapper [point] has different lat_lon", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); + assertThat("mapper [point] has different [lat_lon]", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); + assertThat("mapper [point] has different [ignore_malformed]", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); // correct mapping and ensure no failures stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") 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 07a769faa61..b6ebeb90acf 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 @@ -22,6 +22,7 @@ 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; +import org.junit.Before; public class GeoPointFieldTypeTests extends FieldTypeTestCase { @Override @@ -29,20 +30,33 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase { return new GeoPointFieldMapper.GeoPointFieldType(); } - @Override - protected int numProperties() { - return 4 + super.numProperties(); - } - - @Override - protected void modifyProperty(MappedFieldType ft, int propNum) { - GeoPointFieldMapper.GeoPointFieldType gft = (GeoPointFieldMapper.GeoPointFieldType)ft; - switch (propNum) { - case 0: gft.setGeohashEnabled(new StringFieldMapper.StringFieldType(), 1, true); break; - case 1: gft.setLatLonEnabled(new DoubleFieldMapper.DoubleFieldType(), new DoubleFieldMapper.DoubleFieldType()); break; - case 2: gft.setIgnoreMalformed(!gft.ignoreMalformed()); break; - case 3: gft.setCoerce(!gft.coerce()); break; - default: super.modifyProperty(ft, propNum - 4); - } + @Before + public void setupProperties() { + addModifier(new Modifier("geohash", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoPointFieldMapper.GeoPointFieldType)ft).setGeohashEnabled(new StringFieldMapper.StringFieldType(), 1, true); + } + }); + addModifier(new Modifier("lat_lon", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoPointFieldMapper.GeoPointFieldType)ft).setLatLonEnabled(new DoubleFieldMapper.DoubleFieldType(), new DoubleFieldMapper.DoubleFieldType()); + } + }); + addModifier(new Modifier("ignore_malformed", false, true) { + @Override + public void modify(MappedFieldType ft) { + GeoPointFieldMapper.GeoPointFieldType gft = (GeoPointFieldMapper.GeoPointFieldType)ft; + gft.setIgnoreMalformed(!gft.ignoreMalformed()); + } + }); + addModifier(new Modifier("coerce", false, true) { + @Override + public void modify(MappedFieldType ft) { + GeoPointFieldMapper.GeoPointFieldType gft = (GeoPointFieldMapper.GeoPointFieldType)ft; + gft.setCoerce(!gft.coerce()); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java index b30589b8c93..b7161c3463b 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java @@ -377,10 +377,10 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase { assertThat(mergeResult.hasConflicts(), equalTo(true)); assertThat(mergeResult.buildConflicts().length, equalTo(4)); ArrayList conflicts = new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())); - assertThat("mapper [shape] has different strategy", isIn(conflicts)); - assertThat("mapper [shape] has different tree", isIn(conflicts)); - assertThat("mapper [shape] has different tree_levels", isIn(conflicts)); - assertThat("mapper [shape] has different precision", isIn(conflicts)); + assertThat("mapper [shape] has different [strategy]", isIn(conflicts)); + assertThat("mapper [shape] has different [tree]", isIn(conflicts)); + assertThat("mapper [shape] has different [tree_levels]", isIn(conflicts)); + assertThat("mapper [shape] has different [precision]", isIn(conflicts)); // verify nothing changed FieldMapper fieldMapper = stage1.mappers().getMapper("shape"); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java index 527d79c0ee9..7ce99aa737a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldTypeTests.java @@ -21,31 +21,51 @@ package org.elasticsearch.index.mapper.geo; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class GeoShapeFieldTypeTests extends FieldTypeTestCase { @Override protected MappedFieldType createDefaultFieldType() { - GeoShapeFieldMapper.GeoShapeFieldType gft = new GeoShapeFieldMapper.GeoShapeFieldType(); - gft.setNames(new MappedFieldType.Names("testgeoshape")); - return gft; + return new GeoShapeFieldMapper.GeoShapeFieldType(); } - @Override - protected int numProperties() { - return 6 + super.numProperties(); - } - - @Override - protected void modifyProperty(MappedFieldType ft, int propNum) { - GeoShapeFieldMapper.GeoShapeFieldType gft = (GeoShapeFieldMapper.GeoShapeFieldType)ft; - switch (propNum) { - case 0: gft.setTree("quadtree"); break; - case 1: gft.setStrategyName("term"); break; - case 2: gft.setTreeLevels(10); break; - case 3: gft.setPrecisionInMeters(20); break; - case 4: gft.setDefaultDistanceErrorPct(0.5); break; - case 5: gft.setOrientation(ShapeBuilder.Orientation.LEFT); break; - default: super.modifyProperty(ft, propNum - 6); - } + @Before + public void setupProperties() { + addModifier(new Modifier("tree", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setTree("quadtree"); + } + }); + addModifier(new Modifier("strategy", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setStrategyName("term"); + } + }); + addModifier(new Modifier("tree_levels", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setTreeLevels(10); + } + }); + addModifier(new Modifier("precision", false, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setPrecisionInMeters(20); + } + }); + addModifier(new Modifier("distance_error_pct", true, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setDefaultDistanceErrorPct(0.5); + } + }); + addModifier(new Modifier("orientation", true, true) { + @Override + public void modify(MappedFieldType ft) { + ((GeoShapeFieldMapper.GeoShapeFieldType)ft).setOrientation(ShapeBuilder.Orientation.LEFT); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java index 8f0ea33d37e..83aa779a61d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldTypeTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.internal; import org.elasticsearch.index.mapper.FieldTypeTestCase; import org.elasticsearch.index.mapper.MappedFieldType; +import org.junit.Before; public class FieldNamesFieldTypeTests extends FieldTypeTestCase { @Override @@ -27,17 +28,14 @@ public class FieldNamesFieldTypeTests extends FieldTypeTestCase { return new FieldNamesFieldMapper.FieldNamesFieldType(); } - @Override - protected int numProperties() { - return 1 + super.numProperties(); - } - - @Override - protected void modifyProperty(MappedFieldType ft, int propNum) { - FieldNamesFieldMapper.FieldNamesFieldType fnft = (FieldNamesFieldMapper.FieldNamesFieldType)ft; - switch (propNum) { - case 0: fnft.setEnabled(!fnft.isEnabled()); break; - default: super.modifyProperty(ft, propNum - 1); - } + @Before + public void setupProperties() { + addModifier(new Modifier("enabled", true, true) { + @Override + public void modify(MappedFieldType ft) { + FieldNamesFieldMapper.FieldNamesFieldType fnft = (FieldNamesFieldMapper.FieldNamesFieldType)ft; + fnft.setEnabled(!fnft.isEnabled()); + } + }); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java index 655069ed9e2..eec0002a6ef 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java @@ -173,15 +173,15 @@ public class JavaMultiFieldMergeTests extends ESSingleNodeTestCase { DocumentMapper docMapper4 = parser.parse(mapping); mergeResult = docMapper.merge(docMapper4.mapping(), true, false); assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(true)); - assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different index values")); - assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different store values")); + assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different [index] values")); + assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different [store] values")); mergeResult = docMapper.merge(docMapper4.mapping(), false, false); assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(true)); assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); - assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different index values")); - assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different store values")); + assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different [index] values")); + assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different [store] values")); // There are conflicts, but the `name.not_indexed3` has been added, b/c that field has no conflicts assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java index f122de90202..e6b08fb4978 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java @@ -515,7 +515,7 @@ public class SimpleStringMappingTests extends ESSingleNodeTestCase { mergeResult = defaultMapper.merge(parser.parse(updatedMapping).mapping(), true, false); assertTrue(mergeResult.hasConflicts()); assertEquals(1, mergeResult.buildConflicts().length); - assertTrue(mergeResult.buildConflicts()[0].contains("cannot enable norms")); + assertTrue(mergeResult.buildConflicts()[0].contains("different [omit_norms]")); } /** diff --git a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java index e33f898427d..057dc41f0f9 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java @@ -579,11 +579,10 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { MergeResult mergeResult = docMapper.merge(parser.parse(mapping).mapping(), true, false); List expectedConflicts = new ArrayList<>(Arrays.asList( - "mapper [_timestamp] has different index values", - "mapper [_timestamp] has different store values", + "mapper [_timestamp] has different [index] values", + "mapper [_timestamp] has different [store] values", "Cannot update default in _timestamp value. Value is 1970-01-01 now encountering 1970-01-02", - "Cannot update path in _timestamp value. Value is foo path in merged mapping is bar", - "mapper [_timestamp] has different tokenize values")); + "Cannot update path in _timestamp value. Value is foo path in merged mapping is bar")); for (String conflict : mergeResult.buildConflicts()) { assertTrue("found unexpected conflict [" + conflict + "]", expectedConflicts.remove(conflict)); @@ -618,12 +617,12 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { MergeResult mergeResult = docMapper.merge(parser.parse(mapping).mapping(), true, false); List expectedConflicts = new ArrayList<>(); - expectedConflicts.add("mapper [_timestamp] has different index values"); - expectedConflicts.add("mapper [_timestamp] has different tokenize values"); + expectedConflicts.add("mapper [_timestamp] has different [index] values"); + expectedConflicts.add("mapper [_timestamp] has different [tokenize] values"); if (indexValues.get(0).equals("not_analyzed") == false) { // if the only index value left is not_analyzed, then the doc values setting will be the same, but in the // other two cases, it will change - expectedConflicts.add("mapper [_timestamp] has different doc_values values"); + expectedConflicts.add("mapper [_timestamp] has different [doc_values] values"); } for (String conflict : mergeResult.buildConflicts()) { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java index 4d4d06bd292..4ae039a3610 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java @@ -55,14 +55,14 @@ public class UpdateMappingOnClusterIT extends ESIntegTestCase { String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/all_mapping_create_index.json"); String mappingUpdate = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/all_mapping_update_with_conflicts.json"); String[] errorMessage = {"[_all] enabled is true now encountering false", - "[_all] cannot enable norms (`norms.enabled`)", - "[_all] has different store values", - "[_all] has different store_term_vector values", - "[_all] has different store_term_vector_offsets values", - "[_all] has different store_term_vector_positions values", - "[_all] has different store_term_vector_payloads values", - "[_all] has different analyzer", - "[_all] has different similarity"}; + "[_all] has different [omit_norms] values", + "[_all] has different [store] values", + "[_all] has different [store_term_vector] values", + "[_all] has different [store_term_vector_offsets] values", + "[_all] has different [store_term_vector_positions] values", + "[_all] has different [store_term_vector_payloads] values", + "[_all] has different [analyzer]", + "[_all] has different [similarity]"}; // fielddata and search_analyzer should not report conflict testConflict(mapping, mappingUpdate, errorMessage); }