From 2932918c3acb347bd5c68e442310fc2d11ee1e88 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 2 Jun 2020 16:16:15 -0700 Subject: [PATCH] Remove the 'array value parser' marker interface. This PR replaces the marker interface with the method FieldMapper#parsesArrayValue. I find this cleaner and it will help with the fields retrieval work (#55363). The refactor also ensures that only field mappers can declare they parse array values. Previously other types like ObjectMapper could implement the marker interface and be passed array values, which doesn't make sense. --- .../mapper/AbstractGeometryFieldMapper.java | 5 ++++ .../index/mapper/ArrayValueMapperParser.java | 29 ------------------- .../index/mapper/CompletionFieldMapper.java | 7 ++++- .../index/mapper/DocumentParser.java | 8 +++-- .../index/mapper/FieldMapper.java | 10 +++++++ .../index/mapper/GeoPointFieldMapper.java | 3 +- .../index/mapper/PointFieldMapper.java | 4 +-- .../mapper/DenseVectorFieldMapper.java | 8 +++-- 8 files changed, 35 insertions(+), 39 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/ArrayValueMapperParser.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java index e76cf11c56603..721a4033965f5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -305,6 +305,11 @@ protected void parseCreateField(ParseContext context) throws IOException { protected abstract void addDocValuesFields(String name, Processed geometry, List fields, ParseContext context); protected abstract void addMultiFields(ParseContext context, Processed geometry) throws IOException; + @Override + public final boolean parsesArrayValue() { + return true; + } + /** parsing logic for geometry indexing */ @Override public void parse(ParseContext context) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ArrayValueMapperParser.java b/server/src/main/java/org/elasticsearch/index/mapper/ArrayValueMapperParser.java deleted file mode 100644 index 44eeb917f31b5..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/ArrayValueMapperParser.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.mapper; - -/** - * A marker interface indicating that this mapper can handle array value, and the array - * itself should be passed to it. - * - * - */ -public interface ArrayValueMapperParser { -} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index edd934ae119ed..4109913122d4f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -83,7 +83,7 @@ * This field can also be extended to add search criteria to suggestions * for query-time filtering and boosting (see {@link ContextMappings} */ -public class CompletionFieldMapper extends FieldMapper implements ArrayValueMapperParser { +public class CompletionFieldMapper extends FieldMapper { public static final String CONTENT_TYPE = "completion"; /** @@ -426,6 +426,11 @@ public CompletionFieldType fieldType() { return (CompletionFieldType) super.fieldType(); } + @Override + public boolean parsesArrayValue() { + return true; + } + /** * Parses and indexes inputs * diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index a1cda9f3e4990..3466369768df8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -522,7 +522,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper, // There is a concrete mapper for this field already. Need to check if the mapper // expects an array, if so we pass the context straight to the mapper and if not // we serialize the array components - if (mapper instanceof ArrayValueMapperParser) { + if (parsesArrayValue(mapper)) { parseObjectOrField(context, mapper); } else { parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName); @@ -543,7 +543,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper, Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path()); mapper = builder.build(builderContext); assert mapper != null; - if (mapper instanceof ArrayValueMapperParser) { + if (parsesArrayValue(mapper)) { context.addDynamicMapper(mapper); context.path().add(arrayFieldName); parseObjectOrField(context, mapper); @@ -562,6 +562,10 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper, } } + private static boolean parsesArrayValue(Mapper mapper) { + return mapper instanceof FieldMapper && ((FieldMapper) mapper).parsesArrayValue(); + } + private static void parseNonDynamicArray(ParseContext context, ObjectMapper mapper, final String lastFieldName, String arrayFieldName) throws IOException { XContentParser parser = context.parser(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 77a17114518c3..0cbd4d75c76fa 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -273,6 +273,16 @@ public CopyTo copyTo() { return copyTo; } + /** + * Whether this mapper can handle an array value during document parsing. If true, + * when an array is encountered during parsing, the document parser will pass the + * whole array to the mapper. If false, the array is split into individual values + * and each value is passed to the mapper for parsing. + */ + public boolean parsesArrayValue() { + return false; + } + /** * Parse the field value using the provided {@link ParseContext}. */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java index 00c7c63392d9f..5b4ac56b6138e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -44,8 +44,7 @@ * * Uses lucene 6 LatLonPoint encoding */ -public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper, List> - implements ArrayValueMapperParser { +public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper, List> { public static final String CONTENT_TYPE = "geo_point"; public static class Builder extends AbstractPointGeometryFieldMapper.Builder { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java index a986127dfc551..e16f1d348d70c 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper; -import org.elasticsearch.index.mapper.ArrayValueMapperParser; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.xpack.spatial.common.CartesianPoint; @@ -30,8 +29,7 @@ * * Uses lucene 8 XYPoint encoding */ -public class PointFieldMapper extends AbstractPointGeometryFieldMapper, List> - implements ArrayValueMapperParser { +public class PointFieldMapper extends AbstractPointGeometryFieldMapper, List> { public static final String CONTENT_TYPE = "point"; public static class Builder extends AbstractPointGeometryFieldMapper.Builder { diff --git a/x-pack/plugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/DenseVectorFieldMapper.java b/x-pack/plugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/DenseVectorFieldMapper.java index e5062bf6d3bb8..acfe77002b21e 100644 --- a/x-pack/plugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/DenseVectorFieldMapper.java +++ b/x-pack/plugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/DenseVectorFieldMapper.java @@ -18,7 +18,6 @@ import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.fielddata.IndexFieldData; -import org.elasticsearch.index.mapper.ArrayValueMapperParser; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.Mapper; @@ -40,7 +39,7 @@ /** * A {@link FieldMapper} for indexing a dense vector of floats. */ -public class DenseVectorFieldMapper extends FieldMapper implements ArrayValueMapperParser { +public class DenseVectorFieldMapper extends FieldMapper { public static final String CONTENT_TYPE = "dense_vector"; public static short MAX_DIMS_COUNT = 2048; //maximum allowed number of dimensions @@ -173,6 +172,11 @@ public DenseVectorFieldType fieldType() { return (DenseVectorFieldType) super.fieldType(); } + @Override + public boolean parsesArrayValue() { + return true; + } + @Override public void parse(ParseContext context) throws IOException { if (context.externalValueSet()) {