Merge multiple ignored source entires for the same field (#111994) (#112116)

(cherry picked from commit a35da24)

# Conflicts:
#	modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/qa/StandardVersusLogsIndexModeRandomDataChallengeRestIT.java
#	test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java

Author: lkts

docs/changelog/111994.yaml

@@ -0,0 +1,6 @@
+pr: 111994
+summary: Merge multiple ignored source entires for the same field
+area: Logs
+type: bug
+issues:
+ - 111694

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

@@ -83,7 +83,7 @@ void write(XContentBuilder builder) throws IOException {
             XContentDataHelper.decodeAndWrite(builder, value());
         }
 
-        private String getFieldName() {
+        String getFieldName() {
             return parentOffset() == 0 ? name() : name().substring(parentOffset());
         }
     }

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

@@ -9,12 +9,12 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
-import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
@@ -32,6 +32,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TreeMap;
 import java.util.stream.Stream;
 
@@ -74,7 +75,7 @@ DynamicFieldsBuilder getDynamicFieldsBuilder() {
          * If no dynamic settings are explicitly configured, we default to {@link #TRUE}
          */
         static Dynamic getRootDynamic(MappingLookup mappingLookup) {
-            ObjectMapper.Dynamic rootDynamic = mappingLookup.getMapping().getRoot().dynamic;
+            Dynamic rootDynamic = mappingLookup.getMapping().getRoot().dynamic;
             return rootDynamic == null ? Defaults.DYNAMIC : rootDynamic;
         }
     }
@@ -142,13 +143,13 @@ public final void addDynamic(String name, String prefix, Mapper mapper, Document
                 int firstDotIndex = name.indexOf('.');
                 String immediateChild = name.substring(0, firstDotIndex);
                 String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild;
-                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChildFullName, context);
+                Builder parentBuilder = findObjectBuilder(immediateChildFullName, context);
                 parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context);
                 add(parentBuilder);
             }
         }
 
-        private static ObjectMapper.Builder findObjectBuilder(String fullName, DocumentParserContext context) {
+        private static Builder findObjectBuilder(String fullName, DocumentParserContext context) {
             // does the object mapper already exist? if so, use that
             ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName);
             if (objectMapper != null) {
@@ -215,7 +216,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, MappingParser
             throws MapperParsingException {
             parserContext.incrementMappingObjectDepth(); // throws MapperParsingException if depth limit is exceeded
             Explicit<Boolean> subobjects = parseSubobjects(node);
-            ObjectMapper.Builder builder = new Builder(name, subobjects);
+            Builder builder = new Builder(name, subobjects);
             parseObjectFields(node, parserContext, builder);
             parserContext.decrementMappingObjectDepth();
             return builder;
@@ -237,7 +238,7 @@ protected static boolean parseObjectOrDocumentTypeProperties(
             String fieldName,
             Object fieldNode,
             MappingParserContext parserContext,
-            ObjectMapper.Builder builder
+            Builder builder
         ) {
             if (fieldName.equals("dynamic")) {
                 String value = fieldNode.toString();
@@ -284,11 +285,7 @@ protected static Explicit<Boolean> parseSubobjects(Map<String, Object> node) {
             return Defaults.SUBOBJECTS;
         }
 
-        protected static void parseProperties(
-            ObjectMapper.Builder objBuilder,
-            Map<String, Object> propsNode,
-            MappingParserContext parserContext
-        ) {
+        protected static void parseProperties(Builder objBuilder, Map<String, Object> propsNode, MappingParserContext parserContext) {
             Iterator<Map.Entry<String, Object>> iterator = propsNode.entrySet().iterator();
             while (iterator.hasNext()) {
                 Map.Entry<String, Object> entry = iterator.next();
@@ -347,7 +344,7 @@ protected static void parseProperties(
                         for (int i = fieldNameParts.length - 2; i >= 0; --i) {
                             String intermediateObjectName = fieldNameParts[i];
                             validateFieldName(intermediateObjectName, parserContext.indexVersionCreated());
-                            ObjectMapper.Builder intermediate = new ObjectMapper.Builder(intermediateObjectName, Defaults.SUBOBJECTS);
+                            Builder intermediate = new Builder(intermediateObjectName, Defaults.SUBOBJECTS);
                             intermediate.add(fieldBuilder);
                             fieldBuilder = intermediate;
                         }
@@ -417,8 +414,8 @@ private static void validateFieldName(String fieldName, IndexVersion indexCreate
     /**
      * @return a Builder that will produce an empty ObjectMapper with the same configuration as this one
      */
-    public ObjectMapper.Builder newBuilder(IndexVersion indexVersionCreated) {
-        ObjectMapper.Builder builder = new ObjectMapper.Builder(leafName(), subobjects);
+    public Builder newBuilder(IndexVersion indexVersionCreated) {
+        Builder builder = new Builder(leafName(), subobjects);
         builder.enabled = this.enabled;
         builder.dynamic = this.dynamic;
         return builder;
@@ -507,7 +504,7 @@ protected record MergeResult(
         Explicit<Boolean> enabled,
         Explicit<Boolean> subObjects,
         Explicit<Boolean> trackArraySource,
-        ObjectMapper.Dynamic dynamic,
+        Dynamic dynamic,
         Map<String, Mapper> mappers
     ) {
         static MergeResult build(ObjectMapper existing, ObjectMapper mergeWithObject, MapperMergeContext parentMergeContext) {
@@ -789,9 +786,9 @@ public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
 
         @Override
         public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
-            List<SourceLoader.SyntheticFieldLoader.DocValuesLoader> loaders = new ArrayList<>();
+            List<DocValuesLoader> loaders = new ArrayList<>();
             for (SourceLoader.SyntheticFieldLoader field : fields) {
-                SourceLoader.SyntheticFieldLoader.DocValuesLoader loader = field.docValuesLoader(leafReader, docIdsInLeaf);
+                DocValuesLoader loader = field.docValuesLoader(leafReader, docIdsInLeaf);
                 if (loader != null) {
                     loaders.add(loader);
                 }
@@ -803,7 +800,7 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
         }
 
         private class ObjectDocValuesLoader implements DocValuesLoader {
-            private final List<SourceLoader.SyntheticFieldLoader.DocValuesLoader> loaders;
+            private final List<DocValuesLoader> loaders;
 
             private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
                 this.loaders = loaders;
@@ -812,7 +809,7 @@ private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
             @Override
             public boolean advanceToDoc(int docId) throws IOException {
                 boolean anyLeafHasDocValues = false;
-                for (SourceLoader.SyntheticFieldLoader.DocValuesLoader docValueLoader : loaders) {
+                for (DocValuesLoader docValueLoader : loaders) {
                     boolean leafHasValue = docValueLoader.advanceToDoc(docId);
                     anyLeafHasDocValues |= leafHasValue;
                 }
@@ -848,18 +845,24 @@ public void write(XContentBuilder b) throws IOException {
 
             if (ignoredValues != null && ignoredValues.isEmpty() == false) {
                 // Use an ordered map between field names and writer functions, to order writing by field name.
-                Map<String, CheckedConsumer<XContentBuilder, IOException>> orderedFields = new TreeMap<>();
+                Map<String, FieldWriter> orderedFields = new TreeMap<>();
                 for (IgnoredSourceFieldMapper.NameValue value : ignoredValues) {
-                    orderedFields.put(value.name(), value::write);
+                    var existing = orderedFields.get(value.name());
+                    if (existing == null) {
+                        orderedFields.put(value.name(), new FieldWriter.IgnoredSource(value));
+                    } else if (existing instanceof FieldWriter.IgnoredSource isw) {
+                        isw.mergeWith(value);
+                    }
                 }
                 for (SourceLoader.SyntheticFieldLoader field : fields) {
                     if (field.hasValue()) {
                         // Skip if the field source is stored separately, to avoid double-printing.
-                        orderedFields.putIfAbsent(field.fieldName(), field::write);
+                        orderedFields.computeIfAbsent(field.fieldName(), k -> new FieldWriter.FieldLoader(field));
                     }
                 }
+
                 for (var writer : orderedFields.values()) {
-                    writer.accept(b);
+                    writer.writeTo(b);
                 }
                 ignoredValues = null;
             } else {
@@ -890,6 +893,42 @@ public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameVa
         public String fieldName() {
             return ObjectMapper.this.fullPath();
         }
+
+        interface FieldWriter {
+            void writeTo(XContentBuilder builder) throws IOException;
+
+            record FieldLoader(SourceLoader.SyntheticFieldLoader loader) implements FieldWriter {
+                @Override
+                public void writeTo(XContentBuilder builder) throws IOException {
+                    loader.write(builder);
+                }
+            }
+
+            class IgnoredSource implements FieldWriter {
+                private final String fieldName;
+                private final String leafName;
+                private final List<BytesRef> values;
+
+                IgnoredSource(IgnoredSourceFieldMapper.NameValue initialValue) {
+                    this.fieldName = initialValue.name();
+                    this.leafName = initialValue.getFieldName();
+                    this.values = new ArrayList<>();
+                    this.values.add(initialValue.value());
+                }
+
+                @Override
+                public void writeTo(XContentBuilder builder) throws IOException {
+                    XContentDataHelper.writeMerged(builder, leafName, values);
+                }
+
+                public FieldWriter mergeWith(IgnoredSourceFieldMapper.NameValue nameValue) {
+                    assert Objects.equals(nameValue.name(), fieldName) : "IgnoredSource is merged with wrong field data";
+
+                    values.add(nameValue.value());
+                    return this;
+                }
+            }
+        }
     }
 
     protected boolean isRoot() {

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

@@ -26,6 +26,8 @@
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 
 /**
  * Helper class for processing field data of any type, as provided by the {@link XContentParser}.
@@ -92,6 +94,70 @@ static void decodeAndWrite(XContentBuilder b, BytesRef r) throws IOException {
         }
     }
 
+    /**
+     * Writes encoded values to provided builder. If there are multiple values they are merged into
+     * a single resulting array.
+     * @param b destination
+     * @param fieldName name of the field that is written
+     * @param encodedParts subset of field data encoded using methods of this class. Can contain arrays which will be flattened.
+     * @throws IOException
+     */
+    static void writeMerged(XContentBuilder b, String fieldName, List<BytesRef> encodedParts) throws IOException {
+        if (encodedParts.isEmpty()) {
+            return;
+        }
+
+        if (encodedParts.size() == 1) {
+            b.field(fieldName);
+            XContentDataHelper.decodeAndWrite(b, encodedParts.get(0));
+            return;
+        }
+
+        b.startArray(fieldName);
+
+        for (var encodedValue : encodedParts) {
+            Optional<XContentType> encodedXContentType = switch ((char) encodedValue.bytes[encodedValue.offset]) {
+                case CBOR_OBJECT_ENCODING, JSON_OBJECT_ENCODING, YAML_OBJECT_ENCODING, SMILE_OBJECT_ENCODING -> Optional.of(
+                    getXContentType(encodedValue)
+                );
+                default -> Optional.empty();
+            };
+            if (encodedXContentType.isEmpty()) {
+                // This is a plain value, we can just write it
+                XContentDataHelper.decodeAndWrite(b, encodedValue);
+            } else {
+                // Encoded value could be an array which needs to be flattened
+                // since we are already inside an array.
+                try (
+                    XContentParser parser = encodedXContentType.get()
+                        .xContent()
+                        .createParser(
+                            XContentParserConfiguration.EMPTY,
+                            encodedValue.bytes,
+                            encodedValue.offset + 1,
+                            encodedValue.length - 1
+                        )
+                ) {
+                    if (parser.currentToken() == null) {
+                        parser.nextToken();
+                    }
+
+                    // It's an array, we will flatten it.
+                    if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
+                        while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                            b.copyCurrentStructure(parser);
+                        }
+                    } else {
+                        // It is a single complex structure (an object), write it as is.
+                        b.copyCurrentStructure(parser);
+                    }
+                }
+            }
+        }
+
+        b.endArray();
+    }
+
     /**
      * Returns the {@link XContentType} to use for creating an XContentBuilder to decode the passed value.
      */