Simplify the return type of FieldMapper#parse. (#32654)

jtibshirani · jtibshirani · commit 4da9f18850f3 · 2018-09-04T12:27:33.000-07:00
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
@@ -379,7 +379,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         context.path().add(simpleName());
         XContentParser.Token token = context.parser().currentToken();
         String name = null;
@@ -441,7 +441,6 @@ public Mapper parse(ParseContext context) throws IOException {
         context.doc().add(field);
         context.doc().add(new SortedDocValuesField(fieldType().name(), binaryValue));
         context.path().remove();
-        return null;
     }
 
     @Override
diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java
@@ -402,7 +402,7 @@ public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldT
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         QueryShardContext queryShardContext = this.queryShardContext.get();
         if (context.doc().getField(queryBuilderField.name()) != null) {
             // If a percolator query has been defined in an array object then multiple percolator queries
@@ -425,7 +425,6 @@ public Mapper parse(ParseContext context) throws IOException {
         createQueryBuilderField(indexVersion, queryBuilderField, queryBuilder, context);
         Query query = toQuery(queryShardContext, isMapUnmappedFieldAsText(), queryBuilder);
         processQuery(query, context);
-        return null;
     }
 
     static void createQueryBuilderField(Version indexVersion, BinaryFieldMapper qbField,
diff --git a/plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java b/plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java
@@ -149,9 +149,8 @@ public void postParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // nothing to do here, we call the parent in postParse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AllFieldMapper.java
@@ -244,9 +244,8 @@ public void postParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // we parse in post parse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java
@@ -430,15 +430,15 @@ public CompletionFieldType fieldType() {
      *  else adds inputs as a {@link org.apache.lucene.search.suggest.document.SuggestField}
      */
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // parse
         XContentParser parser = context.parser();
         Token token = parser.currentToken();
         Map<String, CompletionInputMetaData> inputMap = new HashMap<>(1);
 
         // ignore null values
         if (token == Token.VALUE_NULL) {
-            return null;
+            return;
         } else if (token == Token.START_ARRAY) {
             while ((token = parser.nextToken()) != Token.END_ARRAY) {
                 parse(context, token, parser, inputMap);
@@ -477,7 +477,6 @@ public Mapper parse(ParseContext context) throws IOException {
             context.doc().add(field);
         }
         multiFields.parse(this, context);
-        return null;
     }
 
     /**
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -479,10 +479,7 @@ private static void parseObjectOrField(ParseContext context, Mapper mapper) thro
             parseObjectOrNested(context, (ObjectMapper) mapper);
         } else if (mapper instanceof FieldMapper) {
             FieldMapper fieldMapper = (FieldMapper) mapper;
-            Mapper update = fieldMapper.parse(context);
-            if (update != null) {
-                context.addDynamicMapper(update);
-            }
+            fieldMapper.parse(context);
             parseCopyFields(context, fieldMapper.copyTo().copyToFields());
         } else if (mapper instanceof FieldAliasMapper) {
             throw new IllegalArgumentException("Cannot write to a field alias [" + mapper.name() + "].");
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -287,11 +287,9 @@ public CopyTo copyTo() {
     }
 
     /**
-     * Parse using the provided {@link ParseContext} and return a mapping
-     * update if dynamic mappings modified the mappings, or {@code null} if
-     * mappings were not modified.
+     * Parse the field value using the provided {@link ParseContext}.
      */
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         final List<IndexableField> fields = new ArrayList<>(2);
         try {
             parseCreateField(context, fields);
@@ -303,7 +301,6 @@ public Mapper parse(ParseContext context) throws IOException {
                     fieldType().typeName());
         }
         multiFields.parse(this, context);
-        return null;
     }
 
     /**
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java
@@ -225,9 +225,8 @@ public void postParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // Adding values to the _field_names field is handled by the mappers for each field type
-        return null;
     }
 
     static Iterable<String> extractFieldNames(final String fullPath) {
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
@@ -285,7 +285,7 @@ protected void parse(ParseContext originalContext, GeoPoint point) throws IOExce
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         context.path().add(simpleName());
 
         GeoPoint sparse = context.parseExternalValue(GeoPoint.class);
@@ -340,7 +340,6 @@ public Mapper parse(ParseContext context) throws IOException {
         }
 
         context.path().remove();
-        return null;
     }
 
     /**
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
@@ -492,13 +492,13 @@ public GeoShapeFieldType fieldType() {
         return (GeoShapeFieldType) super.fieldType();
     }
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         try {
             Shape shape = context.parseExternalValue(Shape.class);
             if (shape == null) {
                 ShapeBuilder shapeBuilder = ShapeParser.parse(context.parser(), this);
                 if (shapeBuilder == null) {
-                    return null;
+                    return;
                 }
                 shape = shapeBuilder.build();
             }
@@ -510,7 +510,7 @@ public Mapper parse(ParseContext context) throws IOException {
                     for (Shape s : shapes) {
                         indexShape(context, s);
                     }
-                    return null;
+                    return;
                 } else if (shape instanceof Point == false) {
                     throw new MapperParsingException("[{" + fieldType().name() + "}] is configured for points only but a " +
                         ((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass()) + " was found");
@@ -524,7 +524,6 @@ public Mapper parse(ParseContext context) throws IOException {
             }
             context.addIgnoredField(fieldType.name());
         }
-        return null;
     }
 
     private void indexShape(ParseContext context, Shape shape) {
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IgnoredFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IgnoredFieldMapper.java
@@ -129,9 +129,8 @@ public void postParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // done in post-parse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java
@@ -157,11 +157,10 @@ public void preParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // no need ot parse here, we either get the routing in the sourceToParse
         // or we don't have routing, if we get it in sourceToParse, we process it in preParse
         // which will always be called
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SeqNoFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SeqNoFieldMapper.java
@@ -242,9 +242,8 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // fields are added in parseCreateField
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -223,9 +223,8 @@ public void preParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // nothing to do here, we will call it in pre parse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java
@@ -322,9 +322,8 @@ public void preParse(ParseContext context) throws IOException {
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // we parse in pre parse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/UidFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/UidFieldMapper.java
@@ -205,9 +205,8 @@ public void preParse(ParseContext context) throws IOException {
     public void postParse(ParseContext context) throws IOException {}
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // nothing to do here, we do everything in preParse
-        return null;
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/VersionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/VersionFieldMapper.java
@@ -117,9 +117,8 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         // _version added in preparse
-        return null;
     }
 
     @Override
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java
@@ -168,7 +168,7 @@ public ExternalMapper(String simpleName, MappedFieldType fieldType,
     }
 
     @Override
-    public Mapper parse(ParseContext context) throws IOException {
+    public void parse(ParseContext context) throws IOException {
         byte[] bytes = "Hello world".getBytes(Charset.defaultCharset());
         binMapper.parse(context.createExternalValueContext(bytes));
 
@@ -190,7 +190,6 @@ public Mapper parse(ParseContext context) throws IOException {
         stringMapper.parse(context);
 
         multiFields.parse(this, context);
-        return null;
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -379,7 +379,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field`
`379`	`379`	`}`
`380`	`380`
`381`	`381`	`@Override`
`382`		`- public Mapper parse(ParseContext context) throws IOException {`
	`382`	`+ public void parse(ParseContext context) throws IOException {`
`383`	`383`	`context.path().add(simpleName());`
`384`	`384`	`XContentParser.Token token = context.parser().currentToken();`
`385`	`385`	`String name = null;`
`@@ -441,7 +441,6 @@ public Mapper parse(ParseContext context) throws IOException {`
`441`	`441`	`context.doc().add(field);`
`442`	`442`	`context.doc().add(new SortedDocValuesField(fieldType().name(), binaryValue));`
`443`	`443`	`context.path().remove();`
`444`		`- return null;`
`445`	`444`	`}`
`446`	`445`
`447`	`446`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -402,7 +402,7 @@ public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldT`
`402`	`402`	`}`
`403`	`403`
`404`	`404`	`@Override`
`405`		`- public Mapper parse(ParseContext context) throws IOException {`
	`405`	`+ public void parse(ParseContext context) throws IOException {`
`406`	`406`	`QueryShardContext queryShardContext = this.queryShardContext.get();`
`407`	`407`	`if (context.doc().getField(queryBuilderField.name()) != null) {`
`408`	`408`	`// If a percolator query has been defined in an array object then multiple percolator queries`
`@@ -425,7 +425,6 @@ public Mapper parse(ParseContext context) throws IOException {`
`425`	`425`	`createQueryBuilderField(indexVersion, queryBuilderField, queryBuilder, context);`
`426`	`426`	`Query query = toQuery(queryShardContext, isMapUnmappedFieldAsText(), queryBuilder);`
`427`	`427`	`processQuery(query, context);`
`428`		`- return null;`
`429`	`428`	`}`
`430`	`429`
`431`	`430`	`static void createQueryBuilderField(Version indexVersion, BinaryFieldMapper qbField,`
Original file line number	Diff line number	Diff line change
`@@ -149,9 +149,8 @@ public void postParse(ParseContext context) throws IOException {`
`149`	`149`	`}`
`150`	`150`
`151`	`151`	`@Override`
`152`		`- public Mapper parse(ParseContext context) throws IOException {`
	`152`	`+ public void parse(ParseContext context) throws IOException {`
`153`	`153`	`// nothing to do here, we call the parent in postParse`
`154`		`- return null;`
`155`	`154`	`}`
`156`	`155`
`157`	`156`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -244,9 +244,8 @@ public void postParse(ParseContext context) throws IOException {`
`244`	`244`	`}`
`245`	`245`
`246`	`246`	`@Override`
`247`		`- public Mapper parse(ParseContext context) throws IOException {`
	`247`	`+ public void parse(ParseContext context) throws IOException {`
`248`	`248`	`// we parse in post parse`
`249`		`- return null;`
`250`	`249`	`}`
`251`	`250`
`252`	`251`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -287,11 +287,9 @@ public CopyTo copyTo() {`
`287`	`287`	`}`
`288`	`288`
`289`	`289`	`/**`
`290`		`- * Parse using the provided {@link ParseContext} and return a mapping`
`291`		`- * update if dynamic mappings modified the mappings, or {@code null} if`
`292`		`- * mappings were not modified.`
	`290`	`+ * Parse the field value using the provided {@link ParseContext}.`
`293`	`291`	`*/`
`294`		`- public Mapper parse(ParseContext context) throws IOException {`
	`292`	`+ public void parse(ParseContext context) throws IOException {`
`295`	`293`	`final List<IndexableField> fields = new ArrayList<>(2);`
`296`	`294`	`try {`
`297`	`295`	`parseCreateField(context, fields);`
`@@ -303,7 +301,6 @@ public Mapper parse(ParseContext context) throws IOException {`
`303`	`301`	`fieldType().typeName());`
`304`	`302`	`}`
`305`	`303`	`multiFields.parse(this, context);`
`306`		`- return null;`
`307`	`304`	`}`
`308`	`305`
`309`	`306`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -225,9 +225,8 @@ public void postParse(ParseContext context) throws IOException {`
`225`	`225`	`}`
`226`	`226`
`227`	`227`	`@Override`
`228`		`- public Mapper parse(ParseContext context) throws IOException {`
	`228`	`+ public void parse(ParseContext context) throws IOException {`
`229`	`229`	`// Adding values to the _field_names field is handled by the mappers for each field type`
`230`		`- return null;`
`231`	`230`	`}`
`232`	`231`
`233`	`232`	`static Iterable<String> extractFieldNames(final String fullPath) {`
Original file line number	Diff line number	Diff line change
`@@ -285,7 +285,7 @@ protected void parse(ParseContext originalContext, GeoPoint point) throws IOExce`
`285`	`285`	`}`
`286`	`286`
`287`	`287`	`@Override`
`288`		`- public Mapper parse(ParseContext context) throws IOException {`
	`288`	`+ public void parse(ParseContext context) throws IOException {`
`289`	`289`	`context.path().add(simpleName());`
`290`	`290`
`291`	`291`	`GeoPoint sparse = context.parseExternalValue(GeoPoint.class);`
`@@ -340,7 +340,6 @@ public Mapper parse(ParseContext context) throws IOException {`
`340`	`340`	`}`
`341`	`341`
`342`	`342`	`context.path().remove();`
`343`		`- return null;`
`344`	`343`	`}`
`345`	`344`
`346`	`345`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -492,13 +492,13 @@ public GeoShapeFieldType fieldType() {`
`492`	`492`	`return (GeoShapeFieldType) super.fieldType();`
`493`	`493`	`}`
`494`	`494`	`@Override`
`495`		`- public Mapper parse(ParseContext context) throws IOException {`
	`495`	`+ public void parse(ParseContext context) throws IOException {`
`496`	`496`	`try {`
`497`	`497`	`Shape shape = context.parseExternalValue(Shape.class);`
`498`	`498`	`if (shape == null) {`
`499`	`499`	`ShapeBuilder shapeBuilder = ShapeParser.parse(context.parser(), this);`
`500`	`500`	`if (shapeBuilder == null) {`
`501`		`- return null;`
	`501`	`+ return;`
`502`	`502`	`}`
`503`	`503`	`shape = shapeBuilder.build();`
`504`	`504`	`}`
`@@ -510,7 +510,7 @@ public Mapper parse(ParseContext context) throws IOException {`
`510`	`510`	`for (Shape s : shapes) {`
`511`	`511`	`indexShape(context, s);`
`512`	`512`	`}`
`513`		`- return null;`
	`513`	`+ return;`
`514`	`514`	`} else if (shape instanceof Point == false) {`
`515`	`515`	`throw new MapperParsingException("[{" + fieldType().name() + "}] is configured for points only but a " +`
`516`	`516`	`((shape instanceof JtsGeometry) ? ((JtsGeometry)shape).getGeom().getGeometryType() : shape.getClass()) + " was found");`
`@@ -524,7 +524,6 @@ public Mapper parse(ParseContext context) throws IOException {`
`524`	`524`	`}`
`525`	`525`	`context.addIgnoredField(fieldType.name());`
`526`	`526`	`}`
`527`		`- return null;`
`528`	`527`	`}`
`529`	`528`
`530`	`529`	`private void indexShape(ParseContext context, Shape shape) {`