From 2e5e45161ecbc7d10f66ffef64b8d10868969e01 Mon Sep 17 00:00:00 2001 From: Yu Date: Fri, 7 Jul 2017 11:49:09 +0200 Subject: [PATCH] Disable date field mapping changing (#25285) Make date field mapping unchangeable. Closes #25271 --- .../index/mapper/DateFieldMapper.java | 19 +++++++-------- .../index/mapper/ParentFieldMapper.java | 8 +------ .../index/mapper/DateFieldMapperTests.java | 23 +++++++++++++++++++ .../index/mapper/DateFieldTypeTests.java | 4 ++-- .../mapper/DocumentMapperMergeTests.java | 7 ------ 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index f94334e3822..229b42af6ee 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -54,6 +54,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.ArrayList; import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter; /** A {@link FieldMapper} for ip addresses. */ @@ -211,16 +212,12 @@ public class DateFieldMapper extends FieldMapper { @Override public void checkCompatibility(MappedFieldType fieldType, List conflicts, boolean strict) { super.checkCompatibility(fieldType, conflicts, strict); - if (strict) { - DateFieldType other = (DateFieldType)fieldType; - if (Objects.equals(dateTimeFormatter().format(), other.dateTimeFormatter().format()) == false) { - conflicts.add("mapper [" + name() - + "] is used by multiple types. Set update_all_types to true to update [format] across all types."); - } - if (Objects.equals(dateTimeFormatter().locale(), other.dateTimeFormatter().locale()) == false) { - conflicts.add("mapper [" + name() - + "] is used by multiple types. Set update_all_types to true to update [locale] across all types."); - } + DateFieldType other = (DateFieldType) fieldType; + if (Objects.equals(dateTimeFormatter().format(), other.dateTimeFormatter().format()) == false) { + conflicts.add("mapper [" + name() + "] has different [format] values"); + } + if (Objects.equals(dateTimeFormatter().locale(), other.dateTimeFormatter().locale()) == false) { + conflicts.add("mapper [" + name() + "] has different [locale] values"); } } @@ -490,8 +487,8 @@ public class DateFieldMapper extends FieldMapper { @Override protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { + final DateFieldMapper other = (DateFieldMapper) mergeWith; super.doMerge(mergeWith, updateAllTypes); - DateFieldMapper other = (DateFieldMapper) mergeWith; this.includeInAll = other.includeInAll; if (other.ignoreMalformed.explicit()) { this.ignoreMalformed = other.ignoreMalformed; diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java index 49f5b820f8e..482935437ed 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java @@ -296,19 +296,13 @@ public class ParentFieldMapper extends MetadataFieldMapper { @Override protected void doMerge(Mapper mergeWith, boolean updateAllTypes) { + ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith; ParentFieldType currentFieldType = (ParentFieldType) fieldType.clone(); super.doMerge(mergeWith, updateAllTypes); - ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith; if (fieldMergeWith.parentType != null && Objects.equals(parentType, fieldMergeWith.parentType) == false) { throw new IllegalArgumentException("The _parent field's type option can't be changed: [" + parentType + "]->[" + fieldMergeWith.parentType + "]"); } - List conflicts = new ArrayList<>(); - fieldType().checkCompatibility(fieldMergeWith.fieldType, conflicts, true); - if (conflicts.isEmpty() == false) { - throw new IllegalArgumentException("Merge conflicts: " + conflicts); - } - if (active()) { fieldType = currentFieldType; } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index 24bfc930306..747e4b498da 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -37,6 +37,7 @@ import java.io.IOException; import java.util.Collection; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.notNullValue; public class DateFieldMapperTests extends ESSingleNodeTestCase { @@ -345,4 +346,26 @@ public class DateFieldMapperTests extends ESSingleNodeTestCase { assertEquals(randomDate.withZone(DateTimeZone.UTC).getMillis(), fields[0].numericValue().longValue()); } + + public void testMergeDate() throws IOException { + String initMapping = XContentFactory.jsonBuilder().startObject().startObject("movie") + .startObject("properties") + .startObject("release_date").field("type", "date").field("format", "yyyy/MM/dd").endObject() + .endObject().endObject().endObject().string(); + DocumentMapper initMapper = indexService.mapperService().merge("movie", new CompressedXContent(initMapping), + MapperService.MergeReason.MAPPING_UPDATE, randomBoolean()); + + assertThat(initMapper.mappers().getMapper("release_date"), notNullValue()); + assertFalse(initMapper.mappers().getMapper("release_date").fieldType().stored()); + + String updateFormatMapping = XContentFactory.jsonBuilder().startObject().startObject("movie") + .startObject("properties") + .startObject("release_date").field("type", "date").field("format", "epoch_millis").endObject() + .endObject().endObject().endObject().string(); + + Exception e = expectThrows(IllegalArgumentException.class, + () -> indexService.mapperService().merge("movie", new CompressedXContent(updateFormatMapping), + MapperService.MergeReason.MAPPING_UPDATE, randomBoolean())); + assertThat(e.getMessage(), containsString("[mapper [release_date] has different [format] values]")); + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java index f6a224e8a4b..925ae210b08 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java @@ -58,13 +58,13 @@ public class DateFieldTypeTests extends FieldTypeTestCase { @Before public void setupProperties() { setDummyNullValue(10); - addModifier(new Modifier("format", true) { + addModifier(new Modifier("format", false) { @Override public void modify(MappedFieldType ft) { ((DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT)); } }); - addModifier(new Modifier("locale", true) { + addModifier(new Modifier("locale", false) { @Override public void modify(MappedFieldType ft) { ((DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("date_optional_time", Locale.CANADA)); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java index 198992b41a0..e0f3bb9ccb1 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java @@ -25,13 +25,6 @@ import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.analysis.NamedAnalyzer; -import org.elasticsearch.index.mapper.DocumentFieldMappers; -import org.elasticsearch.index.mapper.DocumentMapper; -import org.elasticsearch.index.mapper.DocumentMapperParser; -import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.mapper.Mapping; -import org.elasticsearch.index.mapper.ObjectMapper; -import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.test.ESSingleNodeTestCase; import java.io.IOException;