Remove Mapper.Builder#mapperSize

Because of elastic#103865, DocumentParserContext#addDynamicMapper receives a Mapper, not a Mapper.Builder again. Therefore, we don't need a mapperSize method for the builder. This simplifies things a lot.
felixbarny · Jan 5, 2024 · a716549 · a716549
1 parent 4d53bcd
commit a716549
Show file tree

Hide file tree

Showing 23 changed files with 19 additions and 173 deletions.
diff --git a/...-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java b/...-geo/src/test/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapperTests.java
@@ -104,18 +104,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         checker.registerUpdateCheck(b -> b.field("distance_error_pct", 0.8), m -> {});
     }
 
-    @Override
-    public void testMapperBuilderSize() throws IOException {
-        super.testMapperBuilderSize();
-        assertWarnings("Parameter [strategy] is deprecated and will be removed in a future version");
-    }
-
-    @Override
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        super.testMapperBuilderSizeMultiField();
-        assertWarnings("Parameter [strategy] is deprecated and will be removed in a future version");
-    }
-
     @Override
     protected Collection<? extends Plugin> getPlugins() {
         return List.of(new TestLegacyGeoShapeFieldMapperPlugin());

diff --git a/...xtras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java b/...xtras/src/main/java/org/elasticsearch/index/mapper/extras/SearchAsYouTypeFieldMapper.java
@@ -270,12 +270,6 @@ public SearchAsYouTypeFieldMapper build(MapperBuilderContext context) {
                 this
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return super.mapperSize() + 1 // prefixField
-                + maxShingleSize.getValue() - 1;
-        }
     }
 
     private static int countPosition(TokenStream stream) throws IOException {

diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java b/modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java
@@ -127,11 +127,6 @@ public ParentJoinFieldMapper build(MapperBuilderContext context) {
                 relations.get()
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return 1 + relations.get().size();
-        }
     }
 
     public static final TypeParser PARSER = new TypeParser((n, c) -> {

diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java
@@ -178,16 +178,6 @@ public PercolatorFieldMapper build(MapperBuilderContext context) {
             );
         }
 
-        @Override
-        public int mapperSize() {
-            return super.mapperSize() // this + multi fields
-                + 1  // queryTermsField,
-                + 1  // extractionResultField,
-                + 1  // queryBuilderField,
-                + 1  // minimumShouldMatchFieldMapper,
-                + 1; // rangeFieldMapper
-        }
-
         static KeywordFieldMapper createExtractQueryFieldBuilder(
             String name,
             MapperBuilderContext context,

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java
@@ -155,11 +155,6 @@ public FieldAliasMapper build(MapperBuilderContext context) {
             String fullName = context.buildFullName(name);
             return new FieldAliasMapper(name, fullName, path);
         }
-
-        @Override
-        public int mapperSize() {
-            return 1;
-        }
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -1411,11 +1411,6 @@ private static boolean isDeprecatedParameter(String propName, IndexVersion index
             }
             return DEPRECATED_PARAMS.contains(propName);
         }
-
-        @Override
-        public int mapperSize() {
-            return 1 + multiFieldsBuilder.mapperSize();
-        }
     }
 
     public static BiConsumer<String, MappingParserContext> notInMultiFields(String type) {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
@@ -37,13 +37,6 @@ public String name() {
 
         /** Returns a newly built mapper. */
         public abstract Mapper build(MapperBuilderContext context);
-
-        /**
-         * Returns the size this mapper counts against the {@linkplain MapperService#INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING field limit}.
-         * <p>
-         * Needs to be in sync with {@link Mapper#mapperSize()} which is based on {@link Mapper#iterator()}
-         */
-        public abstract int mapperSize();
     }
 
     public interface TypeParser {
@@ -146,7 +139,7 @@ public static FieldType freezeAndDeduplicateFieldType(FieldType fieldType) {
     /**
      * Returns the size this mapper counts against the {@linkplain MapperService#INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING field limit}.
      * <p>
-     * Needs to be in sync with {@link Mapper.Builder#mapperSize()} and {@link MappingLookup#getTotalFieldsCount()}.
+     * Needs to be in sync with {@link MappingLookup#getTotalFieldsCount()}.
      */
     public int mapperSize() {
         int size = 1;

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java
@@ -180,11 +180,6 @@ public ObjectMapper build(MapperBuilderContext context) {
                 buildMappers(context.createChildContext(name))
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return 1 + mappersBuilders.stream().mapToInt(Mapper.Builder::mapperSize).sum();
-        }
     }
 
     public static class TypeParser implements Mapper.TypeParser {

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
@@ -119,11 +119,6 @@ public RootObjectMapper build(MapperBuilderContext context) {
                 numericDetection
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return mappersBuilders.stream().mapToInt(Mapper.Builder::mapperSize).sum() + runtimeFields.size();
-        }
     }
 
     private final Explicit<DateFormatter[]> dynamicDateTimeFormatters;

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperTests.java
@@ -17,25 +17,25 @@
 public class FieldAliasMapperTests extends MapperServiceTestCase {
 
     public void testParsing() throws IOException {
-        XContentBuilder mappingXContent = XContentFactory.jsonBuilder()
-            .startObject()
-            .startObject("_doc")
-            .startObject("properties")
-            .startObject("alias-field")
-            .field("type", "alias")
-            .field("path", "concrete-field")
-            .endObject()
-            .startObject("concrete-field")
-            .field("type", "keyword")
-            .endObject()
-            .endObject()
-            .endObject()
-            .endObject();
-        String mapping = Strings.toString(mappingXContent);
+        String mapping = Strings.toString(
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .startObject("_doc")
+                .startObject("properties")
+                .startObject("alias-field")
+                .field("type", "alias")
+                .field("path", "concrete-field")
+                .endObject()
+                .startObject("concrete-field")
+                .field("type", "keyword")
+                .endObject()
+                .endObject()
+                .endObject()
+                .endObject()
+        );
         DocumentMapper mapper = createDocumentMapper(mapping);
         assertEquals(mapping, mapper.mappingSource().toString());
-        RootObjectMapper.Builder builder = getRootObjectMapperBuilder(mappingXContent);
-        assertEquals(2, builder.mapperSize());
+        assertEquals(2, mapper.mapping().getRoot().mapperSize());
     }
 
     public void testParsingWithMissingPath() {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java
@@ -63,12 +63,6 @@ protected Object getSampleValueForDocument() {
         return "POINT (14.0 15.0)";
     }
 
-    @Override
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        super.testMapperBuilderSizeMultiField();
-        assertWarnings("Adding multifields to [geo_shape] mappers has no effect and will be forbidden in future");
-    }
-
     public void testDefaultConfiguration() throws IOException {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
         Mapper fieldMapper = mapper.mappers().getMapper("field");

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java
@@ -536,7 +536,6 @@ public void testNestedObjectWithMultiFieldsMapperSize() throws IOException {
                 ).addMultiField(new KeywordFieldMapper.Builder("multi_field_size_5", IndexVersion.current()))
             )
         );
-        assertThat(mapperBuilder.mapperSize(), equalTo(5));
         assertThat(mapperBuilder.build(MapperBuilderContext.root(false, false)).mapperSize(), equalTo(5));
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java
@@ -160,15 +160,7 @@ public void testRuntimeSection() throws IOException {
         }));
         MapperService mapperService = createMapperService(mapping);
         assertEquals(mapping, mapperService.documentMapper().mappingSource().toString());
-    }
-
-    public void testRuntimeMappingBuilderSize() throws IOException {
-        RootObjectMapper.Builder builder = getRootObjectMapperBuilder(runtimeMapping(b -> {
-            b.startObject("field1").field("type", "double").endObject();
-            b.startObject("field2").field("type", "date").endObject();
-            b.startObject("field3").field("type", "ip").endObject();
-        }));
-        assertEquals(3, builder.mapperSize());
+        assertEquals(3, mapperService.documentMapper().mapping().getRoot().mapperSize());
     }
 
     public void testRuntimeSectionRejectedUpdate() throws IOException {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java
@@ -92,9 +92,6 @@ protected boolean supportsIgnoreMalformed() {
         return false;
     }
 
-    @Override
-    public void testMapperBuilderSizeMultiField() {}
-
     public void testDefaults() throws Exception {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
         ParsedDocument parsedDoc = mapper.parse(source(b -> b.startObject("field").field("key", "value").endObject()));

diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java
@@ -225,18 +225,6 @@ protected final MapperService createMapperService(IndexVersion version, Settings
         );
     }
 
-    @SuppressWarnings("unchecked")
-    protected RootObjectMapper.Builder getRootObjectMapperBuilder(XContentBuilder mapping) throws IOException {
-        MapperService mapperService = createMapperService(mapping(b -> {}));
-        Map<String, Object> mappingAsMap = MappingParser.convertToMap(new CompressedXContent(BytesReference.bytes(mapping)));
-        RootObjectMapper.Builder builder = RootObjectMapper.parse(
-            "_doc",
-            (Map<String, Object>) mappingAsMap.get("_doc"),
-            mapperService.parserContext()
-        );
-        return builder;
-    }
-
     /**
      *  This is the injection point for tests that require mock scripts.  Test cases should override this to return the
      *  mock script factory of their choice.

diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
@@ -425,32 +425,6 @@ public final void testMinimalToMaximal() throws IOException {
         assertParseMaximalWarnings();
     }
 
-    public void testMapperBuilderSize() throws IOException {
-        RootObjectMapper.Builder builder = getRootObjectMapperBuilder(fieldMapping(this::minimalMapping));
-        assertEquals(builder.build(MapperBuilderContext.root(false, false)).mapperSize(), builder.mapperSize());
-    }
-
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        RootObjectMapper.Builder builder = getRootObjectMapperBuilder(fieldMapping(b -> {
-            minimalMapping(b);
-            b.startObject("fields");
-            {
-                b.startObject("multi_field_1");
-                {
-                    b.field("type", "keyword");
-                }
-                b.endObject();
-                b.startObject("multi_field_2");
-                {
-                    b.field("type", "keyword");
-                }
-                b.endObject();
-            }
-            b.endObject();
-        }));
-        assertEquals(builder.build(MapperBuilderContext.root(false, false)).mapperSize(), builder.mapperSize());
-    }
-
     protected final void assertParseMinimalWarnings() {
         String[] warnings = getParseMinimalWarnings();
         if (warnings.length > 0) {

diff --git a/...java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/...java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -254,11 +254,6 @@ public AggregateDoubleMetricFieldMapper build(MapperBuilderContext context) {
 
             return new AggregateDoubleMetricFieldMapper(name, metricFieldType, metricMappers, this);
         }
-
-        @Override
-        public int mapperSize() {
-            return 1;
-        }
     }
 
     public static final FieldMapper.TypeParser PARSER = new TypeParser(

diff --git a/.../main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java b/.../main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java
@@ -103,11 +103,6 @@ public ConstantKeywordFieldMapper build(MapperBuilderContext context) {
                 new ConstantKeywordFieldType(context.buildFullName(name), value.getValue(), meta.getValue())
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return 1;
-        }
     }
 
     public static final TypeParser PARSER = new TypeParser((n, c) -> new Builder(n));

diff --git a/...yword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java b/...yword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java
@@ -309,11 +309,6 @@ public FieldMapper build(MapperBuilderContext context) {
                 countFieldMapper
             );
         }
-
-        @Override
-        public int mapperSize() {
-            return super.mapperSize() + 1; // countFieldMapper
-        }
     }
 
     public static TypeParser PARSER = new TypeParser((n, c) -> new CountedKeywordFieldMapper.Builder(n));

diff --git a/...ck/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/mapper/SemanticTextFieldMapper.java b/...ck/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/mapper/SemanticTextFieldMapper.java
@@ -91,11 +91,6 @@ protected Parameter<?>[] getParameters() {
         public SemanticTextFieldMapper build(MapperBuilderContext context) {
             return new SemanticTextFieldMapper(name(), new SemanticTextFieldType(name(), modelId.getValue(), meta.getValue()), copyTo);
         }
-
-        @Override
-        public int mapperSize() {
-            return 1;
-        }
     }
 
     public static class SemanticTextFieldType extends SimpleMappedFieldType implements InferenceModelFieldType {

diff --git a/...a/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java b/...a/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java
@@ -66,12 +66,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         });
     }
 
-    @Override
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        super.testMapperBuilderSizeMultiField();
-        assertWarnings("Adding multifields to [geo_shape] mappers has no effect and will be forbidden in future");
-    }
-
     protected AbstractShapeGeometryFieldType<?> fieldType(Mapper fieldMapper) {
         AbstractShapeGeometryFieldMapper<?> shapeFieldMapper = (AbstractShapeGeometryFieldMapper<?>) fieldMapper;
         return (AbstractShapeGeometryFieldType<?>) shapeFieldMapper.fieldType();

diff --git a/...ial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java b/...ial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java
@@ -63,12 +63,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         });
     }
 
-    @Override
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        super.testMapperBuilderSizeMultiField();
-        assertWarnings("Adding multifields to [point] mappers has no effect and will be forbidden in future");
-    }
-
     public void testAggregationsDocValuesDisabled() throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);

diff --git a/...ial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java b/...ial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java
@@ -87,12 +87,6 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         });
     }
 
-    @Override
-    public void testMapperBuilderSizeMultiField() throws IOException {
-        super.testMapperBuilderSizeMultiField();
-        assertWarnings("Adding multifields to [shape] mappers has no effect and will be forbidden in future");
-    }
-
     protected AbstractShapeGeometryFieldType<?> fieldType(Mapper fieldMapper) {
         AbstractShapeGeometryFieldMapper<?> shapeFieldMapper = (AbstractShapeGeometryFieldMapper<?>) fieldMapper;
         return (AbstractShapeGeometryFieldType<?>) shapeFieldMapper.fieldType();