diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java index 7143006382dd4..c7728774c819c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java @@ -364,6 +364,14 @@ public DocumentDimensions getDimensions() { public abstract ContentPath path(); + public final MapperBuilderContext createMapperBuilderContext() { + String p = path().pathAsText(""); + if (p.endsWith(".")) { + p = p.substring(0, p.length() - 1); + } + return new MapperBuilderContext(p); + } + public abstract XContentParser parser(); public abstract LuceneDocument rootDoc(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java index 6d56b230b6a53..dd2baa686cfc1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java @@ -159,7 +159,9 @@ void createDynamicFieldFromValue(final DocumentParserContext context, XContentPa */ static Mapper createDynamicObjectMapper(DocumentParserContext context, String name) { Mapper mapper = createObjectMapperFromTemplate(context, name); - return mapper != null ? mapper : new ObjectMapper.Builder(name).enabled(true).build(MapperBuilderContext.forPath(context.path())); + return mapper != null + ? mapper + : new ObjectMapper.Builder(name).enabled(ObjectMapper.Defaults.ENABLED).build(context.createMapperBuilderContext()); } /** @@ -167,7 +169,7 @@ static Mapper createDynamicObjectMapper(DocumentParserContext context, String na */ static Mapper createObjectMapperFromTemplate(DocumentParserContext context, String name) { Mapper.Builder templateBuilder = findTemplateBuilderForObject(context, name); - return templateBuilder == null ? null : templateBuilder.build(MapperBuilderContext.forPath(context.path())); + return templateBuilder == null ? null : templateBuilder.build(context.createMapperBuilderContext()); } /** @@ -302,7 +304,7 @@ private static final class Concrete implements Strategy { } void createDynamicField(Mapper.Builder builder, DocumentParserContext context) throws IOException { - Mapper mapper = builder.build(MapperBuilderContext.forPath(context.path())); + Mapper mapper = builder.build(context.createMapperBuilderContext()); context.addDynamicMapper(mapper); parseField.accept(context, mapper); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java index 29eb960da27e6..814ed2b1713df 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java @@ -55,7 +55,7 @@ public String path() { } @Override - public Mapper merge(Mapper mergeWith) { + public Mapper merge(Mapper mergeWith, MapperBuilderContext mapperBuilderContext) { if ((mergeWith instanceof FieldAliasMapper) == false) { throw new IllegalArgumentException( "Cannot merge a field alias mapping [" + name() + "] with a mapping that is not for a field alias." diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index c0ea1a4d4d794..b7219705cdcca 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -321,7 +321,7 @@ private static void checkNestedScopeCompatibility(String source, String target) public abstract Builder getMergeBuilder(); @Override - public final FieldMapper merge(Mapper mergeWith) { + public final FieldMapper merge(Mapper mergeWith, MapperBuilderContext mapperBuilderContext) { if (mergeWith == this) { return this; } @@ -343,9 +343,9 @@ public final FieldMapper merge(Mapper mergeWith) { return (FieldMapper) mergeWith; } Conflicts conflicts = new Conflicts(name()); - builder.merge((FieldMapper) mergeWith, conflicts); + builder.merge((FieldMapper) mergeWith, conflicts, mapperBuilderContext); conflicts.check(); - return builder.build(MapperBuilderContext.forPath(Builder.parentPath(name()))); + return builder.build(mapperBuilderContext); } protected void checkIncomingMergeType(FieldMapper mergeWith) { @@ -408,7 +408,7 @@ public Builder update(FieldMapper toMerge, MapperBuilderContext context) { add(toMerge); } else { FieldMapper existing = mapperBuilders.get(toMerge.simpleName()).apply(context); - add(existing.merge(toMerge)); + add(existing.merge(toMerge, context)); } return this; } @@ -1138,12 +1138,13 @@ public Builder init(FieldMapper initializer) { return this; } - protected void merge(FieldMapper in, Conflicts conflicts) { + protected void merge(FieldMapper in, Conflicts conflicts, MapperBuilderContext mapperBuilderContext) { for (Parameter param : getParameters()) { param.merge(in, conflicts); } + MapperBuilderContext childContext = mapperBuilderContext.createChildContext(in.simpleName()); for (FieldMapper newSubField : in.multiFields.mappers) { - multiFieldsBuilder.update(newSubField, MapperBuilderContext.forPath(parentPath(newSubField.name()))); + multiFieldsBuilder.update(newSubField, childContext); } this.copyTo.reset(in.copyTo); validate(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java index 724664e13a6f2..6b5e526e5430a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java @@ -25,6 +25,7 @@ protected Builder(String name) { this.name = internFieldName(name); } + // TODO rename this to leafName? public String name() { return this.name; } @@ -53,11 +54,13 @@ public Mapper(String simpleName) { /** Returns the simple name, which identifies this mapper against other mappers at the same level in the mappers hierarchy * TODO: make this protected once Mapper and FieldMapper are merged together */ + // TODO rename this to leafName? public final String simpleName() { return simpleName; } /** Returns the canonical name which uniquely identifies the mapper against other mappers in a type. */ + // TODO rename this to fullPath??? public abstract String name(); /** @@ -67,7 +70,7 @@ public final String simpleName() { /** Return the merge of {@code mergeWith} into this. * Both {@code this} and {@code mergeWith} will be left unmodified. */ - public abstract Mapper merge(Mapper mergeWith); + public abstract Mapper merge(Mapper mergeWith, MapperBuilderContext mapperBuilderContext); /** * Validate any cross-field references made by this mapper diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperBuilderContext.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperBuilderContext.java index 45a5fa24ed538..2d689a6091dba 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperBuilderContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperBuilderContext.java @@ -10,29 +10,26 @@ import org.elasticsearch.common.Strings; +import java.util.Objects; + /** * Holds context for building Mapper objects from their Builders */ -public class MapperBuilderContext { +public final class MapperBuilderContext { /** * The root context, to be used when building a tree of mappers */ - public static final MapperBuilderContext ROOT = new MapperBuilderContext(null); - - // TODO remove this - public static MapperBuilderContext forPath(ContentPath path) { - String p = path.pathAsText(""); - if (p.endsWith(".")) { - p = p.substring(0, p.length() - 1); - } - return new MapperBuilderContext(p); - } + public static final MapperBuilderContext ROOT = new MapperBuilderContext(); private final String path; - private MapperBuilderContext(String path) { - this.path = path; + private MapperBuilderContext() { + this.path = null; + } + + MapperBuilderContext(String path) { + this.path = Objects.requireNonNull(path); } /** @@ -47,7 +44,7 @@ public MapperBuilderContext createChildContext(String name) { /** * Builds the full name of the field, taking into account parent objects */ - public final String buildFullName(String name) { + public String buildFullName(String name) { if (Strings.isEmpty(path)) { return name; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java index 40bb37d364254..f2ce6ec3a6f34 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java @@ -127,7 +127,7 @@ Mapping mappingUpdate(RootObjectMapper rootObjectMapper) { * @return the resulting merged mapping. */ Mapping merge(Mapping mergeWith, MergeReason reason) { - RootObjectMapper mergedRoot = root.merge(mergeWith.root, reason); + RootObjectMapper mergedRoot = root.merge(mergeWith.root, reason, MapperBuilderContext.ROOT); // When merging metadata fields as part of applying an index template, new field definitions // completely overwrite existing ones instead of being merged. This behavior matches how we @@ -139,7 +139,7 @@ Mapping merge(Mapping mergeWith, MergeReason reason) { if (mergeInto == null || reason == MergeReason.INDEX_TEMPLATE) { merged = metaMergeWith; } else { - merged = (MetadataFieldMapper) mergeInto.merge(metaMergeWith); + merged = (MetadataFieldMapper) mergeInto.merge(metaMergeWith, MapperBuilderContext.ROOT); } mergedMetadataMappers.put(merged.getClass(), merged); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java index c228b70db8eab..b2a05e632804f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java @@ -170,7 +170,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } @Override - public ObjectMapper merge(Mapper mergeWith, MapperService.MergeReason reason) { + public ObjectMapper merge(Mapper mergeWith, MapperService.MergeReason reason, MapperBuilderContext mapperBuilderContext) { if ((mergeWith instanceof NestedObjectMapper) == false) { throw new IllegalArgumentException("can't merge a non nested mapping [" + mergeWith.name() + "] with a nested mapping"); } @@ -191,7 +191,7 @@ public ObjectMapper merge(Mapper mergeWith, MapperService.MergeReason reason) { throw new MapperException("the [include_in_root] parameter can't be updated on a nested object mapping"); } } - toMerge.doMerge(mergeWithObject, reason); + toMerge.doMerge(mergeWithObject, reason, mapperBuilderContext); return toMerge; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 2d88ff7c657a5..303d5ded51e5c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -163,7 +163,7 @@ protected final Map buildMappers(boolean root, MapperBuilderCont } Mapper existing = mappers.get(mapper.simpleName()); if (existing != null) { - mapper = existing.merge(mapper); + mapper = existing.merge(mapper, mapperBuilderContext); } mappers.put(mapper.simpleName(), mapper); } @@ -414,8 +414,8 @@ public final boolean subobjects() { } @Override - public ObjectMapper merge(Mapper mergeWith) { - return merge(mergeWith, MergeReason.MAPPING_UPDATE); + public ObjectMapper merge(Mapper mergeWith, MapperBuilderContext mapperBuilderContext) { + return merge(mergeWith, MergeReason.MAPPING_UPDATE, mapperBuilderContext); } @Override @@ -425,7 +425,7 @@ public void validate(MappingLookup mappers) { } } - public ObjectMapper merge(Mapper mergeWith, MergeReason reason) { + public ObjectMapper merge(Mapper mergeWith, MergeReason reason, MapperBuilderContext mapperBuilderContext) { if ((mergeWith instanceof ObjectMapper) == false) { throw new IllegalArgumentException("can't merge a non object mapping [" + mergeWith.name() + "] with an object mapping"); } @@ -435,11 +435,11 @@ public ObjectMapper merge(Mapper mergeWith, MergeReason reason) { } ObjectMapper mergeWithObject = (ObjectMapper) mergeWith; ObjectMapper merged = clone(); - merged.doMerge(mergeWithObject, reason); + merged.doMerge(mergeWithObject, reason, mapperBuilderContext); return merged; } - protected void doMerge(final ObjectMapper mergeWith, MergeReason reason) { + protected void doMerge(final ObjectMapper mergeWith, MergeReason reason, MapperBuilderContext mapperBuilderContext) { if (mergeWith.dynamic != null) { this.dynamic = mergeWith.dynamic; @@ -469,7 +469,8 @@ protected void doMerge(final ObjectMapper mergeWith, MergeReason reason) { if (mergeIntoMapper == null) { merged = mergeWithMapper; } else if (mergeIntoMapper instanceof ObjectMapper objectMapper) { - merged = objectMapper.merge(mergeWithMapper, reason); + MapperBuilderContext childContext = mapperBuilderContext.createChildContext(objectMapper.simpleName()); + merged = objectMapper.merge(mergeWithMapper, reason, childContext); } else { assert mergeIntoMapper instanceof FieldMapper || mergeIntoMapper instanceof FieldAliasMapper; if (mergeWithMapper instanceof ObjectMapper) { @@ -483,7 +484,7 @@ protected void doMerge(final ObjectMapper mergeWith, MergeReason reason) { if (reason == MergeReason.INDEX_TEMPLATE) { merged = mergeWithMapper; } else { - merged = mergeIntoMapper.merge(mergeWithMapper); + merged = mergeIntoMapper.merge(mergeWithMapper, mapperBuilderContext); } } if (mergedMappers == null) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PlaceHolderFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/PlaceHolderFieldMapper.java index ec2b5bd58e7f4..b242e3f425287 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PlaceHolderFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PlaceHolderFieldMapper.java @@ -65,10 +65,10 @@ public FieldMapper.Builder init(FieldMapper initializer) { } @Override - protected void merge(FieldMapper in, Conflicts conflicts) { + protected void merge(FieldMapper in, Conflicts conflicts, MapperBuilderContext mapperBuilderContext) { assert in instanceof PlaceHolderFieldMapper; unknownParams.putAll(((PlaceHolderFieldMapper) in).unknownParams); - super.merge(in, conflicts); + super.merge(in, conflicts, mapperBuilderContext); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java index d1f446f8ab070..f2a288550faac 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -322,13 +322,13 @@ RuntimeField getRuntimeField(String name) { } @Override - public RootObjectMapper merge(Mapper mergeWith, MergeReason reason) { - return (RootObjectMapper) super.merge(mergeWith, reason); + public RootObjectMapper merge(Mapper mergeWith, MergeReason reason, MapperBuilderContext mapperBuilderContext) { + return (RootObjectMapper) super.merge(mergeWith, reason, mapperBuilderContext); } @Override - protected void doMerge(ObjectMapper mergeWith, MergeReason reason) { - super.doMerge(mergeWith, reason); + protected void doMerge(ObjectMapper mergeWith, MergeReason reason, MapperBuilderContext mapperBuilderContext) { + super.doMerge(mergeWith, reason, mapperBuilderContext); RootObjectMapper mergeWithObject = (RootObjectMapper) mergeWith; if (mergeWithObject.numericDetection.explicit()) { this.numericDetection = mergeWithObject.numericDetection; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java index 769c79e2495eb..095938af5820f 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java @@ -158,9 +158,7 @@ public void testFieldAliasWithDifferentNestedScopes() { } private static FieldMapper createFieldMapper(String parent, String name) { - return new BooleanFieldMapper.Builder(name, ScriptCompiler.NONE, Version.CURRENT).build( - MapperBuilderContext.forPath(new ContentPath(parent)) - ); + return new BooleanFieldMapper.Builder(name, ScriptCompiler.NONE, Version.CURRENT).build(new MapperBuilderContext(parent)); } private static ObjectMapper createObjectMapper(String name) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperMergeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperMergeTests.java index b2d2e7bfdd392..cb46a450ffc70 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperMergeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperMergeTests.java @@ -49,7 +49,7 @@ public void testMerge() { ObjectMapper mergeWith = createMapping(false, true, true, true); // WHEN merging mappings - final ObjectMapper merged = rootObjectMapper.merge(mergeWith); + final ObjectMapper merged = rootObjectMapper.merge(mergeWith, MapperBuilderContext.ROOT); // THEN "baz" new field is added to merged mapping final ObjectMapper mergedFoo = (ObjectMapper) merged.getMapper("foo"); @@ -63,7 +63,7 @@ public void testMergeWhenDisablingField() { // WHEN merging mappings // THEN a MapperException is thrown with an excepted message - MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith)); + MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith, MapperBuilderContext.ROOT)); assertEquals("the [enabled] parameter can't be updated for the object mapping [foo]", e.getMessage()); } @@ -74,17 +74,17 @@ public void testMergeDisabledField() { mappers.put("disabled", new ObjectMapper.Builder("disabled").build(MapperBuilderContext.ROOT)); RootObjectMapper mergeWith = createRootObjectMapper("type1", true, Collections.unmodifiableMap(mappers)); - RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith); + RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperBuilderContext.ROOT); assertFalse(((ObjectMapper) merged.getMapper("disabled")).isEnabled()); } public void testMergeEnabled() { ObjectMapper mergeWith = createMapping(true, true, true, false); - MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith)); + MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith, MapperBuilderContext.ROOT)); assertEquals("the [enabled] parameter can't be updated for the object mapping [disabled]", e.getMessage()); - ObjectMapper result = rootObjectMapper.merge(mergeWith, MapperService.MergeReason.INDEX_TEMPLATE); + ObjectMapper result = rootObjectMapper.merge(mergeWith, MapperService.MergeReason.INDEX_TEMPLATE, MapperBuilderContext.ROOT); assertTrue(result.isEnabled()); } @@ -93,10 +93,10 @@ public void testMergeEnabledForRootMapper() { ObjectMapper firstMapper = createRootObjectMapper(type, true, Collections.emptyMap()); ObjectMapper secondMapper = createRootObjectMapper(type, false, Collections.emptyMap()); - MapperException e = expectThrows(MapperException.class, () -> firstMapper.merge(secondMapper)); + MapperException e = expectThrows(MapperException.class, () -> firstMapper.merge(secondMapper, MapperBuilderContext.ROOT)); assertEquals("the [enabled] parameter can't be updated for the object mapping [" + type + "]", e.getMessage()); - ObjectMapper result = firstMapper.merge(secondMapper, MapperService.MergeReason.INDEX_TEMPLATE); + ObjectMapper result = firstMapper.merge(secondMapper, MapperService.MergeReason.INDEX_TEMPLATE, MapperBuilderContext.ROOT); assertFalse(result.isEnabled()); } @@ -109,14 +109,13 @@ public void testMergeDisabledRootMapper() { Collections.singletonMap("test", new TestRuntimeField("test", "long")) ).build(MapperBuilderContext.ROOT); - RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith); + RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperBuilderContext.ROOT); assertFalse(merged.isEnabled()); assertEquals(1, merged.runtimeFields().size()); assertEquals("test", merged.runtimeFields().iterator().next().name()); } public void testMergeNested() { - NestedObjectMapper firstMapper = new NestedObjectMapper.Builder("nested1", Version.CURRENT).includeInParent(true) .includeInRoot(true) .build(MapperBuilderContext.ROOT); @@ -124,14 +123,105 @@ public void testMergeNested() { .includeInRoot(true) .build(MapperBuilderContext.ROOT); - MapperException e = expectThrows(MapperException.class, () -> firstMapper.merge(secondMapper)); + MapperException e = expectThrows(MapperException.class, () -> firstMapper.merge(secondMapper, MapperBuilderContext.ROOT)); assertThat(e.getMessage(), containsString("[include_in_parent] parameter can't be updated on a nested object mapping")); - NestedObjectMapper result = (NestedObjectMapper) firstMapper.merge(secondMapper, MapperService.MergeReason.INDEX_TEMPLATE); + NestedObjectMapper result = (NestedObjectMapper) firstMapper.merge( + secondMapper, + MapperService.MergeReason.INDEX_TEMPLATE, + MapperBuilderContext.ROOT + ); assertFalse(result.isIncludeInParent()); assertTrue(result.isIncludeInRoot()); } + public void testMergedFieldNamesFieldWithDotsSubobjectsFalseAtRoot() { + RootObjectMapper mergeInto = createRootSubobjectFalseLeafWithDots(); + RootObjectMapper mergeWith = createRootSubobjectFalseLeafWithDots(); + + final ObjectMapper merged = mergeInto.merge(mergeWith, MapperBuilderContext.ROOT); + + final KeywordFieldMapper keywordFieldMapper = (KeywordFieldMapper) merged.getMapper("host.name"); + assertEquals("host.name", keywordFieldMapper.name()); + assertEquals("host.name", keywordFieldMapper.simpleName()); + } + + public void testMergedFieldNamesFieldWithDotsSubobjectsFalse() { + RootObjectMapper mergeInto = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("foo", createObjectSubobjectsFalseLeafWithDots()) + ); + RootObjectMapper mergeWith = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("foo", createObjectSubobjectsFalseLeafWithDots()) + ); + + final ObjectMapper merged = mergeInto.merge(mergeWith, MapperBuilderContext.ROOT); + + ObjectMapper foo = (ObjectMapper) merged.getMapper("foo"); + ObjectMapper metrics = (ObjectMapper) foo.getMapper("metrics"); + final KeywordFieldMapper keywordFieldMapper = (KeywordFieldMapper) metrics.getMapper("host.name"); + assertEquals("foo.metrics.host.name", keywordFieldMapper.name()); + assertEquals("host.name", keywordFieldMapper.simpleName()); + } + + public void testMergedFieldNamesMultiFields() { + RootObjectMapper mergeInto = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("text", createTextKeywordMultiField("text", MapperBuilderContext.ROOT)) + ); + RootObjectMapper mergeWith = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("text", createTextKeywordMultiField("text", MapperBuilderContext.ROOT)) + ); + + final ObjectMapper merged = mergeInto.merge(mergeWith, MapperBuilderContext.ROOT); + + TextFieldMapper text = (TextFieldMapper) merged.getMapper("text"); + assertEquals("text", text.name()); + assertEquals("text", text.simpleName()); + KeywordFieldMapper keyword = (KeywordFieldMapper) text.multiFields().iterator().next(); + assertEquals("text.keyword", keyword.name()); + assertEquals("keyword", keyword.simpleName()); + } + + public void testMergedFieldNamesMultiFieldsWithinSubobjectsFalse() { + RootObjectMapper mergeInto = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("foo", createObjectSubobjectsFalseLeafWithMultiField()) + ); + RootObjectMapper mergeWith = createRootObjectMapper( + "_doc", + true, + Collections.singletonMap("foo", createObjectSubobjectsFalseLeafWithMultiField()) + ); + + final ObjectMapper merged = mergeInto.merge(mergeWith, MapperBuilderContext.ROOT); + + ObjectMapper foo = (ObjectMapper) merged.getMapper("foo"); + ObjectMapper metrics = (ObjectMapper) foo.getMapper("metrics"); + final TextFieldMapper textFieldMapper = (TextFieldMapper) metrics.getMapper("host.name"); + assertEquals("foo.metrics.host.name", textFieldMapper.name()); + assertEquals("host.name", textFieldMapper.simpleName()); + FieldMapper fieldMapper = textFieldMapper.multiFields.iterator().next(); + assertEquals("foo.metrics.host.name.keyword", fieldMapper.name()); + assertEquals("keyword", fieldMapper.simpleName()); + } + + private static RootObjectMapper createRootSubobjectFalseLeafWithDots() { + FieldMapper fieldMapper = new KeywordFieldMapper.Builder("host.name", Version.CURRENT).build(MapperBuilderContext.ROOT); + assertEquals("host.name", fieldMapper.simpleName()); + assertEquals("host.name", fieldMapper.name()); + return (RootObjectMapper) new RootObjectMapper.Builder("_doc").subobjects(false) + .addMappers(Collections.singletonMap("host.name", fieldMapper)) + .build(MapperBuilderContext.ROOT); + } + private static RootObjectMapper createRootObjectMapper(String name, boolean enabled, Map mappers) { return (RootObjectMapper) new RootObjectMapper.Builder(name).enabled(enabled).addMappers(mappers).build(MapperBuilderContext.ROOT); } @@ -140,7 +230,38 @@ private static ObjectMapper createObjectMapper(String name, boolean enabled, Map return new ObjectMapper.Builder(name).enabled(enabled).addMappers(mappers).build(MapperBuilderContext.ROOT); } + private static ObjectMapper createObjectSubobjectsFalseLeafWithDots() { + KeywordFieldMapper fieldMapper = new KeywordFieldMapper.Builder("host.name", Version.CURRENT).build( + new MapperBuilderContext("foo.metrics") + ); + assertEquals("host.name", fieldMapper.simpleName()); + assertEquals("foo.metrics.host.name", fieldMapper.name()); + ObjectMapper metrics = new ObjectMapper.Builder("metrics").subobjects(false) + .addMappers(Collections.singletonMap("host.name", fieldMapper)) + .build(new MapperBuilderContext("foo")); + return new ObjectMapper.Builder("foo").addMappers(Collections.singletonMap("metrics", metrics)).build(MapperBuilderContext.ROOT); + } + + private ObjectMapper createObjectSubobjectsFalseLeafWithMultiField() { + TextFieldMapper textKeywordMultiField = createTextKeywordMultiField("host.name", new MapperBuilderContext("foo.metrics")); + assertEquals("host.name", textKeywordMultiField.simpleName()); + assertEquals("foo.metrics.host.name", textKeywordMultiField.name()); + FieldMapper fieldMapper = textKeywordMultiField.multiFields.iterator().next(); + assertEquals("keyword", fieldMapper.simpleName()); + assertEquals("foo.metrics.host.name.keyword", fieldMapper.name()); + ObjectMapper metrics = new ObjectMapper.Builder("metrics").subobjects(false) + .addMappers(Collections.singletonMap("host.name", textKeywordMultiField)) + .build(new MapperBuilderContext("foo")); + return new ObjectMapper.Builder("foo").addMappers(Collections.singletonMap("metrics", metrics)).build(MapperBuilderContext.ROOT); + } + private TextFieldMapper createTextFieldMapper(String name) { return new TextFieldMapper.Builder(name, createDefaultIndexAnalyzers()).build(MapperBuilderContext.ROOT); } + + private TextFieldMapper createTextKeywordMultiField(String name, MapperBuilderContext mapperBuilderContext) { + TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name, createDefaultIndexAnalyzers()); + builder.multiFieldsBuilder.add(new KeywordFieldMapper.Builder("keyword", Version.CURRENT)); + return builder.build(mapperBuilderContext); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java index 7ea9b92668a83..b5cced31ced17 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java @@ -332,7 +332,7 @@ public void testMerging() { TestMapper badMerge = fromMapping(""" {"type":"test_mapper","fixed":true,"fixed2":true,"required":"value"}"""); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.merge(badMerge)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.merge(badMerge, MapperBuilderContext.ROOT)); String expectedError = """ Mapper for [field] conflicts with existing mapper: \tCannot update parameter [fixed] from [false] to [true] @@ -344,7 +344,7 @@ public void testMerging() { // TODO: should we have to include 'fixed' here? Or should updates take as 'defaults' the existing values? TestMapper goodMerge = fromMapping(""" {"type":"test_mapper","fixed":false,"variable":"updated","required":"value"}"""); - TestMapper merged = (TestMapper) mapper.merge(goodMerge); + TestMapper merged = (TestMapper) mapper.merge(goodMerge, MapperBuilderContext.ROOT); assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); // original mapping is unaffected assertEquals(""" @@ -362,7 +362,7 @@ public void testMultifields() throws IOException { String addSubField = """ {"type":"test_mapper","variable":"foo","required":"value","fields":{"sub2":{"type":"keyword"}}}"""; TestMapper toMerge = fromMapping(addSubField); - TestMapper merged = (TestMapper) mapper.merge(toMerge); + TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperBuilderContext.ROOT); assertEquals(XContentHelper.stripWhitespace(""" { "field": { @@ -383,7 +383,10 @@ public void testMultifields() throws IOException { String badSubField = """ {"type":"test_mapper","variable":"foo","required":"value","fields":{"sub2":{"type":"binary"}}}"""; TestMapper badToMerge = fromMapping(badSubField); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> merged.merge(badToMerge)); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> merged.merge(badToMerge, MapperBuilderContext.ROOT) + ); assertEquals("mapper [field.sub2] cannot be changed from type [keyword] to [binary]", e.getMessage()); } @@ -398,13 +401,13 @@ public void testCopyTo() { TestMapper toMerge = fromMapping(""" {"type":"test_mapper","variable":"updated","required":"value","copy_to":["foo","bar"]}"""); - TestMapper merged = (TestMapper) mapper.merge(toMerge); + TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperBuilderContext.ROOT); assertEquals(""" {"field":{"type":"test_mapper","variable":"updated","required":"value","copy_to":["foo","bar"]}}""", Strings.toString(merged)); TestMapper removeCopyTo = fromMapping(""" {"type":"test_mapper","variable":"updated","required":"value"}"""); - TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo); + TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo, MapperBuilderContext.ROOT); assertEquals(""" {"field":{"type":"test_mapper","variable":"updated","required":"value"}}""", Strings.toString(noCopyTo)); } @@ -468,7 +471,7 @@ public void testCustomSerialization() { String conflict = """ {"type":"test_mapper","wrapper":"new value","required":"value"}"""; TestMapper toMerge = fromMapping(conflict); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.merge(toMerge)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.merge(toMerge, MapperBuilderContext.ROOT)); assertEquals( "Mapper for [field] conflicts with existing mapper:\n" + "\tCannot update parameter [wrapper] from [wrapper_wrapped value] to [wrapper_new value]", @@ -544,7 +547,7 @@ public void testAnalyzers() { TestMapper original = mapper; TestMapper toMerge = fromMapping(mapping); - e = expectThrows(IllegalArgumentException.class, () -> original.merge(toMerge)); + e = expectThrows(IllegalArgumentException.class, () -> original.merge(toMerge, MapperBuilderContext.ROOT)); assertEquals( "Mapper for [field] conflicts with existing mapper:\n" + "\tCannot update parameter [analyzer] from [default] to [_standard]", e.getMessage()