From df1ac5b2a44a026324758c5a52f3b6fb0f2f2361 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Tue, 28 Jul 2026 11:37:42 -0700 Subject: [PATCH 1/7] Add plugin SPI for dynamic field-type inference and templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a generic extension point so mapper plugins can participate in dynamic mapping for their own field types, without core hard-coding any plugin-specific type knowledge. A single hook, tryPluginInference(), fires in innerParseObject() before the token-type switch (covering arrays, objects, and scalars). It offers an unmapped field to two plugin mechanisms: - DynamicFieldTypeInferencer: inspects the buffered value and returns a mapping config to claim the field, or null to pass. - DynamicTemplateTypeHandler: backs a plugin-registered match_mapping_type (validated against the plugin registry when it is not a builtin XContentFieldType), completing the template config before the TypeParser builds the mapper. isConfigComplete() gates eager index-creation validation. Both receive a FieldValueParserSupplier, which hands out a fresh XContentParser over the buffered field bytes on demand — lazy, no boxing, preserves JSON fidelity. If no plugin claims the field, the buffered bytes are replayed through the existing path, so all current behavior is preserved and the hook fast-exits when no plugin is registered. Plugins register via new MapperPlugin SPI methods collected by IndicesModule into MapperRegistry. All new types are @ExperimentalApi. Signed-off-by: ved naykude --- .../index/mapper/DocumentParser.java | 284 +++++++++ .../mapper/DynamicFieldTypeInferencer.java | 51 ++ .../index/mapper/DynamicTemplate.java | 79 ++- .../mapper/DynamicTemplateTypeHandler.java | 62 ++ .../mapper/FieldValueParserSupplier.java | 81 +++ .../index/mapper/MapperService.java | 14 + .../index/mapper/RootObjectMapper.java | 71 ++- .../org/opensearch/indices/IndicesModule.java | 30 +- .../indices/mapper/MapperRegistry.java | 30 + .../org/opensearch/plugins/MapperPlugin.java | 22 + .../index/mapper/DynamicTemplateTests.java | 9 +- .../mapper/PluginDynamicTemplateTests.java | 547 ++++++++++++++++++ 12 files changed, 1251 insertions(+), 29 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/FieldValueParserSupplier.java create mode 100644 server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java 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 e0820d485b5f6..bcf39d44c9f79 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -622,6 +622,17 @@ private static void innerParseObject( parser.skipChildren(); } } else { + // Before branching by token type, offer the field to plugin inferencers. + // This is the single convergence point where we know the field name, the + // incoming token type, and paths — before the code fans out to parseObject / + // parseArray / parseValue. Placing the hook here means one method covers arrays, + // objects, and scalars rather than requiring three separate hooks. The hook only + // fires for unmapped fields; if the field has a mapper we skip directly to the + // switch below via the early-return inside tryPluginInference. + if (tryPluginInference(context, mapper, currentFieldName, paths)) { + token = parser.nextToken(); + continue; + } // Process different token types during object parsing switch (token) { case START_OBJECT: @@ -1097,6 +1108,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper, case TRUE: case STRICT_ALLOW_TEMPLATES: case FALSE_ALLOW_TEMPLATES: + // Try template matching with OBJECT type (existing behavior). Mapper.Builder builder = findTemplateBuilder( context, arrayFieldName, @@ -1145,6 +1157,223 @@ private static boolean parsesArrayValue(Mapper mapper) { return mapper instanceof FieldMapper fieldMapper && fieldMapper.parsesArrayValue(); } + /** + * Offers an unmapped field to all registered plugin inferencers and plugin-registered dynamic template types + * before the normal token-type branching takes place in {@link #innerParseObject}. + * + *

This is called at the single convergence point in {@code innerParseObject} where the field name, + * the incoming token type, and the parser position are all known simultaneously — before the code fans + * out into {@code parseObject} / {@code parseArray} / {@code parseValue}. Placing the hook here means + * one method handles arrays, objects, and scalars without requiring separate hooks per token type, + * making the SPI genuinely generic rather than array-specific. + * + *

Fast-path exits (no buffering, no deserialization): + *

+ * + *

When buffering does occur, the content is kept for replay: if no plugin claims the field, the + * same bytes are replayed through the normal unmapped-field logic so existing behavior is preserved. + * + * @return {@code true} if this method consumed the parser (either a plugin claimed the field or the + * content was replayed through the existing path); {@code false} to fall through to the normal + * token-type switch in {@code innerParseObject}. + */ + private static boolean tryPluginInference(ParseContext context, ObjectMapper parentMapper, String fieldName, String[] paths) + throws IOException { + // Fast path: no plugins registered — zero overhead + List inferencers = context.mapperService().getDynamicFieldTypeInferencers(); + Map templateTypes = context.mapperService().getDynamicTemplateTypes(); + if (inferencers.isEmpty() && templateTypes.isEmpty()) { + return false; + } + + // Fast path: field is already mapped — let the normal path handle it + Mapper existingMapper = getMapper(context, parentMapper, fieldName, paths); + if (existingMapper != null) { + return false; + } + + // Only fire for dynamic=TRUE / STRICT_ALLOW_TEMPLATES / FALSE_ALLOW_TEMPLATES + final String[] resolvedPaths = paths != null ? paths : splitAndValidatePath(fieldName); + Tuple parentMapperTuple = getDynamicParentMapper(context, resolvedPaths, parentMapper); + ObjectMapper resolvedParent = parentMapperTuple.v2(); + ObjectMapper.Dynamic dynamic = dynamicOrDefault(resolvedParent, context); + if (dynamic == ObjectMapper.Dynamic.STRICT || dynamic == ObjectMapper.Dynamic.FALSE) { + // Release path-slots added by getDynamicParentMapper before returning + for (int i = 0; i < parentMapperTuple.v1(); i++) { + context.path().remove(); + } + return false; + } + + XContentParser parser = context.parser(); + MediaType contentType = parser.contentType(); + + // Buffer the complete field value — needed for replay regardless of whether + // a plugin claims the field or not (streaming parser can only be read once) + byte[] rawContent; + try (XContentBuilder bufferBuilder = XContentBuilder.builder(contentType.xContent())) { + bufferBuilder.copyCurrentStructure(parser); + rawContent = BytesReference.toBytes(BytesReference.bytes(bufferBuilder)); + } + + // Hand plugins a factory that produces a fresh parser over the buffered bytes rather than a + // pre-deserialized object. Core stays free of any representation contract: each plugin streams + // the tokens it needs. Plugins whose config is already complete never call get(), so no parsing + // happens for them. + final FieldValueParserSupplier fieldValueParser = new FieldValueParserSupplier( + contentType, + parser.getXContentRegistry(), + parser.getDeprecationHandler(), + rawContent + ); + + final String resolvedFieldName = resolvedPaths[resolvedPaths.length - 1]; + + // Step 1: Check plugin-registered dynamic templates first — explicit user intent beats + // auto-inference. A user who writes match_mapping_type: "knn_vector" with dimension: 64 + // has declared their intent; we must not reject it because it falls below the inferencer + // threshold. Template matching is already scoped by the user's match/path_match patterns + // on the DynamicTemplate, so it only fires for fields the user intended. + for (Map.Entry entry : templateTypes.entrySet()) { + Mapper.Builder templateBuilder = findPluginTemplateBuilder( + context, + resolvedFieldName, + entry, + dynamic, + resolvedParent.fullPath(), + fieldValueParser + ); + if (templateBuilder != null) { + Mapper.BuilderContext templateBuilderContext = new Mapper.BuilderContext( + context.indexSettings().getSettings(), + context.path() + ); + Mapper templateMapper = templateBuilder.build(templateBuilderContext); + context.addDynamicMapper(templateMapper); + try ( + XContentParser replayParser = contentType.xContent() + .createParser(context.parser().getXContentRegistry(), context.parser().getDeprecationHandler(), rawContent) + ) { + replayParser.nextToken(); + ParseContext replayContext = context.switchParser(replayParser); + context.path().add(resolvedFieldName); + parseObjectOrField(replayContext, templateMapper); + context.path().remove(); + } + for (int i = 0; i < parentMapperTuple.v1(); i++) { + context.path().remove(); + } + return true; + } + } + + // Step 2: No template matched — run the inferencer as the auto-detection fallback. + // This is the path for fields with no user-defined template: the inferencer checks + // whether the field looks like a plugin-managed type (e.g. numeric array >= 128 elements). + Map inferredFieldMapping = null; + for (DynamicFieldTypeInferencer inferencer : inferencers) { + try { + inferredFieldMapping = inferencer.inferFieldType(fieldValueParser); + } catch (Exception e) { + // A buggy inferencer must not break document parsing + continue; + } + if (inferredFieldMapping != null) { + break; + } + } + + if (inferredFieldMapping == null) { // put these into one if + // No template and no inferencer claimed this field — fall through to existing path + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + return true; + } + + String inferredType = (String) inferredFieldMapping.get("type"); + if (inferredType == null) { + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + return true; + } + + // Step 3: No template — use inferencer result directly. + Mapper.TypeParser.ParserContext parserContext = context.docMapperParser().parserContext(); + Mapper.TypeParser typeParser = parserContext.typeParser(inferredType); + if (typeParser == null) { + // Unknown type — fall through to existing path rather than failing + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + return true; + } + + Mapper.Builder builder = typeParser.parse(resolvedFieldName, inferredFieldMapping, parserContext); + Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path()); + Mapper inferredMapper = builder.build(builderContext); + context.addDynamicMapper(inferredMapper); + + // Replay buffered content through the new mapper + try ( + XContentParser replayParser = contentType.xContent() + .createParser(parser.getXContentRegistry(), parser.getDeprecationHandler(), rawContent) + ) { + replayParser.nextToken(); // position at the start of the value + ParseContext replayContext = context.switchParser(replayParser); + context.path().add(resolvedFieldName); + parseObjectOrField(replayContext, inferredMapper); + context.path().remove(); + } + + for (int i = 0; i < parentMapperTuple.v1(); i++) { + context.path().remove(); + } + return true; + } + + /** + * Replays raw-buffered field content through the normal unmapped-field logic — dynamic + * templates, parseDynamicValue, parseNonDynamicArray — as if the buffer had never been created. + */ + private static void replayThroughExistingPath( + ParseContext context, + ObjectMapper parentMapper, + String fieldName, + byte[] rawContent, + int pathsAddedByGetDynamicParentMapper + ) throws IOException { + XContentParser originalParser = context.parser(); + try ( + XContentParser replayParser = originalParser.contentType() + .xContent() + .createParser(originalParser.getXContentRegistry(), originalParser.getDeprecationHandler(), rawContent) + ) { + replayParser.nextToken(); // position at value start + ParseContext replayContext = context.switchParser(replayParser); + XContentParser.Token replayToken = replayParser.currentToken(); + String[] replayPaths = splitAndValidatePath(fieldName); + switch (replayToken) { + case START_OBJECT: + parseObject(replayContext, parentMapper, fieldName, replayPaths); + break; + case START_ARRAY: + parseArray(replayContext, parentMapper, fieldName, replayPaths); + break; + case VALUE_NULL: + parseNullValue(replayContext, parentMapper, fieldName, replayPaths); + break; + default: + if (replayToken != null && replayToken.isValue()) { + parseValue(replayContext, parentMapper, fieldName, replayToken, replayPaths); + } + } + } finally { + for (int i = 0; i < pathsAddedByGetDynamicParentMapper; i++) { + context.path().remove(); + } + } + } + private static void parseNonDynamicArray(ParseContext context, ObjectMapper mapper, final String lastFieldName, String arrayFieldName) throws IOException { @@ -1794,4 +2023,59 @@ private static Mapper.Builder findTemplateBuilder( } return builder; } + + /** + * Attempts to find a plugin-registered dynamic template whose {@code match_mapping_type} equals + * {@code pluginType} and whose path/name patterns match the field being parsed. + * + *

If a matching template is found, calls {@link DynamicTemplateTypeHandler#adjustMappingConfig} + * with a factory that produces a fresh parser over the buffered field bytes, so the handler can + * inject any required parameters (e.g. dimension) before the {@link Mapper.TypeParser} builds the + * mapper. This runs before TypeParser so that parameters that can only be inferred from the data + * are present when the mapper is constructed. Handlers whose config is already complete never call + * {@code get()}, so no parsing happens for fully-specified templates. + * + * @param name the simple field name being parsed (last path component) + * @param entry the plugin type string (e.g. {@code "knn_vector"}) and its handler + * @param fieldValueParser produces a fresh parser over the buffered field bytes + * @return a {@link Mapper.Builder} ready to build the mapper, or {@code null} if no template matched + */ + @SuppressWarnings("rawtypes") + private static Mapper.Builder findPluginTemplateBuilder( + ParseContext context, + String name, + Map.Entry entry, + ObjectMapper.Dynamic dynamic, + String fieldFullPath, + FieldValueParserSupplier fieldValueParser + ) throws IOException { + String pluginType = entry.getKey(); + DynamicTemplateTypeHandler handler = entry.getValue(); + DynamicTemplate dynamicTemplate = findPluginTemplate(context.root(), context.path(), name, pluginType); + if (dynamicTemplate == null) { + return null; + } + String mappingType = dynamicTemplate.mappingType(pluginType); + Mapper.TypeParser.ParserContext parserContext = context.docMapperParser().parserContext(); + Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); + if (typeParser == null) { + throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]"); + } + Map mappingConfig = dynamicTemplate.mappingForName(name, pluginType); + // The handler completes the config (injects its own type when omitted, and any data-derived + // params such as dimension) before the TypeParser builds the mapper. + handler.adjustMappingConfig(mappingConfig, fieldValueParser); + return typeParser.parse(name, mappingConfig, parserContext); + } + + /** Scans dynamic templates on the root mapper for one whose {@code match_mapping_type} equals {@code pluginType} and whose path/name patterns match. */ + private static DynamicTemplate findPluginTemplate(RootObjectMapper root, ContentPath path, String name, String pluginType) { + final String pathAsString = path.pathAsText(name); + for (DynamicTemplate dynamicTemplate : root.dynamicTemplates()) { + if (dynamicTemplate.matchesPluginType(pathAsString, name, pluginType)) { + return dynamicTemplate; + } + } + return null; + } } diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java b/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java new file mode 100644 index 0000000000000..d070a3b916e11 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java @@ -0,0 +1,51 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.Map; + +/** + * SPI for plugins to register dynamic field type inference logic. + * + *

When DocumentParser encounters an unmapped field and no dynamic template matches, it buffers + * the field content and calls {@link #inferFieldType} on each registered inferencer in order. The + * first non-null config map wins; the type is read from the {@code "type"} key, the mapper is built, + * and the field content is replayed through it. If no inferencer claims the field, existing fallback + * behavior applies. + * + *

Rather than deserializing the field value into a fixed Java representation, core hands the + * inferencer a {@link FieldValueParserSupplier} that produces a fresh {@link XContentParser} over the + * buffered bytes. Each call to {@code fieldValueParser.get()} returns an independent parser positioned before + * the field value, so the plugin can inspect the content however it needs — for example streaming + * through tokens to count array elements. This keeps core free of any representation contract: each + * plugin decides how to interpret the value. + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface DynamicFieldTypeInferencer { + + /** + * Inspect the buffered field value and decide whether to claim it. + * + * @param fieldValueParser produces a fresh {@link XContentParser} over the buffered field bytes; + * call {@code get()} and advance the parser to read the value. The returned + * parser should be closed by the caller (e.g. via try-with-resources). + * @return a mutable mapping config map with at minimum a {@code "type"} key (e.g. + * {@code {"type": "knn_vector", "dimension": 384}}), or {@code null} to pass to the + * next inferencer. The map MUST be mutable — TypeParser implementations call + * {@code node.remove()} on it during parsing. + * @throws IOException if reading from the parser fails + */ + Map inferFieldType(FieldValueParserSupplier fieldValueParser) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java index da62b20c98563..56d10812b3da4 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java @@ -195,7 +195,18 @@ public static XContentFieldType fromString(String value) { public abstract String defaultMappingType(); } + /** Parses a dynamic template without plugin-type validation: any unknown match_mapping_type is kept as a plugin match type. */ public static DynamicTemplate parse(String name, Map conf) throws MapperParsingException { + return parse(name, conf, null); + } + + /** + * Parses a dynamic template. If {@code knownPluginTypes} is non-null, a {@code match_mapping_type} + * that is neither an {@link XContentFieldType} nor a registered plugin type fails here, before any + * {@link DynamicTemplate} is constructed. + */ + static DynamicTemplate parse(String name, Map conf, Map knownPluginTypes) + throws MapperParsingException { String match = null; String pathMatch = null; String unmatch = null; @@ -235,8 +246,28 @@ public static DynamicTemplate parse(String name, Map conf) throw } XContentFieldType xcontentFieldType = null; - if (matchMappingType != null && matchMappingType.equals("*") == false) { - xcontentFieldType = XContentFieldType.fromString(matchMappingType); + String pluginMatchType = null; + if (matchMappingType != null && !matchMappingType.equals("*")) { + for (XContentFieldType t : XContentFieldType.values()) { + if (t.toString().equals(matchMappingType)) { + xcontentFieldType = t; + break; + } + } + if (xcontentFieldType == null) { + pluginMatchType = matchMappingType; + // Validate plugin type before constructing the template + if (knownPluginTypes != null && !knownPluginTypes.containsKey(pluginMatchType)) { + List allTypes = new ArrayList<>(); + for (XContentFieldType t : XContentFieldType.values()) { + allTypes.add(t.toString()); + } + allTypes.addAll(knownPluginTypes.keySet()); + throw new IllegalArgumentException( + "No field type matched on [" + pluginMatchType + "], possible values are " + allTypes + ); + } + } } final MatchType matchType = MatchType.fromString(matchPattern); @@ -256,7 +287,7 @@ public static DynamicTemplate parse(String name, Map conf) throw } } - return new DynamicTemplate(name, pathMatch, pathUnmatch, match, unmatch, xcontentFieldType, matchType, mapping); + return new DynamicTemplate(name, pathMatch, pathUnmatch, match, unmatch, xcontentFieldType, pluginMatchType, matchType, mapping); } private final String name; @@ -273,6 +304,8 @@ public static DynamicTemplate parse(String name, Map conf) throw private final XContentFieldType xcontentFieldType; + private final String pluginMatchType; + private final Map mapping; private DynamicTemplate( @@ -282,6 +315,7 @@ private DynamicTemplate( String match, String unmatch, XContentFieldType xcontentFieldType, + String pluginMatchType, MatchType matchType, Map mapping ) { @@ -292,6 +326,7 @@ private DynamicTemplate( this.unmatch = unmatch; this.matchType = matchType; this.xcontentFieldType = xcontentFieldType; + this.pluginMatchType = pluginMatchType; this.mapping = mapping; } @@ -303,25 +338,26 @@ public String pathMatch() { return pathMatch; } + private boolean matchesPathAndName(String path, String name) { + if (pathMatch != null && !matchType.matches(pathMatch, path)) return false; + if (match != null && !matchType.matches(match, name)) return false; + if (pathUnmatch != null && matchType.matches(pathUnmatch, path)) return false; + if (unmatch != null && matchType.matches(unmatch, name)) return false; + return true; + } + public boolean match(String path, String name, XContentFieldType xcontentFieldType) { - if (pathMatch != null && !matchType.matches(pathMatch, path)) { - return false; - } - if (match != null && !matchType.matches(match, name)) { - return false; - } - if (pathUnmatch != null && matchType.matches(pathUnmatch, path)) { - return false; - } - if (unmatch != null && matchType.matches(unmatch, name)) { - return false; - } - if (this.xcontentFieldType != null && this.xcontentFieldType != xcontentFieldType) { - return false; - } + if (!matchesPathAndName(path, name)) return false; + if (this.xcontentFieldType != null && this.xcontentFieldType != xcontentFieldType) return false; return true; } + /** Returns true if this template's {@code match_mapping_type} equals {@code pluginType} and the path/name patterns match. */ + public boolean matchesPluginType(String path, String name, String pluginType) { + if (this.pluginMatchType == null || !this.pluginMatchType.equals(pluginType)) return false; + return matchesPathAndName(path, name); + } + public String mappingType(String dynamicType) { String type; if (mapping.containsKey("type")) { @@ -401,6 +437,11 @@ XContentFieldType getXContentFieldType() { return xcontentFieldType; } + /** Returns the plugin-registered match_mapping_type string, or null if this is a standard template. */ + public String getPluginMatchType() { + return pluginMatchType; + } + Map getMapping() { return mapping; } @@ -422,6 +463,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } if (xcontentFieldType != null) { builder.field("match_mapping_type", xcontentFieldType); + } else if (pluginMatchType != null) { + builder.field("match_mapping_type", pluginMatchType); } else if (match == null && pathMatch == null) { builder.field("match_mapping_type", "*"); } diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java new file mode 100644 index 0000000000000..bd1ee79ad5704 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java @@ -0,0 +1,62 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.Map; + +/** + * Handler for plugin-registered dynamic template types. + * Called when a dynamic template matches on a plugin-registered type string + * (e.g. "knn_vector") to allow the plugin to adjust the mapping configuration + * before the mapper is built. + * + *

Rather than deserializing the field value for the handler, core hands it a + * {@link FieldValueParserSupplier} that produces a fresh {@link XContentParser} over the buffered + * bytes. A handler whose template config is already complete never calls {@code get()}, + * so no parsing happens for fully-specified templates. A handler that needs a + * data-derived parameter (e.g. a vector's dimension) creates a parser and reads what it + * needs. + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface DynamicTemplateTypeHandler { + + /** + * Adjust the mapping configuration before the TypeParser builds the mapper. + * Called when a dynamic template matches but before the mapper is constructed. + * + * @param mappingConfig the mutable mapping config from the template (modified in place) + * @param fieldValueParser produces a fresh {@link XContentParser} over the buffered field bytes; + * only call {@code get()} if the config is missing a parameter that must + * be derived from the data. Close the returned parser (e.g. via + * try-with-resources). + * @throws IOException if reading from the parser fails + */ + void adjustMappingConfig(Map mappingConfig, FieldValueParserSupplier fieldValueParser) throws IOException; + + /** + * Returns {@code true} if the given template mapping config is fully specified — i.e. building a + * mapper from it requires no parameter derived from a document ({@link #adjustMappingConfig} would + * not need to read the field value). When this returns {@code true}, core validates the template + * eagerly at index-creation time by handing the config to the {@link Mapper.TypeParser}, which + * reports any invalid content. When it returns {@code false}, validation is deferred to + * document-parse time, where the data-derived parameters become available. + * + * @param mappingConfig the mapping config from the matched template + * @return whether the config can be validated without inspecting a document + */ + default boolean isConfigComplete(Map mappingConfig) { + return false; + } +} diff --git a/server/src/main/java/org/opensearch/index/mapper/FieldValueParserSupplier.java b/server/src/main/java/org/opensearch/index/mapper/FieldValueParserSupplier.java new file mode 100644 index 0000000000000..5dec7b88bf0ca --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/FieldValueParserSupplier.java @@ -0,0 +1,81 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.MediaType; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; + +/** + * Supplies a fresh {@link XContentParser} positioned at the start of a buffered field value. + * + *

Handed to dynamic-mapping plugin extension points ({@link DynamicFieldTypeInferencer} and + * {@link DynamicTemplateTypeHandler}) so they can inspect an unmapped field's value without core + * committing to any deserialized representation. Each call to {@link #get()} creates an independent + * parser over the same buffered bytes, so a plugin may read the value more than once. The caller + * closes the returned parser (e.g. via try-with-resources). + * + *

Plugins whose configuration is already complete need not call {@link #get()} at all — index-creation + * validation constructs the supplier with {@link #withoutValue()}, whose {@link #get()} throws, so a + * complete config that never reads the value is validated without any field data being available. + * + * @opensearch.experimental + */ +@ExperimentalApi +public class FieldValueParserSupplier { + + private final MediaType contentType; + private final NamedXContentRegistry registry; + private final DeprecationHandler deprecationHandler; + private final byte[] rawContent; + + /** + * Creates a supplier over the buffered field value. {@link #get()} produces a fresh parser + * positioned at the value's first token on each call. + */ + public FieldValueParserSupplier( + MediaType contentType, + NamedXContentRegistry registry, + DeprecationHandler deprecationHandler, + byte[] rawContent + ) { + this.contentType = contentType; + this.registry = registry; + this.deprecationHandler = deprecationHandler; + this.rawContent = rawContent; + } + + /** + * A supplier with no field value behind it — {@link #get()} throws. Used at index-creation time, + * where a fully specified template config must be validated without reading any document. + */ + public static FieldValueParserSupplier withoutValue() { + return new FieldValueParserSupplier(null, null, null, null); + } + + /** + * Returns a fresh {@link XContentParser} positioned at the first token of the field value. The + * caller is responsible for closing it (e.g. via try-with-resources). + * + * @throws IOException if the parser cannot be created + * @throws IllegalStateException if this supplier has no field value ({@link #withoutValue()}) + */ + public XContentParser get() throws IOException { + if (rawContent == null) { + throw new IllegalStateException("No field value is available to parse"); + } + XContentParser parser = contentType.xContent().createParser(registry, deprecationHandler, rawContent); + parser.nextToken(); // position at the start of the value + return parser; + } +} 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 ccad43bd73c6e..b686999354c13 100644 --- a/server/src/main/java/org/opensearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/opensearch/index/mapper/MapperService.java @@ -818,6 +818,20 @@ public Set getMetadataFields() { return Collections.unmodifiableSet(mapperRegistry.getMetadataMapperParsers().keySet()); } + /** + * Returns the list of registered dynamic field type inferencers. + */ + public List getDynamicFieldTypeInferencers() { + return mapperRegistry.getDynamicFieldTypeInferencers(); + } + + /** + * Returns the map of registered dynamic template type handlers. + */ + public Map getDynamicTemplateTypes() { + return mapperRegistry.getDynamicTemplateTypes(); + } + /** * An analyzer wrapper that can lookup fields within the index mappings */ diff --git a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java index 609f4b25d6840..2b6e0ae878fb7 100644 --- a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java @@ -282,11 +282,17 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam Map.Entry entry = tmpl.entrySet().iterator().next(); String templateName = entry.getKey(); Map templateParams = (Map) entry.getValue(); - DynamicTemplate template = DynamicTemplate.parse(templateName, templateParams); - if (template != null) { + DynamicTemplate template = DynamicTemplate.parse( + templateName, + templateParams, + parserContext.mapperService().getDynamicTemplateTypes() + ); + if (template.getPluginMatchType() == null) { validateDynamicTemplate(parserContext, template); - templates.add(template); + } else { + validatePluginDynamicTemplate(parserContext, template); } + templates.add(template); } builder.dynamicTemplates(templates); return true; @@ -451,7 +457,7 @@ private Mapper.Builder findTemplateBuilder(ParseContext context, String name, XC public DynamicTemplate findTemplate(ContentPath path, String name, XContentFieldType matchType) { final String pathAsString = path.pathAsText(name); for (DynamicTemplate dynamicTemplate : dynamicTemplates.value()) { - if (dynamicTemplate.match(pathAsString, name, matchType)) { + if (dynamicTemplate.getPluginMatchType() == null && dynamicTemplate.match(pathAsString, name, matchType)) { return dynamicTemplate; } } @@ -713,6 +719,63 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars } } + /** + * Validates a dynamic template whose {@code match_mapping_type} is a plugin-registered type + * (e.g. {@code knn_vector}) at index-creation time. + * + *

Plugin templates are allowed to be intentionally incomplete: a parameter such as a vector's + * {@code dimension} may be derived from the first indexed document rather than declared in the + * template. Such templates cannot be validated up front, so we only validate eagerly when the + * plugin reports — via {@link DynamicTemplateTypeHandler#isConfigComplete} — that the config is + * fully specified. In that case we hand the config to the plugin's {@link Mapper.TypeParser}, and + * the type parser (not core) reports any invalid content, exactly as it would for an explicit + * field mapping of the same type. Otherwise validation is deferred to document-parse time. + */ + private static void validatePluginDynamicTemplate(Mapper.TypeParser.ParserContext parserContext, DynamicTemplate dynamicTemplate) { + // {name} placeholders can't be resolved until a concrete field name is known — skip, as the + // builtin path does. + if (containsSnippet(dynamicTemplate.getMapping(), "{name}")) { + return; + } + + String pluginType = dynamicTemplate.getPluginMatchType(); + DynamicTemplateTypeHandler handler = parserContext.mapperService().getDynamicTemplateTypes().get(pluginType); + if (handler == null) { + // Unknown plugin types are already rejected in DynamicTemplate.parse; nothing to do here. + return; + } + + String mappingType = dynamicTemplate.mappingType(pluginType); + Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); + if (typeParser == null) { + // No parser to validate against — defer to document-parse time, which reports the error. + return; + } + + String templateName = "__dynamic__" + dynamicTemplate.getName(); + Map fieldTypeConfig = dynamicTemplate.mappingForName(templateName, pluginType); + if (handler.isConfigComplete(fieldTypeConfig) == false) { + // Config relies on data-derived parameters; it can only be validated once a document is seen. + return; + } + + // Config is fully specified. Let the handler normalize it (e.g. inject its own type when the + // template omitted it) — a complete config never reads the field value, so the supplier is + // never invoked at index-creation time. Then hand it to the type parser, which validates the + // content and reports any invalid config. + try { + // No document is available at index creation, so the supplier's get() throws. A complete + // config never reads the field value, so this is a no-op normalization (e.g. type injection). + handler.adjustMappingConfig(fieldTypeConfig, FieldValueParserSupplier.withoutValue()); + } catch (IllegalStateException | IOException e) { + // A complete config performs no I/O and must not read the field value. If a handler + // violates that contract, treat it as non-fatal and defer validation to document-parse time + // rather than failing index creation. + return; + } + typeParser.parse(templateName, fieldTypeConfig, parserContext); + } + private static boolean containsSnippet(Map map, String snippet) { for (Map.Entry entry : map.entrySet()) { String key = entry.getKey().toString(); diff --git a/server/src/main/java/org/opensearch/indices/IndicesModule.java b/server/src/main/java/org/opensearch/indices/IndicesModule.java index b3e7950020dff..a13259e7018f3 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesModule.java +++ b/server/src/main/java/org/opensearch/indices/IndicesModule.java @@ -52,6 +52,8 @@ import org.opensearch.index.mapper.DateFieldMapper; import org.opensearch.index.mapper.DerivedFieldMapper; import org.opensearch.index.mapper.DocCountFieldMapper; +import org.opensearch.index.mapper.DynamicFieldTypeInferencer; +import org.opensearch.index.mapper.DynamicTemplateTypeHandler; import org.opensearch.index.mapper.FieldAliasMapper; import org.opensearch.index.mapper.FieldNamesFieldMapper; import org.opensearch.index.mapper.FlatObjectFieldMapper; @@ -114,11 +116,37 @@ public IndicesModule(List mapperPlugins) { this.mapperRegistry = new MapperRegistry( getMappers(mapperPlugins), getMetadataMappers(mapperPlugins), - getFieldFilter(mapperPlugins) + getFieldFilter(mapperPlugins), + getDynamicFieldTypeInferencers(mapperPlugins), + getDynamicTemplateTypes(mapperPlugins) ); registerBuiltinWritables(); } + /** Collects dynamic field type inferencers from all mapper plugins in registration order. */ + private static List getDynamicFieldTypeInferencers(List mapperPlugins) { + List inferencers = new ArrayList<>(); + for (MapperPlugin mapperPlugin : mapperPlugins) { + inferencers.addAll(mapperPlugin.getDynamicFieldTypeInferencers()); + } + return inferencers; + } + + /** Merges dynamic template type handlers from all mapper plugins; throws if two plugins register the same type string. */ + private static Map getDynamicTemplateTypes(List mapperPlugins) { + Map templateTypes = new LinkedHashMap<>(); + for (MapperPlugin mapperPlugin : mapperPlugins) { + Map pluginTypes = mapperPlugin.getDynamicTemplateTypes(); + for (Map.Entry entry : pluginTypes.entrySet()) { + if (templateTypes.containsKey(entry.getKey())) { + throw new IllegalArgumentException("dynamic template type [" + entry.getKey() + "] is already registered"); + } + templateTypes.put(entry.getKey(), entry.getValue()); + } + } + return templateTypes; + } + private void registerBuiltinWritables() { namedWritables.add(new NamedWriteableRegistry.Entry(Condition.class, MaxAgeCondition.NAME, MaxAgeCondition::new)); namedWritables.add(new NamedWriteableRegistry.Entry(Condition.class, MaxDocsCondition.NAME, MaxDocsCondition::new)); diff --git a/server/src/main/java/org/opensearch/indices/mapper/MapperRegistry.java b/server/src/main/java/org/opensearch/indices/mapper/MapperRegistry.java index 7974de3514ce3..d3cbd57c25916 100644 --- a/server/src/main/java/org/opensearch/indices/mapper/MapperRegistry.java +++ b/server/src/main/java/org/opensearch/indices/mapper/MapperRegistry.java @@ -33,12 +33,16 @@ package org.opensearch.indices.mapper; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.index.mapper.DynamicFieldTypeInferencer; +import org.opensearch.index.mapper.DynamicTemplateTypeHandler; import org.opensearch.index.mapper.Mapper; import org.opensearch.index.mapper.MetadataFieldMapper; import org.opensearch.plugins.MapperPlugin; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.function.Function; import java.util.function.Predicate; @@ -54,15 +58,29 @@ public final class MapperRegistry { private final Map mapperParsers; private final Map metadataMapperParsers; private final Function> fieldFilter; + private final List dynamicFieldTypeInferencers; + private final Map dynamicTemplateTypes; public MapperRegistry( Map mapperParsers, Map metadataMapperParsers, Function> fieldFilter + ) { + this(mapperParsers, metadataMapperParsers, fieldFilter, Collections.emptyList(), Collections.emptyMap()); + } + + public MapperRegistry( + Map mapperParsers, + Map metadataMapperParsers, + Function> fieldFilter, + List dynamicFieldTypeInferencers, + Map dynamicTemplateTypes ) { this.mapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(mapperParsers)); this.metadataMapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(metadataMapperParsers)); this.fieldFilter = fieldFilter; + this.dynamicFieldTypeInferencers = Collections.unmodifiableList(new ArrayList<>(dynamicFieldTypeInferencers)); + this.dynamicTemplateTypes = Collections.unmodifiableMap(new LinkedHashMap<>(dynamicTemplateTypes)); } /** @@ -98,4 +116,16 @@ public boolean isMetadataField(String field) { public Function> getFieldFilter() { return fieldFilter; } + + /** + * Returns the registered dynamic field type inferencers, in priority order. + */ + public List getDynamicFieldTypeInferencers() { + return dynamicFieldTypeInferencers; + } + + /** Returns the registered dynamic template type handlers keyed by their match_mapping_type string. */ + public Map getDynamicTemplateTypes() { + return dynamicTemplateTypes; + } } diff --git a/server/src/main/java/org/opensearch/plugins/MapperPlugin.java b/server/src/main/java/org/opensearch/plugins/MapperPlugin.java index 1480f46582ff5..a65cbe869b579 100644 --- a/server/src/main/java/org/opensearch/plugins/MapperPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/MapperPlugin.java @@ -32,6 +32,8 @@ package org.opensearch.plugins; +import org.opensearch.index.mapper.DynamicFieldTypeInferencer; +import org.opensearch.index.mapper.DynamicTemplateTypeHandler; import org.opensearch.index.mapper.Mapper; import org.opensearch.index.mapper.MappingTransformer; import org.opensearch.index.mapper.MetadataFieldMapper; @@ -100,4 +102,24 @@ default Function> getFieldFilter() { default List getMappingTransformers() { return Collections.emptyList(); } + + /** + * Returns dynamic field type inferencers provided by this plugin. + * These are consulted when an unmapped array field is encountered during document indexing + * and no template matches. The first inferencer to claim a field wins. + */ + default List getDynamicFieldTypeInferencers() { + return Collections.emptyList(); + } + + /** + * Returns dynamic template types registered by this plugin. + * These allow plugins to register custom match_mapping_type strings + * (e.g. "knn_vector") that users can reference in dynamic templates. + * The key is the type string, and the value is the handler that adjusts + * the mapper builder when a template matches. + */ + default Map getDynamicTemplateTypes() { + return Collections.emptyMap(); + } } diff --git a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java index 3d53f3fbebd87..7aecf220fd21e 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java @@ -58,12 +58,9 @@ public void testParseUnknownMatchType() { Map templateDef2 = new HashMap<>(); templateDef2.put("match_mapping_type", "text"); templateDef2.put("mapping", Collections.singletonMap("store", true)); - // if a wrong match type is specified, we ignore the template - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> DynamicTemplate.parse("my_template", templateDef2)); - assertEquals( - "No field type matched on [text], possible values are [object, string, long, double, boolean, date, binary]", - e.getMessage() - ); + // Unknown match_mapping_type is stored as pluginMatchType — validation against the registry happens in RootObjectMapper + DynamicTemplate template = DynamicTemplate.parse("my_template", templateDef2); + assertEquals("text", template.getPluginMatchType()); } public void testParseInvalidRegex() { diff --git a/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java new file mode 100644 index 0000000000000..b9c5b55f1bd1b --- /dev/null +++ b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java @@ -0,0 +1,547 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.mapper.DynamicTemplate.XContentFieldType; +import org.opensearch.plugins.MapperPlugin; +import org.opensearch.plugins.Plugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for plugin-extensible dynamic template types (match_mapping_type SPI) + * and field type inferencers (DynamicFieldTypeInferencer SPI). + * + * Uses a minimal stub plugin: + * - Registers "mock_type" as a plugin match_mapping_type + * - Registers an inferencer that claims numeric scalars >= 100 as "long" + * (scalars are safe — Long mapper handles them without parsesArrayValue issues) + */ +public class PluginDynamicTemplateTests extends MapperServiceTestCase { + + private static final String MOCK_TYPE = "mock_type"; + // A plugin type whose handler always reads the field value, i.e. it depends on data-derived + // parameters and therefore cannot be validated at index-creation time. + private static final String MOCK_INFERRED_TYPE = "mock_inferred_type"; + + /** No-op handler — template config used as-is. Reports its config as always complete. */ + static class MockTemplateTypeHandler implements DynamicTemplateTypeHandler { + @Override + public void adjustMappingConfig(Map mappingConfig, FieldValueParserSupplier fieldValueParser) {} + + @Override + public boolean isConfigComplete(Map mappingConfig) { + return true; + } + } + + /** Handler for a data-derived type: config is never complete, so validation is deferred. */ + static class MockInferredTemplateTypeHandler implements DynamicTemplateTypeHandler { + @Override + public void adjustMappingConfig(Map mappingConfig, FieldValueParserSupplier fieldValueParser) throws IOException { + try (XContentParser parser = fieldValueParser.get()) { + parser.currentToken(); + } + } + + @Override + public boolean isConfigComplete(Map mappingConfig) { + return false; + } + } + + /** + * Claims numeric scalars >= 100 as "long". + * Using scalars (not arrays) avoids needing a parsesArrayValue mapper in core tests. + */ + static class MockInferencer implements DynamicFieldTypeInferencer { + @Override + public Map inferFieldType(FieldValueParserSupplier fieldValueParser) throws IOException { + try (XContentParser parser = fieldValueParser.get()) { + if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) return null; + if (parser.doubleValue() < 100) return null; + } + Map config = new HashMap<>(); + config.put("type", "long"); + return config; + } + } + + /** + * TypeParser for a plugin field type that requires a {@code required_param} and throws a + * {@link MapperParsingException} when it is missing — mirroring how the knn_vector parser rejects + * a mapping with no {@code dimension}. Used to prove eager index-creation-time validation throws. + */ + static class RequiresParamTypeParser implements Mapper.TypeParser { + @Override + public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { + if (node.containsKey("required_param") == false) { + throw new MapperParsingException("required_param missing for field [" + name + "]"); + } + node.remove("type"); + node.remove("required_param"); + return new MockFieldMapper.Builder(name); + } + } + + static class MockMapperPlugin extends Plugin implements MapperPlugin { + @Override + public Map getDynamicTemplateTypes() { + Map handlers = new HashMap<>(); + handlers.put(MOCK_TYPE, new MockTemplateTypeHandler()); + handlers.put(MOCK_INFERRED_TYPE, new MockInferredTemplateTypeHandler()); + return handlers; + } + + @Override + public Map getMappers() { + Map mappers = new HashMap<>(); + mappers.put(MOCK_TYPE, new RequiresParamTypeParser()); + mappers.put(MOCK_INFERRED_TYPE, new RequiresParamTypeParser()); + return mappers; + } + + @Override + public List getDynamicFieldTypeInferencers() { + return Collections.singletonList(new MockInferencer()); + } + } + + @Override + protected Collection getPlugins() { + return Collections.singletonList(new MockMapperPlugin()); + } + + // ===================================================================== + // DynamicTemplate unit tests + // ===================================================================== + + public void testPluginMatchTypeStoredOnParse() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertEquals(MOCK_TYPE, template.getPluginMatchType()); + assertNull(template.getXContentFieldType()); + } + + public void testBuiltinMatchTypeNotStoredAsPlugin() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", "string"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertNull(template.getPluginMatchType()); + assertEquals(XContentFieldType.STRING, template.getXContentFieldType()); + } + + public void testAllBuiltinTypesNotStoredAsPlugin() { + for (XContentFieldType t : XContentFieldType.values()) { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", t.toString()); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t_" + t, conf); + assertNull("builtin type " + t + " must not be stored as pluginMatchType", template.getPluginMatchType()); + assertEquals(t, template.getXContentFieldType()); + } + } + + public void testMatchesPluginTypeTrue() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("match", "big_*"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertTrue(template.matchesPluginType("big_field", "big_field", MOCK_TYPE)); + } + + public void testMatchesPluginTypeFalseWrongType() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertFalse(template.matchesPluginType("field", "field", "other_type")); + } + + public void testMatchesPluginTypeFalseNamePatternMismatch() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("match", "big_*"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertFalse(template.matchesPluginType("small_field", "small_field", MOCK_TYPE)); + } + + public void testMatchesPluginTypeWithPathMatch() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("path_match", "obj.*"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertTrue(template.matchesPluginType("obj.field", "field", MOCK_TYPE)); + assertFalse(template.matchesPluginType("other.field", "field", MOCK_TYPE)); + } + + public void testMatchesPluginTypeWithUnmatch() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("unmatch", "excluded_*"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertTrue(template.matchesPluginType("big_field", "big_field", MOCK_TYPE)); + assertFalse(template.matchesPluginType("excluded_field", "excluded_field", MOCK_TYPE)); + } + + public void testBuiltinMatchNeverReturnsPluginTemplate() throws IOException { + // findTemplate() on RootObjectMapper must skip plugin templates + MapperService mapperService = createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + RootObjectMapper root = mapperService.documentMapper().mapping().root(); + ContentPath path = new ContentPath(); + for (XContentFieldType t : XContentFieldType.values()) { + assertNull("findTemplate must not return plugin template for builtin type " + t, root.findTemplate(path, "any_field", t)); + } + } + + public void testPluginTemplateSerializesMatchMappingType() throws Exception { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", MOCK_TYPE); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + XContentBuilder builder = JsonXContent.contentBuilder(); + template.toXContent(builder, ToXContent.EMPTY_PARAMS); + assertThat(builder.toString(), containsString("\"match_mapping_type\":\"" + MOCK_TYPE + "\"")); + } + + public void testWildcardNotStoredAsPlugin() { + Map conf = new HashMap<>(); + conf.put("match_mapping_type", "*"); + conf.put("mapping", Collections.singletonMap("type", "keyword")); + DynamicTemplate template = DynamicTemplate.parse("t", conf); + assertNull(template.getPluginMatchType()); + assertNull(template.getXContentFieldType()); + } + + // ===================================================================== + // Index creation validation tests + // ===================================================================== + + public void testUnregisteredPluginTypeThrowsAtIndexCreation() throws IOException { + XContentBuilder mapping = topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("bad_template"); + b.field("match_mapping_type", "unregistered_type"); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + }); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping)); + assertThat(e.getMessage(), containsString("No field type matched on [unregistered_type]")); + assertThat(e.getMessage(), containsString(MOCK_TYPE)); + assertThat(e.getMessage(), containsString("string")); + assertThat(e.getMessage(), containsString("long")); + } + + public void testTypoInPluginTypeThrowsWithFullList() throws IOException { + XContentBuilder mapping = topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("typo_template"); + b.field("match_mapping_type", "mock_typo"); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + }); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping)); + assertThat(e.getMessage(), containsString("mock_typo")); + assertThat(e.getMessage(), containsString(MOCK_TYPE)); + } + + public void testRegisteredPluginTypeAcceptedAtIndexCreation() throws IOException { + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + + public void testBuiltinTypeStillWorksAlongsidePluginType() throws IOException { + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("string_template"); + b.field("match_mapping_type", "string"); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + + // ===================================================================== + // Eager plugin-template validation at index creation + // ===================================================================== + + public void testCompletePluginTemplateWithValidConfigAccepted() throws IOException { + // Handler opens no parser (complete config) and the type parser is satisfied → index creation succeeds. + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", MOCK_TYPE); + b.field("required_param", "present"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + + public void testCompletePluginTemplateWithInvalidConfigThrowsAtIndexCreation() throws IOException { + // Complete config (handler opens no parser) but required_param is missing → the type parser + // throws at index creation instead of silently accepting the broken template. + XContentBuilder mapping = topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", MOCK_TYPE); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + }); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping)); + assertThat(e.getMessage(), containsString("required_param missing")); + } + + public void testInferredPluginTemplateWithIncompleteConfigNotValidatedAtIndexCreation() throws IOException { + // The handler for this type always opens the parser (data-derived config), so even though + // required_param is absent the template must NOT be validated/rejected at index creation. + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("inferred_template"); + b.field("match_mapping_type", MOCK_INFERRED_TYPE); + b.startObject("mapping"); + b.field("type", MOCK_INFERRED_TYPE); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + + public void testPluginTemplateWithNamePlaceholderSkipsEagerValidation() throws IOException { + // {name} can't be resolved up front, so validation is skipped even though required_param is missing. + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("named_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", MOCK_TYPE); + b.field("field_name", "{name}"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + + // ===================================================================== + // Document indexing — inferencer tests + // ===================================================================== + + public void testInferencerClaimsLargeNumericScalar() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + // 200 >= 100 threshold — MockInferencer claims it as "long" + ParsedDocument doc = mapper.parse(source(b -> b.field("big_number", 200))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("big_number"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("long")); + } + + public void testInferencerDoesNotClaimSmallNumericScalar() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + // 50 < 100 threshold — MockInferencer returns null, normal path maps as long + ParsedDocument doc = mapper.parse(source(b -> b.field("small_number", 50))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("small_number"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("long")); + } + + public void testInferencerDoesNotClaimString() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + // String — MockInferencer returns null (not a Number), normal path maps as text + ParsedDocument doc = mapper.parse(source(b -> b.field("str_field", "hello"))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("str_field"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("text")); + } + + public void testInferencerDoesNotClaimBoolean() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + ParsedDocument doc = mapper.parse(source(b -> b.field("bool_field", true))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("bool_field"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("boolean")); + } + + public void testExplicitMappingBeatsInference() throws IOException { + // Explicit keyword mapping — inference must not fire even for value >= 100 + DocumentMapper mapper = createDocumentMapper(topMapping(b -> { + b.startObject("properties"); + b.startObject("my_field"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + })); + ParsedDocument doc = mapper.parse(source(b -> b.field("my_field", 200))); + assertNull("explicit mapping must beat inferencer", doc.dynamicMappingsUpdate()); + } + + public void testAlreadyInferredFieldSkipsInferenceOnSecondDoc() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + // First doc triggers inference + ParsedDocument first = mapper.parse(source(b -> b.field("big_number", 200))); + assertNotNull(first.dynamicMappingsUpdate()); + + // Simulate mapping update applied — second doc should not re-infer + MapperService mapperService = createMapperService(topMapping(b -> { + b.startObject("properties"); + b.startObject("big_number"); + b.field("type", "long"); + b.endObject(); + b.endObject(); + })); + ParsedDocument second = mapperService.documentMapper().parse(source(b -> b.field("big_number", 300))); + assertNull("already mapped field must not re-trigger inference", second.dynamicMappingsUpdate()); + } + + public void testNoPluginsNormalPathRuns() throws IOException { + // Without plugins, tryPluginInference returns false immediately + MapperService noPluginService = new MapperServiceTestCase() { + @Override + protected Collection getPlugins() { + return Collections.emptyList(); + } + }.createMapperService(topMapping(b -> {})); + + ParsedDocument doc = noPluginService.documentMapper().parse(source(b -> b.field("my_field", "hello"))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("my_field"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("text")); + } + + public void testBuiltinTemplateFallbackWhenInferencerDoesNotClaim() throws IOException { + // Builtin string → keyword template still fires when inferencer doesn't claim + DocumentMapper mapper = createDocumentMapper(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("strings_as_keyword"); + b.field("match_mapping_type", "string"); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + ParsedDocument doc = mapper.parse(source(b -> b.field("my_field", "hello"))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("my_field"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("keyword")); + } + + /** Plugin that registers only a template type handler — no inferencer */ + static class TemplateOnlyPlugin extends Plugin implements MapperPlugin { + @Override + public Map getDynamicTemplateTypes() { + return Collections.singletonMap(MOCK_TYPE, new MockTemplateTypeHandler()); + } + } + + public void testFastPathNotExitedWhenOnlyTemplateTypesRegistered() throws IOException { + // Plugin registers template type but no inferencer. + // tryPluginInference must NOT fast-exit — template types alone are enough to proceed. + MapperService mapperService = new MapperServiceTestCase() { + @Override + protected Collection getPlugins() { + return Collections.singletonList(new TemplateOnlyPlugin()); + } + }.createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("mock_template"); + b.field("match_mapping_type", MOCK_TYPE); + b.startObject("mapping"); + b.field("type", "keyword"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + assertNotNull(mapperService.documentMapper()); + } +} From 7863ebc5118be54976ea308a4fb23177c63d4478 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Wed, 29 Jul 2026 12:15:26 -0700 Subject: [PATCH 2/7] Fail on ambiguous dynamic type inference Per OpenSearch triage: instead of taking the first plugin claim by registration/load order, DocumentParser now consults ALL registered plugin template types and ALL inferencers for an unmapped field and throws if more than one claims it: - Two plugin dynamic-template types matching the same field -> throw. - Two field-type inferencers claiming the same field -> throw. Zero claims still fall through to existing behavior; exactly one is used as before. The exception names the conflicting claimants so the misconfiguration is clear. Adds PluginInferenceConflictTests covering both conflict paths and the single-match (no-throw) case. Signed-off-by: ved naykude --- .../index/mapper/DocumentParser.java | 118 +++++++++---- .../index/mapper/RootObjectMapper.java | 12 +- .../mapper/PluginInferenceConflictTests.java | 158 ++++++++++++++++++ .../indices/IndicesModuleTests.java | 55 ++++++ 4 files changed, 313 insertions(+), 30 deletions(-) create mode 100644 server/src/test/java/org/opensearch/index/mapper/PluginInferenceConflictTests.java 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 bcf39d44c9f79..cc4cc1e3aabf2 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -32,6 +32,8 @@ package org.opensearch.index.mapper; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.lucene.document.Field; import org.apache.lucene.index.FieldInfos; import org.apache.lucene.index.IndexableField; @@ -76,6 +78,8 @@ */ final class DocumentParser { + private static final Logger logger = LogManager.getLogger(DocumentParser.class); + private final IndexSettings indexSettings; private final DocumentMapperParser docMapperParser; private final DocumentMapper docMapper; @@ -1238,8 +1242,14 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par // has declared their intent; we must not reject it because it falls below the inferencer // threshold. Template matching is already scoped by the user's match/path_match patterns // on the DynamicTemplate, so it only fires for fields the user intended. + // + // We evaluate ALL registered template types rather than stopping at the first match: if two + // plugin types both match the same field, that is an ambiguous configuration and we fail + // loudly (per OpenSearch triage) rather than silently letting registration order decide. + Mapper.Builder templateBuilder = null; + String templateMatchedType = null; for (Map.Entry entry : templateTypes.entrySet()) { - Mapper.Builder templateBuilder = findPluginTemplateBuilder( + Mapper.Builder candidate = findPluginTemplateBuilder( context, resolvedFieldName, entry, @@ -1247,43 +1257,88 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par resolvedParent.fullPath(), fieldValueParser ); - if (templateBuilder != null) { - Mapper.BuilderContext templateBuilderContext = new Mapper.BuilderContext( - context.indexSettings().getSettings(), - context.path() - ); - Mapper templateMapper = templateBuilder.build(templateBuilderContext); - context.addDynamicMapper(templateMapper); - try ( - XContentParser replayParser = contentType.xContent() - .createParser(context.parser().getXContentRegistry(), context.parser().getDeprecationHandler(), rawContent) - ) { - replayParser.nextToken(); - ParseContext replayContext = context.switchParser(replayParser); - context.path().add(resolvedFieldName); + if (candidate != null) { + if (templateBuilder != null) { + throw new MapperParsingException( + "field [" + + resolvedFieldName + + "] matched more than one dynamic template plugin type: [" + + templateMatchedType + + "] and [" + + entry.getKey() + + "]; the mapping is ambiguous" + ); + } + templateBuilder = candidate; + templateMatchedType = entry.getKey(); + } + } + if (templateBuilder != null) { + Mapper.BuilderContext templateBuilderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path()); + Mapper templateMapper = templateBuilder.build(templateBuilderContext); + context.addDynamicMapper(templateMapper); + try ( + XContentParser replayParser = contentType.xContent() + .createParser(context.parser().getXContentRegistry(), context.parser().getDeprecationHandler(), rawContent) + ) { + replayParser.nextToken(); + ParseContext replayContext = context.switchParser(replayParser); + context.path().add(resolvedFieldName); + try { parseObjectOrField(replayContext, templateMapper); + } finally { + // Release the field-name slot even if replay throws, so ContentPath is not left + // corrupt for subsequent fields in the same document. context.path().remove(); } + } finally { for (int i = 0; i < parentMapperTuple.v1(); i++) { context.path().remove(); } - return true; } + return true; } - // Step 2: No template matched — run the inferencer as the auto-detection fallback. - // This is the path for fields with no user-defined template: the inferencer checks - // whether the field looks like a plugin-managed type (e.g. numeric array >= 128 elements). + // Step 2: No template matched — run the inferencers as the auto-detection fallback. + // This is the path for fields with no user-defined template: each inferencer checks whether + // the field looks like a plugin-managed type (e.g. numeric array >= 128 elements). + // + // We consult ALL registered inferencers rather than stopping at the first claim: if two + // inferencers both claim the same field, that is ambiguous and we fail loudly (per OpenSearch + // triage) rather than letting plugin load order silently pick a winner. Map inferredFieldMapping = null; + DynamicFieldTypeInferencer claimingInferencer = null; for (DynamicFieldTypeInferencer inferencer : inferencers) { + Map claim; try { - inferredFieldMapping = inferencer.inferFieldType(fieldValueParser); + claim = inferencer.inferFieldType(fieldValueParser); } catch (Exception e) { - // A buggy inferencer must not break document parsing + // A buggy inferencer must not break document parsing, but the failure must be visible: + // log it rather than swallowing silently, then move on to the next inferencer. + logger.warn( + () -> new org.apache.logging.log4j.message.ParameterizedMessage( + "dynamic field type inferencer [{}] threw while inspecting field [{}]; skipping it", + inferencer.getClass().getName(), + resolvedFieldName + ), + e + ); continue; } - if (inferredFieldMapping != null) { - break; + if (claim != null) { + if (inferredFieldMapping != null) { + throw new MapperParsingException( + "field [" + + resolvedFieldName + + "] was claimed by more than one dynamic field type inferencer: [" + + claimingInferencer.getClass().getName() + + "] and [" + + inferencer.getClass().getName() + + "]; the inferred type is ambiguous" + ); + } + inferredFieldMapping = claim; + claimingInferencer = inferencer; } } @@ -1321,12 +1376,17 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par replayParser.nextToken(); // position at the start of the value ParseContext replayContext = context.switchParser(replayParser); context.path().add(resolvedFieldName); - parseObjectOrField(replayContext, inferredMapper); - context.path().remove(); - } - - for (int i = 0; i < parentMapperTuple.v1(); i++) { - context.path().remove(); + try { + parseObjectOrField(replayContext, inferredMapper); + } finally { + // Release the field-name slot even if replay throws, so ContentPath is not left + // corrupt for subsequent fields in the same document. + context.path().remove(); + } + } finally { + for (int i = 0; i < parentMapperTuple.v1(); i++) { + context.path().remove(); + } } return true; } diff --git a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java index 2b6e0ae878fb7..c5aa3482d4b2f 100644 --- a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java @@ -74,6 +74,7 @@ @PublicApi(since = "1.0.0") public class RootObjectMapper extends ObjectMapper { private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(RootObjectMapper.class); + private static final org.apache.logging.log4j.Logger logger = org.apache.logging.log4j.LogManager.getLogger(RootObjectMapper.class); /** * Default parameters for root object @@ -770,7 +771,16 @@ private static void validatePluginDynamicTemplate(Mapper.TypeParser.ParserContex } catch (IllegalStateException | IOException e) { // A complete config performs no I/O and must not read the field value. If a handler // violates that contract, treat it as non-fatal and defer validation to document-parse time - // rather than failing index creation. + // rather than failing index creation — but log it so the contract violation is visible. + logger.warn( + () -> new org.apache.logging.log4j.message.ParameterizedMessage( + "dynamic template type handler for [{}] threw while normalizing a complete config for template [{}]; " + + "deferring validation to document-parse time", + dynamicTemplate.getName(), + templateName + ), + e + ); return; } typeParser.parse(templateName, fieldTypeConfig, parserContext); diff --git a/server/src/test/java/org/opensearch/index/mapper/PluginInferenceConflictTests.java b/server/src/test/java/org/opensearch/index/mapper/PluginInferenceConflictTests.java new file mode 100644 index 0000000000000..e2987b1071fb9 --- /dev/null +++ b/server/src/test/java/org/opensearch/index/mapper/PluginInferenceConflictTests.java @@ -0,0 +1,158 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.plugins.MapperPlugin; +import org.opensearch.plugins.Plugin; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; + +/** + * Tests that core fails loudly when more than one plugin mechanism claims the same unmapped field — + * two inferencers both claiming, or two plugin template types both matching. Per OpenSearch triage + * (Froh): loop through all claimants and throw on ambiguity rather than letting registration/load + * order silently pick a winner. + */ +public class PluginInferenceConflictTests extends MapperServiceTestCase { + + private static final String TYPE_A = "type_a"; + private static final String TYPE_B = "type_b"; + + /** An inferencer that claims every numeric scalar as {@code claimedType}. */ + static class AlwaysClaimsNumberInferencer implements DynamicFieldTypeInferencer { + private final String claimedType; + + AlwaysClaimsNumberInferencer(String claimedType) { + this.claimedType = claimedType; + } + + @Override + public Map inferFieldType(FieldValueParserSupplier fieldValueParser) throws IOException { + try (XContentParser parser = fieldValueParser.get()) { + if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) return null; + } + Map config = new HashMap<>(); + config.put("type", claimedType); + return config; + } + } + + /** A no-op template handler whose config is always complete (validated eagerly, no field read). */ + static class NoopHandler implements DynamicTemplateTypeHandler { + @Override + public void adjustMappingConfig(Map mappingConfig, FieldValueParserSupplier fieldValueParser) {} + + @Override + public boolean isConfigComplete(Map mappingConfig) { + return true; + } + } + + /** Builds a MockFieldMapper for a mock plugin type (no required params). */ + static class MockTypeParser implements Mapper.TypeParser { + @Override + public Mapper.Builder parse(String name, Map node, ParserContext parserContext) { + node.remove("type"); + return new MockFieldMapper.Builder(name); + } + } + + /** + * Registers two inferencers that both claim numeric scalars, and two plugin template types that + * both match any field — so a single unmapped field is claimed by two mechanisms of each kind. + */ + static class ConflictingPlugin extends Plugin implements MapperPlugin { + @Override + public List getDynamicFieldTypeInferencers() { + return Arrays.asList(new AlwaysClaimsNumberInferencer(TYPE_A), new AlwaysClaimsNumberInferencer(TYPE_B)); + } + + @Override + public Map getDynamicTemplateTypes() { + Map handlers = new HashMap<>(); + handlers.put(TYPE_A, new NoopHandler()); + handlers.put(TYPE_B, new NoopHandler()); + return handlers; + } + + @Override + public Map getMappers() { + Map mappers = new HashMap<>(); + mappers.put(TYPE_A, new MockTypeParser()); + mappers.put(TYPE_B, new MockTypeParser()); + return mappers; + } + } + + @Override + protected Collection getPlugins() { + return Collections.singletonList(new ConflictingPlugin()); + } + + public void testTwoInferencersClaimSameFieldThrows() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("n", 5)))); + assertThat(e.getMessage(), containsString("claimed by more than one dynamic field type inferencer")); + assertThat(e.getMessage(), containsString("n")); + } + + public void testTwoPluginTemplatesMatchSameFieldThrows() throws IOException { + // Two templates, each targeting a different plugin type but the same field name pattern. + DocumentMapper mapper = createDocumentMapper(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("t_a"); + b.field("match_mapping_type", TYPE_A); + b.field("match", "v_*"); + b.startObject("mapping").field("type", TYPE_A).endObject(); + b.endObject(); + b.endObject(); + b.startObject(); + b.startObject("t_b"); + b.field("match_mapping_type", TYPE_B); + b.field("match", "v_*"); + b.startObject("mapping").field("type", TYPE_B).endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("v_x", 5)))); + assertThat(e.getMessage(), containsString("matched more than one dynamic template plugin type")); + assertThat(e.getMessage(), containsString("v_x")); + } + + public void testSingleTemplateMatchDoesNotThrow() throws IOException { + // Sanity: with only ONE plugin template matching (a name pattern no other template shares), + // there is no ambiguity — the field is claimed and mapped without a conflict exception. + DocumentMapper mapper = createDocumentMapper(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("only_a"); + b.field("match_mapping_type", TYPE_A); + b.field("match", "only_*"); + b.startObject("mapping").field("type", TYPE_A).endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + // "only_field" is a string, so the numeric inferencers don't fire; only template t_a matches. + ParsedDocument doc = mapper.parse(source(b -> b.field("only_field", "text-value"))); + assertNotNull(doc.dynamicMappingsUpdate()); + assertNotNull("single-matching template must map the field, not throw", doc.dynamicMappingsUpdate().root().getMapper("only_field")); + } +} diff --git a/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java b/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java index c54dfa0fab277..077f2485f2874 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java @@ -290,4 +290,59 @@ public Function> getFieldFilter() { assertNotSame(MapperPlugin.NOOP_FIELD_PREDICATE, fieldFilter.apply("hidden_index")); assertNotSame(MapperPlugin.NOOP_FIELD_PREDICATE, fieldFilter.apply("filtered")); } + + /** A no-op dynamic template type handler for registration tests. */ + private static org.opensearch.index.mapper.DynamicTemplateTypeHandler noopHandler() { + return new org.opensearch.index.mapper.DynamicTemplateTypeHandler() { + @Override + public void adjustMappingConfig( + Map mappingConfig, + org.opensearch.index.mapper.FieldValueParserSupplier fieldValueParser + ) {} + }; + } + + /** A no-op inferencer for registration tests. */ + private static org.opensearch.index.mapper.DynamicFieldTypeInferencer noopInferencer() { + return fieldValueParser -> null; + } + + public void testInferencersAndTemplateTypesRegisteredFromPlugins() { + List plugins = Collections.singletonList(new MapperPlugin() { + @Override + public List getDynamicFieldTypeInferencers() { + return Collections.singletonList(noopInferencer()); + } + + @Override + public Map getDynamicTemplateTypes() { + return Collections.singletonMap("my_type", noopHandler()); + } + }); + MapperRegistry registry = new IndicesModule(plugins).getMapperRegistry(); + assertEquals(1, registry.getDynamicFieldTypeInferencers().size()); + assertTrue(registry.getDynamicTemplateTypes().containsKey("my_type")); + } + + public void testDuplicateTemplateTypeAcrossPluginsRejectedAtStartup() { + List plugins = Arrays.asList(new MapperPlugin() { + @Override + public Map getDynamicTemplateTypes() { + return Collections.singletonMap("dup_type", noopHandler()); + } + }, new MapperPlugin() { + @Override + public Map getDynamicTemplateTypes() { + return Collections.singletonMap("dup_type", noopHandler()); + } + }); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IndicesModule(plugins)); + assertThat(e.getMessage(), containsString("dynamic template type [dup_type] is already registered")); + } + + public void testNoDynamicInferencersOrTemplateTypesByDefault() { + MapperRegistry registry = new IndicesModule(Collections.emptyList()).getMapperRegistry(); + assertTrue(registry.getDynamicFieldTypeInferencers().isEmpty()); + assertTrue(registry.getDynamicTemplateTypes().isEmpty()); + } } From 000d53400ee64912f044697352fcc8b3dd3e5878 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Wed, 29 Jul 2026 15:10:26 -0700 Subject: [PATCH 3/7] Add tests for inferencer error/edge branches Cover the previously-untested fall-through and error paths in DocumentParser.tryPluginInference and RootObjectMapper eager validation: - Throwing inferencer is caught and the field falls through to normal inference (not a parse failure). - Inferencer returning a config with no "type", or an unknown type with no registered TypeParser, falls through instead of failing. - A template handler that reports its config complete but throws during adjustMappingConfig is treated as non-fatal at index creation (deferred). Adds PluginInferencerEdgeCaseTests and a throwing-handler case in PluginDynamicTemplateTests. Improves patch coverage on the new branches. Signed-off-by: ved naykude --- .../mapper/PluginDynamicTemplateTests.java | 39 ++++++ .../mapper/PluginInferencerEdgeCaseTests.java | 124 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 server/src/test/java/org/opensearch/index/mapper/PluginInferencerEdgeCaseTests.java diff --git a/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java index b9c5b55f1bd1b..f7e9a9a3320d8 100644 --- a/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java @@ -41,6 +41,9 @@ public class PluginDynamicTemplateTests extends MapperServiceTestCase { // A plugin type whose handler always reads the field value, i.e. it depends on data-derived // parameters and therefore cannot be validated at index-creation time. private static final String MOCK_INFERRED_TYPE = "mock_inferred_type"; + // A plugin type whose handler reports its config complete but throws when normalizing it — used to + // verify eager index-creation validation treats a handler contract violation as non-fatal (defer). + private static final String MOCK_THROWING_TYPE = "mock_throwing_type"; /** No-op handler — template config used as-is. Reports its config as always complete. */ static class MockTemplateTypeHandler implements DynamicTemplateTypeHandler { @@ -68,6 +71,22 @@ public boolean isConfigComplete(Map mappingConfig) { } } + /** + * Reports its config as complete (so index-creation validation runs eagerly) but throws from + * adjustMappingConfig — a handler-contract violation. Core must treat this as non-fatal and defer. + */ + static class ThrowingTemplateTypeHandler implements DynamicTemplateTypeHandler { + @Override + public void adjustMappingConfig(Map mappingConfig, FieldValueParserSupplier fieldValueParser) { + throw new IllegalStateException("handler contract violation"); + } + + @Override + public boolean isConfigComplete(Map mappingConfig) { + return true; + } + } + /** * Claims numeric scalars >= 100 as "long". * Using scalars (not arrays) avoids needing a parsesArrayValue mapper in core tests. @@ -108,6 +127,7 @@ public Map getDynamicTemplateTypes() { Map handlers = new HashMap<>(); handlers.put(MOCK_TYPE, new MockTemplateTypeHandler()); handlers.put(MOCK_INFERRED_TYPE, new MockInferredTemplateTypeHandler()); + handlers.put(MOCK_THROWING_TYPE, new ThrowingTemplateTypeHandler()); return handlers; } @@ -116,6 +136,7 @@ public Map getMappers() { Map mappers = new HashMap<>(); mappers.put(MOCK_TYPE, new RequiresParamTypeParser()); mappers.put(MOCK_INFERRED_TYPE, new RequiresParamTypeParser()); + mappers.put(MOCK_THROWING_TYPE, new RequiresParamTypeParser()); return mappers; } @@ -386,6 +407,24 @@ public void testInferredPluginTemplateWithIncompleteConfigNotValidatedAtIndexCre })); } + public void testHandlerThrowingDuringEagerValidationIsNonFatal() throws IOException { + // The handler reports its config complete but throws from adjustMappingConfig. Core must treat + // this contract violation as non-fatal (log + defer) rather than failing index creation. + createMapperService(topMapping(b -> { + b.startArray("dynamic_templates"); + b.startObject(); + b.startObject("throwing_template"); + b.field("match_mapping_type", MOCK_THROWING_TYPE); + b.startObject("mapping"); + b.field("type", MOCK_THROWING_TYPE); + b.field("required_param", "present"); + b.endObject(); + b.endObject(); + b.endObject(); + b.endArray(); + })); + } + public void testPluginTemplateWithNamePlaceholderSkipsEagerValidation() throws IOException { // {name} can't be resolved up front, so validation is skipped even though required_param is missing. createMapperService(topMapping(b -> { diff --git a/server/src/test/java/org/opensearch/index/mapper/PluginInferencerEdgeCaseTests.java b/server/src/test/java/org/opensearch/index/mapper/PluginInferencerEdgeCaseTests.java new file mode 100644 index 0000000000000..9a69ffad24b40 --- /dev/null +++ b/server/src/test/java/org/opensearch/index/mapper/PluginInferencerEdgeCaseTests.java @@ -0,0 +1,124 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.plugins.MapperPlugin; +import org.opensearch.plugins.Plugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +/** + * Exercises the error/edge branches of the inferencer path in {@code DocumentParser.tryPluginInference}: + * a buggy inferencer that throws is caught and skipped; an inferencer that returns a config with no + * {@code type}, or an unknown {@code type}, falls through to the normal path instead of failing. + * + *

Each test installs a plugin with a single inferencer of a fixed behavior — no shared mutable + * state — so the tests are independent and safe under any execution order. + */ +public class PluginInferencerEdgeCaseTests extends MapperServiceTestCase { + + enum Behavior { + THROW, // inferencer throws — must be caught, field falls through to normal path + NO_TYPE, // returns a config map without a "type" key — falls through + UNKNOWN_TYPE // returns a config with a type no TypeParser is registered for — falls through + } + + /** Claims numeric scalars, then behaves per its fixed {@link Behavior}. Immutable, no shared state. */ + private static class ConfigurableInferencer implements DynamicFieldTypeInferencer { + private final Behavior behavior; + + ConfigurableInferencer(Behavior behavior) { + this.behavior = behavior; + } + + @Override + public Map inferFieldType(FieldValueParserSupplier fieldValueParser) throws IOException { + try (XContentParser parser = fieldValueParser.get()) { + if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) return null; + } + switch (behavior) { + case THROW: + throw new RuntimeException("boom from inferencer"); + case NO_TYPE: { + Map config = new HashMap<>(); + config.put("some_param", "x"); // deliberately no "type" + return config; + } + case UNKNOWN_TYPE: + default: { + Map config = new HashMap<>(); + config.put("type", "no_such_registered_type"); + return config; + } + } + } + } + + private static class EdgeCasePlugin extends Plugin implements MapperPlugin { + private final Behavior behavior; + + EdgeCasePlugin(Behavior behavior) { + this.behavior = behavior; + } + + @Override + public List getDynamicFieldTypeInferencers() { + return Collections.singletonList(new ConfigurableInferencer(behavior)); + } + } + + // Set by each test before createDocumentMapper() is called; consumed by getPlugins(). + private Behavior behavior; + + @Override + protected Collection getPlugins() { + return Collections.singletonList(new EdgeCasePlugin(behavior)); + } + + /** A throwing inferencer must not break parsing; the field falls through to normal inference (long). */ + public void testThrowingInferencerIsCaughtAndFieldFallsThrough() throws IOException { + behavior = Behavior.THROW; + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + ParsedDocument doc = mapper.parse(source(b -> b.field("n", 5))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("n"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("long")); + } + + /** A config with no "type" key is ignored; the field falls through to normal inference. */ + public void testInferredConfigWithoutTypeFallsThrough() throws IOException { + behavior = Behavior.NO_TYPE; + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + ParsedDocument doc = mapper.parse(source(b -> b.field("n", 5))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("n"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("long")); + } + + /** A config whose "type" has no registered TypeParser is ignored; the field falls through. */ + public void testInferredUnknownTypeFallsThrough() throws IOException { + behavior = Behavior.UNKNOWN_TYPE; + DocumentMapper mapper = createDocumentMapper(topMapping(b -> {})); + ParsedDocument doc = mapper.parse(source(b -> b.field("n", 5))); + assertNotNull(doc.dynamicMappingsUpdate()); + Mapper fieldMapper = doc.dynamicMappingsUpdate().root().getMapper("n"); + assertNotNull(fieldMapper); + assertThat(fieldMapper.typeName(), equalTo("long")); + } +} From b31f7a0598a82bd3c89de23bd7c90edff57f5594 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Fri, 31 Jul 2026 12:30:01 -0700 Subject: [PATCH 4/7] Address review feedback on dynamic-mapping SPI - Restore public DynamicTemplate.parse(name, conf) to fail fast on an unknown match_mapping_type (pre-plugin-SPI behavior); the plugin path uses the registry-aware overload. No API change. - Remove the always-false handler-null guard in validatePluginDynamicTemplate (the plugin type is registry-validated at parse time). - Use proper log4j imports instead of fully-qualified names; reword the deferred-validation warning; extract the __dynamic__ template-name prefix to a constant; drop a stray inline comment. - Update tests to the registry-aware parse and the restored strict behavior. Signed-off-by: ved naykude --- .../index/mapper/DocumentParser.java | 7 +++--- .../index/mapper/DynamicTemplate.java | 18 ++++++++------ .../index/mapper/RootObjectMapper.java | 24 +++++++++---------- .../index/mapper/DynamicTemplateTests.java | 8 ++++--- .../mapper/PluginDynamicTemplateTests.java | 23 ++++++++++++------ 5 files changed, 48 insertions(+), 32 deletions(-) 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 3da5ab52cfffc..4f7e31af4a8c1 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -34,6 +34,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.document.Field; import org.apache.lucene.index.FieldInfos; import org.apache.lucene.index.IndexableField; @@ -1341,8 +1342,8 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par // A buggy inferencer must not break document parsing, but the failure must be visible: // log it rather than swallowing silently, then move on to the next inferencer. logger.warn( - () -> new org.apache.logging.log4j.message.ParameterizedMessage( - "dynamic field type inferencer [{}] threw while inspecting field [{}]; skipping it", + () -> new ParameterizedMessage( + "Skipping dynamic field type inferencer [{}]: it threw while inspecting field [{}]", inferencer.getClass().getName(), resolvedFieldName ), @@ -1367,7 +1368,7 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par } } - if (inferredFieldMapping == null) { // put these into one if + if (inferredFieldMapping == null) { // No template and no inferencer claimed this field — fall through to existing path replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); return true; diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java index 56d10812b3da4..e3cb59509866a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -195,15 +196,18 @@ public static XContentFieldType fromString(String value) { public abstract String defaultMappingType(); } - /** Parses a dynamic template without plugin-type validation: any unknown match_mapping_type is kept as a plugin match type. */ + /** + * Parses a dynamic template with no plugin registry available: a {@code match_mapping_type} that is + * not a known {@link XContentFieldType} fails immediately, preserving the pre-plugin-SPI behavior. + * The plugin-aware overload is used by the mapping-parse path, which supplies the registry. + */ public static DynamicTemplate parse(String name, Map conf) throws MapperParsingException { - return parse(name, conf, null); + return parse(name, conf, Collections.emptyMap()); } /** - * Parses a dynamic template. If {@code knownPluginTypes} is non-null, a {@code match_mapping_type} - * that is neither an {@link XContentFieldType} nor a registered plugin type fails here, before any - * {@link DynamicTemplate} is constructed. + * Parses a dynamic template. A {@code match_mapping_type} that is neither an {@link XContentFieldType} + * nor a key in {@code knownPluginTypes} fails here, before any {@link DynamicTemplate} is constructed. */ static DynamicTemplate parse(String name, Map conf, Map knownPluginTypes) throws MapperParsingException { @@ -256,8 +260,8 @@ static DynamicTemplate parse(String name, Map conf, Map allTypes = new ArrayList<>(); for (XContentFieldType t : XContentFieldType.values()) { allTypes.add(t.toString()); diff --git a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java index 6b5d044f3b5f3..0bfe284b97b33 100644 --- a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java @@ -32,6 +32,9 @@ package org.opensearch.index.mapper; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.index.LeafReader; import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.Operations; @@ -74,7 +77,8 @@ @PublicApi(since = "1.0.0") public class RootObjectMapper extends ObjectMapper { private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(RootObjectMapper.class); - private static final org.apache.logging.log4j.Logger logger = org.apache.logging.log4j.LogManager.getLogger(RootObjectMapper.class); + private static final Logger logger = LogManager.getLogger(RootObjectMapper.class); + private static final String DYNAMIC_TEMPLATE_NAME_PREFIX = "__dynamic__"; /** * Default parameters for root object @@ -688,7 +692,7 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars continue; } - String templateName = "__dynamic__" + dynamicTemplate.name(); + String templateName = DYNAMIC_TEMPLATE_NAME_PREFIX + dynamicTemplate.name(); Map fieldTypeConfig = dynamicTemplate.mappingForName(templateName, defaultDynamicType); try { Mapper.Builder dummyBuilder = typeParser.parse(templateName, fieldTypeConfig, parserContext); @@ -745,11 +749,8 @@ private static void validatePluginDynamicTemplate(Mapper.TypeParser.ParserContex } String pluginType = dynamicTemplate.getPluginMatchType(); + // The plugin type was validated against the registry at parse time, so a handler is always present. DynamicTemplateTypeHandler handler = parserContext.mapperService().getDynamicTemplateTypes().get(pluginType); - if (handler == null) { - // Unknown plugin types are already rejected in DynamicTemplate.parse; nothing to do here. - return; - } String mappingType = dynamicTemplate.mappingType(pluginType); Mapper.TypeParser typeParser = parserContext.typeParser(mappingType); @@ -758,7 +759,7 @@ private static void validatePluginDynamicTemplate(Mapper.TypeParser.ParserContex return; } - String templateName = "__dynamic__" + dynamicTemplate.getName(); + String templateName = DYNAMIC_TEMPLATE_NAME_PREFIX + dynamicTemplate.getName(); Map fieldTypeConfig = dynamicTemplate.mappingForName(templateName, pluginType); if (handler.isConfigComplete(fieldTypeConfig) == false) { // Config relies on data-derived parameters; it can only be validated once a document is seen. @@ -778,11 +779,10 @@ private static void validatePluginDynamicTemplate(Mapper.TypeParser.ParserContex // violates that contract, treat it as non-fatal and defer validation to document-parse time // rather than failing index creation — but log it so the contract violation is visible. logger.warn( - () -> new org.apache.logging.log4j.message.ParameterizedMessage( - "dynamic template type handler for [{}] threw while normalizing a complete config for template [{}]; " - + "deferring validation to document-parse time", - dynamicTemplate.getName(), - templateName + () -> new ParameterizedMessage( + "Deferring index-creation validation of dynamic template [{}]: its handler threw while " + + "normalizing a config it reported as complete", + dynamicTemplate.getName() ), e ); diff --git a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java index 7aecf220fd21e..40e06080dad79 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DynamicTemplateTests.java @@ -42,6 +42,8 @@ import java.util.HashMap; import java.util.Map; +import static org.hamcrest.Matchers.containsString; + public class DynamicTemplateTests extends OpenSearchTestCase { public void testParseUnknownParam() throws Exception { @@ -58,9 +60,9 @@ public void testParseUnknownMatchType() { Map templateDef2 = new HashMap<>(); templateDef2.put("match_mapping_type", "text"); templateDef2.put("mapping", Collections.singletonMap("store", true)); - // Unknown match_mapping_type is stored as pluginMatchType — validation against the registry happens in RootObjectMapper - DynamicTemplate template = DynamicTemplate.parse("my_template", templateDef2); - assertEquals("text", template.getPluginMatchType()); + // With no plugin registry (the public 2-arg parse), an unknown match_mapping_type fails immediately. + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> DynamicTemplate.parse("my_template", templateDef2)); + assertThat(e.getMessage(), containsString("No field type matched on [text]")); } public void testParseInvalidRegex() { diff --git a/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java index f7e9a9a3320d8..b1704dc4a3116 100644 --- a/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/PluginDynamicTemplateTests.java @@ -155,11 +155,20 @@ protected Collection getPlugins() { // DynamicTemplate unit tests // ===================================================================== + /** Parses a template with the mock plugin registry, so a plugin match_mapping_type is accepted. */ + private static DynamicTemplate parseWithRegistry(String name, Map conf) { + Map registry = new HashMap<>(); + registry.put(MOCK_TYPE, new MockTemplateTypeHandler()); + registry.put(MOCK_INFERRED_TYPE, new MockInferredTemplateTypeHandler()); + registry.put(MOCK_THROWING_TYPE, new ThrowingTemplateTypeHandler()); + return DynamicTemplate.parse(name, conf, registry); + } + public void testPluginMatchTypeStoredOnParse() { Map conf = new HashMap<>(); conf.put("match_mapping_type", MOCK_TYPE); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertEquals(MOCK_TYPE, template.getPluginMatchType()); assertNull(template.getXContentFieldType()); } @@ -189,7 +198,7 @@ public void testMatchesPluginTypeTrue() { conf.put("match_mapping_type", MOCK_TYPE); conf.put("match", "big_*"); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertTrue(template.matchesPluginType("big_field", "big_field", MOCK_TYPE)); } @@ -197,7 +206,7 @@ public void testMatchesPluginTypeFalseWrongType() { Map conf = new HashMap<>(); conf.put("match_mapping_type", MOCK_TYPE); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertFalse(template.matchesPluginType("field", "field", "other_type")); } @@ -206,7 +215,7 @@ public void testMatchesPluginTypeFalseNamePatternMismatch() { conf.put("match_mapping_type", MOCK_TYPE); conf.put("match", "big_*"); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertFalse(template.matchesPluginType("small_field", "small_field", MOCK_TYPE)); } @@ -215,7 +224,7 @@ public void testMatchesPluginTypeWithPathMatch() { conf.put("match_mapping_type", MOCK_TYPE); conf.put("path_match", "obj.*"); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertTrue(template.matchesPluginType("obj.field", "field", MOCK_TYPE)); assertFalse(template.matchesPluginType("other.field", "field", MOCK_TYPE)); } @@ -225,7 +234,7 @@ public void testMatchesPluginTypeWithUnmatch() { conf.put("match_mapping_type", MOCK_TYPE); conf.put("unmatch", "excluded_*"); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); assertTrue(template.matchesPluginType("big_field", "big_field", MOCK_TYPE)); assertFalse(template.matchesPluginType("excluded_field", "excluded_field", MOCK_TYPE)); } @@ -255,7 +264,7 @@ public void testPluginTemplateSerializesMatchMappingType() throws Exception { Map conf = new HashMap<>(); conf.put("match_mapping_type", MOCK_TYPE); conf.put("mapping", Collections.singletonMap("type", "keyword")); - DynamicTemplate template = DynamicTemplate.parse("t", conf); + DynamicTemplate template = parseWithRegistry("t", conf); XContentBuilder builder = JsonXContent.contentBuilder(); template.toXContent(builder, ToXContent.EMPTY_PARAMS); assertThat(builder.toString(), containsString("\"match_mapping_type\":\"" + MOCK_TYPE + "\"")); From 497ce79b1350a9322d0a268c6f168cc5f2d87c5f Mon Sep 17 00:00:00 2001 From: ved naykude Date: Fri, 31 Jul 2026 15:53:42 -0700 Subject: [PATCH 5/7] Keep dynamic-mapping SPI generic and validate plugin type early Remove plugin-specific examples (knn_vector, dimension) from the core dynamic-mapping SPI docs and comments so the interfaces stay generic. Also validate a plugin match_mapping_type against the registry before assigning it as the plugin match type. Signed-off-by: ved naykude --- .../org/opensearch/index/mapper/DocumentParser.java | 10 +++++----- .../index/mapper/DynamicFieldTypeInferencer.java | 2 +- .../org/opensearch/index/mapper/DynamicTemplate.java | 8 ++++---- .../index/mapper/DynamicTemplateTypeHandler.java | 6 ++---- .../org/opensearch/index/mapper/RootObjectMapper.java | 6 +++--- 5 files changed, 15 insertions(+), 17 deletions(-) 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 4f7e31af4a8c1..55602c5e2aa6d 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -1264,8 +1264,8 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par final String resolvedFieldName = resolvedPaths[resolvedPaths.length - 1]; // Step 1: Check plugin-registered dynamic templates first — explicit user intent beats - // auto-inference. A user who writes match_mapping_type: "knn_vector" with dimension: 64 - // has declared their intent; we must not reject it because it falls below the inferencer + // auto-inference. A user who writes a plugin-typed match_mapping_type with explicit params + // has declared their intent; we must not reject it because it falls below an inferencer // threshold. Template matching is already scoped by the user's match/path_match patterns // on the DynamicTemplate, so it only fires for fields the user intended. // @@ -2121,13 +2121,13 @@ private static Mapper.Builder findTemplateBuilder( * *

If a matching template is found, calls {@link DynamicTemplateTypeHandler#adjustMappingConfig} * with a factory that produces a fresh parser over the buffered field bytes, so the handler can - * inject any required parameters (e.g. dimension) before the {@link Mapper.TypeParser} builds the + * inject any required parameters before the {@link Mapper.TypeParser} builds the * mapper. This runs before TypeParser so that parameters that can only be inferred from the data * are present when the mapper is constructed. Handlers whose config is already complete never call * {@code get()}, so no parsing happens for fully-specified templates. * * @param name the simple field name being parsed (last path component) - * @param entry the plugin type string (e.g. {@code "knn_vector"}) and its handler + * @param entry the plugin type string and its handler * @param fieldValueParser produces a fresh parser over the buffered field bytes * @return a {@link Mapper.Builder} ready to build the mapper, or {@code null} if no template matched */ @@ -2154,7 +2154,7 @@ private static Mapper.Builder findPluginTemplateBuilder( } Map mappingConfig = dynamicTemplate.mappingForName(name, pluginType); // The handler completes the config (injects its own type when omitted, and any data-derived - // params such as dimension) before the TypeParser builds the mapper. + // params) before the TypeParser builds the mapper. handler.adjustMappingConfig(mappingConfig, fieldValueParser); return typeParser.parse(name, mappingConfig, parserContext); } diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java b/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java index d070a3b916e11..79f58313532b9 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicFieldTypeInferencer.java @@ -42,7 +42,7 @@ public interface DynamicFieldTypeInferencer { * call {@code get()} and advance the parser to read the value. The returned * parser should be closed by the caller (e.g. via try-with-resources). * @return a mutable mapping config map with at minimum a {@code "type"} key (e.g. - * {@code {"type": "knn_vector", "dimension": 384}}), or {@code null} to pass to the + * {@code {"type": "my_type", "some_param": 384}}), or {@code null} to pass to the * next inferencer. The map MUST be mutable — TypeParser implementations call * {@code node.remove()} on it during parsing. * @throws IOException if reading from the parser fails diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java index e3cb59509866a..d9fe807a7b41b 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplate.java @@ -259,18 +259,18 @@ static DynamicTemplate parse(String name, Map conf, Map allTypes = new ArrayList<>(); for (XContentFieldType t : XContentFieldType.values()) { allTypes.add(t.toString()); } allTypes.addAll(knownPluginTypes.keySet()); throw new IllegalArgumentException( - "No field type matched on [" + pluginMatchType + "], possible values are " + allTypes + "No field type matched on [" + matchMappingType + "], possible values are " + allTypes ); } + pluginMatchType = matchMappingType; } } diff --git a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java index bd1ee79ad5704..9f8624c741609 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java +++ b/server/src/main/java/org/opensearch/index/mapper/DynamicTemplateTypeHandler.java @@ -17,15 +17,13 @@ /** * Handler for plugin-registered dynamic template types. * Called when a dynamic template matches on a plugin-registered type string - * (e.g. "knn_vector") to allow the plugin to adjust the mapping configuration - * before the mapper is built. + * to allow the plugin to adjust the mapping configuration before the mapper is built. * *

Rather than deserializing the field value for the handler, core hands it a * {@link FieldValueParserSupplier} that produces a fresh {@link XContentParser} over the buffered * bytes. A handler whose template config is already complete never calls {@code get()}, * so no parsing happens for fully-specified templates. A handler that needs a - * data-derived parameter (e.g. a vector's dimension) creates a parser and reads what it - * needs. + * data-derived parameter creates a parser and reads what it needs. * * @opensearch.experimental */ diff --git a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java index 0bfe284b97b33..11c9e78107190 100644 --- a/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/RootObjectMapper.java @@ -731,10 +731,10 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars /** * Validates a dynamic template whose {@code match_mapping_type} is a plugin-registered type - * (e.g. {@code knn_vector}) at index-creation time. + * at index-creation time. * - *

Plugin templates are allowed to be intentionally incomplete: a parameter such as a vector's - * {@code dimension} may be derived from the first indexed document rather than declared in the + *

Plugin templates are allowed to be intentionally incomplete: a parameter may be derived from + * the first indexed document rather than declared in the * template. Such templates cannot be validated up front, so we only validate eagerly when the * plugin reports — via {@link DynamicTemplateTypeHandler#isConfigComplete} — that the config is * fully specified. In that case we hand the config to the plugin's {@link Mapper.TypeParser}, and From 1e88a41f99ed3e93093c734d8ea4102c99aaa308 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Mon, 3 Aug 2026 18:06:39 -0700 Subject: [PATCH 6/7] Name both plugins in duplicate template type error When two mapper plugins register the same dynamic template type, name both plugins in the error so the misconfiguration is easy to trace. Signed-off-by: ved naykude --- .../org/opensearch/indices/IndicesModule.java | 15 ++++++++++++++- .../opensearch/indices/IndicesModuleTests.java | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/IndicesModule.java b/server/src/main/java/org/opensearch/indices/IndicesModule.java index a13259e7018f3..d5b50c459312b 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesModule.java +++ b/server/src/main/java/org/opensearch/indices/IndicesModule.java @@ -96,6 +96,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -135,13 +136,25 @@ private static List getDynamicFieldTypeInferencers(L /** Merges dynamic template type handlers from all mapper plugins; throws if two plugins register the same type string. */ private static Map getDynamicTemplateTypes(List mapperPlugins) { Map templateTypes = new LinkedHashMap<>(); + // Tracks which plugin registered each type so a collision can name both plugins involved. + Map registeredBy = new HashMap<>(); for (MapperPlugin mapperPlugin : mapperPlugins) { + String pluginName = mapperPlugin.getClass().getName(); Map pluginTypes = mapperPlugin.getDynamicTemplateTypes(); for (Map.Entry entry : pluginTypes.entrySet()) { if (templateTypes.containsKey(entry.getKey())) { - throw new IllegalArgumentException("dynamic template type [" + entry.getKey() + "] is already registered"); + throw new IllegalArgumentException( + "dynamic template type [" + + entry.getKey() + + "] is registered by both [" + + registeredBy.get(entry.getKey()) + + "] and [" + + pluginName + + "]" + ); } templateTypes.put(entry.getKey(), entry.getValue()); + registeredBy.put(entry.getKey(), pluginName); } } return templateTypes; diff --git a/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java b/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java index 077f2485f2874..f610fbee199ee 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java @@ -337,7 +337,10 @@ public Map getDy } }); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IndicesModule(plugins)); - assertThat(e.getMessage(), containsString("dynamic template type [dup_type] is already registered")); + assertThat(e.getMessage(), containsString("dynamic template type [dup_type] is registered by both")); + // The message names both colliding plugins so the misconfiguration is easy to trace. + assertThat(e.getMessage(), containsString(plugins.get(0).getClass().getName())); + assertThat(e.getMessage(), containsString(plugins.get(1).getClass().getName())); } public void testNoDynamicInferencersOrTemplateTypesByDefault() { From dbd28f06f5dcb1b5c144527bb8aeb0496f717747 Mon Sep 17 00:00:00 2001 From: ved naykude Date: Mon, 3 Aug 2026 18:29:58 -0700 Subject: [PATCH 7/7] Release ContentPath slots on all inference exit paths Wrap the plugin-inference body in a try/finally so path slots added by getDynamicParentMapper are released even when buffering or an ambiguous claim throws, preventing ContentPath corruption for later fields in the same document. Signed-off-by: ved naykude --- .../index/mapper/DocumentParser.java | 57 +++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) 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 55602c5e2aa6d..74147c4cf34d8 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentParser.java @@ -1230,15 +1230,43 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par final String[] resolvedPaths = paths != null ? paths : splitAndValidatePath(fieldName); Tuple parentMapperTuple = getDynamicParentMapper(context, resolvedPaths, parentMapper); ObjectMapper resolvedParent = parentMapperTuple.v2(); + final int parentPathSlots = parentMapperTuple.v1(); ObjectMapper.Dynamic dynamic = dynamicOrDefault(resolvedParent, context); if (dynamic == ObjectMapper.Dynamic.STRICT || dynamic == ObjectMapper.Dynamic.FALSE) { // Release path-slots added by getDynamicParentMapper before returning - for (int i = 0; i < parentMapperTuple.v1(); i++) { + for (int i = 0; i < parentPathSlots; i++) { context.path().remove(); } return false; } + // getDynamicParentMapper added parentPathSlots to context.path(); release them on every exit + // (including an exception, e.g. buffering failure or an ambiguous-claim MapperParsingException) + // so ContentPath is not left corrupt for subsequent fields in the same document. + try { + return attemptPluginInference(context, dynamic, resolvedParent, resolvedPaths, inferencers, templateTypes); + } finally { + for (int i = 0; i < parentPathSlots; i++) { + context.path().remove(); + } + } + } + + /** + * Body of the plugin-inference attempt. Path slots added by {@code getDynamicParentMapper} are + * released by the caller's {@code finally}, so this method only manages the field-name slot it adds + * for replay. Returns {@code true} if a plugin claimed the field (template or inferencer) or it was + * replayed through the existing path. + */ + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static boolean attemptPluginInference( + ParseContext context, + ObjectMapper.Dynamic dynamic, + ObjectMapper resolvedParent, + String[] resolvedPaths, + List inferencers, + Map templateTypes + ) throws IOException { XContentParser parser = context.parser(); MediaType contentType = parser.contentType(); @@ -1317,10 +1345,6 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par // corrupt for subsequent fields in the same document. context.path().remove(); } - } finally { - for (int i = 0; i < parentMapperTuple.v1(); i++) { - context.path().remove(); - } } return true; } @@ -1370,13 +1394,13 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par if (inferredFieldMapping == null) { // No template and no inferencer claimed this field — fall through to existing path - replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent); return true; } String inferredType = (String) inferredFieldMapping.get("type"); if (inferredType == null) { - replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent); return true; } @@ -1385,7 +1409,7 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par Mapper.TypeParser typeParser = parserContext.typeParser(inferredType); if (typeParser == null) { // Unknown type — fall through to existing path rather than failing - replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent, parentMapperTuple.v1()); + replayThroughExistingPath(context, resolvedParent, resolvedFieldName, rawContent); return true; } @@ -1409,10 +1433,6 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par // corrupt for subsequent fields in the same document. context.path().remove(); } - } finally { - for (int i = 0; i < parentMapperTuple.v1(); i++) { - context.path().remove(); - } } return true; } @@ -1421,13 +1441,8 @@ private static boolean tryPluginInference(ParseContext context, ObjectMapper par * Replays raw-buffered field content through the normal unmapped-field logic — dynamic * templates, parseDynamicValue, parseNonDynamicArray — as if the buffer had never been created. */ - private static void replayThroughExistingPath( - ParseContext context, - ObjectMapper parentMapper, - String fieldName, - byte[] rawContent, - int pathsAddedByGetDynamicParentMapper - ) throws IOException { + private static void replayThroughExistingPath(ParseContext context, ObjectMapper parentMapper, String fieldName, byte[] rawContent) + throws IOException { XContentParser originalParser = context.parser(); try ( XContentParser replayParser = originalParser.contentType() @@ -1453,10 +1468,6 @@ private static void replayThroughExistingPath( parseValue(replayContext, parentMapper, fieldName, replayToken, replayPaths); } } - } finally { - for (int i = 0; i < pathsAddedByGetDynamicParentMapper; i++) { - context.path().remove(); - } } }