From 3b7928d568929afa064d4a78ecff78523a0f7911 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 26 Feb 2015 13:47:53 -0800 Subject: [PATCH] Mappings: Lock down _field_names field Now that we have an explicit `enabled` flag, we can lock down the field type so it is not mungeable. see #8143 closes #9912 --- docs/reference/migration/migrate_2_0.asciidoc | 1 + .../index/mapper/internal/FieldNamesFieldMapper.java | 7 +++++-- .../index/query/ExistsFilterParser.java | 8 +++++--- .../index/query/MissingFilterParser.java | 7 ++++--- .../java/org/elasticsearch/get/GetActionTests.java | 9 ++++++--- .../mapper/internal/FieldNamesFieldMapperTests.java | 12 +++++++++--- .../search/query/ExistsMissingTests.java | 4 +--- .../test/ElasticsearchIntegrationTest.java | 2 +- 8 files changed, 32 insertions(+), 18 deletions(-) diff --git a/docs/reference/migration/migrate_2_0.asciidoc b/docs/reference/migration/migrate_2_0.asciidoc index f6605eefd9f..132b6638ca0 100644 --- a/docs/reference/migration/migrate_2_0.asciidoc +++ b/docs/reference/migration/migrate_2_0.asciidoc @@ -256,6 +256,7 @@ to provide special features. They now have limited configuration options. * `_index` configuration is limited to enabling the field. * `_routing` configuration is limited to requiring the field. * `_boost` has been removed. +* `_field_names` configuration is limited to disabling the field. === Codecs diff --git a/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java index 632e76da475..3605fbaa48d 100644 --- a/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java @@ -89,6 +89,7 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implement } @Override + @Deprecated public Builder index(boolean index) { enabled(index); return super.index(index); @@ -113,7 +114,9 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implement } FieldNamesFieldMapper.Builder builder = fieldNames(); - parseField(builder, builder.name, node, parserContext); + if (parserContext.indexVersionCreated().before(Version.V_2_0_0)) { + parseField(builder, builder.name, node, parserContext); + } for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = iterator.next(); @@ -260,7 +263,7 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implement if (includeDefaults || enabledState != Defaults.ENABLED_STATE) { builder.field("enabled", enabledState.enabled); } - if (includeDefaults || fieldType().equals(Defaults.FIELD_TYPE) == false) { + if (writePre2xSettings && (includeDefaults || fieldType().equals(Defaults.FIELD_TYPE) == false)) { super.doXContentBody(builder, includeDefaults, params); } diff --git a/src/main/java/org/elasticsearch/index/query/ExistsFilterParser.java b/src/main/java/org/elasticsearch/index/query/ExistsFilterParser.java index 8e0dbef349c..dfe42cdfa38 100644 --- a/src/main/java/org/elasticsearch/index/query/ExistsFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/ExistsFilterParser.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.lucene.HashedBytesRef; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.lucene.search.XBooleanFilter; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.FieldMappers; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; @@ -82,7 +83,8 @@ public class ExistsFilterParser implements FilterParser { } public static Filter newFilter(QueryParseContext parseContext, String fieldPattern, String filterName) { - final FieldMappers fieldNamesMapper = parseContext.mapperService().indexName(FieldNamesFieldMapper.CONTENT_TYPE); + final FieldMappers fieldNamesMappers = parseContext.mapperService().indexName(FieldNamesFieldMapper.NAME); + final FieldNamesFieldMapper fieldNamesMapper = (FieldNamesFieldMapper)fieldNamesMappers.mapper(); MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern); if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) { @@ -104,14 +106,14 @@ public class ExistsFilterParser implements FilterParser { nonNullFieldMappers = smartNameFieldMappers; } Filter filter = null; - if (fieldNamesMapper!= null && fieldNamesMapper.mapper().fieldType().indexOptions() != IndexOptions.NONE) { + if (fieldNamesMapper!= null && fieldNamesMapper.enabled()) { final String f; if (smartNameFieldMappers != null && smartNameFieldMappers.hasMapper()) { f = smartNameFieldMappers.mapper().names().indexName(); } else { f = field; } - filter = fieldNamesMapper.mapper().termFilter(f, parseContext); + filter = fieldNamesMapper.termFilter(f, parseContext); } // if _field_names are not indexed, we need to go the slow way if (filter == null && smartNameFieldMappers != null && smartNameFieldMappers.hasMapper()) { diff --git a/src/main/java/org/elasticsearch/index/query/MissingFilterParser.java b/src/main/java/org/elasticsearch/index/query/MissingFilterParser.java index d6f65027570..ee3c306a24b 100644 --- a/src/main/java/org/elasticsearch/index/query/MissingFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/MissingFilterParser.java @@ -95,7 +95,8 @@ public class MissingFilterParser implements FilterParser { throw new QueryParsingException(parseContext.index(), "missing must have either existence, or null_value, or both set to true"); } - final FieldMappers fieldNamesMapper = parseContext.mapperService().indexName(FieldNamesFieldMapper.NAME); + final FieldMappers fieldNamesMappers = parseContext.mapperService().indexName(FieldNamesFieldMapper.NAME); + final FieldNamesFieldMapper fieldNamesMapper = (FieldNamesFieldMapper)fieldNamesMappers.mapper(); MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern); if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) { // automatic make the object mapper pattern @@ -124,14 +125,14 @@ public class MissingFilterParser implements FilterParser { nonNullFieldMappers = smartNameFieldMappers; } Filter filter = null; - if (fieldNamesMapper != null && fieldNamesMapper.mapper().fieldType().indexOptions() != IndexOptions.NONE) { + if (fieldNamesMapper != null && fieldNamesMapper.enabled()) { final String f; if (smartNameFieldMappers != null && smartNameFieldMappers.hasMapper()) { f = smartNameFieldMappers.mapper().names().indexName(); } else { f = field; } - filter = fieldNamesMapper.mapper().termFilter(f, parseContext); + filter = fieldNamesMapper.termFilter(f, parseContext); } // if _field_names are not indexed, we need to go the slow way if (filter == null && smartNameFieldMappers != null && smartNameFieldMappers.hasMapper()) { diff --git a/src/test/java/org/elasticsearch/get/GetActionTests.java b/src/test/java/org/elasticsearch/get/GetActionTests.java index bb2b52c9ee6..8741bfc1690 100644 --- a/src/test/java/org/elasticsearch/get/GetActionTests.java +++ b/src/test/java/org/elasticsearch/get/GetActionTests.java @@ -1144,16 +1144,20 @@ public class GetActionTests extends ElasticsearchIntegrationTest { @Test public void testGeneratedStringFieldsStored() throws IOException { indexSingleDocumentWithStringFieldsGeneratedFromText(true, randomBoolean()); - String[] fieldsList = {"_all", "_field_names"}; + String[] fieldsList = {"_all"}; + String[] alwaysNotStoredFieldsList = {"_field_names"}; // before refresh - document is only in translog assertGetFieldsNull(indexOrAlias(), "doc", "1", fieldsList); assertGetFieldsException(indexOrAlias(), "doc", "1", fieldsList); + assertGetFieldsNull(indexOrAlias(), "doc", "1", alwaysNotStoredFieldsList); refresh(); //after refresh - document is in translog and also indexed assertGetFieldsAlwaysWorks(indexOrAlias(), "doc", "1", fieldsList); + assertGetFieldsNull(indexOrAlias(), "doc", "1", alwaysNotStoredFieldsList); flush(); //after flush - document is in not anymore translog - only indexed assertGetFieldsAlwaysWorks(indexOrAlias(), "doc", "1", fieldsList); + assertGetFieldsNull(indexOrAlias(), "doc", "1", alwaysNotStoredFieldsList); } void indexSingleDocumentWithStringFieldsGeneratedFromText(boolean stored, boolean sourceEnabled) { @@ -1167,8 +1171,7 @@ public class GetActionTests extends ElasticsearchIntegrationTest { " \"mappings\": {\n" + " \"doc\": {\n" + " \"_source\" : {\"enabled\" : " + sourceEnabled + "}," + - " \"_all\" : {\"enabled\" : true, \"store\":\"" + storedString + "\" }," + - " \"_field_names\" : {\"store\":\"" + storedString + "\" }" + + " \"_all\" : {\"enabled\" : true, \"store\":\"" + storedString + "\" }" + " }\n" + " }\n" + "}"; diff --git a/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapperTests.java b/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapperTests.java index 77559e7baa9..25c6dc8d60b 100644 --- a/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapperTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapperTests.java @@ -19,6 +19,10 @@ package org.elasticsearch.index.mapper.internal; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.settings.ImmutableSettings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; @@ -114,7 +118,8 @@ public class FieldNamesFieldMapperTests extends ElasticsearchSingleNodeTest { .startObject("_field_names").field("index", "no").endObject() .endObject().endObject().string(); - DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + Settings indexSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build(); + DocumentMapper docMapper = createIndex("test", indexSettings).mapperService().documentMapperParser().parse(mapping); FieldNamesFieldMapper fieldNamesMapper = docMapper.rootMapper(FieldNamesFieldMapper.class); assertFalse(fieldNamesMapper.enabled()); @@ -127,12 +132,13 @@ public class FieldNamesFieldMapperTests extends ElasticsearchSingleNodeTest { assertNull(doc.rootDoc().get("_field_names")); } - public void testFieldTypeSettings() throws Exception { + public void testFieldTypeSettingsBackcompat() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_field_names").field("store", "yes").endObject() .endObject().endObject().string(); - DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + Settings indexSettings = ImmutableSettings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build(); + DocumentMapper docMapper = createIndex("test", indexSettings).mapperService().documentMapperParser().parse(mapping); FieldNamesFieldMapper fieldNamesMapper = docMapper.rootMapper(FieldNamesFieldMapper.class); assertTrue(fieldNamesMapper.fieldType().stored()); } diff --git a/src/test/java/org/elasticsearch/search/query/ExistsMissingTests.java b/src/test/java/org/elasticsearch/search/query/ExistsMissingTests.java index 202a0b0643b..25dfd8051f8 100644 --- a/src/test/java/org/elasticsearch/search/query/ExistsMissingTests.java +++ b/src/test/java/org/elasticsearch/search/query/ExistsMissingTests.java @@ -45,9 +45,7 @@ public class ExistsMissingTests extends ElasticsearchIntegrationTest { .startObject() .startObject("type") .startObject(FieldNamesFieldMapper.NAME) - // by setting randomly index to no we also test the pre-1.3 behavior - .field("index", randomFrom("no", "not_analyzed")) - .field("store", randomFrom("no", "yes")) + .field("enabled", randomBoolean()) .endObject() .startObject("properties") .startObject("foo") diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java b/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java index 1d732e18a91..89aba3bd19f 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java +++ b/src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java @@ -335,7 +335,7 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase .field("compress", randomBoolean()) .endObject(); } - if (compatibilityVersion().onOrAfter(Version.V_1_3_0)) { + if (compatibilityVersion().onOrAfter(Version.V_1_3_0) && compatibilityVersion().before(Version.V_2_0_0)) { // some tests rely on this BWC version behavior that we wanna keep mappings.startObject(FieldNamesFieldMapper.NAME) .startObject("fielddata")