Remove 'external values', and replace with swapped out XContentParsers (#72203) (#72448)

The majority of field mappers read a single value from their positioned
XContentParser, and do not need to call nextToken. There is a general
assumption that the same holds for any multifields defined on them, and
so the XContentParser is passed down to their multifields builder as-is.
This assumption does not hold for mappers that accept json objects,
and so we have a second mechanism for passing values around called
'external values', where a mapper can set a specific value on its context
and child mappers can then check for these external values before reading
from xcontent. The disadvantage of this is that every field mapper now
needs to check its context for external values. Because the values are
defined by their java class, we can also know that in the vast majority of
cases this functionality is unused. We have only two mappers that actually
make use of this, CompletionFieldMapper and GeoPointFieldMapper.

This commit removes external values entirely, and replaces it with the ability
to pass a modified XContentParser to multifields. FieldMappers can just check
the parser attached to their context for data and don't need to worry about
multiple sources.

Plugins implementing field mappers will need to take the removal of external
values into account. Implementations that are passing structured objects
as external values should instead use ParseContext.switchParser and
wrap the objects using MapXContentParser.wrapObject().

GeoPointFieldMapper passes on a fake parser that just wraps its input data
formatted as a geohash; CompletionFieldMapper has a slightly more complicated
parser that in general wraps its metadata, but if textOrNull() is called without
the parser being advanced just returns its text input.

Relates to #56063

Author: romseygeek

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/FilterXContentParser.java

@@ -0,0 +1,240 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.common.xcontent;
+
+import org.elasticsearch.common.CheckedFunction;
+
+import java.io.IOException;
+import java.nio.CharBuffer;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+
+/**
+ * Filters an existing XContentParser by using a delegate
+ */
+public abstract class FilterXContentParser implements XContentParser {
+
+    protected final XContentParser in;
+
+    protected FilterXContentParser(XContentParser in) {
+        this.in = in;
+    }
+
+    @Override
+    public XContentType contentType() {
+        return in.contentType();
+    }
+
+    @Override
+    public Token nextToken() throws IOException {
+        return in.nextToken();
+    }
+
+    @Override
+    public void skipChildren() throws IOException {
+        in.skipChildren();
+    }
+
+    @Override
+    public Token currentToken() {
+        return in.currentToken();
+    }
+
+    @Override
+    public String currentName() throws IOException {
+        return in.currentName();
+    }
+
+    @Override
+    public Map<String, Object> map() throws IOException {
+        return in.map();
+    }
+
+    @Override
+    public Map<String, Object> mapOrdered() throws IOException {
+        return in.mapOrdered();
+    }
+
+    @Override
+    public Map<String, String> mapStrings() throws IOException {
+        return in.mapStrings();
+    }
+
+    @Override
+    public <T> Map<String, T> map(
+        Supplier<Map<String, T>> mapFactory, CheckedFunction<XContentParser, T, IOException> mapValueParser) throws IOException {
+        return in.map(mapFactory, mapValueParser);
+    }
+
+    @Override
+    public List<Object> list() throws IOException {
+        return in.list();
+    }
+
+    @Override
+    public List<Object> listOrderedMap() throws IOException {
+        return in.listOrderedMap();
+    }
+
+    @Override
+    public String text() throws IOException {
+        return in.text();
+    }
+
+    @Override
+    public String textOrNull() throws IOException {
+        return in.textOrNull();
+    }
+
+    @Override
+    public CharBuffer charBufferOrNull() throws IOException {
+        return in.charBufferOrNull();
+    }
+
+    @Override
+    public CharBuffer charBuffer() throws IOException {
+        return in.charBuffer();
+    }
+
+    @Override
+    public Object objectText() throws IOException {
+        return in.objectText();
+    }
+
+    @Override
+    public Object objectBytes() throws IOException {
+        return in.objectBytes();
+    }
+
+    @Override
+    public boolean hasTextCharacters() {
+        return in.hasTextCharacters();
+    }
+
+    @Override
+    public char[] textCharacters() throws IOException {
+        return in.textCharacters();
+    }
+
+    @Override
+    public int textLength() throws IOException {
+        return in.textLength();
+    }
+
+    @Override
+    public int textOffset() throws IOException {
+        return in.textOffset();
+    }
+
+    @Override
+    public Number numberValue() throws IOException {
+        return in.numberValue();
+    }
+
+    @Override
+    public NumberType numberType() throws IOException {
+        return in.numberType();
+    }
+
+    @Override
+    public short shortValue(boolean coerce) throws IOException {
+        return in.shortValue(coerce);
+    }
+
+    @Override
+    public int intValue(boolean coerce) throws IOException {
+        return in.intValue(coerce);
+    }
+
+    @Override
+    public long longValue(boolean coerce) throws IOException {
+        return in.longValue(coerce);
+    }
+
+    @Override
+    public float floatValue(boolean coerce) throws IOException {
+        return in.floatValue(coerce);
+    }
+
+    @Override
+    public double doubleValue(boolean coerce) throws IOException {
+        return in.doubleValue(coerce);
+    }
+
+    @Override
+    public short shortValue() throws IOException {
+        return in.shortValue();
+    }
+
+    @Override
+    public int intValue() throws IOException {
+        return in.intValue();
+    }
+
+    @Override
+    public long longValue() throws IOException {
+        return in.longValue();
+    }
+
+    @Override
+    public float floatValue() throws IOException {
+        return in.floatValue();
+    }
+
+    @Override
+    public double doubleValue() throws IOException {
+        return in.doubleValue();
+    }
+
+    @Override
+    public boolean isBooleanValue() throws IOException {
+        return in.isBooleanValue();
+    }
+
+    @Override
+    public boolean booleanValue() throws IOException {
+        return in.booleanValue();
+    }
+
+    @Override
+    public byte[] binaryValue() throws IOException {
+        return in.binaryValue();
+    }
+
+    @Override
+    public XContentLocation getTokenLocation() {
+        return in.getTokenLocation();
+    }
+
+    @Override
+    public <T> T namedObject(Class<T> categoryClass, String name, Object context) throws IOException {
+        return in.namedObject(categoryClass, name, context);
+    }
+
+    @Override
+    public NamedXContentRegistry getXContentRegistry() {
+        return in.getXContentRegistry();
+    }
+
+    @Override
+    public boolean isClosed() {
+        return in.isClosed();
+    }
+
+    @Override
+    public void close() throws IOException {
+        in.close();
+    }
+
+    @Override
+    public DeprecationHandler getDeprecationHandler() {
+        return in.getDeprecationHandler();
+    }
+}

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java

@@ -318,12 +318,7 @@ public FieldMapper.Builder getMergeBuilder() {
 
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
-        final String value;
-        if (context.externalValueSet()) {
-            value = context.externalValue().toString();
-        } else {
-            value = context.parser().textOrNull();
-        }
+        final String value = context.parser().textOrNull();
 
         if (value == null) {
             return;

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeatureFieldMapper.java

@@ -136,10 +136,7 @@ public RankFeatureFieldType fieldType() {
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
         float value;
-        if (context.externalValueSet()) {
-            Object v = context.externalValue();
-            value = objectToFloat(v);
-        } else if (context.parser().currentToken() == Token.VALUE_NULL) {
+        if (context.parser().currentToken() == Token.VALUE_NULL) {
             // skip
             return;
         } else {

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapper.java

@@ -57,7 +57,7 @@ name, new RankFeaturesFieldType(buildFullName(contentPath), meta.getValue(), pos
         }
     }
 
-    public static final TypeParser PARSER = new TypeParser((n, c) -> new Builder(n));
+    public static final TypeParser PARSER = new TypeParser((n, c) -> new Builder(n), notInMultiFields(CONTENT_TYPE));
 
     public static final class RankFeaturesFieldType extends MappedFieldType {
 
@@ -118,9 +118,6 @@ public RankFeaturesFieldType fieldType() {
 
     @Override
     public void parse(ParseContext context) throws IOException {
-        if (context.externalValueSet()) {
-            throw new IllegalArgumentException("[rank_features] fields can't be used in multi-fields");
-        }
 
         if (context.parser().currentToken() != Token.START_OBJECT) {
             throw new IllegalArgumentException("[rank_features] fields must be json objects, expected a START_OBJECT but got: " +

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java

@@ -330,9 +330,7 @@ protected void parseCreateField(ParseContext context) throws IOException {
         XContentParser parser = context.parser();
         Object value;
         Number numericValue = null;
-        if (context.externalValueSet()) {
-            value = context.externalValue();
-        } else if (parser.currentToken() == Token.VALUE_NULL) {
+        if (parser.currentToken() == Token.VALUE_NULL) {
             value = null;
         } else if (coerce.value()
                 && parser.currentToken() == Token.VALUE_STRING

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java

@@ -571,7 +571,7 @@ public SearchAsYouTypeFieldMapper(String simpleName,
 
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
-        final String value = context.externalValueSet() ? context.externalValue().toString() : context.parser().textOrNull();
+        final String value = context.parser().textOrNull();
         if (value == null) {
             return;
         }

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java

@@ -111,12 +111,7 @@ protected TokenCountFieldMapper(String simpleName, MappedFieldType defaultFieldT
 
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
-        final String value;
-        if (context.externalValueSet()) {
-            value = context.externalValue().toString();
-        } else {
-            value = context.parser().textOrNull();
-        }
+        final String value = context.parser().textOrNull();
 
         if (value == null && nullValue == null) {
             return;

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java

@@ -20,6 +20,8 @@
 import java.util.Arrays;
 import java.util.Collection;
 
+import static org.hamcrest.Matchers.containsString;
+
 public class RankFeaturesFieldMapperTests extends MapperTestCase {
 
     @Override
@@ -140,6 +142,18 @@ public void testRejectMultiValuedFields() throws MapperParsingException, IOExcep
                 "the same document", e.getCause().getMessage());
     }
 
+    public void testCannotBeUsedInMultifields() {
+        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+            b.field("type", "keyword");
+            b.startObject("fields");
+            b.startObject("feature");
+            b.field("type", "rank_features");
+            b.endObject();
+            b.endObject();
+        })));
+        assertThat(e.getMessage(), containsString("Field [feature] of type [rank_features] can't be used in multifields"));
+    }
+
     @Override
     protected Object generateRandomInputValue(MappedFieldType ft) {
         assumeFalse("Test implemented in a follow up", true);

plugins/analysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java

@@ -438,15 +438,11 @@ public FieldMapper.Builder getMergeBuilder() {
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
         final String value;
-        if (context.externalValueSet()) {
-            value = context.externalValue().toString();
+        XContentParser parser = context.parser();
+        if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
+            value = nullValue;
         } else {
-            XContentParser parser = context.parser();
-            if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
-                value = nullValue;
-            } else {
-                value = parser.textOrNull();
-            }
+            value = parser.textOrNull();
         }
 
         if (value == null || value.length() > ignoreAbove) {

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

@@ -524,12 +524,7 @@ protected AnnotatedTextFieldMapper(String simpleName, FieldType fieldType, Annot
 
     @Override
     protected void parseCreateField(ParseContext context) throws IOException {
-        final String value;
-        if (context.externalValueSet()) {
-            value = context.externalValue().toString();
-        } else {
-            value = context.parser().textOrNull();
-        }
+        final String value = context.parser().textOrNull();
 
         if (value == null) {
             return;

plugins/mapper-murmur3/src/main/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapper.java

@@ -124,12 +124,7 @@ protected String contentType() {
     @Override
     protected void parseCreateField(ParseContext context)
             throws IOException {
-        final Object value;
-        if (context.externalValueSet()) {
-            value = context.externalValue();
-        } else {
-            value = context.parser().textOrNull();
-        }
+        final String value = context.parser().textOrNull();
         if (value != null) {
             final BytesRef bytes = new BytesRef(value.toString());
             final long hash = MurmurHash3.hash128(bytes.bytes, bytes.offset, bytes.length, 0, new MurmurHash3.Hash128()).h1;