diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 5f8ed5eae48d3..067061443d885 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -403,8 +403,8 @@ private static DocumentParserContext nestedContext(DocumentParserContext context } static void parseObjectOrField(DocumentParserContext context, Mapper mapper) throws IOException { - if (mapper instanceof ObjectMapper) { - parseObjectOrNested(context, (ObjectMapper) mapper); + if (mapper instanceof ObjectMapper objectMapper) { + parseObjectOrNested(context, objectMapper); } else if (mapper instanceof FieldMapper fieldMapper) { fieldMapper.parse(context); if (context.isWithinCopyTo() == false) { @@ -447,9 +447,10 @@ private static void throwOnCopyToOnObject(Mapper mapper, List copyToFiel ); } - private static void parseObject(final DocumentParserContext context, ObjectMapper mapper, String currentFieldName) throws IOException { + private static void parseObject(final DocumentParserContext context, ObjectMapper parentObjectMapper, String currentFieldName) + throws IOException { assert currentFieldName != null; - Mapper objectMapper = getMapper(context, mapper, currentFieldName); + Mapper objectMapper = getMapper(context, parentObjectMapper, currentFieldName); if (objectMapper != null) { context.path().add(currentFieldName); if (objectMapper instanceof ObjectMapper objMapper) { @@ -461,16 +462,17 @@ private static void parseObject(final DocumentParserContext context, ObjectMappe context.path().setWithinLeafObject(false); context.path().remove(); } else { - parseObjectDynamic(context, mapper, currentFieldName); + parseObjectDynamic(context, parentObjectMapper, currentFieldName); } } - private static void parseObjectDynamic(DocumentParserContext context, ObjectMapper mapper, String currentFieldName) throws IOException { - ObjectMapper.Dynamic dynamic = dynamicOrDefault(mapper, context); + private static void parseObjectDynamic(DocumentParserContext context, ObjectMapper parentObjectMapper, String currentFieldName) + throws IOException { + ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentObjectMapper, context); if (dynamic == ObjectMapper.Dynamic.STRICT) { - throw new StrictDynamicMappingException(mapper.fullPath(), currentFieldName); + throw new StrictDynamicMappingException(parentObjectMapper.fullPath(), currentFieldName); } else if (dynamic == ObjectMapper.Dynamic.FALSE) { - failIfMatchesRoutingPath(context, mapper, currentFieldName); + failIfMatchesRoutingPath(context, parentObjectMapper, currentFieldName); // not dynamic, read everything up to end object context.parser().skipChildren(); } else { @@ -483,13 +485,13 @@ private static void parseObjectDynamic(DocumentParserContext context, ObjectMapp dynamicObjectMapper = DynamicFieldsBuilder.createDynamicObjectMapper(context, currentFieldName); context.addDynamicMapper(dynamicObjectMapper); } - if (mapper.subobjects() == false) { + if (parentObjectMapper.subobjects() == false) { if (dynamicObjectMapper instanceof NestedObjectMapper) { throw new MapperParsingException( "Tried to add nested object [" + dynamicObjectMapper.simpleName() + "] to object [" - + mapper.name() + + parentObjectMapper.name() + "] which does not support subobjects" ); } @@ -498,7 +500,7 @@ private static void parseObjectDynamic(DocumentParserContext context, ObjectMapp "Tried to add subobject [" + dynamicObjectMapper.simpleName() + "] to object [" - + mapper.name() + + parentObjectMapper.name() + "] which does not support subobjects" ); } 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 c7728774c819c..9b6353b862b24 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java @@ -234,20 +234,44 @@ public final void addDynamicMapper(Mapper mapper) { && newFieldsSeen.add(mapper.name())) { mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size()); } - if (mapper instanceof ObjectMapper) { - dynamicObjectMappers.put(mapper.name(), (ObjectMapper) mapper); + if (mapper instanceof ObjectMapper objectMapper) { + dynamicObjectMappers.put(objectMapper.name(), objectMapper); + // dynamic object mappers may have been obtained from applying a dynamic template, in which case their definition may contain + // sub-fields as well as sub-objects that need to be added to the mappings + for (Mapper submapper : objectMapper.mappers.values()) { + // we could potentially skip the step of adding these to the dynamic mappers, because their parent is already added to + // that list, and what is important is that all of the intermediate objects are added to the dynamic object mappers so that + // they can be looked up once sub-fields need to be added to them. For simplicity, we treat these like any other object + addDynamicMapper(submapper); + } } + // TODO we may want to stop adding object mappers to the dynamic mappers list: most times they will be mapped when parsing their + // sub-fields (see ObjectMapper.Builder#addDynamic), which causes extra work as the two variants of the same object field + // will be merged together when creating the final dynamic update. The only cases where object fields need extra treatment are + // dynamically mapped objects when the incoming document defines no sub-fields in them: + // 1) by default, they would be empty containers in the mappings, is it then important to map them? + // 2) they can be the result of applying a dynamic template which may define sub-fields or set dynamic, enabled or subobjects. dynamicMappers.add(mapper); } /** - * Get dynamic mappers created while parsing. + * Get dynamic mappers created as a result of parsing an incoming document. Responsible for exposing all the newly created + * fields that need to be merged into the existing mappings. Used to create the required mapping update at the end of document parsing. + * Consists of a flat set of {@link Mapper}s that will need to be added to their respective parent {@link ObjectMapper}s in order + * to become part of the resulting dynamic mapping update. */ public final List getDynamicMappers() { return dynamicMappers; } - public final ObjectMapper getDynamicObjectMapper(String name) { + /** + * Get a dynamic object mapper by name. Allows consumers to lookup objects that have been dynamically added as a result + * of parsing an incoming document. Used to find the parent object for new fields that are being dynamically mapped whose parent is + * also not mapped yet. Such new fields will need to be dynamically added to their parent according to its dynamic behaviour. + * Holds a flat set of object mappers, meaning that an object field named foo.bar can be looked up directly with its + * dotted name. + */ + final ObjectMapper getDynamicObjectMapper(String name) { return dynamicObjectMappers.get(name); } @@ -259,7 +283,9 @@ public final void addDynamicRuntimeField(RuntimeField runtimeField) { } /** - * Get dynamic runtime fields created while parsing. + * Get dynamic runtime fields created while parsing. Holds a flat set of {@link RuntimeField}s. + * Runtime fields get dynamically mapped when {@link org.elasticsearch.index.mapper.ObjectMapper.Dynamic#RUNTIME} is used, + * or when dynamic templates specify a runtime section. */ public final List getDynamicRuntimeFields() { return Collections.unmodifiableList(dynamicRuntimeFields); 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 a54536090a79f..3e629b4a21119 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -102,14 +102,14 @@ Builder addMappers(Map mappers) { } /** - * Adds a dynamically created Mapper to this builder. + * Adds a dynamically created {@link Mapper} to this builder. * * @param name the name of the Mapper, including object prefixes * @param prefix the object prefix of this mapper * @param mapper the mapper to add * @param context the DocumentParserContext in which the mapper has been built */ - public void addDynamic(String name, String prefix, Mapper mapper, DocumentParserContext context) { + public final void addDynamic(String name, String prefix, Mapper mapper, DocumentParserContext context) { // If the mapper to add has no dots, or the current object mapper has subobjects set to false, // we just add it as it is for sure a leaf mapper if (name.contains(".") == false || subobjects.value() == false) { @@ -118,29 +118,29 @@ public void addDynamic(String name, String prefix, Mapper mapper, DocumentParser // otherwise we strip off the first object path of the mapper name, load or create // the relevant object mapper, and then recurse down into it, passing the remainder // of the mapper name. So for a mapper 'foo.bar.baz', we locate 'foo' and then - // call addDynamic on it with the name 'bar.baz'. + // call addDynamic on it with the name 'bar.baz', and next call addDynamic on 'bar' with the name 'baz'. else { int firstDotIndex = name.indexOf("."); - String childName = name.substring(0, firstDotIndex); - String fullChildName = prefix == null ? childName : prefix + "." + childName; - ObjectMapper.Builder childBuilder = findChild(fullChildName, context); - childBuilder.addDynamic(name.substring(firstDotIndex + 1), fullChildName, mapper, context); - add(childBuilder); + String immediateChild = name.substring(0, firstDotIndex); + String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild; + ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChildFullName, context); + parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context); + add(parentBuilder); } } - private static ObjectMapper.Builder findChild(String fullChildName, DocumentParserContext context) { - // does the child mapper already exist? if so, use that - ObjectMapper child = context.mappingLookup().objectMappers().get(fullChildName); - if (child != null) { - return child.newBuilder(context.indexSettings().getIndexVersionCreated()); + private static ObjectMapper.Builder findObjectBuilder(String fullName, DocumentParserContext context) { + // does the object mapper already exist? if so, use that + ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName); + if (objectMapper != null) { + return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated()); } - // has the child mapper been added as a dynamic update already? - child = context.getDynamicObjectMapper(fullChildName); - if (child != null) { - return child.newBuilder(context.indexSettings().getIndexVersionCreated()); + // has the object mapper been added as a dynamic update already? + objectMapper = context.getDynamicObjectMapper(fullName); + if (objectMapper != null) { + return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated()); } - throw new IllegalArgumentException("Missing intermediate object " + fullChildName); + throw new IllegalStateException("Missing intermediate object " + fullName); } protected final Map buildMappers(boolean root, MapperBuilderContext context) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index 30d57b24ebfbd..6c3f50c30de35 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -734,6 +734,20 @@ public void testObjectMappingUpdate() throws Exception { assertNotNull(((ObjectMapper) barMapper).getMapper("baz")); } + public void testEmptyObjectGetsMapped() throws Exception { + MapperService mapperService = createMapperService(); + DocumentMapper docMapper = mapperService.documentMapper(); + ParsedDocument doc = docMapper.parse(source(b -> { + b.startObject("foo"); + b.endObject(); + })); + Mapping mapping = doc.dynamicMappingsUpdate(); + assertNotNull(mapping); + Mapper foo = mapping.getRoot().getMapper("foo"); + assertThat(foo, instanceOf(ObjectMapper.class)); + assertEquals(0, ((ObjectMapper) foo).mappers.size()); + } + public void testDynamicGeoPointArrayWithTemplate() throws Exception { DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startArray("dynamic_templates"); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java index d620fc3767ad9..ad02f39e509ed 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java @@ -1383,4 +1383,376 @@ public void testSubobjectsFalseWithInnerNestedFromDynamicTemplate() { exception.getRootCause().getMessage() ); } + + public void testDynamicSubobject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("map_artifact_identifiers"); + { + b.field("match_mapping_type", "object"); + b.field("path_match", "artifacts.*"); + b.startObject("mapping"); + { + b.startObject("properties"); + { + b.startObject("identifiers"); + { + b.startObject("properties"); + b.startObject("name").field("type", "keyword").endObject(); + b.startObject("label").field("type", "keyword").endObject(); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "dynamic1": { + "identifiers": { + "value": 100, + "name": "diagnostic-configuration-v1" + } + }, + "dynamic2": { + "identifiers": { + "value": 500, + "name": "diagnostic-configuration-v2" + } + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper dynamic1 = (ObjectMapper) artifacts.getMapper("dynamic1"); + ObjectMapper identifiers1 = (ObjectMapper) dynamic1.getMapper("identifiers"); + Mapper name1 = identifiers1.getMapper("name"); + assertThat(name1, instanceOf(KeywordFieldMapper.class)); + assertNotNull(identifiers1.getMapper("value")); + Mapper label1 = identifiers1.getMapper("label"); + assertThat(label1, instanceOf(KeywordFieldMapper.class)); + + ObjectMapper dynamic2 = (ObjectMapper) artifacts.getMapper("dynamic2"); + ObjectMapper identifiers2 = (ObjectMapper) dynamic2.getMapper("identifiers"); + Mapper name2 = identifiers2.getMapper("name"); + assertThat(name2, instanceOf(KeywordFieldMapper.class)); + assertNotNull(identifiers2.getMapper("value")); + Mapper label2 = identifiers2.getMapper("label"); + assertThat(label2, instanceOf(KeywordFieldMapper.class)); + + LuceneDocument rootDoc = doc.rootDoc(); + assertNotNull(rootDoc.getField("artifacts.dynamic1.identifiers.name")); + assertNotNull(rootDoc.getField("artifacts.dynamic1.identifiers.value")); + assertNotNull(rootDoc.getField("artifacts.dynamic2.identifiers.name")); + assertNotNull(rootDoc.getField("artifacts.dynamic2.identifiers.value")); + } + + public void testDynamicSubobjectWithInnerObject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("map_artifact_identifiers"); + { + b.field("match_mapping_type", "object"); + b.field("path_match", "artifacts.*"); + b.startObject("mapping"); + { + b.startObject("properties"); + { + b.startObject("identifiers"); + { + b.startObject("properties"); + { + b.startObject("name").field("type", "keyword").endObject(); + b.startObject("subobject"); + { + b.startObject("properties"); + b.startObject("label").field("type", "keyword").endObject(); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "dynamic": { + "identifiers": { + "subobject" : { + "label": "test", + "value" : 1000 + } + } + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper dynamic = (ObjectMapper) artifacts.getMapper("dynamic"); + ObjectMapper identifiers = (ObjectMapper) dynamic.getMapper("identifiers"); + Mapper name = identifiers.getMapper("name"); + assertThat(name, instanceOf(KeywordFieldMapper.class)); + ObjectMapper subobject = (ObjectMapper) identifiers.getMapper("subobject"); + Mapper label = subobject.getMapper("label"); + assertThat(label, instanceOf(KeywordFieldMapper.class)); + assertNotNull(subobject.getMapper("value")); + + LuceneDocument rootDoc = doc.rootDoc(); + assertNotNull(rootDoc.getField("artifacts.dynamic.identifiers.subobject.label")); + assertNotNull(rootDoc.getField("artifacts.dynamic.identifiers.subobject.value")); + } + + public void testDynamicSubobjectsWithFieldsAndDynamic() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("map_artifact_identifiers"); + { + b.field("match_mapping_type", "object"); + b.field("path_match", "artifacts.*"); + b.startObject("mapping"); + { + b.startObject("properties"); + { + b.startObject("identifiers"); + { + b.field("dynamic", false); + b.startObject("properties"); + { + b.startObject("subobject"); + { + b.field("type", "object"); + b.field("dynamic", true); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "dynamic": { + "identifiers": { + "anything": "test", + "subobject" : { + "anything" : "test" + } + } + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper dynamic = (ObjectMapper) artifacts.getMapper("dynamic"); + ObjectMapper identifiers = (ObjectMapper) dynamic.getMapper("identifiers"); + assertEquals(ObjectMapper.Dynamic.FALSE, identifiers.dynamic); + assertEquals(1, identifiers.mappers.size()); + ObjectMapper subobject = (ObjectMapper) identifiers.getMapper("subobject"); + assertEquals(ObjectMapper.Dynamic.TRUE, subobject.dynamic); + assertNotNull(subobject.getMapper("anything")); + } + + public void testDynamicSubobjectWithInnerObjectDocWithEmptyObject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("map_artifact_identifiers"); + { + b.field("match_mapping_type", "object"); + b.field("path_match", "artifacts.*"); + b.startObject("mapping"); + { + b.startObject("properties"); + { + b.startObject("identifiers"); + { + b.startObject("properties"); + { + b.startObject("name").field("type", "keyword").endObject(); + b.startObject("subobject"); + { + b.startObject("properties"); + b.startObject("label").field("type", "keyword").endObject(); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "dynamic": { + "identifiers": { + } + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper dynamic = (ObjectMapper) artifacts.getMapper("dynamic"); + ObjectMapper identifiers = (ObjectMapper) dynamic.getMapper("identifiers"); + Mapper name = identifiers.getMapper("name"); + assertThat(name, instanceOf(KeywordFieldMapper.class)); + ObjectMapper subobject = (ObjectMapper) identifiers.getMapper("subobject"); + Mapper label = subobject.getMapper("label"); + assertThat(label, instanceOf(KeywordFieldMapper.class)); + } + + public void testEnabledFalseDocWithEmptyObject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("disabled_object"); + { + b.field("match_mapping_type", "object"); + b.field("match", "disabled"); + b.startObject("mapping"); + b.field("enabled", false); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + // here we provide the empty object to make sure that it gets mapped even if it does not have sub-fields defined + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "disabled": { + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper disabled = (ObjectMapper) artifacts.getMapper("disabled"); + assertFalse(disabled.enabled.value()); + } + + public void testDynamicStrictDocWithEmptyObject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("strict_object"); + { + b.field("match_mapping_type", "object"); + b.field("match", "strict"); + b.startObject("mapping"); + b.field("dynamic", "strict"); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + // here we provide the empty object to make sure that it gets mapped even if it does not have sub-fields defined + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "strict": { + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper strict = (ObjectMapper) artifacts.getMapper("strict"); + assertEquals(ObjectMapper.Dynamic.STRICT, strict.dynamic()); + } + + public void testSubobjectsFalseDocWithEmptyObject() throws IOException { + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + { + b.startObject("disabled_object"); + { + b.field("match_mapping_type", "object"); + b.field("match", "leaf"); + b.startObject("mapping"); + b.field("subobjects", false); + b.endObject(); + } + b.endObject(); + } + b.endObject(); + b.endArray(); + })); + // here we provide the empty object to make sure that it gets mapped even if it does not have sub-fields defined + ParsedDocument doc = mapperService.documentMapper().parse(source(""" + { + "artifacts": { + "leaf": { + } + } + } + """)); + + Mapping mapping = doc.dynamicMappingsUpdate(); + ObjectMapper artifacts = (ObjectMapper) mapping.getRoot().getMapper("artifacts"); + ObjectMapper leaf = (ObjectMapper) artifacts.getMapper("leaf"); + assertFalse(leaf.subobjects()); + } }