Merge branch 'main' into test/ensure-vectors-searchable-bwc-test

benwtrent · Jun 18, 2024 · 785d317 · 785d317
2 parents 643547d + b147d07
commit 785d317
Show file tree

Hide file tree

Showing 18 changed files with 339 additions and 73 deletions.
diff --git a/docs/changelog/109651.yaml b/docs/changelog/109651.yaml
@@ -0,0 +1,5 @@
+pr: 109651
+summary: Support synthetic source for `geo_point` when `ignore_malformed` is used
+area: Mapping
+type: enhancement
+issues: []
diff --git a/docs/reference/mapping/types/geo-point.asciidoc b/docs/reference/mapping/types/geo-point.asciidoc
@@ -220,8 +220,7 @@ any issues, but features in technical preview are not subject to the support SLA
 of official GA features.
 
 `geo_point` fields support <<synthetic-source,synthetic `_source`>> in their
-default configuration. Synthetic `_source` cannot be used together with
-<<ignore-malformed,`ignore_malformed`>>, <<copy-to,`copy_to`>>, or with
+default configuration. Synthetic `_source` cannot be used together with <<copy-to,`copy_to`>> or with
 <<doc-values,`doc_values`>> disabled.
 
 Synthetic source always sorts `geo_point` fields (first by latitude and then

diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/CopyingXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/CopyingXContentParser.java
@@ -0,0 +1,43 @@
+/*
+ * 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.xcontent;
+
+import java.io.IOException;
+
+/**
+ * A parser that copies data that was parsed into a {@link XContentBuilder}.
+ * This parser naturally has some memory and runtime overhead to perform said copying.
+ * Use with {@link XContentSubParser} to preserve the entire object.
+ */
+public class CopyingXContentParser extends FilterXContentParserWrapper {
+    private final XContentBuilder builder;
+
+    public CopyingXContentParser(XContentParser delegate) throws IOException {
+        super(delegate);
+        this.builder = XContentBuilder.builder(delegate.contentType().xContent());
+        switch (delegate.currentToken()) {
+            case START_OBJECT -> builder.startObject();
+            case START_ARRAY -> builder.startArray();
+            default -> throw new IllegalArgumentException(
+                "can only copy parsers pointed to START_OBJECT or START_ARRAY but found: " + delegate.currentToken()
+            );
+        }
+    }
+
+    @Override
+    public Token nextToken() throws IOException {
+        XContentParser.Token next = delegate().nextToken();
+        builder.copyCurrentEvent(delegate());
+        return next;
+    }
+
+    public XContentBuilder getBuilder() {
+        return builder;
+    }
+}
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java
@@ -1220,6 +1220,18 @@ public XContentBuilder rawValue(String value) throws IOException {
         return this;
     }
 
+    /**
+     * Copies current event from parser into this builder.
+     * The difference with {@link XContentBuilder#copyCurrentStructure(XContentParser)}
+     * is that this method does not copy sub-objects as a single entity.
+     * @param parser
+     * @throws IOException
+     */
+    public XContentBuilder copyCurrentEvent(XContentParser parser) throws IOException {
+        generator.copyCurrentEvent(parser);
+        return this;
+    }
+
     public XContentBuilder copyCurrentStructure(XContentParser parser) throws IOException {
         generator.copyCurrentStructure(parser);
         return this;

diff --git a/...egacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java b/...egacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java
@@ -58,7 +58,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -379,18 +378,18 @@ private LegacyGeoShapeParser() {}
         public void parse(
             XContentParser parser,
             CheckedConsumer<ShapeBuilder<?, ?, ?>, IOException> consumer,
-            Consumer<Exception> onMalformed
+            MalformedValueHandler malformedHandler
         ) throws IOException {
             try {
                 if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
                     while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                        parse(parser, consumer, onMalformed);
+                        parse(parser, consumer, malformedHandler);
                     }
                 } else {
                     consumer.accept(ShapeParser.parse(parser));
                 }
             } catch (ElasticsearchParseException e) {
-                onMalformed.accept(e);
+                malformedHandler.notify(e);
             }
         }
 

diff --git a/muted-tests.yml b/muted-tests.yml
@@ -68,8 +68,7 @@ tests:
   issue: "https://github.com/elastic/elasticsearch/issues/109838"
   method: "testShutdownReadinessService"
 - class: "org.elasticsearch.packaging.test.PackageTests"
-  issue: "https://github.com/elastic/elasticsearch/issues/109841"
-  method: "test50Remove"
+  issue: "https://github.com/elastic/elasticsearch/issues/109852"
 
 # Examples:
 #

diff --git a/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/packaging/src/test/java/org/elasticsearch/packaging/test/PackageTests.java
@@ -210,7 +210,6 @@ public void test50Remove() throws Exception {
         assertThat(SYSTEMD_SERVICE, fileDoesNotExist());
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/109852")
     public void test60Reinstall() throws Exception {
         try {
             install();
@@ -224,7 +223,6 @@ public void test60Reinstall() throws Exception {
         }
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/109852")
     public void test70RestartServer() throws Exception {
         try {
             install();

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
@@ -8,12 +8,14 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.search.Query;
+import org.elasticsearch.common.CheckedBiConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryFormatterFactory;
 import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.xcontent.DeprecationHandler;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
+import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xcontent.support.MapXContentParser;
@@ -52,12 +54,12 @@ public abstract static class Parser<T> {
          * Parse the given xContent value to one or more objects of type {@link T}. The value can be
          * in any supported format.
          */
-        public abstract void parse(XContentParser parser, CheckedConsumer<T, IOException> consumer, Consumer<Exception> onMalformed)
+        public abstract void parse(XContentParser parser, CheckedConsumer<T, IOException> consumer, MalformedValueHandler malformedHandler)
             throws IOException;
 
         private void fetchFromSource(Object sourceMap, Consumer<T> consumer) {
             try (XContentParser parser = wrapObject(sourceMap)) {
-                parse(parser, v -> consumer.accept(normalizeFromSource(v)), e -> {}); /* ignore malformed */
+                parse(parser, v -> consumer.accept(normalizeFromSource(v)), NoopMalformedValueHandler.INSTANCE);
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
             }
@@ -84,6 +86,36 @@ private static XContentParser wrapObject(Object sourceMap) throws IOException {
         }
     }
 
+    public interface MalformedValueHandler {
+        void notify(Exception parsingException) throws IOException;
+
+        void notify(Exception parsingException, XContentBuilder malformedDataForSyntheticSource) throws IOException;
+    }
+
+    public record NoopMalformedValueHandler() implements MalformedValueHandler {
+        public static final NoopMalformedValueHandler INSTANCE = new NoopMalformedValueHandler();
+
+        @Override
+        public void notify(Exception parsingException) {}
+
+        @Override
+        public void notify(Exception parsingException, XContentBuilder malformedDataForSyntheticSource) {}
+    }
+
+    public record DefaultMalformedValueHandler(CheckedBiConsumer<Exception, XContentBuilder, IOException> consumer)
+        implements
+            MalformedValueHandler {
+        @Override
+        public void notify(Exception parsingException) throws IOException {
+            consumer.accept(parsingException, null);
+        }
+
+        @Override
+        public void notify(Exception parsingException, XContentBuilder malformedDataForSyntheticSource) throws IOException {
+            consumer.accept(parsingException, malformedDataForSyntheticSource);
+        }
+    }
+
     public abstract static class AbstractGeometryFieldType<T> extends MappedFieldType {
 
         protected final Parser<T> geometryParser;
@@ -220,17 +252,20 @@ public final void parse(DocumentParserContext context) throws IOException {
                 new IllegalArgumentException("Cannot index data directly into a field with a [script] parameter")
             );
         }
-        parser.parse(context.parser(), v -> index(context, v), e -> {
-            if (ignoreMalformed()) {
-                context.addIgnoredField(fieldType().name());
-            } else {
-                throw new DocumentParsingException(
-                    context.parser().getTokenLocation(),
-                    "failed to parse field [" + fieldType().name() + "] of type [" + contentType() + "]",
-                    e
-                );
-            }
-        });
+        parser.parse(context.parser(), v -> index(context, v), new DefaultMalformedValueHandler((e, b) -> onMalformedValue(context, b, e)));
+    }
+
+    protected void onMalformedValue(DocumentParserContext context, XContentBuilder malformedDataForSyntheticSource, Exception cause)
+        throws IOException {
+        if (ignoreMalformed()) {
+            context.addIgnoredField(fieldType().name());
+        } else {
+            throw new DocumentParsingException(
+                context.parser().getTokenLocation(),
+                "failed to parse field [" + fieldType().name() + "] of type [" + contentType() + "]",
+                cause
+            );
+        }
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
@@ -18,7 +18,6 @@
 import java.io.IOException;
 import java.util.Map;
 import java.util.Objects;
-import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -71,7 +70,7 @@ public T getNullValue() {
     /** A base parser implementation for point formats */
     protected abstract static class PointParser<T> extends Parser<T> {
         protected final String field;
-        private final CheckedFunction<XContentParser, T, IOException> objectParser;
+        protected final CheckedFunction<XContentParser, T, IOException> objectParser;
         private final T nullValue;
         private final boolean ignoreZValue;
         protected final boolean ignoreMalformed;
@@ -98,7 +97,7 @@ protected PointParser(
         protected abstract T createPoint(double x, double y);
 
         @Override
-        public void parse(XContentParser parser, CheckedConsumer<T, IOException> consumer, Consumer<Exception> onMalformed)
+        public void parse(XContentParser parser, CheckedConsumer<T, IOException> consumer, MalformedValueHandler malformedHandler)
             throws IOException {
             if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
                 XContentParser.Token token = parser.nextToken();
@@ -132,7 +131,7 @@ public void parse(XContentParser parser, CheckedConsumer<T, IOException> consume
                                 consumer.accept(nullValue);
                             }
                         } else {
-                            parseAndConsumeFromObject(parser, consumer, onMalformed);
+                            parseAndConsumeFromObject(parser, consumer, malformedHandler);
                         }
                         token = parser.nextToken();
                     }
@@ -142,20 +141,20 @@ public void parse(XContentParser parser, CheckedConsumer<T, IOException> consume
                     consumer.accept(nullValue);
                 }
             } else {
-                parseAndConsumeFromObject(parser, consumer, onMalformed);
+                parseAndConsumeFromObject(parser, consumer, malformedHandler);
             }
         }
 
-        private void parseAndConsumeFromObject(
+        protected void parseAndConsumeFromObject(
             XContentParser parser,
             CheckedConsumer<T, IOException> consumer,
-            Consumer<Exception> onMalformed
-        ) {
+            MalformedValueHandler malformedHandler
+        ) throws IOException {
             try {
                 T point = objectParser.apply(parser);
                 consumer.accept(validate(point));
             } catch (Exception e) {
-                onMalformed.accept(e);
+                malformedHandler.notify(e);
             }
         }
     }