From 288aaba311e49e1b9a44ef04e8b76ecbdb44d023 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Thu, 17 Feb 2022 18:11:12 -0600 Subject: [PATCH] [Remove] Default Mapping Default mappings were deprecated in Legacy version 6x and cannot be added to indexes created in legacy 7+ or any version of OpenSearch. All support for default mappings are removed for OpenSearch 2+. Signed-off-by: Nicholas Walter Knize --- .../mapper/ParentJoinFieldMapperTests.java | 4 +- .../search/morelikethis/MoreLikeThisIT.java | 14 +- .../admin/indices/get/GetIndexResponse.java | 7 +- .../mapping/get/GetFieldMappingsResponse.java | 7 +- .../mapping/get/GetMappingsResponse.java | 6 +- .../action/bulk/TransportBulkAction.java | 2 +- .../action/bulk/TransportShardBulkAction.java | 2 +- .../action/index/MappingUpdatedAction.java | 4 - .../cluster/metadata/IndexMetadata.java | 41 +----- .../metadata/IndexTemplateMetadata.java | 9 +- .../cluster/metadata/MappingMetadata.java | 6 - .../metadata/MetadataCreateIndexService.java | 12 +- .../metadata/MetadataMappingService.java | 51 +++----- .../index/mapper/DocumentMapperParser.java | 16 +-- .../index/mapper/DocumentParser.java | 4 - .../index/mapper/MapperService.java | 123 +++--------------- .../cluster/metadata/IndexMetadataTests.java | 28 ---- .../MetadataCreateIndexServiceTests.java | 11 +- .../index/mapper/DocumentParserTests.java | 27 ++-- .../index/mapper/DynamicTemplatesTests.java | 4 +- .../index/mapper/MapperServiceTests.java | 81 +++++++----- .../index/mapper/NestedObjectMapperTests.java | 8 +- .../index/query/IdsQueryBuilderTests.java | 5 - .../dynamictemplate/simple/test-mapping.json | 2 +- .../index/mapper/simple/test-mapping.json | 2 +- 25 files changed, 138 insertions(+), 338 deletions(-) diff --git a/modules/parent-join/src/test/java/org/opensearch/join/mapper/ParentJoinFieldMapperTests.java b/modules/parent-join/src/test/java/org/opensearch/join/mapper/ParentJoinFieldMapperTests.java index 62040b3893e83..628345a625d1b 100644 --- a/modules/parent-join/src/test/java/org/opensearch/join/mapper/ParentJoinFieldMapperTests.java +++ b/modules/parent-join/src/test/java/org/opensearch/join/mapper/ParentJoinFieldMapperTests.java @@ -474,7 +474,7 @@ public void testUpdateRelations() throws Exception { .endObject() ); docMapper = indexService.mapperService() - .merge("_doc", new CompressedXContent(updateMapping), MapperService.MergeReason.MAPPING_UPDATE); + .merge("type", new CompressedXContent(updateMapping), MapperService.MergeReason.MAPPING_UPDATE); ParentJoinFieldMapper mapper = ParentJoinFieldMapper.getMapper(indexService.mapperService()); assertNotNull(mapper); assertEquals("join_field", mapper.name()); @@ -501,7 +501,7 @@ public void testUpdateRelations() throws Exception { .endObject() ); docMapper = indexService.mapperService() - .merge("_doc", new CompressedXContent(updateMapping), MapperService.MergeReason.MAPPING_UPDATE); + .merge("type", new CompressedXContent(updateMapping), MapperService.MergeReason.MAPPING_UPDATE); ParentJoinFieldMapper mapper = ParentJoinFieldMapper.getMapper(indexService.mapperService()); assertNotNull(mapper); assertEquals("join_field", mapper.name()); diff --git a/server/src/internalClusterTest/java/org/opensearch/search/morelikethis/MoreLikeThisIT.java b/server/src/internalClusterTest/java/org/opensearch/search/morelikethis/MoreLikeThisIT.java index 913ce487fda31..a7909daeab599 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/morelikethis/MoreLikeThisIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/morelikethis/MoreLikeThisIT.java @@ -43,6 +43,7 @@ import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; +import org.opensearch.index.mapper.MapperService; import org.opensearch.index.query.MoreLikeThisQueryBuilder; import org.opensearch.index.query.MoreLikeThisQueryBuilder.Item; import org.opensearch.index.query.QueryBuilder; @@ -740,16 +741,23 @@ public void testMoreLikeThisArtificialDocs() throws Exception { assertHitCount(response, 1); } - @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/2107") public void testMoreLikeThisMalformedArtificialDocs() throws Exception { logger.info("Creating the index ..."); - assertAcked(prepareCreate("test").addMapping("type1", "text", "type=text,analyzer=whitespace", "date", "type=date")); + assertAcked( + prepareCreate("test").addMapping( + MapperService.SINGLE_MAPPING_NAME, + "text", + "type=text,analyzer=whitespace", + "date", + "type=date" + ) + ); ensureGreen("test"); logger.info("Creating an index with a single document ..."); indexRandom( true, - client().prepareIndex("test", "type1", "1") + client().prepareIndex("test", MapperService.SINGLE_MAPPING_NAME, "1") .setSource(jsonBuilder().startObject().field("text", "Hello World!").field("date", "2009-01-01").endObject()) ); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java index aa1d5feb66c9e..7efe88e9bbc83 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java @@ -47,7 +47,6 @@ import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.common.xcontent.XContentParser.Token; -import org.opensearch.index.mapper.MapperService; import java.io.IOException; import java.util.ArrayList; @@ -301,10 +300,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } else { MappingMetadata mappings = null; for (final ObjectObjectCursor typeEntry : indexMappings) { - if (typeEntry.key.equals(MapperService.DEFAULT_MAPPING) == false) { - assert mappings == null; - mappings = typeEntry.value; - } + assert mappings == null; + mappings = typeEntry.value; } if (mappings == null) { // no mappings yet diff --git a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java index 562c92da8673b..d486a102d1a21 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java @@ -47,7 +47,6 @@ import org.opensearch.common.xcontent.XContentParser; import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.mapper.Mapper; -import org.opensearch.index.mapper.MapperService; import org.opensearch.rest.BaseRestHandler; import java.io.IOException; @@ -165,10 +164,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (includeTypeName == false) { Map mappings = null; for (Map.Entry> typeEntry : indexEntry.getValue().entrySet()) { - if (typeEntry.getKey().equals(MapperService.DEFAULT_MAPPING) == false) { - assert mappings == null; - mappings = typeEntry.getValue(); - } + assert mappings == null; + mappings = typeEntry.getValue(); } if (mappings != null) { addFieldMappingsToBuilder(builder, params, mappings); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java index f0f5265367549..0087271147f4a 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java @@ -135,10 +135,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (includeTypeName == false) { MappingMetadata mappings = null; for (final ObjectObjectCursor typeEntry : indexEntry.value) { - if (typeEntry.key.equals("_default_") == false) { - assert mappings == null; - mappings = typeEntry.value; - } + assert mappings == null; + mappings = typeEntry.value; } if (mappings == null) { // no mappings yet diff --git a/server/src/main/java/org/opensearch/action/bulk/TransportBulkAction.java b/server/src/main/java/org/opensearch/action/bulk/TransportBulkAction.java index e3d1ba3f834aa..7121636eabc90 100644 --- a/server/src/main/java/org/opensearch/action/bulk/TransportBulkAction.java +++ b/server/src/main/java/org/opensearch/action/bulk/TransportBulkAction.java @@ -552,7 +552,7 @@ protected void doRun() { prohibitCustomRoutingOnDataStream(docWriteRequest, metadata); IndexRequest indexRequest = (IndexRequest) docWriteRequest; final IndexMetadata indexMetadata = metadata.index(concreteIndex); - MappingMetadata mappingMd = indexMetadata.mappingOrDefault(); + MappingMetadata mappingMd = indexMetadata.mapping(); Version indexCreated = indexMetadata.getCreationVersion(); indexRequest.resolveRouting(metadata); indexRequest.process(indexCreated, mappingMd, concreteIndex.getName()); diff --git a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java index 1ce4a346e5dc3..5073adf13dc41 100644 --- a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java +++ b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java @@ -311,7 +311,7 @@ static boolean executeBulkItemRequest( case UPDATED: IndexRequest indexRequest = updateResult.action(); IndexMetadata metadata = context.getPrimary().indexSettings().getIndexMetadata(); - MappingMetadata mappingMd = metadata.mappingOrDefault(); + MappingMetadata mappingMd = metadata.mapping(); indexRequest.process(metadata.getCreationVersion(), mappingMd, updateRequest.concreteIndex()); context.setRequestToExecute(indexRequest); break; diff --git a/server/src/main/java/org/opensearch/cluster/action/index/MappingUpdatedAction.java b/server/src/main/java/org/opensearch/cluster/action/index/MappingUpdatedAction.java index 4bc7e61a67240..d7100a194a4bb 100644 --- a/server/src/main/java/org/opensearch/cluster/action/index/MappingUpdatedAction.java +++ b/server/src/main/java/org/opensearch/cluster/action/index/MappingUpdatedAction.java @@ -51,7 +51,6 @@ import org.opensearch.common.util.concurrent.UncategorizedExecutionException; import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.Index; -import org.opensearch.index.mapper.MapperService; import org.opensearch.index.mapper.Mapping; import java.util.concurrent.Semaphore; @@ -111,9 +110,6 @@ public void setClient(Client client) { * potentially waiting for a master node to be available. */ public void updateMappingOnMaster(Index index, String type, Mapping mappingUpdate, ActionListener listener) { - if (type.equals(MapperService.DEFAULT_MAPPING)) { - throw new IllegalArgumentException("_default_ mapping should not be updated"); - } final RunOnce release = new RunOnce(() -> semaphore.release()); try { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index 147c8987169c7..c02358d47b066 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -67,7 +67,6 @@ import org.opensearch.common.xcontent.XContentParser; import org.opensearch.gateway.MetadataStateFormat; import org.opensearch.index.Index; -import org.opensearch.index.mapper.MapperService; import org.opensearch.index.seqno.SequenceNumbers; import org.opensearch.index.shard.ShardId; import org.opensearch.rest.RestStatus; @@ -677,22 +676,11 @@ public ImmutableOpenMap getMappings() { @Nullable public MappingMetadata mapping() { for (ObjectObjectCursor cursor : mappings) { - if (cursor.key.equals(MapperService.DEFAULT_MAPPING) == false) { - return cursor.value; - } + return cursor.value; } return null; } - /** - * Get the default mapping. - * NOTE: this is always {@code null} for 7.x indices which are disallowed to have a default mapping. - */ - @Nullable - public MappingMetadata defaultMapping() { - return mappings.get(MapperService.DEFAULT_MAPPING); - } - public static final String INDEX_RESIZE_SOURCE_UUID_KEY = "index.resize.source.uuid"; public static final String INDEX_RESIZE_SOURCE_NAME_KEY = "index.resize.source.name"; public static final Setting INDEX_RESIZE_SOURCE_UUID = Setting.simpleString(INDEX_RESIZE_SOURCE_UUID_KEY); @@ -704,25 +692,6 @@ public Index getResizeSourceIndex() { : null; } - /** - * Sometimes, the default mapping exists and an actual mapping is not created yet (introduced), - * in this case, we want to return the default mapping in case it has some default mapping definitions. - *

- * Note, once the mapping type is introduced, the default mapping is applied on the actual typed MappingMetadata, - * setting its routing, timestamp, and so on if needed. - */ - @Nullable - public MappingMetadata mappingOrDefault() { - MappingMetadata mapping = null; - for (ObjectCursor m : mappings.values()) { - if (mapping == null || mapping.type().equals(MapperService.DEFAULT_MAPPING)) { - mapping = m.value; - } - } - - return mapping; - } - ImmutableOpenMap getCustomData() { return this.customData; } @@ -1337,14 +1306,6 @@ public IndexMetadata build() { ImmutableOpenMap.Builder tmpAliases = aliases; Settings tmpSettings = settings; - // update default mapping on the MappingMetadata - if (mappings.containsKey(MapperService.DEFAULT_MAPPING)) { - MappingMetadata defaultMapping = mappings.get(MapperService.DEFAULT_MAPPING); - for (ObjectCursor cursor : mappings.values()) { - cursor.value.updateDefaultMapping(defaultMapping); - } - } - /* * We expect that the metadata has been properly built to set the number of shards and the number of replicas, and do not rely * on the default values here. Those must have been set upstream. diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexTemplateMetadata.java index 7cf3c3da24c52..d08fe3b926c66 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexTemplateMetadata.java @@ -51,7 +51,6 @@ import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.index.mapper.MapperService; import java.io.IOException; import java.util.ArrayList; @@ -432,11 +431,9 @@ private static void toInnerXContent( if (includeTypeName == false) { Map documentMapping = null; for (ObjectObjectCursor cursor : indexTemplateMetadata.mappings()) { - if (!cursor.key.equals(MapperService.DEFAULT_MAPPING)) { - assert documentMapping == null; - Map mapping = XContentHelper.convertToMap(cursor.value.uncompressed(), true).v2(); - documentMapping = reduceMapping(cursor.key, mapping); - } + assert documentMapping == null; + Map mapping = XContentHelper.convertToMap(cursor.value.uncompressed(), true).v2(); + documentMapping = reduceMapping(cursor.key, mapping); } if (documentMapping != null) { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java index e3ab1d491131a..02fe7ee8db889 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java @@ -140,12 +140,6 @@ private void initMappers(Map withoutType) { } } - void updateDefaultMapping(MappingMetadata defaultMapping) { - if (routing == Routing.EMPTY) { - routing = defaultMapping.routing(); - } - } - public String type() { return this.type; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index b4cfaef223a66..4e2c475e6c4ce 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -98,7 +98,6 @@ import java.nio.file.Path; import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -474,7 +473,6 @@ private ClusterState applyCreateIndexWithTemporaryService( request.index(), aliases, indexService.mapperService()::documentMapper, - () -> indexService.mapperService().documentMapper(MapperService.DEFAULT_MAPPING), temporaryIndexMeta.getSettings(), temporaryIndexMeta.getRoutingNumShards(), sourceMetadata, @@ -1099,7 +1097,6 @@ static IndexMetadata buildIndexMetadata( String indexName, List aliases, Supplier documentMapperSupplier, - Supplier defaultDocumentMapperSupplier, Settings indexSettings, int routingNumShards, @Nullable IndexMetadata sourceMetadata, @@ -1109,11 +1106,10 @@ static IndexMetadata buildIndexMetadata( indexMetadataBuilder.system(isSystem); // now, update the mappings with the actual source Map mappingsMetadata = new HashMap<>(); - for (DocumentMapper mapper : Arrays.asList(documentMapperSupplier.get(), defaultDocumentMapperSupplier.get())) { - if (mapper != null) { - MappingMetadata mappingMd = new MappingMetadata(mapper); - mappingsMetadata.put(mapper.type(), mappingMd); - } + DocumentMapper mapper = documentMapperSupplier.get(); + if (mapper != null) { + MappingMetadata mappingMd = new MappingMetadata(mapper); + mappingsMetadata.put(mapper.type(), mappingMd); } for (MappingMetadata mappingMd : mappingsMetadata.values()) { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java index 9ed2a0f9257fc..7392dab6b0ac2 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataMappingService.java @@ -190,15 +190,11 @@ private boolean refreshIndexMapping(IndexService indexService, IndexMetadata.Bui try { List updatedTypes = new ArrayList<>(); MapperService mapperService = indexService.mapperService(); - for (DocumentMapper mapper : Arrays.asList( - mapperService.documentMapper(), - mapperService.documentMapper(MapperService.DEFAULT_MAPPING) - )) { - if (mapper != null) { - final String type = mapper.type(); - if (!mapper.mappingSource().equals(builder.mapping(type).source())) { - updatedTypes.add(type); - } + DocumentMapper mapper = mapperService.documentMapper(); + if (mapper != null) { + final String type = mapper.type(); + if (!mapper.mappingSource().equals(builder.mapping(type).source())) { + updatedTypes.add(type); } } @@ -206,13 +202,8 @@ private boolean refreshIndexMapping(IndexService indexService, IndexMetadata.Bui if (updatedTypes.isEmpty() == false) { logger.warn("[{}] re-syncing mappings with cluster state because of types [{}]", index, updatedTypes); dirty = true; - for (DocumentMapper mapper : Arrays.asList( - mapperService.documentMapper(), - mapperService.documentMapper(MapperService.DEFAULT_MAPPING) - )) { - if (mapper != null) { - builder.putMapping(new MappingMetadata(mapper)); - } + if (mapper != null) { + builder.putMapping(new MappingMetadata(mapper)); } } } catch (Exception e) { @@ -286,7 +277,6 @@ private ClusterState applyRequest( // we used for the validation, it makes this mechanism little less scary (a little) updateList.add(indexMetadata); // try and parse it (no need to add it here) so we can bail early in case of parsing exception - DocumentMapper newMapper; DocumentMapper existingMapper = mapperService.documentMapper(); String typeForUpdate = mapperService.getTypeForUpdate(mappingType, mappingUpdateSource); @@ -299,15 +289,10 @@ private ClusterState applyRequest( ); } - if (MapperService.DEFAULT_MAPPING.equals(request.type())) { - // _default_ types do not go through merging, but we do test the new settings. Also don't apply the old default - newMapper = mapperService.parse(request.type(), mappingUpdateSource, false); - } else { - newMapper = mapperService.parse(request.type(), mappingUpdateSource, existingMapper == null); - if (existingMapper != null) { - // first, simulate: just call merge and ignore the result - existingMapper.merge(newMapper.mapping(), MergeReason.MAPPING_UPDATE); - } + DocumentMapper newMapper = mapperService.parse(request.type(), mappingUpdateSource); + if (existingMapper != null) { + // first, simulate: just call merge and ignore the result + existingMapper.merge(newMapper.mapping(), MergeReason.MAPPING_UPDATE); } if (mappingType == null) { mappingType = newMapper.type(); @@ -319,9 +304,7 @@ private ClusterState applyRequest( } assert mappingType != null; - if (MapperService.DEFAULT_MAPPING.equals(mappingType) == false - && MapperService.SINGLE_MAPPING_NAME.equals(mappingType) == false - && mappingType.charAt(0) == '_') { + if (MapperService.SINGLE_MAPPING_NAME.equals(mappingType) == false && mappingType.charAt(0) == '_') { throw new InvalidTypeNameException("Document mapping type name can't start with '_', found: [" + mappingType + "]"); } Metadata.Builder builder = Metadata.builder(metadata); @@ -367,13 +350,9 @@ private ClusterState applyRequest( IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetadata); // Mapping updates on a single type may have side-effects on other types so we need to // update mapping metadata on all types - for (DocumentMapper mapper : Arrays.asList( - mapperService.documentMapper(), - mapperService.documentMapper(MapperService.DEFAULT_MAPPING) - )) { - if (mapper != null) { - indexMetadataBuilder.putMapping(new MappingMetadata(mapper.mappingSource())); - } + DocumentMapper mapper = mapperService.documentMapper(); + if (mapper != null) { + indexMetadataBuilder.putMapping(new MappingMetadata(mapper.mappingSource())); } if (updatedMapping) { indexMetadataBuilder.mappingVersion(1 + indexMetadataBuilder.mappingVersion()); diff --git a/server/src/main/java/org/opensearch/index/mapper/DocumentMapperParser.java b/server/src/main/java/org/opensearch/index/mapper/DocumentMapperParser.java index 8b406c4691018..2d6880c6b4186 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentMapperParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentMapperParser.java @@ -114,10 +114,6 @@ public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter } public DocumentMapper parse(@Nullable String type, CompressedXContent source) throws MapperParsingException { - return parse(type, source, null); - } - - public DocumentMapper parse(@Nullable String type, CompressedXContent source, String defaultSource) throws MapperParsingException { Map mapping = null; if (source != null) { Map root = XContentHelper.convertToMap(source.compressedReference(), true, XContentType.JSON).v2(); @@ -128,22 +124,14 @@ public DocumentMapper parse(@Nullable String type, CompressedXContent source, St if (mapping == null) { mapping = new HashMap<>(); } - return parse(type, mapping, defaultSource); + return parse(type, mapping); } @SuppressWarnings({ "unchecked" }) - private DocumentMapper parse(String type, Map mapping, String defaultSource) throws MapperParsingException { + private DocumentMapper parse(String type, Map mapping) throws MapperParsingException { if (type == null) { throw new MapperParsingException("Failed to derive type"); } - - if (defaultSource != null) { - Tuple> t = extractMapping(MapperService.DEFAULT_MAPPING, defaultSource); - if (t.v2() != null) { - XContentHelper.mergeDefaults(mapping, t.v2()); - } - } - Mapper.TypeParser.ParserContext parserContext = parserContext(); // parse RootObjectMapper DocumentMapper.Builder docBuilder = new DocumentMapper.Builder( diff --git a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java index c7317ef639eff..30579f501a50c 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -141,10 +141,6 @@ private static void internalParseDocument( } private void validateType(SourceToParse source) { - if (docMapper.type().equals(MapperService.DEFAULT_MAPPING)) { - throw new IllegalArgumentException("It is forbidden to index into the default mapping [" + MapperService.DEFAULT_MAPPING + "]"); - } - if (Objects.equals(source.type(), docMapper.type()) == false && MapperService.SINGLE_MAPPING_NAME.equals(source.type()) == false) { // used // by // typeless diff --git a/server/src/main/java/org/opensearch/index/mapper/MapperService.java b/server/src/main/java/org/opensearch/index/mapper/MapperService.java index 765f5dc2d2f24..f42dadbbd8bf5 100644 --- a/server/src/main/java/org/opensearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/opensearch/index/mapper/MapperService.java @@ -41,7 +41,6 @@ import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.MappingMetadata; -import org.opensearch.common.Nullable; import org.opensearch.common.Strings; import org.opensearch.common.compress.CompressedXContent; import org.opensearch.common.logging.DeprecationLogger; @@ -81,6 +80,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.BooleanSupplier; import java.util.function.Function; @@ -115,7 +115,6 @@ public enum MergeReason { MAPPING_RECOVERY; } - public static final String DEFAULT_MAPPING = "_default_"; public static final String SINGLE_MAPPING_NAME = "_doc"; public static final Setting INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING = Setting.longSetting( "index.mapping.nested_fields.limit", @@ -175,10 +174,7 @@ public enum MergeReason { private final IndexAnalyzers indexAnalyzers; - private volatile String defaultMappingSource; - private volatile DocumentMapper mapper; - private volatile DocumentMapper defaultMapper; private final DocumentMapperParser documentParser; private final Version indexVersionCreated; @@ -231,12 +227,6 @@ public MapperService( && indexSettings.getIndexVersionCreated().onOrAfter(LegacyESVersion.V_7_0_0)) { throw new IllegalArgumentException("Setting " + INDEX_MAPPER_DYNAMIC_SETTING.getKey() + " was removed after version 6.0.0"); } - - defaultMappingSource = "{\"_default_\":{}}"; - - if (logger.isTraceEnabled()) { - logger.trace("default mapping source[{}]", defaultMappingSource); - } } public boolean hasNested() { @@ -286,9 +276,6 @@ public boolean updateMapping(final IndexMetadata currentIndexMetadata, final Ind if (mapper != null) { existingMappers.add(mapper.type()); } - if (defaultMapper != null) { - existingMappers.add(DEFAULT_MAPPING); - } final Map updatedEntries; try { // only update entries if needed @@ -304,13 +291,8 @@ public boolean updateMapping(final IndexMetadata currentIndexMetadata, final Ind for (DocumentMapper documentMapper : updatedEntries.values()) { String mappingType = documentMapper.type(); - MappingMetadata mappingMetadata; - if (mappingType.equals(MapperService.DEFAULT_MAPPING)) { - mappingMetadata = newIndexMetadata.defaultMapping(); - } else { - mappingMetadata = newIndexMetadata.mapping(); - assert mappingType.equals(mappingMetadata.type()); - } + MappingMetadata mappingMetadata = newIndexMetadata.mapping(); + assert mappingType.equals(mappingMetadata.type()); CompressedXContent incomingMappingSource = mappingMetadata.source(); String op = existingMappers.contains(mappingType) ? "updated" : "added"; @@ -351,20 +333,6 @@ private void assertMappingVersion( // if the mapping version is unchanged, then there should not be any updates and all mappings should be the same assert updatedEntries.isEmpty() : updatedEntries; - MappingMetadata defaultMapping = newIndexMetadata.defaultMapping(); - if (defaultMapping != null) { - final CompressedXContent currentSource = currentIndexMetadata.defaultMapping().source(); - final CompressedXContent newSource = defaultMapping.source(); - assert currentSource.equals(newSource) : "expected current mapping [" - + currentSource - + "] for type [" - + defaultMapping.type() - + "] " - + "to be the same as new mapping [" - + newSource - + "]"; - } - MappingMetadata mapping = newIndexMetadata.mapping(); if (mapping != null) { final CompressedXContent currentSource = currentIndexMetadata.mapping().source(); @@ -400,13 +368,8 @@ private void assertMappingVersion( + "]"; assert updatedEntries.isEmpty() == false; for (final DocumentMapper documentMapper : updatedEntries.values()) { - final MappingMetadata currentMapping; - if (documentMapper.type().equals(MapperService.DEFAULT_MAPPING)) { - currentMapping = currentIndexMetadata.defaultMapping(); - } else { - currentMapping = currentIndexMetadata.mapping(); - assert currentMapping == null || documentMapper.type().equals(currentMapping.type()); - } + final MappingMetadata currentMapping = currentIndexMetadata.mapping(); + assert currentMapping == null || documentMapper.type().equals(currentMapping.type()); if (currentMapping != null) { final CompressedXContent currentSource = currentMapping.source(); final CompressedXContent newSource = documentMapper.mappingSource(); @@ -462,52 +425,21 @@ private synchronized Map internalMerge(IndexMetadata ind } private synchronized Map internalMerge(Map mappings, MergeReason reason) { - DocumentMapper defaultMapper = null; - String defaultMappingSource = null; - - if (mappings.containsKey(DEFAULT_MAPPING)) { - // verify we can parse it - // NOTE: never apply the default here - try { - defaultMapper = documentParser.parse(DEFAULT_MAPPING, mappings.get(DEFAULT_MAPPING)); - } catch (Exception e) { - throw new MapperParsingException("Failed to parse mapping [{}]: {}", e, DEFAULT_MAPPING, e.getMessage()); - } - defaultMappingSource = mappings.get(DEFAULT_MAPPING).string(); - } - - final String defaultMappingSourceOrLastStored; - if (defaultMappingSource != null) { - defaultMappingSourceOrLastStored = defaultMappingSource; - } else { - defaultMappingSourceOrLastStored = this.defaultMappingSource; - } - DocumentMapper documentMapper = null; for (Map.Entry entry : mappings.entrySet()) { String type = entry.getKey(); - if (type.equals(DEFAULT_MAPPING)) { - continue; - } - if (documentMapper != null) { throw new IllegalArgumentException("Cannot put multiple mappings: " + mappings.keySet()); } - final boolean applyDefault = - // the default was already applied if we are recovering - reason != MergeReason.MAPPING_RECOVERY - // only apply the default mapping if we don't have the type yet - && this.mapper == null; - try { - documentMapper = documentParser.parse(type, entry.getValue(), applyDefault ? defaultMappingSourceOrLastStored : null); + documentMapper = documentParser.parse(type, entry.getValue()); } catch (Exception e) { throw new MapperParsingException("Failed to parse mapping [{}]: {}", e, entry.getKey(), e.getMessage()); } } - return internalMerge(defaultMapper, defaultMappingSource, documentMapper, reason); + return internalMerge(documentMapper, reason); } static void validateTypeName(String type) { @@ -535,23 +467,19 @@ static void validateTypeName(String type) { } } - private synchronized Map internalMerge( - @Nullable DocumentMapper defaultMapper, - @Nullable String defaultMappingSource, - DocumentMapper mapper, - MergeReason reason - ) { + private synchronized Map internalMerge(DocumentMapper mapper, MergeReason reason) { Map results = new LinkedHashMap<>(2); - if (defaultMapper != null) { - if (indexSettings.getIndexVersionCreated().onOrAfter(LegacyESVersion.V_7_0_0)) { - throw new IllegalArgumentException(DEFAULT_MAPPING_ERROR_MESSAGE); - } else if (reason == MergeReason.MAPPING_UPDATE) { // only log in case of explicit mapping updates - deprecationLogger.deprecate("default_mapping_not_allowed", DEFAULT_MAPPING_ERROR_MESSAGE); + { + if (mapper != null && this.mapper != null && Objects.equals(this.mapper.type(), mapper.type()) == false) { + throw new IllegalArgumentException( + "Rejecting mapping update to [" + + index().getName() + + "] as the final mapping would have more than 1 type: " + + Arrays.asList(this.mapper.type(), mapper.type()) + ); } - assert defaultMapper.type().equals(DEFAULT_MAPPING); - results.put(DEFAULT_MAPPING, defaultMapper); } DocumentMapper newMapper = null; @@ -580,10 +508,6 @@ private synchronized Map internalMerge( } // commit the change - if (defaultMappingSource != null) { - this.defaultMappingSource = defaultMappingSource; - this.defaultMapper = defaultMapper; - } if (newMapper != null) { this.mapper = newMapper; } @@ -595,7 +519,7 @@ private synchronized Map internalMerge( private boolean assertSerialization(DocumentMapper mapper) { // capture the source now, it may change due to concurrent parsing final CompressedXContent mappingSource = mapper.mappingSource(); - DocumentMapper newMapper = parse(mapper.type(), mappingSource, false); + DocumentMapper newMapper = parse(mapper.type(), mappingSource); if (newMapper.mappingSource().equals(mappingSource) == false) { throw new IllegalStateException( @@ -609,8 +533,8 @@ private boolean assertSerialization(DocumentMapper mapper) { return true; } - public DocumentMapper parse(String mappingType, CompressedXContent mappingSource, boolean applyDefault) throws MapperParsingException { - return documentParser.parse(mappingType, mappingSource, applyDefault ? defaultMappingSource : null); + public DocumentMapper parse(String mappingType, CompressedXContent mappingSource) throws MapperParsingException { + return documentParser.parse(mappingType, mappingSource); } /** @@ -621,17 +545,12 @@ public DocumentMapper documentMapper() { } /** - * Return the {@link DocumentMapper} for the given type. By using the special - * {@value #DEFAULT_MAPPING} type, you can get a {@link DocumentMapper} for - * the default mapping. + * Return the {@link DocumentMapper} for the given type. */ public DocumentMapper documentMapper(String type) { if (mapper != null && type.equals(mapper.type())) { return mapper; } - if (DEFAULT_MAPPING.equals(type)) { - return defaultMapper; - } return null; } @@ -683,7 +602,7 @@ public DocumentMapperForType documentMapperWithAutoCreate(String type) { if (mapper != null) { return new DocumentMapperForType(mapper, null); } - mapper = parse(type, null, true); + mapper = parse(type, null); return new DocumentMapperForType(mapper, mapper.mapping()); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java index cde2a762786af..87860b8c536ef 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java @@ -32,7 +32,6 @@ package org.opensearch.cluster.metadata; -import org.opensearch.Version; import org.opensearch.action.admin.indices.rollover.MaxAgeCondition; import org.opensearch.action.admin.indices.rollover.MaxDocsCondition; import org.opensearch.action.admin.indices.rollover.MaxSizeCondition; @@ -52,7 +51,6 @@ import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.index.mapper.MapperService; import org.opensearch.index.shard.ShardId; import org.opensearch.indices.IndicesModule; import org.opensearch.test.OpenSearchTestCase; @@ -363,32 +361,6 @@ public void testNumberOfRoutingShards() { assertEquals("the number of source shards [2] must be a factor of [3]", iae.getMessage()); } - public void testMappingOrDefault() throws IOException { - Settings settings = Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 2) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1) - .build(); - IndexMetadata meta = IndexMetadata.builder("index").settings(settings).build(); - assertNull(meta.mappingOrDefault()); - - meta = IndexMetadata.builder("index").settings(settings).putMapping("type", "{}").build(); - assertNotNull(meta.mappingOrDefault()); - assertEquals("type", meta.mappingOrDefault().type()); - - meta = IndexMetadata.builder("index").settings(settings).putMapping(MapperService.DEFAULT_MAPPING, "{}").build(); - assertNotNull(meta.mappingOrDefault()); - assertEquals(MapperService.DEFAULT_MAPPING, meta.mappingOrDefault().type()); - - meta = IndexMetadata.builder("index") - .settings(settings) - .putMapping("type", "{}") - .putMapping(MapperService.DEFAULT_MAPPING, "{}") - .build(); - assertNotNull(meta.mappingOrDefault()); - assertEquals("type", meta.mappingOrDefault().type()); - } - public void testMissingNumberOfShards() { final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> IndexMetadata.builder("test").build()); assertThat(e.getMessage(), containsString("must specify number of shards for index [test]")); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 45fc92ab66062..5caa9eb212e15 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1100,16 +1100,7 @@ public void testBuildIndexMetadata() { .put(SETTING_NUMBER_OF_SHARDS, 1) .build(); List aliases = singletonList(AliasMetadata.builder("alias1").build()); - IndexMetadata indexMetadata = buildIndexMetadata( - "test", - aliases, - () -> null, - () -> null, - indexSettings, - 4, - sourceIndexMetadata, - false - ); + IndexMetadata indexMetadata = buildIndexMetadata("test", aliases, () -> null, indexSettings, 4, sourceIndexMetadata, false); assertThat(indexMetadata.getAliases().size(), is(1)); assertThat(indexMetadata.getAliases().keys().iterator().next().value, is("alias1")); diff --git a/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java index 2e2e4990f2e65..9b355a8064660 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java @@ -1058,33 +1058,36 @@ public void testSimpleMapper() throws Exception { public void testParseToJsonAndParse() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/simple/test-mapping.json"); MapperService mapperService = createMapperService(mapping(b -> {})); - merge("person", mapperService, mapping); + merge(MapperService.SINGLE_MAPPING_NAME, mapperService, mapping); String builtMapping = mapperService.documentMapper().mappingSource().string(); // reparse it - DocumentMapper builtDocMapper = createDocumentMapper("_doc", builtMapping); + DocumentMapper builtDocMapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, builtMapping); BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/opensearch/index/mapper/simple/test1.json")); - Document doc = builtDocMapper.parse(new SourceToParse("test", "_doc", "1", json, XContentType.JSON)).rootDoc(); + Document doc = builtDocMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", json, XContentType.JSON)) + .rootDoc(); assertThat(doc.getBinaryValue(builtDocMapper.idFieldMapper().name()), equalTo(Uid.encodeId("1"))); assertThat(doc.get(builtDocMapper.mappers().getMapper("name.first").name()), equalTo("fred")); } public void testSimpleParser() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/simple/test-mapping.json"); - DocumentMapper docMapper = createDocumentMapper("person", mapping); + DocumentMapper docMapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, mapping); assertThat((String) docMapper.meta().get("param1"), equalTo("value1")); BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/opensearch/index/mapper/simple/test1.json")); - Document doc = docMapper.parse(new SourceToParse("test", "_doc", "1", json, XContentType.JSON)).rootDoc(); + Document doc = docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", json, XContentType.JSON)) + .rootDoc(); assertThat(doc.getBinaryValue(docMapper.idFieldMapper().name()), equalTo(Uid.encodeId("1"))); assertThat(doc.get(docMapper.mappers().getMapper("name.first").name()), equalTo("fred")); } public void testSimpleParserNoTypeNoId() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/simple/test-mapping.json"); - DocumentMapper docMapper = createDocumentMapper("person", mapping); + DocumentMapper docMapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, mapping); BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/opensearch/index/mapper/simple/test1-notype-noid.json")); - Document doc = docMapper.parse(new SourceToParse("test", "_doc", "1", json, XContentType.JSON)).rootDoc(); + Document doc = docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", json, XContentType.JSON)) + .rootDoc(); assertThat(doc.getBinaryValue(docMapper.idFieldMapper().name()), equalTo(Uid.encodeId("1"))); assertThat(doc.get(docMapper.mappers().getMapper("name.first").name()), equalTo("fred")); } @@ -1092,12 +1095,12 @@ public void testSimpleParserNoTypeNoId() throws Exception { public void testAttributes() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/simple/test-mapping.json"); - DocumentMapper docMapper = createDocumentMapper("person", mapping); + DocumentMapper docMapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, mapping); assertThat((String) docMapper.meta().get("param1"), equalTo("value1")); String builtMapping = docMapper.mappingSource().string(); - DocumentMapper builtDocMapper = createDocumentMapper("_doc", builtMapping); + DocumentMapper builtDocMapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, builtMapping); assertThat((String) builtDocMapper.meta().get("param1"), equalTo("value1")); } @@ -1106,7 +1109,7 @@ public void testNoDocumentSent() throws Exception { BytesReference json = new BytesArray("".getBytes(StandardCharsets.UTF_8)); MapperParsingException e = expectThrows( MapperParsingException.class, - () -> docMapper.parse(new SourceToParse("test", "_doc", "1", json, XContentType.JSON)) + () -> docMapper.parse(new SourceToParse("test", MapperService.SINGLE_MAPPING_NAME, "1", json, XContentType.JSON)) ); assertThat(e.getMessage(), equalTo("failed to parse, document is empty")); } @@ -1472,7 +1475,7 @@ public void testTypeless() throws IOException { String mapping = Strings.toString( XContentFactory.jsonBuilder() .startObject() - .startObject("type") + .startObject(MapperService.SINGLE_MAPPING_NAME) .startObject("properties") .startObject("foo") .field("type", "keyword") @@ -1481,7 +1484,7 @@ public void testTypeless() throws IOException { .endObject() .endObject() ); - DocumentMapper mapper = createDocumentMapper("type", mapping); + DocumentMapper mapper = createDocumentMapper(MapperService.SINGLE_MAPPING_NAME, mapping); ParsedDocument doc = mapper.parse(source(b -> b.field("foo", "1234"))); assertNull(doc.dynamicMappingsUpdate()); // no update since we reused the existing type diff --git a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplatesTests.java index f5e4ea8b2aaa8..70b58525e2772 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplatesTests.java @@ -76,7 +76,7 @@ public void testMatchTypeOnly() throws Exception { public void testSimple() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json"); - MapperService mapperService = createMapperService("person", mapping); + MapperService mapperService = createMapperService("_doc", mapping); String docJson = copyToStringFromClasspath("/org/opensearch/index/mapper/dynamictemplate/simple/test-data.json"); ParsedDocument parsedDoc = mapperService.documentMapper().parse(source(docJson)); @@ -131,7 +131,7 @@ public void testSimple() throws Exception { public void testSimpleWithXContentTraverse() throws Exception { String mapping = copyToStringFromClasspath("/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json"); - MapperService mapperService = createMapperService("person", mapping); + MapperService mapperService = createMapperService("_doc", mapping); String docJson = copyToStringFromClasspath("/org/opensearch/index/mapper/dynamictemplate/simple/test-data.json"); ParsedDocument parsedDoc = mapperService.documentMapper().parse(source(docJson)); diff --git a/server/src/test/java/org/opensearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/opensearch/index/mapper/MapperServiceTests.java index 376a5b6360d00..2fe59756755fb 100644 --- a/server/src/test/java/org/opensearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/MapperServiceTests.java @@ -33,7 +33,6 @@ package org.opensearch.index.mapper; import org.apache.lucene.analysis.TokenStream; -import org.opensearch.ExceptionsHelper; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.Strings; import org.opensearch.common.bytes.BytesReference; @@ -66,12 +65,12 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; -import java.util.concurrent.ExecutionException; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.startsWith; public class MapperServiceTests extends OpenSearchSingleNodeTestCase { @@ -116,34 +115,6 @@ public void testTypeValidation() { MapperService.validateTypeName("_doc"); // no exception } - public void testIndexIntoDefaultMapping() throws Throwable { - // 1. test implicit index creation - ExecutionException e = expectThrows( - ExecutionException.class, - () -> client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "1").setSource("{}", XContentType.JSON).execute().get() - ); - Throwable throwable = ExceptionsHelper.unwrapCause(e.getCause()); - if (throwable instanceof IllegalArgumentException) { - assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage()); - } else { - throw e; - } - - // 2. already existing index - IndexService indexService = createIndex("index2"); - e = expectThrows( - ExecutionException.class, - () -> { client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "2").setSource().execute().get(); } - ); - throwable = ExceptionsHelper.unwrapCause(e.getCause()); - if (throwable instanceof IllegalArgumentException) { - assertEquals("It is forbidden to index into the default mapping [_default_]", throwable.getMessage()); - } else { - throw e; - } - assertNull(indexService.mapperService().documentMapper(MapperService.DEFAULT_MAPPING)); - } - public void testPreflightUpdateDoesNotChangeMapping() throws Throwable { final MapperService mapperService = createIndex("test1").mapperService(); final CompressedXContent mapping = createMappingSpecifyingNumberOfFields(1); @@ -359,14 +330,56 @@ public void testTotalFieldsLimitWithFieldAlias() throws Throwable { assertEquals("Limit of total fields [" + numberOfNonAliasFields + "] has been exceeded", e.getMessage()); } - public void testDefaultMappingIsRejectedOn7() throws IOException { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_default_").endObject().endObject()); + public void testForbidMultipleTypes() throws IOException { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject()); MapperService mapperService = createIndex("test").mapperService(); + mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); + + String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type2").endObject().endObject()); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> mapperService.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE) + ); + assertThat(e.getMessage(), startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); + } + + /** + * This test checks that the multi-type validation is done before we do any other kind of validation on the mapping that's added, + * see https://github.com/elastic/elasticsearch/issues/29313 + */ + public void testForbidMultipleTypesWithConflictingMappings() throws IOException { + String mapping = Strings.toString( + XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field1") + .field("type", "integer_range") + .endObject() + .endObject() + .endObject() + .endObject() + ); + MapperService mapperService = createIndex("test").mapperService(); + mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); + + String mapping2 = Strings.toString( + XContentFactory.jsonBuilder() + .startObject() + .startObject("type2") + .startObject("properties") + .startObject("field1") + .field("type", "integer") + .endObject() + .endObject() + .endObject() + .endObject() + ); IllegalArgumentException e = expectThrows( IllegalArgumentException.class, - () -> mapperService.merge("_default_", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE) + () -> mapperService.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE) ); - assertEquals(MapperService.DEFAULT_MAPPING_ERROR_MESSAGE, e.getMessage()); + assertThat(e.getMessage(), startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); } public void testFieldNameLengthLimit() throws Throwable { diff --git a/server/src/test/java/org/opensearch/index/mapper/NestedObjectMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/NestedObjectMapperTests.java index c456e3ee11e3e..045cc97275eb7 100644 --- a/server/src/test/java/org/opensearch/index/mapper/NestedObjectMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/NestedObjectMapperTests.java @@ -1148,7 +1148,7 @@ public void testMergeNestedMappings() throws IOException { String mapping1 = Strings.toString( XContentFactory.jsonBuilder() .startObject() - .startObject("type") + .startObject(MapperService.SINGLE_MAPPING_NAME) .startObject("properties") .startObject("nested1") .field("type", "nested") @@ -1162,14 +1162,14 @@ public void testMergeNestedMappings() throws IOException { // cannot update `include_in_parent` dynamically MapperException e1 = expectThrows( MapperException.class, - () -> mapperService.merge("type", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE) + () -> mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE) ); assertEquals("the [include_in_parent] parameter can't be updated on a nested object mapping", e1.getMessage()); String mapping2 = Strings.toString( XContentFactory.jsonBuilder() .startObject() - .startObject("type") + .startObject(MapperService.SINGLE_MAPPING_NAME) .startObject("properties") .startObject("nested1") .field("type", "nested") @@ -1183,7 +1183,7 @@ public void testMergeNestedMappings() throws IOException { // cannot update `include_in_root` dynamically MapperException e2 = expectThrows( MapperException.class, - () -> mapperService.merge("type", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE) + () -> mapperService.merge(MapperService.SINGLE_MAPPING_NAME, new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE) ); assertEquals("the [include_in_root] parameter can't be updated on a nested object mapping", e2.getMessage()); } diff --git a/server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java index fc38deddf9d4f..6e03acb68e204 100644 --- a/server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java @@ -40,8 +40,6 @@ import org.opensearch.test.AbstractQueryTestCase; import java.io.IOException; -import java.util.HashSet; -import java.util.Set; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.contains; @@ -49,8 +47,6 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase { - private Set assertedWarnings = new HashSet<>(); - @Override protected IdsQueryBuilder doCreateTestQueryBuilder() { int numberOfIds = randomIntBetween(0, 10); @@ -104,7 +100,6 @@ protected QueryBuilder parseQuery(XContentParser parser) throws IOException { @Override public void testMustRewrite() throws IOException { - super.testMustRewrite(); QueryShardContext context = createShardContextWithNoType(); context.setAllowUnmappedFields(true); IdsQueryBuilder queryBuilder = createTestQueryBuilder(); diff --git a/server/src/test/resources/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json b/server/src/test/resources/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json index 4b91bcfb36b5f..457fbdc668241 100644 --- a/server/src/test/resources/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json +++ b/server/src/test/resources/org/opensearch/index/mapper/dynamictemplate/simple/test-mapping.json @@ -1,5 +1,5 @@ { - "person":{ + "_doc":{ "dynamic_templates":[ { "template_1":{ diff --git a/server/src/test/resources/org/opensearch/index/mapper/simple/test-mapping.json b/server/src/test/resources/org/opensearch/index/mapper/simple/test-mapping.json index 0f99af91ecb3a..55e462029ee6b 100644 --- a/server/src/test/resources/org/opensearch/index/mapper/simple/test-mapping.json +++ b/server/src/test/resources/org/opensearch/index/mapper/simple/test-mapping.json @@ -1,5 +1,5 @@ { - "person":{ + "_doc":{ "_meta":{ "param1":"value1" },