Abstracted how Text fields use Keyword fields inside of Text fields

plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java

@@ -20,18 +20,21 @@
 import org.apache.lucene.analysis.tokenattributes.TypeAttribute;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.FieldType;
+import org.apache.lucene.document.StoredField;
 import org.apache.lucene.index.IndexOptions;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.analysis.AnalyzerScope;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
+import org.elasticsearch.index.mapper.CompositeSyntheticFieldLoader;
 import org.elasticsearch.index.mapper.DocumentParserContext;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MapperBuilderContext;
-import org.elasticsearch.index.mapper.SourceFieldMapper;
+import org.elasticsearch.index.mapper.SourceLoader;
 import org.elasticsearch.index.mapper.StringStoredFieldFieldLoader;
+import org.elasticsearch.index.mapper.TextFamilyFieldMapper;
 import org.elasticsearch.index.mapper.TextFieldMapper;
 import org.elasticsearch.index.mapper.TextParams;
 import org.elasticsearch.index.mapper.TextSearchInfo;
@@ -61,7 +64,7 @@
  * This code is largely a copy of TextFieldMapper which is less than ideal -
  * my attempts to subclass TextFieldMapper failed but we can revisit this.
  **/
-public class AnnotatedTextFieldMapper extends FieldMapper {
+public class AnnotatedTextFieldMapper extends TextFamilyFieldMapper {
 
     public static final String CONTENT_TYPE = "annotated_text";
 
@@ -84,28 +87,26 @@ public static class Builder extends FieldMapper.Builder {
         final Parameter<String> indexOptions = TextParams.textIndexOptions(m -> builder(m).indexOptions.getValue());
         final Parameter<Boolean> norms = TextParams.norms(true, m -> builder(m).norms.getValue());
         final Parameter<String> termVectors = TextParams.termVectors(m -> builder(m).termVectors.getValue());
+        private final Parameter<Boolean> store = Parameter.storeParam(m -> builder(m).store.getValue(), false);
 
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
         private final IndexVersion indexCreatedVersion;
         private final TextParams.Analyzers analyzers;
-        private final boolean isSyntheticSourceEnabled;
-        private final Parameter<Boolean> store;
+        private final boolean isWithinMultiField;
 
-        public Builder(String name, IndexVersion indexCreatedVersion, IndexAnalyzers indexAnalyzers, boolean isSyntheticSourceEnabled) {
+        private boolean isSyntheticSourceEnabled;
+
+        public Builder(String name, IndexVersion indexCreatedVersion, IndexAnalyzers indexAnalyzers, boolean isWithinMultiField) {
             super(name);
             this.indexCreatedVersion = indexCreatedVersion;
+            this.isWithinMultiField = isWithinMultiField;
             this.analyzers = new TextParams.Analyzers(
                 indexAnalyzers,
                 m -> builder(m).analyzers.getIndexAnalyzer(),
                 m -> builder(m).analyzers.positionIncrementGap.getValue(),
                 indexCreatedVersion
             );
-            this.isSyntheticSourceEnabled = isSyntheticSourceEnabled;
-            this.store = Parameter.storeParam(
-                m -> builder(m).store.getValue(),
-                () -> isSyntheticSourceEnabled && multiFieldsBuilder.hasSyntheticSourceCompatibleKeywordField() == false
-            );
         }
 
         @Override
@@ -135,6 +136,7 @@ private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilder
                 store.getValue(),
                 tsi,
                 context.isSourceSynthetic(),
+                isWithinMultiField,
                 TextFieldMapper.SyntheticSourceHelper.syntheticSourceDelegate(fieldType, multiFields),
                 meta.getValue()
             );
@@ -154,6 +156,7 @@ public AnnotatedTextFieldMapper build(MapperBuilderContext context) {
                 }
             }
             BuilderParams builderParams = builderParams(this, context);
+            this.isSyntheticSourceEnabled = context.isSourceSynthetic();
             return new AnnotatedTextFieldMapper(
                 leafName(),
                 fieldType,
@@ -165,7 +168,7 @@ public AnnotatedTextFieldMapper build(MapperBuilderContext context) {
     }
 
     public static final TypeParser PARSER = new TypeParser(
-        (n, c) -> new Builder(n, c.indexVersionCreated(), c.getIndexAnalyzers(), SourceFieldMapper.isSynthetic(c.getIndexSettings()))
+        (n, c) -> new Builder(n, c.indexVersionCreated(), c.getIndexAnalyzers(), c.isWithinMultiField())
     );
 
     /**
@@ -484,15 +487,17 @@ private void emitAnnotation(int firstSpannedTextPosInc, int annotationPosLen) th
     }
 
     public static final class AnnotatedTextFieldType extends TextFieldMapper.TextFieldType {
+
         private AnnotatedTextFieldType(
             String name,
             boolean store,
             TextSearchInfo tsi,
             boolean isSyntheticSource,
+            boolean isWithinMultiField,
             KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate,
             Map<String, String> meta
         ) {
-            super(name, true, store, tsi, isSyntheticSource, syntheticSourceDelegate, meta, false, false);
+            super(name, true, store, tsi, isSyntheticSource, isWithinMultiField, syntheticSourceDelegate, meta, false, false);
         }
 
         public AnnotatedTextFieldType(String name, Map<String, String> meta) {
@@ -505,9 +510,9 @@ public String typeName() {
         }
     }
 
+    private final IndexVersion indexCreatedVersion;
     private final FieldType fieldType;
     private final Builder builder;
-
     private final NamedAnalyzer indexAnalyzer;
 
     protected AnnotatedTextFieldMapper(
@@ -517,11 +522,26 @@ protected AnnotatedTextFieldMapper(
         BuilderParams builderParams,
         Builder builder
     ) {
-        super(simpleName, mappedFieldType, builderParams);
+        super(
+            simpleName,
+            builder.indexCreatedVersion,
+            builder.isSyntheticSourceEnabled,
+            builder.isWithinMultiField,
+            mappedFieldType,
+            builderParams
+        );
+
         assert fieldType.tokenized();
+
         this.fieldType = freezeAndDeduplicateFieldType(fieldType);
         this.builder = builder;
         this.indexAnalyzer = wrapAnalyzer(builder.analyzers.getIndexAnalyzer());
+        this.indexCreatedVersion = builder.indexCreatedVersion;
+    }
+
+    @Override
+    public AnnotatedTextFieldType fieldType() {
+        return (AnnotatedTextFieldType) super.fieldType();
     }
 
     @Override
@@ -543,6 +563,18 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
             if (fieldType.omitNorms()) {
                 context.addToFieldNames(fieldType().name());
             }
+        } else if (needsToSupportSyntheticSource() && fieldType.stored() == false) {
+            // if synthetic source needs to be supported, yet the field isn't stored, then we need to rely on something
+            // else to support synthetic source
+
+            // if we can rely on the synthetic source delegate for synthetic source, then return
+            if (fieldType().canUseSyntheticSourceDelegateForSyntheticSource(value)) {
+                return;
+            }
+
+            // otherwise, we need to store a copy of this value so that synthetic source can load it
+            final String fieldName = fieldType().syntheticSourceFallbackFieldName();
+            context.doc().add(new StoredField(fieldName, value));
         }
     }
 
@@ -553,8 +585,7 @@ protected String contentType() {
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(leafName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers, builder.isSyntheticSourceEnabled)
-            .init(this);
+        return new Builder(leafName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers, isWithinMultiField).init(this);
     }
 
     @Override
@@ -568,11 +599,31 @@ protected void write(XContentBuilder b, Object value) throws IOException {
             });
         }
 
+        return new SyntheticSourceSupport.Native(() -> syntheticFieldLoader(fullPath(), leafName()));
+    }
+
+    private SourceLoader.SyntheticFieldLoader syntheticFieldLoader(String fullFieldName, String leafFieldName) {
+        // since we don't know whether the delegate field loader can be used for synthetic source until parsing, we
+        // need to check both this field and the delegate
+
+        // first field loader, representing this field
+        final String fieldName = fieldType().syntheticSourceFallbackFieldName();
+        final var thisFieldLayer = new CompositeSyntheticFieldLoader.StoredFieldLayer(fieldName) {
+            @Override
+            protected void writeValue(Object value, XContentBuilder b) throws IOException {
+                b.value(value.toString());
+            }
+        };
+
+        final CompositeSyntheticFieldLoader fieldLoader = new CompositeSyntheticFieldLoader(leafFieldName, fullFieldName, thisFieldLayer);
+
+        // second loader, representing a delegate field, if one exists
         var kwd = TextFieldMapper.SyntheticSourceHelper.getKeywordFieldMapperForSyntheticSource(this);
         if (kwd != null) {
-            return new SyntheticSourceSupport.Native(() -> kwd.syntheticFieldLoader(fullPath(), leafName()));
+            // merge the two field loaders into one
+            return fieldLoader.mergedWith(kwd.syntheticFieldLoader(fullPath(), leafName()));
         }
 
-        return super.syntheticSourceSupport();
+        return fieldLoader;
     }
 }

server/src/main/java/org/elasticsearch/index/mapper/CompositeSyntheticFieldLoader.java

@@ -132,6 +132,18 @@ public String fieldName() {
         return this.fullFieldName;
     }
 
+    /**
+     * Returns a new {@link CompositeSyntheticFieldLoader} that merges this field loader with the given one.
+     */
+    public CompositeSyntheticFieldLoader mergedWith(CompositeSyntheticFieldLoader other) {
+        if (other == null) {
+            return new CompositeSyntheticFieldLoader(leafFieldName, fullFieldName, List.copyOf(parts));
+        }
+        List<Layer> mergedParts = new ArrayList<>(parts);
+        mergedParts.addAll(other.parts);
+        return new CompositeSyntheticFieldLoader(leafFieldName, fullFieldName, mergedParts);
+    }
+
     /**
      * Represents one layer of loading synthetic source values for a field
      * as a part of {@link CompositeSyntheticFieldLoader}.

server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

@@ -335,6 +335,20 @@ public final void addIgnoredField(IgnoredSourceFieldMapper.NameValue values) {
         }
     }
 
+    /**
+     * This function acts as a more "readable" wrapper around adding ignored fields.
+     *
+     * This is useful when we want to reuse the existing logic that {@link IgnoredSourceFieldMapper} provides for synthetic source, without
+     * explicitly calling addIgnoredField(). Without this, it's a bit confusing why fields that are not meant to be ignored, are being
+     * added to ignored source.
+     */
+    public final void storeFieldForSyntheticSource(String fullPath, String leafName, BytesRef valueBytes, LuceneDocument doc) {
+        if (canAddIgnoredField()) {
+            var fieldData = new IgnoredSourceFieldMapper.NameValue(fullPath, fullPath.lastIndexOf(leafName), valueBytes, doc);
+            ignoredFieldValues.add(fieldData);
+        }
+    }
+
     final void removeLastIgnoredField(String name) {
         if (ignoredFieldValues.isEmpty() == false && ignoredFieldValues.getLast().name().equals(name)) {
             ignoredFieldValues.removeLast();

server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java

@@ -335,10 +335,9 @@ public boolean newDynamicStringField(DocumentParserContext context, String name)
                 );
             } else {
                 return createDynamicField(
-                    new TextFieldMapper.Builder(name, context.indexAnalyzers(), SourceFieldMapper.isSynthetic(context.indexSettings()))
-                        .addMultiField(
-                            new KeywordFieldMapper.Builder("keyword", context.indexSettings().getIndexVersionCreated()).ignoreAbove(256)
-                        ),
+                    new TextFieldMapper.Builder(name, context.indexAnalyzers()).addMultiField(
+                        new KeywordFieldMapper.Builder("keyword", context.indexSettings().getIndexVersionCreated(), true).ignoreAbove(256)
+                    ),
                     context
                 );
             }

server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -610,28 +610,13 @@ public static class Builder {
 
             private final Map<String, Function<MapperBuilderContext, FieldMapper>> mapperBuilders = new HashMap<>();
 
-            private boolean hasSyntheticSourceCompatibleKeywordField;
-
             public Builder add(FieldMapper.Builder builder) {
                 mapperBuilders.put(builder.leafName(), builder::build);
-
-                if (builder instanceof KeywordFieldMapper.Builder kwd) {
-                    if (kwd.hasNormalizer() == false && (kwd.hasDocValues() || kwd.isStored())) {
-                        hasSyntheticSourceCompatibleKeywordField = true;
-                    }
-                }
-
                 return this;
             }
 
             private void add(FieldMapper mapper) {
                 mapperBuilders.put(mapper.leafName(), context -> mapper);
-
-                if (mapper instanceof KeywordFieldMapper kwd) {
-                    if (kwd.hasNormalizer() == false && (kwd.fieldType().hasDocValues() || kwd.fieldType().isStored())) {
-                        hasSyntheticSourceCompatibleKeywordField = true;
-                    }
-                }
             }
 
             private void update(FieldMapper toMerge, MapperMergeContext context) {
@@ -649,10 +634,6 @@ public boolean hasMultiFields() {
                 return mapperBuilders.isEmpty() == false;
             }
 
-            public boolean hasSyntheticSourceCompatibleKeywordField() {
-                return hasSyntheticSourceCompatibleKeywordField;
-            }
-
             public MultiFields build(Mapper.Builder mainFieldBuilder, MapperBuilderContext context) {
                 if (mapperBuilders.isEmpty()) {
                     return empty();