From 4815fe700a63711fb590d45fda63025f4848fc11 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 16 Jan 2019 10:34:46 -0700 Subject: [PATCH 1/8] Add failing rolling upgrade test --- .../test/mixed_cluster/10_basic.yml | 6 ++++++ .../test/old_cluster/10_basic.yml | 18 ++++++++++++++++++ .../test/upgraded_cluster/10_basic.yml | 6 ++++++ 3 files changed, 30 insertions(+) diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml index f50a3fd9ea42d..2605836f8573c 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yml @@ -67,3 +67,9 @@ field3: value - match: { hits.total: 1 } - match: { hits.hits.0._id: q3 } + +--- +"Index with _all is available": + - do: + indices.get: + index: all-index diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml index ffa9e0ce2a6f8..9e06f767d4892 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yml @@ -203,3 +203,21 @@ tasks.get: wait_for_completion: true task_id: $task + +--- +"Create an index with _all explicitly disabled": + - skip: + features: warnings + - do: + warnings: + - "[_all] is deprecated in 6.0+ and will be removed in 7.0. As a replacement, you can use [copy_to] on mapping fields to create your own catch all field." + indices.create: + index: all-index + body: + mappings: + type: + _all: + enabled: false + properties: + field: + type: text diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml index 39c72dfd5334b..57e9c39c8790b 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml @@ -125,3 +125,9 @@ wait_for_completion: true task_id: $task_id - match: { task.headers.X-Opaque-Id: "Reindexing Again" } + +--- +"Index with _all is available": + - do: + indices.get: + index: all-index From c949fbf99476cf0bc8a75b9885e78c6c781d7444 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 24 Jan 2019 11:30:36 +0100 Subject: [PATCH 2/8] Restore a noop _all metadata field for 6x indices This commit restores a noop version of the AllFieldMapper that is instanciated only for indices created in 6x. We need this metadata field mapper to be present in this version in order to allow the upgrade of indices that explicitly disable _all (enabled: false). The mapping of these indices contains a reference to the _all field that we cannot remove in 7 so we'll need to keep this metadata mapper in 7x. Since indices created in 6x will not be compatible with 8, we'll remove this noop mapper in the next major version. Closes #37429 --- .../TransportGetFieldMappingsIndexAction.java | 4 +- ...TransportFieldCapabilitiesIndexAction.java | 3 +- .../index/mapper/AllFieldMapper.java | 170 ++++++++++++++++++ .../index/mapper/DocumentMapper.java | 5 +- .../index/mapper/DocumentMapperParser.java | 2 +- .../elasticsearch/indices/IndicesService.java | 5 +- .../indices/mapper/MapperRegistry.java | 18 +- .../index/mapper/AllFieldMapperTests.java | 67 +++++++ .../indices/IndicesModuleTests.java | 51 ++++-- .../indices/IndicesServiceTests.java | 6 +- 10 files changed, 307 insertions(+), 24 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java index 67b3fe048bea1..c7415391675fc 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java @@ -20,6 +20,7 @@ package org.elasticsearch.action.admin.indices.mapping.get; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse.FieldMappingMetaData; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.single.shard.TransportSingleShardAction; @@ -91,7 +92,8 @@ protected ShardsIterator shards(ClusterState state, InternalRequest request) { protected GetFieldMappingsResponse shardOperation(final GetFieldMappingsIndexRequest request, ShardId shardId) { assert shardId != null; IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex()); - Predicate metadataFieldPredicate = indicesService::isMetaDataField; + Version indexCreatedVersion = indexService.mapperService().getIndexSettings().getIndexVersionCreated(); + Predicate metadataFieldPredicate = (f) -> indicesService.isMetaDataField(indexCreatedVersion, f); Predicate fieldPredicate = metadataFieldPredicate.or(indicesService.getFieldFilter().apply(shardId.getIndexName())); DocumentMapper mapper = indexService.mapperService().documentMapper(); diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java index c46933578f163..01c21544047ed 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java @@ -83,7 +83,8 @@ protected FieldCapabilitiesIndexResponse shardOperation(final FieldCapabilitiesI for (String field : fieldNames) { MappedFieldType ft = mapperService.fullName(field); if (ft != null) { - if (indicesService.isMetaDataField(field) || fieldPredicate.test(ft.name())) { + if (indicesService.isMetaDataField(mapperService.getIndexSettings().getIndexVersionCreated(), field) + || fieldPredicate.test(ft.name())) { FieldCapabilities fieldCap = new FieldCapabilities(field, ft.typeName(), ft.isSearchable(), ft.isAggregatable()); responseMap.put(field, fieldCap); } else { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java new file mode 100644 index 0000000000000..0a61ba2f3c4fb --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java @@ -0,0 +1,170 @@ +/* + * 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; + +import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.Version; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.index.query.QueryShardContext; + +import java.io.IOException; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +/** + * Noop mapper that ensures that mappings created in 6x that explicitly disable the _all field + * can be restored in this version. + * + * TODO: Remove in 8 + */ +public class AllFieldMapper extends MetadataFieldMapper { + public static final String NAME = "_all"; + public static final String CONTENT_TYPE = "_all"; + + public static class Defaults { + public static final String NAME = AllFieldMapper.NAME; + public static final String INDEX_NAME = AllFieldMapper.NAME; + public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_DISABLED; + + public static final MappedFieldType FIELD_TYPE = new AllFieldType(); + + static { + FIELD_TYPE.setIndexOptions(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS); + FIELD_TYPE.setTokenized(true); + FIELD_TYPE.setName(NAME); + FIELD_TYPE.freeze(); + } + } + + public static class Builder extends MetadataFieldMapper.Builder { + public Builder(MappedFieldType existing) { + super(Defaults.NAME, existing == null ? Defaults.FIELD_TYPE : existing, Defaults.FIELD_TYPE); + builder = this; + } + + @Override + public AllFieldMapper build(BuilderContext context) { + return new AllFieldMapper(fieldType, context.indexSettings(), context.indexCreatedVersion()); + } + } + + public static class TypeParser implements MetadataFieldMapper.TypeParser { + @Override + public MetadataFieldMapper.Builder parse(String name, Map node, + ParserContext parserContext) throws MapperParsingException { + Builder builder = new Builder(parserContext.mapperService().fullName(NAME)); + for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { + Map.Entry entry = iterator.next(); + String fieldName = entry.getKey(); + if (fieldName.equals("enabled")) { + boolean enabled = XContentMapValues.nodeBooleanValue(entry.getValue(), "enabled"); + if (enabled) { + throw new IllegalArgumentException("[_all] is disabled in this version."); + } + iterator.remove(); + } + } + return builder; + } + + @Override + public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext context) { + final Settings indexSettings = context.mapperService().getIndexSettings().getSettings(); + return new AllFieldMapper(indexSettings, Defaults.FIELD_TYPE, context.indexVersionCreated()); + } + } + + static final class AllFieldType extends StringFieldType { + AllFieldType() { + } + + protected AllFieldType(AllFieldType ref) { + super(ref); + } + + @Override + public MappedFieldType clone() { + return new AllFieldType(this); + } + + @Override + public String typeName() { + return CONTENT_TYPE; + } + + @Override + public Query existsQuery(QueryShardContext context) { + return new MatchNoDocsQuery(); + } + } + + private AllFieldMapper(Settings indexSettings, MappedFieldType existing, Version indexCreatedVersion) { + this(existing.clone(), indexSettings, indexCreatedVersion); + } + + private AllFieldMapper(MappedFieldType fieldType, Settings indexSettings, Version indexCreatedVersion) { + super(NAME, fieldType, Defaults.FIELD_TYPE, indexSettings); + if (indexCreatedVersion.onOrAfter(Version.V_7_0_0)) { + throw new IllegalArgumentException("[_all] is disabled in this version."); + } + } + + @Override + public void preParse(ParseContext context) throws IOException { + } + + @Override + public void postParse(ParseContext context) throws IOException { + super.parse(context); + } + + @Override + public void parse(ParseContext context) throws IOException { + // we parse in post parse + } + + @Override + protected void parseCreateField(ParseContext context, List fields) throws IOException { + // noop mapper + return; + } + + @Override + protected String contentType() { + return CONTENT_TYPE; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + boolean includeDefaults = params.paramAsBoolean("include_defaults", false); + if (includeDefaults) { + builder.startObject(CONTENT_TYPE); + builder.field("enabled", false); + builder.endObject(); + } + return builder; + } +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 058cf68e8e1f4..044e65c7ec6fb 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -27,6 +27,7 @@ import org.apache.lucene.search.Weight; import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchGenerationException; +import org.elasticsearch.Version; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; @@ -73,7 +74,9 @@ public Builder(RootObjectMapper.Builder builder, MapperService mapperService) { final String type = rootObjectMapper.name(); final DocumentMapper existingMapper = mapperService.documentMapper(type); - final Map metadataMapperParsers = mapperService.mapperRegistry.getMetadataMapperParsers(); + final Version indexCreatedVersion = mapperService.getIndexSettings().getIndexVersionCreated(); + final Map metadataMapperParsers = + mapperService.mapperRegistry.getMetadataMapperParsers(indexCreatedVersion); for (Map.Entry entry : metadataMapperParsers.entrySet()) { final String name = entry.getKey(); final MetadataFieldMapper existingMetadataMapper = existingMapper == null diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index e388dd7ebcd00..f913b9cb6232e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -65,7 +65,7 @@ public DocumentMapperParser(IndexSettings indexSettings, MapperService mapperSer this.similarityService = similarityService; this.queryShardContextSupplier = queryShardContextSupplier; this.typeParsers = mapperRegistry.getMapperParsers(); - this.rootTypeParsers = mapperRegistry.getMetadataMapperParsers(); + this.rootTypeParsers = mapperRegistry.getMetadataMapperParsers(mapperService.getIndexSettings().getIndexVersionCreated()); indexVersionCreated = indexSettings.getIndexVersionCreated(); } diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index b881ba73a28e6..db1deb1b7d30a 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -29,6 +29,7 @@ import org.apache.lucene.util.RamUsageEstimator; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ResourceAlreadyExistsException; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.stats.CommonStats; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags.Flag; @@ -1379,8 +1380,8 @@ public Function> getFieldFilter() { /** * Returns true if the provided field is a registered metadata field (including ones registered via plugins), false otherwise. */ - public boolean isMetaDataField(String field) { - return mapperRegistry.isMetaDataField(field); + public boolean isMetaDataField(Version indexCreatedVersion, String field) { + return mapperRegistry.isMetaDataField(indexCreatedVersion, field); } /** diff --git a/server/src/main/java/org/elasticsearch/indices/mapper/MapperRegistry.java b/server/src/main/java/org/elasticsearch/indices/mapper/MapperRegistry.java index 41d563c2037ea..c79da36200e4f 100644 --- a/server/src/main/java/org/elasticsearch/indices/mapper/MapperRegistry.java +++ b/server/src/main/java/org/elasticsearch/indices/mapper/MapperRegistry.java @@ -19,6 +19,8 @@ package org.elasticsearch.indices.mapper; +import org.elasticsearch.Version; +import org.elasticsearch.index.mapper.AllFieldMapper; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.MetadataFieldMapper; import org.elasticsearch.plugins.MapperPlugin; @@ -36,6 +38,7 @@ public final class MapperRegistry { private final Map mapperParsers; private final Map metadataMapperParsers; + private final Map metadataMapperParsers6x; private final Function> fieldFilter; @@ -43,6 +46,11 @@ public MapperRegistry(Map mapperParsers, Map metadataMapperParsers, Function> fieldFilter) { this.mapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(mapperParsers)); this.metadataMapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(metadataMapperParsers)); + // add the _all field mapper for indices created in 6x + Map metadata6x = new LinkedHashMap<>(); + metadata6x.put(AllFieldMapper.NAME, new AllFieldMapper.TypeParser()); + metadata6x.putAll(metadataMapperParsers); + this.metadataMapperParsers6x = Collections.unmodifiableMap(metadata6x); this.fieldFilter = fieldFilter; } @@ -58,15 +66,15 @@ public Map getMapperParsers() { * Return a map of the meta mappers that have been registered. The * returned map uses the name of the field as a key. */ - public Map getMetadataMapperParsers() { - return metadataMapperParsers; + public Map getMetadataMapperParsers(Version indexCreatedVersion) { + return indexCreatedVersion.onOrAfter(Version.V_7_0_0) ? metadataMapperParsers : metadataMapperParsers6x; } /** - * Returns true if the provide field is a registered metadata field, false otherwise + * Returns true if the provided field is a registered metadata field, false otherwise */ - public boolean isMetaDataField(String field) { - return getMetadataMapperParsers().containsKey(field); + public boolean isMetaDataField(Version indexCreatedVersion, String field) { + return getMetadataMapperParsers(indexCreatedVersion).containsKey(field); } /** diff --git a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java index 62f3495ee172b..f685a00c02526 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java @@ -19,6 +19,8 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; @@ -26,9 +28,74 @@ import org.elasticsearch.index.IndexService; import org.elasticsearch.index.mapper.MapperService.MergeReason; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.test.VersionUtils; + +import static org.hamcrest.CoreMatchers.containsString; public class AllFieldMapperTests extends ESSingleNodeTestCase { + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + + public void testAllDisabled() throws Exception { + { + final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0); + IndexService indexService = createIndex("test_6x", + Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, version) + .build() + ); + String mappingDisabled = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_all") + .field("enabled", false) + .endObject().endObject() + ); + indexService.mapperService().merge("_doc", new CompressedXContent(mappingDisabled), MergeReason.MAPPING_UPDATE); + + String mappingEnabled = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_all") + .field("enabled", true) + .endObject().endObject() + ); + MapperParsingException exc = expectThrows(MapperParsingException.class, + () -> indexService.mapperService().merge("_doc", new CompressedXContent(mappingEnabled), MergeReason.MAPPING_UPDATE)); + assertThat(exc.getMessage(), containsString("[_all] is disabled in this version.")); + } + { + IndexService indexService = createIndex("test"); + String mappingEnabled = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_all") + .field("enabled", true) + .endObject().endObject() + ); + MapperParsingException exc = expectThrows(MapperParsingException.class, + () -> indexService.mapperService().merge("_doc", new CompressedXContent(mappingEnabled), MergeReason.MAPPING_UPDATE)); + assertThat(exc.getMessage(), containsString("unsupported parameters: [_all")); + + String mappingDisabled = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_all") + .field("enabled", false) + .endObject().endObject() + ); + exc = expectThrows(MapperParsingException.class, + () -> indexService.mapperService().merge("_doc", new CompressedXContent(mappingDisabled), MergeReason.MAPPING_UPDATE)); + assertThat(exc.getMessage(), containsString("unsupported parameters: [_all")); + + String mappingAll = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_all").endObject().endObject() + ); + exc = expectThrows(MapperParsingException.class, + () -> indexService.mapperService().merge("_doc", new CompressedXContent(mappingAll), MergeReason.MAPPING_UPDATE)); + assertThat(exc.getMessage(), containsString("unsupported parameters: [_all")); + + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().endObject()); + indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); + assertEquals("{\"_doc\":{}}", indexService.mapperService().documentMapper("_doc").mapping().toString()); + } + } + public void testUpdateDefaultSearchAnalyzer() throws Exception { IndexService indexService = createIndex("test", Settings.builder() .put("index.analysis.analyzer.default_search.type", "custom") diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java index 9b88c6ab8f20c..38322ab1f1ec3 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java @@ -19,6 +19,8 @@ package org.elasticsearch.indices; +import org.elasticsearch.Version; +import org.elasticsearch.index.mapper.AllFieldMapper; import org.elasticsearch.index.mapper.FieldNamesFieldMapper; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.IgnoredFieldMapper; @@ -36,6 +38,7 @@ import org.elasticsearch.indices.mapper.MapperRegistry; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.util.ArrayList; import java.util.Arrays; @@ -87,14 +90,34 @@ public Map getMetadataMappers() { RoutingFieldMapper.NAME, IndexFieldMapper.NAME, SourceFieldMapper.NAME, TypeFieldMapper.NAME, VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, FieldNamesFieldMapper.NAME}; + private static String[] EXPECTED_METADATA_FIELDS_6x = new String[]{AllFieldMapper.NAME, IgnoredFieldMapper.NAME, + IdFieldMapper.NAME, RoutingFieldMapper.NAME, IndexFieldMapper.NAME, SourceFieldMapper.NAME, TypeFieldMapper.NAME, + VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, FieldNamesFieldMapper.NAME}; + + public void testBuiltinMappers() { IndicesModule module = new IndicesModule(Collections.emptyList()); - assertFalse(module.getMapperRegistry().getMapperParsers().isEmpty()); - assertFalse(module.getMapperRegistry().getMetadataMapperParsers().isEmpty()); - Map metadataMapperParsers = module.getMapperRegistry().getMetadataMapperParsers(); - int i = 0; - for (String field : metadataMapperParsers.keySet()) { - assertEquals(EXPECTED_METADATA_FIELDS[i++], field); + { + Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0.minimumCompatibilityVersion()); + assertFalse(module.getMapperRegistry().getMapperParsers().isEmpty()); + assertFalse(module.getMapperRegistry().getMetadataMapperParsers(version).isEmpty()); + Map metadataMapperParsers = + module.getMapperRegistry().getMetadataMapperParsers(version); + int i = 0; + for (String field : metadataMapperParsers.keySet()) { + assertEquals(EXPECTED_METADATA_FIELDS_6x[i++], field); + } + } + { + Version version = VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT); + assertFalse(module.getMapperRegistry().getMapperParsers().isEmpty()); + assertFalse(module.getMapperRegistry().getMetadataMapperParsers(version).isEmpty()); + Map metadataMapperParsers = + module.getMapperRegistry().getMetadataMapperParsers(version); + int i = 0; + for (String field : metadataMapperParsers.keySet()) { + assertEquals(EXPECTED_METADATA_FIELDS[i++], field); + } } } @@ -102,11 +125,15 @@ public void testBuiltinWithPlugins() { IndicesModule noPluginsModule = new IndicesModule(Collections.emptyList()); IndicesModule module = new IndicesModule(fakePlugins); MapperRegistry registry = module.getMapperRegistry(); + Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0.minimumCompatibilityVersion()); assertThat(registry.getMapperParsers().size(), greaterThan(noPluginsModule.getMapperRegistry().getMapperParsers().size())); - assertThat(registry.getMetadataMapperParsers().size(), - greaterThan(noPluginsModule.getMapperRegistry().getMetadataMapperParsers().size())); - Map metadataMapperParsers = module.getMapperRegistry().getMetadataMapperParsers(); + assertThat(registry.getMetadataMapperParsers(version).size(), + greaterThan(noPluginsModule.getMapperRegistry().getMetadataMapperParsers(version).size())); + Map metadataMapperParsers = module.getMapperRegistry().getMetadataMapperParsers(version); Iterator iterator = metadataMapperParsers.keySet().iterator(); + if (version.before(Version.V_7_0_0)) { + assertEquals(AllFieldMapper.NAME, iterator.next()); + } assertEquals(IgnoredFieldMapper.NAME, iterator.next()); String last = null; while(iterator.hasNext()) { @@ -187,13 +214,15 @@ public Map getMetadataMappers() { public void testFieldNamesIsLast() { IndicesModule module = new IndicesModule(Collections.emptyList()); - List fieldNames = new ArrayList<>(module.getMapperRegistry().getMetadataMapperParsers().keySet()); + Version version = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); + List fieldNames = new ArrayList<>(module.getMapperRegistry().getMetadataMapperParsers(version).keySet()); assertEquals(FieldNamesFieldMapper.NAME, fieldNames.get(fieldNames.size() - 1)); } public void testFieldNamesIsLastWithPlugins() { IndicesModule module = new IndicesModule(fakePlugins); - List fieldNames = new ArrayList<>(module.getMapperRegistry().getMetadataMapperParsers().keySet()); + Version version = VersionUtils.randomCompatibleVersion(random(), Version.CURRENT); + List fieldNames = new ArrayList<>(module.getMapperRegistry().getMetadataMapperParsers(version).keySet()); assertEquals(FieldNamesFieldMapper.NAME, fieldNames.get(fieldNames.size() - 1)); } diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index b68ec6d0598e5..60dbad99795f3 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -68,6 +68,7 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.IndexSettingsModule; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.hamcrest.RegexMatcher; import java.io.IOException; @@ -515,9 +516,10 @@ public void testStatsByShardDoesNotDieFromExpectedExceptions() { public void testIsMetaDataField() { IndicesService indicesService = getIndicesService(); - assertFalse(indicesService.isMetaDataField(randomAlphaOfLengthBetween(10, 15))); + final Version randVersion = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.CURRENT); + assertFalse(indicesService.isMetaDataField(randVersion, randomAlphaOfLengthBetween(10, 15))); for (String builtIn : IndicesModule.getBuiltInMetaDataFields()) { - assertTrue(indicesService.isMetaDataField(builtIn)); + assertTrue(indicesService.isMetaDataField(randVersion, builtIn)); } } From 88b5f9bc36f6cfc6efd2b9f9ae0103e47814923b Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 24 Jan 2019 12:32:41 +0100 Subject: [PATCH 3/8] fix mapper serialization when not explicitly disabled --- .../index/mapper/AllFieldMapper.java | 37 +++++++++++-------- .../index/mapper/DocumentMapperParser.java | 4 +- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java index 0a61ba2f3c4fb..1427e67c5389b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java @@ -23,7 +23,6 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.Version; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.support.XContentMapValues; @@ -45,10 +44,6 @@ public class AllFieldMapper extends MetadataFieldMapper { public static final String CONTENT_TYPE = "_all"; public static class Defaults { - public static final String NAME = AllFieldMapper.NAME; - public static final String INDEX_NAME = AllFieldMapper.NAME; - public static final EnabledAttributeMapper ENABLED = EnabledAttributeMapper.UNSET_DISABLED; - public static final MappedFieldType FIELD_TYPE = new AllFieldType(); static { @@ -60,14 +55,21 @@ public static class Defaults { } public static class Builder extends MetadataFieldMapper.Builder { + private boolean disableExplicit = false; + public Builder(MappedFieldType existing) { - super(Defaults.NAME, existing == null ? Defaults.FIELD_TYPE : existing, Defaults.FIELD_TYPE); + super(NAME, existing == null ? Defaults.FIELD_TYPE : existing, Defaults.FIELD_TYPE); builder = this; } + private Builder setDisableExplicit() { + this.disableExplicit = true; + return this; + } + @Override public AllFieldMapper build(BuilderContext context) { - return new AllFieldMapper(fieldType, context.indexSettings(), context.indexCreatedVersion()); + return new AllFieldMapper(fieldType, context.indexSettings(), disableExplicit); } } @@ -84,6 +86,7 @@ public MetadataFieldMapper.Builder parse(String name, Map n if (enabled) { throw new IllegalArgumentException("[_all] is disabled in this version."); } + builder.setDisableExplicit(); iterator.remove(); } } @@ -93,7 +96,7 @@ public MetadataFieldMapper.Builder parse(String name, Map n @Override public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext context) { final Settings indexSettings = context.mapperService().getIndexSettings().getSettings(); - return new AllFieldMapper(indexSettings, Defaults.FIELD_TYPE, context.indexVersionCreated()); + return new AllFieldMapper(indexSettings, Defaults.FIELD_TYPE, false); } } @@ -121,15 +124,15 @@ public Query existsQuery(QueryShardContext context) { } } - private AllFieldMapper(Settings indexSettings, MappedFieldType existing, Version indexCreatedVersion) { - this(existing.clone(), indexSettings, indexCreatedVersion); + private final boolean disableExplicit; + + private AllFieldMapper(Settings indexSettings, MappedFieldType existing, boolean disableExplicit) { + this(existing.clone(), indexSettings, disableExplicit); } - private AllFieldMapper(MappedFieldType fieldType, Settings indexSettings, Version indexCreatedVersion) { + private AllFieldMapper(MappedFieldType fieldType, Settings indexSettings, boolean disableExplicit) { super(NAME, fieldType, Defaults.FIELD_TYPE, indexSettings); - if (indexCreatedVersion.onOrAfter(Version.V_7_0_0)) { - throw new IllegalArgumentException("[_all] is disabled in this version."); - } + this.disableExplicit = disableExplicit; } @Override @@ -160,9 +163,11 @@ protected String contentType() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { boolean includeDefaults = params.paramAsBoolean("include_defaults", false); - if (includeDefaults) { + if (includeDefaults || disableExplicit) { builder.startObject(CONTENT_TYPE); - builder.field("enabled", false); + if (disableExplicit) { + builder.field("enabled", false); + } builder.endObject(); } return builder; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index f913b9cb6232e..db7954e9bd76a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -65,8 +65,8 @@ public DocumentMapperParser(IndexSettings indexSettings, MapperService mapperSer this.similarityService = similarityService; this.queryShardContextSupplier = queryShardContextSupplier; this.typeParsers = mapperRegistry.getMapperParsers(); - this.rootTypeParsers = mapperRegistry.getMetadataMapperParsers(mapperService.getIndexSettings().getIndexVersionCreated()); - indexVersionCreated = indexSettings.getIndexVersionCreated(); + this.indexVersionCreated = indexSettings.getIndexVersionCreated(); + this.rootTypeParsers = mapperRegistry.getMetadataMapperParsers(indexVersionCreated); } public Mapper.TypeParser.ParserContext parserContext(String type) { From cdaa804e87ec5a7a276d31e6a71eb06d8d69924c Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 24 Jan 2019 14:35:58 +0100 Subject: [PATCH 4/8] address review --- .../rest-api-spec/test/upgraded_cluster/10_basic.yml | 8 ++++++++ .../org/elasticsearch/indices/IndicesModuleTests.java | 2 ++ 2 files changed, 10 insertions(+) diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml index 57e9c39c8790b..508a898e0cdb5 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yml @@ -131,3 +131,11 @@ - do: indices.get: index: all-index + + - do: + indices.get_mapping: + index: all-index + + - is_true: all-index.mappings._all + - match: { all-index.mappings._all.enabled: false} + diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java index 38322ab1f1ec3..f31ac0627138e 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesModuleTests.java @@ -103,6 +103,7 @@ public void testBuiltinMappers() { assertFalse(module.getMapperRegistry().getMetadataMapperParsers(version).isEmpty()); Map metadataMapperParsers = module.getMapperRegistry().getMetadataMapperParsers(version); + assertEquals(EXPECTED_METADATA_FIELDS_6x.length, metadataMapperParsers.size()); int i = 0; for (String field : metadataMapperParsers.keySet()) { assertEquals(EXPECTED_METADATA_FIELDS_6x[i++], field); @@ -114,6 +115,7 @@ public void testBuiltinMappers() { assertFalse(module.getMapperRegistry().getMetadataMapperParsers(version).isEmpty()); Map metadataMapperParsers = module.getMapperRegistry().getMetadataMapperParsers(version); + assertEquals(EXPECTED_METADATA_FIELDS.length, metadataMapperParsers.size()); int i = 0; for (String field : metadataMapperParsers.keySet()) { assertEquals(EXPECTED_METADATA_FIELDS[i++], field); From e6519f4ba92ba5233d544e6b5b9b2e3df653ef08 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 25 Jan 2019 10:00:04 +0100 Subject: [PATCH 5/8] remove _all from exists query tests --- .../org/elasticsearch/index/query/ExistsQueryBuilderTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java index ad02209bf5dae..5b71d465b9a46 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java @@ -55,8 +55,6 @@ protected ExistsQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { if (randomBoolean()) { fieldPattern = fieldPattern + "*"; - } else { - fieldPattern = MetaData.ALL; } } return new ExistsQueryBuilder(fieldPattern); From 1e72b4dd41943054a77b38e9d67d26d1dd28b2f4 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 25 Jan 2019 15:23:02 +0100 Subject: [PATCH 6/8] fix checkstyle --- .../org/elasticsearch/index/query/ExistsQueryBuilderTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java index 5b71d465b9a46..a5329856630d5 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java @@ -28,7 +28,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.elasticsearch.Version; -import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; From 7d9b4a1c1bd439f904eb02bc0939e54ca0115b37 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 28 Jan 2019 14:29:01 +0100 Subject: [PATCH 7/8] fix test bug --- .../org/elasticsearch/index/mapper/AllFieldMapperTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java index f685a00c02526..15c758b35b8c0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java @@ -41,7 +41,7 @@ protected boolean forbidPrivateIndexSettings() { public void testAllDisabled() throws Exception { { - final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0); + final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0.minimumCompatibilityVersion()); IndexService indexService = createIndex("test_6x", Settings.builder() .put(IndexMetaData.SETTING_VERSION_CREATED, version) From 1dd9765687e9e04b150422c137d57c2a15f54b64 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 28 Jan 2019 15:03:32 +0100 Subject: [PATCH 8/8] line len --- .../org/elasticsearch/index/mapper/AllFieldMapperTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java index 15c758b35b8c0..34200b51cb317 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/AllFieldMapperTests.java @@ -41,7 +41,8 @@ protected boolean forbidPrivateIndexSettings() { public void testAllDisabled() throws Exception { { - final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_7_0_0.minimumCompatibilityVersion()); + final Version version = VersionUtils.randomVersionBetween(random(), + Version.V_6_0_0, Version.V_7_0_0.minimumCompatibilityVersion()); IndexService indexService = createIndex("test_6x", Settings.builder() .put(IndexMetaData.SETTING_VERSION_CREATED, version)