From 1a1dbf705faaff44d40bd24168ecce15fbfe9540 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 15 Jan 2019 07:32:47 -0800 Subject: [PATCH] Make sure to use the resolved type in DocumentMapperService#extractMappings. (#37451) * Pull out a shared method MapperService#resolveDocumentType. * Make sure to resolve the type when extracting the mappings. Addresses #36811. --- .../test/index/70_mix_typeless_typeful.yml | 38 ++++++++++++++++ .../test/update/90_mix_typeless_typeful.yml | 45 +++++++++++++++++++ .../metadata/MetaDataMappingService.java | 21 ++++----- .../index/mapper/DocumentMapperParser.java | 2 +- .../index/mapper/MapperService.java | 17 +++++++ .../elasticsearch/index/shard/IndexShard.java | 30 ++++--------- 6 files changed, 119 insertions(+), 34 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/70_mix_typeless_typeful.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/70_mix_typeless_typeful.yml index 5e225ec1ad30a..9f4f68f1998ae 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/70_mix_typeless_typeful.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/index/70_mix_typeless_typeful.yml @@ -60,3 +60,41 @@ - match: { _id: $id} - match: { _version: 1} - match: { _source: { foo: bar }} + +--- +"Index call that introduces new field mappings": + + - skip: + version: " - 6.99.99" + reason: Typeless APIs were introduced in 7.0.0 + + - do: + indices.create: # not using include_type_name: false on purpose + index: index + body: + mappings: + not_doc: + properties: + foo: + type: "keyword" + - do: + index: + index: index + id: 2 + body: { new_field: value } + + - match: { _index: "index" } + - match: { _type: "_doc" } + - match: { _id: "2" } + - match: { _version: 1 } + + - do: + get: # using typeful API on purpose + index: index + type: not_doc + id: 2 + + - match: { _index: "index" } + - match: { _type: "not_doc" } + - match: { _id: "2" } + - match: { _version: 1} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/90_mix_typeless_typeful.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/90_mix_typeless_typeful.yml index 066f0989c35b2..4caeb712c2896 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/90_mix_typeless_typeful.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/update/90_mix_typeless_typeful.yml @@ -37,3 +37,48 @@ id: 1 - match: { _source.foo: baz } + +--- +"Update call that introduces new field mappings": + + - skip: + version: " - 6.99.99" + reason: Typeless APIs were introduced in 7.0.0 + + - do: + indices.create: # not using include_type_name: false on purpose + index: index + body: + mappings: + not_doc: + properties: + foo: + type: "keyword" + + - do: + index: + index: index + type: not_doc + id: 1 + body: { foo: bar } + + - do: + update: + index: index + id: 1 + body: + doc: + foo: baz + new_field: value + - do: + get: # using typeful API on purpose + index: index + type: not_doc + id: 1 + + - match: { _index: "index" } + - match: { _type: "not_doc" } + - match: { _id: "1" } + - match: { _version: 2} + - match: { _source.foo: baz } + - match: { _source.new_field: value } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index 002ed86da34c2..06dda07d2289e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -276,8 +276,9 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt } if (mappingType == null) { mappingType = newMapper.type(); - } else if (mappingType.equals(newMapper.type()) == false) { - throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition"); + } else if (mappingType.equals(newMapper.type()) == false + && mapperService.resolveDocumentType(mappingType).equals(newMapper.type()) == false) { + throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition."); } } assert mappingType != null; @@ -295,16 +296,12 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt // we use the exact same indexService and metadata we used to validate above here to actually apply the update final Index index = indexMetaData.getIndex(); final MapperService mapperService = indexMapperServices.get(index); - String typeForUpdate = mappingType; // the type to use to apply the mapping update - if (MapperService.SINGLE_MAPPING_NAME.equals(typeForUpdate)) { - // If the user gave _doc as a special type value or if (s)he is using the new typeless APIs, - // then we apply the mapping update to the existing type. This allows to move to typeless - // APIs with indices whose type name is different from `_doc`. - DocumentMapper mapper = mapperService.documentMapper(); - if (mapper != null) { - typeForUpdate = mapper.type(); - } - } + + // If the user gave _doc as a special type value or if they are using the new typeless APIs, + // then we apply the mapping update to the existing type. This allows to move to typeless + // APIs with indices whose type name is different from `_doc`. + String typeForUpdate = mapperService.resolveDocumentType(mappingType); // the type to use to apply the mapping update + CompressedXContent existingSource = null; DocumentMapper existingMapper = mapperService.documentMapper(typeForUpdate); if (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 e63d5a279f3cd..e388dd7ebcd00 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -197,7 +197,7 @@ private Tuple> extractMapping(String type, Map> mapping; - if (type == null || type.equals(rootName)) { + if (type == null || type.equals(rootName) || mapperService.resolveDocumentType(type).equals(rootName)) { mapping = new Tuple<>(rootName, (Map) root.get(rootName)); } else { mapping = new Tuple<>(type, root); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 6424e75eaf662..add313d7aa87b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -665,6 +665,23 @@ public DocumentMapper documentMapper(String type) { return null; } + /** + * Resolves a type from a mapping-related request into the type that should be used when + * merging and updating mappings. + * + * If the special `_doc` type is provided, then we replace it with the actual type that is + * being used in the mappings. This allows typeless APIs such as 'index' or 'put mappings' + * to work against indices with a custom type name. + */ + public String resolveDocumentType(String type) { + if (MapperService.SINGLE_MAPPING_NAME.equals(type)) { + if (mapper != null) { + return mapper.type(); + } + } + return type; + } + /** * Returns the document mapper created, including a mapping update if the * type has been dynamically created. diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 8a10f4021d20f..e8cb0f519dd13 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -721,7 +721,7 @@ private Engine.IndexResult applyIndexOperation(Engine engine, long seqNo, long o ensureWriteAllowed(origin); Engine.Index operation; try { - final String resolvedType = resolveType(sourceToParse.type()); + final String resolvedType = mapperService.resolveDocumentType(sourceToParse.type()); final SourceToParse sourceWithResolvedType; if (resolvedType.equals(sourceToParse.type())) { sourceWithResolvedType = sourceToParse; @@ -844,11 +844,12 @@ private Engine.DeleteResult applyDeleteOperation(Engine engine, long seqNo, long } catch (MapperParsingException | IllegalArgumentException | TypeMissingException e) { return new Engine.DeleteResult(e, version, operationPrimaryTerm, seqNo, false); } - if (resolveType(type).equals(mapperService.documentMapper().type()) == false) { + if (mapperService.resolveDocumentType(type).equals(mapperService.documentMapper().type()) == false) { // We should never get there due to the fact that we generate mapping updates on deletes, // but we still prefer to have a hard exception here as we would otherwise delete a // document in the wrong type. - throw new IllegalStateException("Deleting document from type [" + resolveType(type) + "] while current type is [" + + throw new IllegalStateException("Deleting document from type [" + + mapperService.resolveDocumentType(type) + "] while current type is [" + mapperService.documentMapper().type() + "]"); } final Term uid = new Term(IdFieldMapper.NAME, Uid.encodeId(id)); @@ -861,8 +862,8 @@ private Engine.Delete prepareDelete(String type, String id, Term uid, long seqNo VersionType versionType, Engine.Operation.Origin origin, long ifSeqNo, long ifPrimaryTerm) { long startTime = System.nanoTime(); - return new Engine.Delete(resolveType(type), id, uid, seqNo, primaryTerm, version, versionType, origin, startTime, - ifSeqNo, ifPrimaryTerm); + return new Engine.Delete(mapperService.resolveDocumentType(type), id, uid, seqNo, primaryTerm, version, versionType, + origin, startTime, ifSeqNo, ifPrimaryTerm); } private Engine.DeleteResult delete(Engine engine, Engine.Delete delete) throws IOException { @@ -885,7 +886,7 @@ private Engine.DeleteResult delete(Engine engine, Engine.Delete delete) throws I public Engine.GetResult get(Engine.Get get) { readAllowed(); DocumentMapper mapper = mapperService.documentMapper(); - if (mapper == null || mapper.type().equals(resolveType(get.type())) == false) { + if (mapper == null || mapper.type().equals(mapperService.resolveDocumentType(get.type())) == false) { return GetResult.NOT_EXISTS; } return getEngine().get(get, this::acquireSearcher); @@ -2319,23 +2320,10 @@ private static void persistMetadata( } } - /** - * If an index/update/get/delete operation is using the special `_doc` type, then we replace - * it with the actual type that is being used in the mappings so that users may use typeless - * APIs with indices that have types. - */ - private String resolveType(String type) { - if (MapperService.SINGLE_MAPPING_NAME.equals(type)) { - DocumentMapper docMapper = mapperService.documentMapper(); - if (docMapper != null) { - return docMapper.type(); - } - } - return type; - } private DocumentMapperForType docMapper(String type) { - return mapperService.documentMapperWithAutoCreate(resolveType(type)); + return mapperService.documentMapperWithAutoCreate( + mapperService.resolveDocumentType(type)); } private EngineConfig newEngineConfig() {