Browse Source

Convert geo field mappers to Parametrized form (#63836)

Relates to #62988
Alan Woodward 5 years ago
parent
commit
bfaf3040c6
43 changed files with 962 additions and 1333 deletions
  1. 18 0
      docs/reference/migration/migrate_8_0/mappings.asciidoc
  2. 1 1
      modules/geo/src/main/java/org/elasticsearch/geo/GeoPlugin.java
  3. 1 1
      server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java
  4. 21 2
      server/src/main/java/org/elasticsearch/common/Strings.java
  5. 3 5
      server/src/main/java/org/elasticsearch/common/geo/GeoPoint.java
  6. 1 1
      server/src/main/java/org/elasticsearch/common/geo/SpatialStrategy.java
  7. 5 10
      server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java
  8. 4 6
      server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java
  9. 16 150
      server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
  10. 10 102
      server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java
  11. 20 157
      server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java
  12. 62 42
      server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
  13. 96 60
      server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
  14. 179 226
      server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java
  15. 29 9
      server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java
  16. 1 1
      server/src/main/java/org/elasticsearch/indices/IndicesModule.java
  17. 1 1
      server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java
  18. 5 5
      server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java
  19. 1 1
      server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java
  20. 2 2
      server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java
  21. 8 23
      server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java
  22. 1 1
      server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java
  23. 32 37
      server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java
  24. 1 1
      server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldTypeTests.java
  25. 31 47
      server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java
  26. 1 1
      server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java
  27. 4 6
      server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java
  28. 21 3
      test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
  29. 1 1
      test/framework/src/main/java/org/elasticsearch/test/TestGeoShapeFieldMapperPlugin.java
  30. 125 226
      x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java
  31. 7 9
      x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java
  32. 1 1
      x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java
  33. 17 24
      x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java
  34. 1 1
      x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldTypeTests.java
  35. 3 3
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java
  36. 2 4
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java
  37. 106 79
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java
  38. 55 33
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java
  39. 52 38
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapper.java
  40. 6 5
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeCentroidAggregatorTests.java
  41. 3 2
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java
  42. 2 1
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java
  43. 6 5
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java

+ 18 - 0
docs/reference/migration/migrate_8_0/mappings.asciidoc

@@ -95,3 +95,21 @@ For a detailed migration guide, see the {ref}/migrate-to-java-time.html[Java
 time migration guide].
 ====
 // end::notable-breaking-changes[]
+
+[[geo-shape-strategy]]
+.The `strategy` parameter on `geo_shape` mappings now rejects unknown strategies
+[%collapsible]
+====
+*Details* +
+The only permissible values for the `strategy` parameter on `geo_shape` mappings
+are `term` and `recursive`. In 7.x if a non-permissible value was used as a
+parameter here, the mapping would silently fall back to using `recursive`.  The
+mapping will now be rejected instead.
+
+*Impact* +
+This will have no impact on existing mappings created with non-permissible
+strategy values, as they will already be serializing themselves as if they
+had been configured as `recursive`.  New indexes will need to use one of the
+permissible strategies, or preferably not define a strategy at all and use
+the far more efficient BKD index.
+====

+ 1 - 1
modules/geo/src/main/java/org/elasticsearch/geo/GeoPlugin.java

@@ -31,6 +31,6 @@ public class GeoPlugin extends Plugin implements MapperPlugin {
 
     @Override
     public Map<String, Mapper.TypeParser> getMappers() {
-        return Collections.singletonMap(GeoShapeFieldMapper.CONTENT_TYPE, new GeoShapeFieldMapper.TypeParser());
+        return Collections.singletonMap(GeoShapeFieldMapper.CONTENT_TYPE, GeoShapeFieldMapper.PARSER);
     }
 }

+ 1 - 1
server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java

@@ -154,7 +154,7 @@ public class GeoShapeIntegrationIT extends ESIntegTestCase {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices()
             .preparePutMapping("test")
             .setSource(update, XContentType.JSON).get());
-        assertThat(e.getMessage(), containsString("using [BKD] strategy cannot be merged with"));
+        assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [BKD] to [recursive]"));
     }
 
     /**

+ 21 - 2
server/src/main/java/org/elasticsearch/common/Strings.java

@@ -411,6 +411,25 @@ public class Strings {
         return collection.toArray(new String[collection.size()]);
     }
 
+    /**
+     * Concatenate two string arrays into a third
+     */
+    public static String[] concatStringArrays(String[] first, String[] second) {
+        if (first == null && second == null) {
+            return Strings.EMPTY_ARRAY;
+        }
+        if (first == null || first.length == 0) {
+            return second;
+        }
+        if (second == null || second.length == 0) {
+            return first;
+        }
+        String[] concat = new String[first.length + second.length];
+        System.arraycopy(first, 0, concat, 0, first.length);
+        System.arraycopy(second, 0, concat, first.length, second.length);
+        return concat;
+    }
+
     /**
      * Tokenize the specified string by commas to a set, trimming whitespace and ignoring empty tokens.
      *
@@ -879,7 +898,7 @@ public class Strings {
             return sb.toString();
         }
     }
-    
+
     public static String toLowercaseAscii(String in) {
         StringBuilder out = new StringBuilder();
         Iterator<Integer> iter = in.codePoints().iterator();
@@ -892,5 +911,5 @@ public class Strings {
             }
         }
         return out.toString();
-    }    
+    }
 }

+ 3 - 5
server/src/main/java/org/elasticsearch/common/geo/GeoPoint.java

@@ -25,10 +25,10 @@ import org.apache.lucene.geo.GeoEncodingUtils;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.util.BitUtil;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.geo.GeoUtils.EffectivePoint;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.Rectangle;
@@ -41,8 +41,6 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Locale;
 
-import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
-
 public class GeoPoint implements ToXContentFragment {
 
     protected double lat;
@@ -265,8 +263,8 @@ public class GeoPoint implements ToXContentFragment {
 
     public static double assertZValue(final boolean ignoreZValue, double zValue) {
         if (ignoreZValue == false) {
-            throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [{}] "
-                + "parameter is [{}]", zValue, IGNORE_Z_VALUE, ignoreZValue);
+            throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [ignore_z_value] "
+                + "parameter is [{}]", zValue, ignoreZValue);
         }
         return zValue;
     }

+ 1 - 1
server/src/main/java/org/elasticsearch/common/geo/SpatialStrategy.java

@@ -54,6 +54,6 @@ public enum SpatialStrategy implements Writeable {
                 return strategy;
             }
         }
-        return null;
+        throw new IllegalArgumentException("Unknown strategy [" + strategyName + "]");
     }
 }

+ 5 - 10
server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java

@@ -19,7 +19,6 @@
 package org.elasticsearch.common.geo.parsers;
 
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoShapeType;
 import org.elasticsearch.common.geo.builders.CircleBuilder;
@@ -50,14 +49,10 @@ abstract class GeoJsonParser {
         GeometryCollectionBuilder geometryCollections = null;
 
         Orientation orientation = (shapeMapper == null)
-            ? AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value()
+            ? Orientation.RIGHT
             : shapeMapper.orientation();
-        Explicit<Boolean> coerce = (shapeMapper == null)
-            ? AbstractShapeGeometryFieldMapper.Defaults.COERCE
-            : shapeMapper.coerce();
-        Explicit<Boolean> ignoreZValue = (shapeMapper == null)
-            ? AbstractShapeGeometryFieldMapper.Defaults.IGNORE_Z_VALUE
-            : shapeMapper.ignoreZValue();
+        boolean coerce = shapeMapper != null && shapeMapper.coerce();
+        boolean ignoreZValue = shapeMapper == null || shapeMapper.ignoreZValue();
 
         String malformedException = null;
 
@@ -78,7 +73,7 @@ abstract class GeoJsonParser {
                         }
                     } else if (ShapeParser.FIELD_COORDINATES.match(fieldName, subParser.getDeprecationHandler())) {
                         subParser.nextToken();
-                        CoordinateNode tempNode = parseCoordinates(subParser, ignoreZValue.value());
+                        CoordinateNode tempNode = parseCoordinates(subParser, ignoreZValue);
                         if (coordinateNode != null && tempNode.numDimensions() != coordinateNode.numDimensions()) {
                             throw new ElasticsearchParseException("Exception parsing coordinates: " +
                                 "number of dimensions do not match");
@@ -134,7 +129,7 @@ abstract class GeoJsonParser {
             return geometryCollections;
         }
 
-        return shapeType.getBuilder(coordinateNode, radius, orientation, coerce.value());
+        return shapeType.getBuilder(coordinateNode, radius, orientation, coerce);
     }
 
     /**

+ 4 - 6
server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java

@@ -19,7 +19,6 @@
 package org.elasticsearch.common.geo.parsers;
 
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoShapeType;
 import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
@@ -78,9 +77,8 @@ public class GeoWKTParser {
                                                  final AbstractShapeGeometryFieldMapper shapeMapper)
             throws IOException, ElasticsearchParseException {
         try (StringReader reader = new StringReader(parser.text())) {
-            Explicit<Boolean> ignoreZValue = (shapeMapper == null) ? AbstractShapeGeometryFieldMapper.Defaults.IGNORE_Z_VALUE :
-                shapeMapper.ignoreZValue();
-            Explicit<Boolean> coerce = (shapeMapper == null) ? AbstractShapeGeometryFieldMapper.Defaults.COERCE : shapeMapper.coerce();
+            boolean coerce = shapeMapper != null && shapeMapper.coerce();
+            boolean ignoreZValue = shapeMapper == null || shapeMapper.ignoreZValue();
             // setup the tokenizer; configured to read words w/o numbers
             StreamTokenizer tokenizer = new StreamTokenizer(reader);
             tokenizer.resetSyntax();
@@ -93,7 +91,7 @@ public class GeoWKTParser {
             tokenizer.wordChars('.', '.');
             tokenizer.whitespaceChars(0, ' ');
             tokenizer.commentChar('#');
-            ShapeBuilder builder = parseGeometry(tokenizer, shapeType, ignoreZValue.value(), coerce.value());
+            ShapeBuilder builder = parseGeometry(tokenizer, shapeType, ignoreZValue, coerce);
             checkEOF(tokenizer);
             return builder;
         }
@@ -258,7 +256,7 @@ public class GeoWKTParser {
             return null;
         }
         PolygonBuilder builder = new PolygonBuilder(parseLinearRing(stream, ignoreZValue, coerce),
-            AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value());
+            ShapeBuilder.Orientation.RIGHT);
         while (nextCloserOrComma(stream).equals(COMMA)) {
             builder.hole(parseLinearRing(stream, ignoreZValue, coerce));
         }

+ 16 - 150
server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java

@@ -18,20 +18,15 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
-import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Explicit;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.geo.GeoJsonGeometryFormat;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.support.MapXContentParser;
-import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.search.lookup.SearchLookup;
 
@@ -40,8 +35,6 @@ import java.io.UncheckedIOException;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
@@ -49,22 +42,15 @@ import java.util.function.Function;
 /**
  * Base field mapper class for all spatial field types
  */
-public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends FieldMapper {
+public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends ParametrizedFieldMapper {
 
-    public static class Names {
-        public static final ParseField IGNORE_MALFORMED = new ParseField("ignore_malformed");
-        public static final ParseField IGNORE_Z_VALUE = new ParseField("ignore_z_value");
+    public static Parameter<Explicit<Boolean>> ignoreMalformedParam(Function<FieldMapper, Explicit<Boolean>> initializer,
+                                                                    boolean ignoreMalformedByDefault) {
+        return Parameter.explicitBoolParam("ignore_malformed", true, initializer, ignoreMalformedByDefault);
     }
 
-    public static class Defaults {
-        public static final Explicit<Boolean> IGNORE_MALFORMED = new Explicit<>(false, false);
-        public static final Explicit<Boolean> IGNORE_Z_VALUE = new Explicit<>(true, false);
-        public static final FieldType FIELD_TYPE = new FieldType();
-        static {
-            FIELD_TYPE.setStored(false);
-            FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
-            FIELD_TYPE.freeze();
-        }
+    public static Parameter<Explicit<Boolean>> ignoreZValueParam(Function<FieldMapper, Explicit<Boolean>> initializer) {
+        return Parameter.explicitBoolParam("ignore_z_value", true, initializer, true);
     }
 
     /**
@@ -118,102 +104,6 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         }
     }
 
-    public abstract static class Builder extends FieldMapper.Builder {
-        protected Boolean ignoreMalformed;
-        protected Boolean ignoreZValue;
-        protected boolean indexed = true;
-
-        public Builder(String name, FieldType fieldType) {
-            super(name, fieldType);
-        }
-
-        public Builder(String name, FieldType fieldType, boolean ignoreMalformed,
-                       boolean ignoreZValue) {
-            super(name, fieldType);
-            this.ignoreMalformed = ignoreMalformed;
-            this.ignoreZValue = ignoreZValue;
-        }
-
-        public Builder ignoreMalformed(boolean ignoreMalformed) {
-            this.ignoreMalformed = ignoreMalformed;
-            return this;
-        }
-
-        protected Explicit<Boolean> ignoreMalformed(BuilderContext context) {
-            if (ignoreMalformed != null) {
-                return new Explicit<>(ignoreMalformed, true);
-            }
-            if (context.indexSettings() != null) {
-                return new Explicit<>(IGNORE_MALFORMED_SETTING.get(context.indexSettings()), false);
-            }
-            return Defaults.IGNORE_MALFORMED;
-        }
-
-        public Explicit<Boolean> ignoreMalformed() {
-            if (ignoreMalformed != null) {
-                return new Explicit<>(ignoreMalformed, true);
-            }
-            return Defaults.IGNORE_MALFORMED;
-        }
-
-        protected Explicit<Boolean> ignoreZValue(BuilderContext context) {
-            if (ignoreZValue != null) {
-                return new Explicit<>(ignoreZValue, true);
-            }
-            return Defaults.IGNORE_Z_VALUE;
-        }
-
-        public Explicit<Boolean> ignoreZValue() {
-            if (ignoreZValue != null) {
-                return new Explicit<>(ignoreZValue, true);
-            }
-            return Defaults.IGNORE_Z_VALUE;
-        }
-
-        public Builder ignoreZValue(final boolean ignoreZValue) {
-            this.ignoreZValue = ignoreZValue;
-            return this;
-        }
-    }
-
-    public abstract static class TypeParser<T extends Builder> implements Mapper.TypeParser {
-        protected abstract T newBuilder(String name, Map<String, Object> params);
-
-        public T parse(String name, Map<String, Object> node, Map<String, Object> params, ParserContext parserContext) {
-            for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
-                Map.Entry<String, Object> entry = iterator.next();
-                String propName = entry.getKey();
-                Object propNode = entry.getValue();
-
-                if (Names.IGNORE_MALFORMED.match(propName, LoggingDeprecationHandler.INSTANCE)) {
-                    params.put(Names.IGNORE_MALFORMED.getPreferredName(), XContentMapValues.nodeBooleanValue(propNode,
-                        name + ".ignore_malformed"));
-                    iterator.remove();
-                } else if (Names.IGNORE_Z_VALUE.getPreferredName().equals(propName)) {
-                    params.put(GeoPointFieldMapper.Names.IGNORE_Z_VALUE.getPreferredName(),
-                        XContentMapValues.nodeBooleanValue(propNode, name + "." + Names.IGNORE_Z_VALUE.getPreferredName()));
-                    iterator.remove();
-                }
-            }
-            T builder = newBuilder(name, params);
-            if (params.containsKey(GeoPointFieldMapper.Names.IGNORE_Z_VALUE.getPreferredName())) {
-                builder.ignoreZValue((Boolean)params.get(GeoPointFieldMapper.Names.IGNORE_Z_VALUE.getPreferredName()));
-            }
-
-            if (params.containsKey(Names.IGNORE_MALFORMED.getPreferredName())) {
-                builder.ignoreMalformed((Boolean)params.get(Names.IGNORE_MALFORMED.getPreferredName()));
-            }
-            return builder;
-        }
-
-        @Override
-        public T parse(String name, Map<String, Object> node, ParserContext parserContext)
-            throws MapperParsingException {
-            Map<String, Object> params = new HashMap<>();
-            return parse(name, node, params, parserContext);
-        }
-    }
-
     public abstract static class AbstractGeometryFieldType extends MappedFieldType {
 
         protected final Parser<?> geometryParser;
@@ -255,35 +145,22 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         }
     }
 
-    protected Explicit<Boolean> ignoreMalformed;
-    protected Explicit<Boolean> ignoreZValue;
+    private final Explicit<Boolean> ignoreMalformed;
+    private final Explicit<Boolean> ignoreZValue;
     private final Indexer<Parsed, Processed> indexer;
     private final Parser<Parsed> parser;
 
-    protected AbstractGeometryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
+    protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                           Explicit<Boolean> ignoreMalformed, Explicit<Boolean> ignoreZValue,
                                           MultiFields multiFields, CopyTo copyTo,
                                           Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
+        super(simpleName, mappedFieldType, multiFields, copyTo);
         this.ignoreMalformed = ignoreMalformed;
         this.ignoreZValue = ignoreZValue;
         this.indexer = indexer;
         this.parser = parser;
     }
 
-    @Override
-    @SuppressWarnings("unchecked")
-    protected void mergeOptions(FieldMapper other, List<String> conflicts) {
-        AbstractGeometryFieldMapper<Parsed, Processed> gsfm = (AbstractGeometryFieldMapper<Parsed, Processed>)other;
-
-        if (gsfm.ignoreMalformed.explicit()) {
-            this.ignoreMalformed = gsfm.ignoreMalformed;
-        }
-        if (gsfm.ignoreZValue.explicit()) {
-            this.ignoreZValue = gsfm.ignoreZValue;
-        }
-    }
-
     @Override
     public AbstractGeometryFieldType fieldType() {
         return (AbstractGeometryFieldType) mappedFieldType;
@@ -324,13 +201,13 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
                 indexedFields.addAll(fields);
             }
             // stored:
-            if (fieldType.stored()) {
+            if (fieldType().isStored()) {
                 addStoredFields(context, shape);
             }
             // docValues:
             if (fieldType().hasDocValues()) {
                 addDocValuesFields(mappedFieldType.name(), shape, fields, context);
-            } else if (fieldType.stored() || fieldType().isSearchable()) {
+            } else if (fieldType().isStored() || fieldType().isSearchable()) {
                 createFieldNamesField(context);
             }
 
@@ -350,22 +227,11 @@ public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends Fie
         }
     }
 
-    @Override
-    public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
-        super.doXContentBody(builder, includeDefaults, params);
-        if (includeDefaults || ignoreMalformed.explicit()) {
-            builder.field(Names.IGNORE_MALFORMED.getPreferredName(), ignoreMalformed.value());
-        }
-        if (includeDefaults || ignoreZValue.explicit()) {
-            builder.field(Names.IGNORE_Z_VALUE.getPreferredName(), ignoreZValue.value());
-        }
-    }
-
-    public Explicit<Boolean> ignoreMalformed() {
-        return ignoreMalformed;
+    public boolean ignoreMalformed() {
+        return ignoreMalformed.value();
     }
 
-    public Explicit<Boolean> ignoreZValue() {
-        return ignoreZValue;
+    public boolean ignoreZValue() {
+        return ignoreZValue.value();
     }
 }

+ 10 - 102
server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java

@@ -18,115 +18,42 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.CheckedBiFunction;
 import org.elasticsearch.common.Explicit;
-import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeometryFormat;
 import org.elasticsearch.common.geo.GeometryParser;
-import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Point;
+import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext;
 
 import java.io.IOException;
 import java.text.ParseException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
+import java.util.function.Function;
 import java.util.function.Supplier;
 
-import static org.elasticsearch.index.mapper.TypeParsers.parseField;
-
 /** Base class for for spatial fields that only support indexing points */
 public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extends AbstractGeometryFieldMapper<Parsed, Processed> {
 
-    public static class Names extends AbstractGeometryFieldMapper.Names {
-        public static final ParseField NULL_VALUE = new ParseField("null_value");
-    }
-
-    public static final FieldType DEFAULT_FIELD_TYPE = new FieldType();
-    static {
-        DEFAULT_FIELD_TYPE.setDimensions(2, Integer.BYTES);
-        DEFAULT_FIELD_TYPE.setStored(false);
-        DEFAULT_FIELD_TYPE.freeze();
-    }
-
-    public abstract static class Builder extends AbstractGeometryFieldMapper.Builder {
-
-        protected ParsedPoint nullValue;
-
-        public Builder(String name, FieldType fieldType) {
-            super(name, fieldType);
-        }
-
-        public void setNullValue(ParsedPoint nullValue) {
-            this.nullValue = nullValue;
-        }
-
-        public abstract AbstractPointGeometryFieldMapper build(BuilderContext context, String simpleName, FieldType fieldType,
-                                                               MultiFields multiFields,
-                                                               Explicit<Boolean> ignoreMalformed,
-                                                               Explicit<Boolean> ignoreZValue,
-                                                               ParsedPoint nullValue, CopyTo copyTo);
-
-
-        @Override
-        public AbstractPointGeometryFieldMapper build(BuilderContext context) {
-            return build(context, name, fieldType,
-                multiFieldsBuilder.build(this, context), ignoreMalformed(context),
-                ignoreZValue(context), nullValue, copyTo);
-        }
-    }
-
-    public abstract static class TypeParser<T extends Builder> extends AbstractGeometryFieldMapper.TypeParser<Builder> {
-        protected abstract ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed);
-
-        @Override
-        public T parse(String name, Map<String, Object> node, Map<String, Object> params, ParserContext parserContext) {
-            T builder = (T)(super.parse(name, node, params, parserContext));
-            parseField(builder, name, node, parserContext);
-            Object nullValue = null;
-            for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
-                Map.Entry<String, Object> entry = iterator.next();
-                String propName = entry.getKey();
-                Object propNode = entry.getValue();
-
-                if (Names.NULL_VALUE.match(propName, LoggingDeprecationHandler.INSTANCE)) {
-                    nullValue = propNode;
-                    iterator.remove();
-                }
-            }
-
-            if (nullValue != null) {
-                builder.setNullValue(parseNullValue(nullValue, (Boolean)builder.ignoreZValue().value(),
-                    (Boolean)builder.ignoreMalformed().value()));
-            }
-
-            return builder;
-        }
+    public static Parameter<ParsedPoint> nullValueParam(Function<FieldMapper, ParsedPoint> initializer,
+                                                        TriFunction<String, ParserContext, Object, ParsedPoint> parser,
+                                                        Supplier<ParsedPoint> def) {
+        return new Parameter<>("null_value", false, def, parser, initializer);
     }
 
-    ParsedPoint nullValue;
+    protected final ParsedPoint nullValue;
 
-    public abstract static class AbstractPointGeometryFieldType
-        extends AbstractGeometryFieldType {
-        protected AbstractPointGeometryFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
-                                                 Parser<?> parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, true, parser, meta);
-        }
-    }
-
-    protected AbstractPointGeometryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
+    protected AbstractPointGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                                MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
                                                Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo,
                                                Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
+        super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
         this.nullValue = nullValue;
     }
 
@@ -135,25 +62,6 @@ public abstract class AbstractPointGeometryFieldMapper<Parsed, Processed> extend
         return true;
     }
 
-    @Override
-    @SuppressWarnings("unchecked")
-    protected void mergeOptions(FieldMapper other, List<String> conflicts) {
-        super.mergeOptions(other, conflicts);
-        AbstractPointGeometryFieldMapper<Parsed, Processed> gpfm = (AbstractPointGeometryFieldMapper<Parsed, Processed>)other;
-        // TODO make this un-updateable
-        if (gpfm.nullValue != null) {
-            this.nullValue = gpfm.nullValue;
-        }
-    }
-
-    @Override
-    public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
-        super.doXContentBody(builder, includeDefaults, params);
-        if (nullValue != null || includeDefaults) {
-            builder.field(Names.NULL_VALUE.getPreferredName(), nullValue);
-        }
-    }
-
     public ParsedPoint getNullValue() {
         return nullValue;
     }

+ 20 - 157
server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java

@@ -18,164 +18,54 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.elasticsearch.common.Explicit;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
-import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.support.XContentMapValues;
-import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper.DeprecatedParameters;
 
-import java.io.IOException;
-import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
 
 /**
  * Base class for {@link GeoShapeFieldMapper} and {@link LegacyGeoShapeFieldMapper}
  */
 public abstract class AbstractShapeGeometryFieldMapper<Parsed, Processed> extends AbstractGeometryFieldMapper<Parsed, Processed> {
 
-    public static class Names extends AbstractGeometryFieldMapper.Names {
-        public static final ParseField ORIENTATION = new ParseField("orientation");
-        public static final ParseField COERCE = new ParseField("coerce");
+    public static Parameter<Explicit<Boolean>> coerceParam(Function<FieldMapper, Explicit<Boolean>> initializer,
+                                                           boolean coerceByDefault) {
+        return Parameter.explicitBoolParam("coerce", true, initializer, coerceByDefault);
     }
 
-    public static class Defaults extends AbstractGeometryFieldMapper.Defaults {
-        public static final Explicit<Orientation> ORIENTATION = new Explicit<>(Orientation.RIGHT, false);
-        public static final Explicit<Boolean> COERCE = new Explicit<>(false, false);
-    }
-
-    public abstract static class Builder extends AbstractGeometryFieldMapper.Builder {
-        protected Boolean coerce;
-        protected Orientation orientation;
-
-        /** default builder - used for external mapper*/
-        public Builder(String name, FieldType fieldType) {
-            super(name, fieldType);
-        }
-
-        public Builder(String name, FieldType fieldType,
-                       boolean coerce, boolean ignoreMalformed, Orientation orientation, boolean ignoreZ) {
-            super(name, fieldType, ignoreMalformed, ignoreZ);
-            this.coerce = coerce;
-            this.orientation = orientation;
-        }
-
-        public Builder coerce(boolean coerce) {
-            this.coerce = coerce;
-            return this;
-        }
-
-        protected Explicit<Boolean> coerce(BuilderContext context) {
-            if (coerce != null) {
-                return new Explicit<>(coerce, true);
-            }
-            if (context.indexSettings() != null) {
-                return new Explicit<>(COERCE_SETTING.get(context.indexSettings()), false);
-            }
-            return Defaults.COERCE;
-        }
-
-        protected Explicit<Boolean> coerce() {
-            if (coerce != null) {
-                return new Explicit<>(coerce, true);
-            }
-            return Defaults.COERCE;
-        }
-
-        public Builder orientation(Orientation orientation) {
-            this.orientation = orientation;
-            return this;
-        }
-
-        protected Explicit<Orientation> orientation() {
-            if (orientation != null) {
-                return new Explicit<>(orientation, true);
-            }
-            return Defaults.ORIENTATION;
-        }
-
-    }
-
-    protected static final String DEPRECATED_PARAMETERS_KEY = "deprecated_parameters";
-
-    public abstract static class TypeParser extends AbstractGeometryFieldMapper.TypeParser<Builder> {
-        protected abstract Builder newBuilder(String name, Map<String, Object> params);
-
-        protected boolean parseXContentParameters(String name, Map.Entry<String, Object> entry, Map<String, Object> params)
-                throws MapperParsingException {
-            if (DeprecatedParameters.parse(name, entry.getKey(), entry.getValue(),
-                (DeprecatedParameters)params.get(DEPRECATED_PARAMETERS_KEY))) {
-                return true;
-            }
-            return false;
-        }
-
-        @Override
-        public Builder parse(String name, Map<String, Object> node, Map<String, Object> params, ParserContext parserContext) {
-            boolean parsedDeprecatedParameters = false;
-            params.put(DEPRECATED_PARAMETERS_KEY, new DeprecatedParameters());
-            for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
-                Map.Entry<String, Object> entry = iterator.next();
-                String fieldName = entry.getKey();
-                Object fieldNode = entry.getValue();
-                if (parseXContentParameters(name, entry, params)) {
-                    parsedDeprecatedParameters = true;
-                    iterator.remove();
-                } else if (Names.ORIENTATION.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                    params.put(Names.ORIENTATION.getPreferredName(), ShapeBuilder.Orientation.fromString(fieldNode.toString()));
-                    iterator.remove();
-                } else if (Names.COERCE.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                    params.put(Names.COERCE.getPreferredName(),
-                        XContentMapValues.nodeBooleanValue(fieldNode, name + "." + Names.COERCE.getPreferredName()));
-                    iterator.remove();
-                }
-            }
-            if (parsedDeprecatedParameters == false) {
-                params.remove(DEPRECATED_PARAMETERS_KEY);
-            }
-
-            Builder builder = super.parse(name, node, params, parserContext);
-
-            if (params.containsKey(Names.COERCE.getPreferredName())) {
-                builder.coerce((Boolean)params.get(Names.COERCE.getPreferredName()));
-            }
-
-            if (params.containsKey(Names.ORIENTATION.getPreferredName())) {
-                builder.orientation((Orientation)params.get(Names.ORIENTATION.getPreferredName()));
-            }
-
-            return builder;
-        }
+    public static Parameter<Explicit<Orientation>> orientationParam(Function<FieldMapper, Explicit<Orientation>> initializer) {
+        return new Parameter<>("orientation", true,
+            () -> new Explicit<>(Orientation.RIGHT, false),
+            (n, c, o) -> new Explicit<>(ShapeBuilder.Orientation.fromString(o.toString()), true),
+            initializer)
+            .setSerializer((b, f, v) -> b.field(f, v.value()), v -> v.value().toString());
     }
 
     public abstract static class AbstractShapeGeometryFieldType extends AbstractGeometryFieldType {
-        protected Orientation orientation = Defaults.ORIENTATION.value();
+
+        private final Orientation orientation;
 
         protected AbstractShapeGeometryFieldType(String name, boolean isSearchable, boolean isStored, boolean hasDocValues,
-                                                 boolean parsesArrayValue, Parser<?> parser, Map<String, String> meta) {
+                                                 boolean parsesArrayValue, Parser<?> parser,
+                                                 Orientation orientation, Map<String, String> meta) {
             super(name, isSearchable, isStored, hasDocValues, parsesArrayValue, parser, meta);
+            this.orientation = orientation;
         }
 
         public Orientation orientation() { return this.orientation; }
-
-        public void setOrientation(Orientation orientation) {
-            this.orientation = orientation;
-        }
     }
 
     protected Explicit<Boolean> coerce;
     protected Explicit<Orientation> orientation;
 
-    protected AbstractShapeGeometryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
+    protected AbstractShapeGeometryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                                Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
                                                Explicit<Boolean> ignoreZValue, Explicit<Orientation> orientation,
                                                MultiFields multiFields, CopyTo copyTo,
                                                Indexer<Parsed, Processed> indexer, Parser<Parsed> parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
+        super(simpleName, mappedFieldType, ignoreMalformed, ignoreZValue, multiFields, copyTo, indexer, parser);
         this.coerce = coerce;
         this.orientation = orientation;
     }
@@ -185,38 +75,11 @@ public abstract class AbstractShapeGeometryFieldMapper<Parsed, Processed> extend
         return false;
     }
 
-    @Override
-    @SuppressWarnings("unchecked")
-    protected final void mergeOptions(FieldMapper other, List<String> conflicts) {
-        super.mergeOptions(other, conflicts);
-        AbstractShapeGeometryFieldMapper<Parsed, Processed> gsfm = (AbstractShapeGeometryFieldMapper<Parsed, Processed>)other;
-        if (gsfm.coerce.explicit()) {
-            this.coerce = gsfm.coerce;
-        }
-        if (gsfm.orientation.explicit()) {
-            this.orientation = gsfm.orientation;
-        }
-        mergeGeoOptions(gsfm, conflicts);
-    }
-
-    protected abstract void mergeGeoOptions(AbstractShapeGeometryFieldMapper<?,?> mergeWith, List<String> conflicts);
-
-    @Override
-    public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
-        super.doXContentBody(builder, includeDefaults, params);
-        if (includeDefaults || coerce.explicit()) {
-            builder.field(AbstractShapeGeometryFieldMapper.Names.COERCE.getPreferredName(), coerce.value());
-        }
-        if (includeDefaults || orientation.explicit()) {
-            builder.field(Names.ORIENTATION.getPreferredName(), orientation.value());
-        }
-    }
-
-    public Explicit<Boolean> coerce() {
-        return coerce;
+    public boolean coerce() {
+        return coerce.value();
     }
 
     public Orientation orientation() {
-        return ((AbstractShapeGeometryFieldType)mappedFieldType).orientation();
+        return orientation.value();
     }
 }

+ 62 - 42
server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -18,11 +18,9 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.LatLonDocValuesField;
 import org.apache.lucene.document.LatLonPoint;
 import org.apache.lucene.document.StoredField;
-import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.ElasticsearchParseException;
@@ -43,6 +41,7 @@ import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -54,44 +53,46 @@ import java.util.function.Supplier;
  * Uses lucene 6 LatLonPoint encoding
  */
 public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<ParsedGeoPoint>, List<? extends GeoPoint>> {
+
     public static final String CONTENT_TYPE = "geo_point";
-    public static final FieldType FIELD_TYPE = new FieldType();
 
-    static {
-        FIELD_TYPE.setStored(false);
-        FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
-        FIELD_TYPE.freeze();
+    private static Builder builder(FieldMapper in) {
+        return ((GeoPointFieldMapper)in).builder;
     }
 
-    public static class Builder extends AbstractPointGeometryFieldMapper.Builder {
-
-        public Builder(String name) {
-            super(name, FIELD_TYPE);
-            hasDocValues = true;
+    public static class Builder extends ParametrizedFieldMapper.Builder {
+
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<ParsedPoint> nullValue;
+        final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+        final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> builder(m).hasDocValues.get(), true);
+        final Parameter<Boolean> stored = Parameter.storeParam(m -> builder(m).stored.get(), false);
+        final Parameter<Map<String, String>> meta = Parameter.metaParam();
+
+        public Builder(String name, boolean ignoreMalformedByDefault) {
+            super(name);
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.nullValue = nullValueParam(
+                m -> builder(m).nullValue.get(),
+                (n, c, o) -> parseNullValue(o, ignoreZValue.get().value(), ignoreMalformed.get().value()),
+                () -> null).acceptsNull();
         }
 
         @Override
-        public GeoPointFieldMapper build(BuilderContext context, String simpleName, FieldType fieldType,
-                                         MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
-                                         Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo) {
-            Parser<List<ParsedGeoPoint>> geoParser = new PointParser<>(name, ParsedGeoPoint::new, (parser, point) -> {
-                GeoUtils.parseGeoPoint(parser, point, ignoreZValue().value());
-                return point;
-            }, (ParsedGeoPoint) nullValue, ignoreZValue.value(), ignoreMalformed.value());
-            GeoPointFieldType ft
-                = new GeoPointFieldType(buildFullName(context), indexed, fieldType.stored(), hasDocValues, geoParser, meta);
-            return new GeoPointFieldMapper(name, fieldType, ft, multiFields,
-                ignoreMalformed, ignoreZValue, nullValue, copyTo, new GeoPointIndexer(ft), geoParser);
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(hasDocValues, indexed, stored, ignoreMalformed, ignoreZValue, nullValue, meta);
         }
-    }
 
-    public static class TypeParser extends AbstractPointGeometryFieldMapper.TypeParser<Builder> {
-        @Override
-        protected Builder newBuilder(String name, Map<String, Object> params) {
-            return new GeoPointFieldMapper.Builder(name);
+        public Builder docValues(boolean hasDocValues) {
+            this.hasDocValues.setValue(hasDocValues);
+            return this;
         }
 
-        protected ParsedGeoPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) {
+        private static ParsedGeoPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) {
+            if (nullValue == null) {
+                return null;
+            }
             ParsedGeoPoint point = new ParsedGeoPoint();
             GeoUtils.parseGeoPoint(nullValue, point, ignoreZValue);
             if (ignoreMalformed == false) {
@@ -106,15 +107,39 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
             }
             return point;
         }
+
+        @Override
+        public ParametrizedFieldMapper build(BuilderContext context) {
+            Parser<List<ParsedGeoPoint>> geoParser = new PointParser<>(name, ParsedGeoPoint::new, (parser, point) -> {
+                GeoUtils.parseGeoPoint(parser, point, ignoreZValue.get().value());
+                return point;
+            }, (ParsedGeoPoint) nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value());
+            GeoPointFieldType ft
+                = new GeoPointFieldType(buildFullName(context), indexed.get(), stored.get(), hasDocValues.get(), geoParser, meta.get());
+            return new GeoPointFieldMapper(name, ft, multiFieldsBuilder.build(this, context),
+                copyTo.build(), new GeoPointIndexer(ft), geoParser, this);
+        }
+
     }
 
-    public GeoPointFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                               MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
-                               Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo,
+    public static TypeParser PARSER = new TypeParser((n, c) -> new Builder(n, IGNORE_MALFORMED_SETTING.get(c.getSettings())));
+
+    private final Builder builder;
+
+    public GeoPointFieldMapper(String simpleName, MappedFieldType mappedFieldType,
+                               MultiFields multiFields, CopyTo copyTo,
                                Indexer<List<ParsedGeoPoint>, List<? extends GeoPoint>> indexer,
-                               Parser<List<ParsedGeoPoint>> parser) {
-        super(simpleName, fieldType, mappedFieldType, multiFields,
-            ignoreMalformed, ignoreZValue, nullValue, copyTo, indexer, parser);
+                               Parser<List<ParsedGeoPoint>> parser,
+                               Builder builder) {
+        super(simpleName, mappedFieldType, multiFields,
+            builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(),
+            copyTo, indexer, parser);
+        this.builder = builder;
+    }
+
+    @Override
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(simpleName(), builder.ignoreMalformed.getDefaultValue().value()).init(this);
     }
 
     @Override
@@ -158,18 +183,13 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<List<P
         return CONTENT_TYPE;
     }
 
-    @Override
-    public GeoPointFieldType fieldType() {
-        return (GeoPointFieldType)mappedFieldType;
-    }
-
-    public static class GeoPointFieldType extends AbstractPointGeometryFieldType implements GeoShapeQueryable {
+    public static class GeoPointFieldType extends AbstractGeometryFieldType implements GeoShapeQueryable {
 
         private final VectorGeoPointShapeQueryProcessor queryProcessor;
 
         private GeoPointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
                                   Parser<?> parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, parser, meta);
+            super(name, indexed, stored, hasDocValues, true, parser, meta);
             this.queryProcessor = new VectorGeoPointShapeQueryProcessor();
         }
 

+ 96 - 60
server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java

@@ -18,18 +18,18 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.LatLonShape;
-import org.apache.lucene.index.IndexOptions;
+import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.query.VectorGeoShapeQueryProcessor;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -54,40 +54,56 @@ import java.util.Map;
  * "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0))
  */
 public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
+
     public static final String CONTENT_TYPE = "geo_shape";
-    public static final FieldType FIELD_TYPE = new FieldType();
-    static {
-        FIELD_TYPE.setDimensions(7, 4, Integer.BYTES);
-        FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
-        FIELD_TYPE.setOmitNorms(true);
-        FIELD_TYPE.freeze();
+
+    private static Builder builder(FieldMapper in) {
+        return ((GeoShapeFieldMapper)in).builder;
     }
 
-    public static class Builder extends AbstractShapeGeometryFieldMapper.Builder {
+    public static class Builder extends ParametrizedFieldMapper.Builder {
+
+        final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<Explicit<Boolean>> coerce;
+        final Parameter<Explicit<Orientation>> orientation = orientationParam(m -> builder(m).orientation.get());
+
+        final Parameter<Map<String, String>> meta = Parameter.metaParam();
+
+        public Builder(String name, boolean ignoreMalformedByDefault, boolean coerceByDefault) {
+            super (name);
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
+        }
 
-        public Builder(String name) {
-            super (name, FIELD_TYPE);
-            this.hasDocValues = false;
+        public Builder ignoreZValue(boolean ignoreZValue) {
+            this.ignoreZValue.setValue(new Explicit<>(ignoreZValue, true));
+            return this;
         }
 
-        private GeoShapeFieldType buildFieldType(BuilderContext context, GeoShapeParser parser) {
-            GeoShapeFieldType ft = new GeoShapeFieldType(buildFullName(context), indexed, fieldType.stored(), hasDocValues, parser, meta);
-            ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation);
-            return ft;
+        @Override
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(indexed, ignoreMalformed, ignoreZValue, coerce, orientation, meta);
         }
 
         @Override
         public GeoShapeFieldMapper build(BuilderContext context) {
-            GeometryParser geometryParser = new GeometryParser(orientation().value().getAsBoolean(), coerce().value(),
-                ignoreZValue().value());
+            GeometryParser geometryParser = new GeometryParser(
+                orientation.get().value().getAsBoolean(),
+                coerce.get().value(),
+                ignoreZValue.get().value());
             GeoShapeParser geoShapeParser = new GeoShapeParser(geometryParser);
-            GeoShapeFieldType ft = buildFieldType(context, geoShapeParser);
-            return new GeoShapeFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context),
-                ignoreZValue(), orientation(),
-                multiFieldsBuilder.build(this, context), copyTo,
-                new GeoShapeIndexer(orientation().value().getAsBoolean(), buildFullName(context)),
-                new GeoShapeParser(new GeometryParser(orientation().value().getAsBoolean(),
-                    coerce(context).value(), ignoreZValue().value())));
+            GeoShapeFieldType ft = new GeoShapeFieldType(
+                buildFullName(context),
+                indexed.get(),
+                orientation.get().value(),
+                geoShapeParser,
+                meta.get());
+            return new GeoShapeFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(),
+                new GeoShapeIndexer(orientation.get().value().getAsBoolean(), buildFullName(context)),
+                geoShapeParser, this);
         }
     }
 
@@ -95,9 +111,9 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
 
         private final VectorGeoShapeQueryProcessor queryProcessor;
 
-        public GeoShapeFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
+        public GeoShapeFieldType(String name, boolean indexed, Orientation orientation,
                                  Parser<Geometry> parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, false, parser, meta);
+            super(name, indexed, false, false, false, parser, orientation, meta);
             this.queryProcessor = new VectorGeoShapeQueryProcessor();
         }
 
@@ -112,25 +128,60 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
         }
     }
 
-    public static final class TypeParser extends AbstractShapeGeometryFieldMapper.TypeParser {
-
-        @Override
-        protected AbstractShapeGeometryFieldMapper.Builder newBuilder(String name, Map<String, Object> params) {
-            if (params.containsKey(DEPRECATED_PARAMETERS_KEY)) {
-                return new LegacyGeoShapeFieldMapper.Builder(name,
-                    (LegacyGeoShapeFieldMapper.DeprecatedParameters)params.get(DEPRECATED_PARAMETERS_KEY));
-            }
-            return new GeoShapeFieldMapper.Builder(name);
+    @SuppressWarnings("deprecation")
+    public static Mapper.TypeParser PARSER = (name, node, parserContext) -> {
+        ParametrizedFieldMapper.Builder builder;
+        boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(parserContext.getSettings());
+        boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings());
+        if (LegacyGeoShapeFieldMapper.containsDeprecatedParameter(node.keySet())) {
+            builder = new LegacyGeoShapeFieldMapper.Builder(
+                name,
+                parserContext.indexVersionCreated(),
+                ignoreMalformedByDefault,
+                coerceByDefault);
+        } else {
+            builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault);
         }
-    }
+        builder.parse(name, parserContext, node);
+        return builder;
+    };
 
-    public GeoShapeFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                               Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
-                               Explicit<Boolean> ignoreZValue, Explicit<ShapeBuilder.Orientation> orientation,
+    private final Builder builder;
+
+    public GeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                MultiFields multiFields, CopyTo copyTo,
-                               Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, coerce, ignoreZValue, orientation,
-            multiFields, copyTo, indexer, parser);
+                               Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser, Builder builder) {
+        super(simpleName,
+            mappedFieldType,
+            builder.ignoreMalformed.get(),
+            builder.coerce.get(),
+            builder.ignoreZValue.get(),
+            builder.orientation.get(),
+            multiFields,
+            copyTo,
+            indexer,
+            parser);
+        this.builder = builder;
+    }
+
+    @Override
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(
+            simpleName(),
+            builder.ignoreMalformed.getDefaultValue().value(),
+            builder.coerce.getDefaultValue().value()
+        ).init(this);
+    }
+
+    @Override
+    @SuppressWarnings("deprecation")
+    protected void checkIncomingMergeType(FieldMapper mergeWith) {
+        if (mergeWith instanceof LegacyGeoShapeFieldMapper) {
+            String strategy = ((LegacyGeoShapeFieldMapper)mergeWith).strategy();
+            throw new IllegalArgumentException("mapper [" + name()
+                + "] of type [geo_shape] cannot change strategy from [BKD] to [" + strategy + "]");
+        }
+        super.checkIncomingMergeType(mergeWith);
     }
 
     @Override
@@ -140,8 +191,7 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
     }
 
     @Override
-    @SuppressWarnings("rawtypes")
-    protected void addDocValuesFields(String name, Geometry geometry, List fields, ParseContext context) {
+    protected void addDocValuesFields(String name, Geometry geometry, List<IndexableField> fields, ParseContext context) {
         // we will throw a mapping exception before we get here
     }
 
@@ -150,16 +200,6 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
         // noop (completion suggester currently not compatible with geo_shape)
     }
 
-    @Override
-    protected void mergeGeoOptions(AbstractShapeGeometryFieldMapper<?,?> mergeWith, List<String> conflicts) {
-        if (mergeWith instanceof LegacyGeoShapeFieldMapper) {
-            LegacyGeoShapeFieldMapper legacy = (LegacyGeoShapeFieldMapper) mergeWith;
-            throw new IllegalArgumentException("[" + fieldType().name() + "] with field mapper [" + fieldType().typeName() + "] " +
-                "using [BKD] strategy cannot be merged with " + "[" + legacy.fieldType().typeName() + "] with [" +
-                legacy.fieldType().strategy() + "] strategy");
-        }
-    }
-
     @Override
     public GeoShapeFieldType fieldType() {
         return (GeoShapeFieldType) super.fieldType();
@@ -170,8 +210,4 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
         return CONTENT_TYPE;
     }
 
-    @Override
-    protected boolean docValuesByDefault() {
-        return false;
-    }
 }

+ 179 - 226
server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java

@@ -18,7 +18,6 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
@@ -31,7 +30,6 @@ import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Explicit;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
@@ -40,11 +38,7 @@ import org.elasticsearch.common.geo.SpatialStrategy;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.common.geo.parsers.ShapeParser;
-import org.elasticsearch.common.logging.DeprecationLogger;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.DistanceUnit;
-import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.geometry.Geometry;
@@ -54,9 +48,12 @@ import org.locationtech.spatial4j.shape.Shape;
 
 import java.io.IOException;
 import java.text.ParseException;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 
 /**
  * FieldMapper for indexing {@link org.locationtech.spatial4j.shape.Shape}s.
@@ -85,156 +82,187 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
     public static final String CONTENT_TYPE = "geo_shape";
 
-    @Deprecated
-    public static class DeprecatedParameters {
-        public static class Names {
-            public static final ParseField STRATEGY = new ParseField("strategy");
-            public static final ParseField TREE = new ParseField("tree");
-            public static final ParseField TREE_LEVELS = new ParseField("tree_levels");
-            public static final ParseField PRECISION = new ParseField("precision");
-            public static final ParseField DISTANCE_ERROR_PCT = new ParseField("distance_error_pct");
-            public static final ParseField POINTS_ONLY = new ParseField("points_only");
-        }
-
-        public static class PrefixTrees {
-            public static final String LEGACY_QUADTREE = "legacyquadtree";
-            public static final String QUADTREE = "quadtree";
-            public static final String GEOHASH = "geohash";
-        }
-
-        public static class Defaults {
-            public static final SpatialStrategy STRATEGY = SpatialStrategy.RECURSIVE;
-            public static final String TREE = "quadtree";
-            public static final String PRECISION = "50m";
-            public static final int QUADTREE_LEVELS = GeoUtils.quadTreeLevelsForPrecision(PRECISION);
-            public static final int GEOHASH_TREE_LEVELS = GeoUtils.geoHashLevelsForPrecision(PRECISION);
-            public static final boolean POINTS_ONLY = false;
-            public static final double DISTANCE_ERROR_PCT = 0.025d;
-        }
-
-        public SpatialStrategy strategy = null;
-        public String tree = null;
-        public Integer treeLevels = null;
-        public String precision = null;
-        public Boolean pointsOnly = null;
-        public Double distanceErrorPct = null;
-
-        public void setSpatialStrategy(SpatialStrategy strategy) {
-            this.strategy = strategy;
-        }
+    public static final Set<String> DEPRECATED_PARAMETERS
+        = Set.of("strategy", "tree", "tree_levels", "precision", "distance_error_pct", "points only");
 
-        public void setTree(String prefixTree) {
-            this.tree = prefixTree;
-        }
-
-        public void setTreeLevels(int treeLevels) {
-            this.treeLevels = treeLevels;
-        }
-
-        public void setPrecision(String precision) {
-            this.precision = precision;
-        }
+    public static boolean containsDeprecatedParameter(Set<String> paramKeys) {
+        return DEPRECATED_PARAMETERS.stream().anyMatch(paramKeys::contains);
+    }
 
-        public void setPointsOnly(boolean pointsOnly) {
-            if (this.strategy == SpatialStrategy.TERM && pointsOnly == false) {
-                throw new ElasticsearchParseException("points_only cannot be set to false for term strategy");
+    public static class Defaults {
+        public static final SpatialStrategy STRATEGY = SpatialStrategy.RECURSIVE;
+        public static final String TREE = "quadtree";
+        public static final String PRECISION = "50m";
+        public static final int QUADTREE_LEVELS = GeoUtils.quadTreeLevelsForPrecision(PRECISION);
+        public static final int GEOHASH_TREE_LEVELS = GeoUtils.geoHashLevelsForPrecision(PRECISION);
+        public static final boolean POINTS_ONLY = false;
+        public static final double DISTANCE_ERROR_PCT = 0.025d;
+
+        public static int defaultTreeLevel(String tree) {
+            switch (tree) {
+                case PrefixTrees.GEOHASH:
+                    return GEOHASH_TREE_LEVELS;
+                case PrefixTrees.LEGACY_QUADTREE:
+                case PrefixTrees.QUADTREE:
+                    return QUADTREE_LEVELS;
+                default:
+                    throw new IllegalArgumentException("Unknown prefix type [" + tree + "]");
             }
-            this.pointsOnly = pointsOnly;
         }
+    }
 
-        public void setDistanceErrorPct(double distanceErrorPct) {
-            this.distanceErrorPct = distanceErrorPct;
-        }
+    public static class PrefixTrees {
+        public static final String LEGACY_QUADTREE = "legacyquadtree";
+        public static final String QUADTREE = "quadtree";
+        public static final String GEOHASH = "geohash";
+    }
 
-        public static boolean parse(String name, String fieldName, Object fieldNode, DeprecatedParameters deprecatedParameters) {
-            if (Names.STRATEGY.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setSpatialStrategy(SpatialStrategy.fromString(fieldNode.toString()));
-            } else if (Names.TREE.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setTree(fieldNode.toString());
-            } else if (Names.TREE_LEVELS.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setTreeLevels(Integer.parseInt(fieldNode.toString()));
-            } else if (Names.PRECISION.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setPrecision(fieldNode.toString());
-            } else if (Names.DISTANCE_ERROR_PCT.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setDistanceErrorPct(Double.parseDouble(fieldNode.toString()));
-            } else if (Names.POINTS_ONLY.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
-                checkPrefixTreeSupport(fieldName);
-                deprecatedParameters.setPointsOnly(
-                    XContentMapValues.nodeBooleanValue(fieldNode, name + "." + DeprecatedParameters.Names.POINTS_ONLY));
-            } else {
-                return false;
-            }
-            return true;
-        }
+    @Deprecated
+    public static class DeprecatedParameters {
 
         private static void checkPrefixTreeSupport(String fieldName) {
             if (ShapesAvailability.JTS_AVAILABLE == false || ShapesAvailability.SPATIAL4J_AVAILABLE == false) {
                 throw new ElasticsearchParseException("Field parameter [{}] is not supported for [{}] field type",
                     fieldName, CONTENT_TYPE);
             }
-            DEPRECATION_LOGGER.deprecate("geo_mapper_field_parameter",
-                "Field parameter [{}] is deprecated and will be removed in a future version.", fieldName);
+
         }
     }
 
-    private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(LegacyGeoShapeFieldMapper.class);
+    private static Builder builder(FieldMapper in) {
+        return ((LegacyGeoShapeFieldMapper)in).builder;
+    }
+
+    public static class Builder extends ParametrizedFieldMapper.Builder {
+
+        Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<Explicit<Boolean>> coerce;
+        Parameter<Explicit<Orientation>> orientation = orientationParam(m -> builder(m).orientation.get());
+
+        Parameter<SpatialStrategy> strategy = new Parameter<>("strategy", false, () -> SpatialStrategy.RECURSIVE,
+            (n, c, o) -> SpatialStrategy.fromString(o.toString()), m -> builder(m).strategy.get())
+            .deprecated();
+        Parameter<String> tree = Parameter.stringParam("tree", false, m -> builder(m).tree.get(), Defaults.TREE)
+            .deprecated();
+        Parameter<Integer> treeLevels = new Parameter<>("tree_levels", false, () -> null,
+            (n, c, o) -> o == null ? null : XContentMapValues.nodeIntegerValue(o),
+            m -> builder(m).treeLevels.get())
+            .deprecated();
+        Parameter<DistanceUnit.Distance> precision = new Parameter<>("precision", false, () -> null,
+            (n, c, o) -> o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()),
+            m -> builder(m).precision.get())
+            .deprecated();
+        Parameter<Double> distanceErrorPct = new Parameter<>("distance_error_pct", true, () -> null,
+            (n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o),
+            m -> builder(m).distanceErrorPct.get())
+            .deprecated()
+            .acceptsNull();
+        Parameter<Boolean> pointsOnly = new Parameter<>("points_only", false,
+            () -> null,
+            (n, c, o) -> XContentMapValues.nodeBooleanValue(o), m -> builder(m).pointsOnly.get())
+            .deprecated().acceptsNull();
+
+        Parameter<Map<String, String>> meta = Parameter.metaParam();
+
+        private final Version indexCreatedVersion;
+
+        public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault) {
+            super(name);
 
-    public static class Builder extends AbstractShapeGeometryFieldMapper.Builder {
+            if (ShapesAvailability.JTS_AVAILABLE == false || ShapesAvailability.SPATIAL4J_AVAILABLE == false) {
+                throw new ElasticsearchParseException("Non-BKD field parameters are not supported for [{}] field type", CONTENT_TYPE);
+            }
 
-        DeprecatedParameters deprecatedParameters;
+            this.indexCreatedVersion = version;
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
+
+            this.pointsOnly.setValidator(v -> {
+                if (v == null) {
+                    return;
+                }
+                if (v == false && SpatialStrategy.TERM == strategy.get()) {
+                    throw new IllegalArgumentException("points_only cannot be set to false for term strategy");
+                }
+            });
+
+            // Set up serialization
+            if (version.onOrAfter(Version.V_7_0_0)) {
+                this.strategy.alwaysSerialize();
+            }
+            this.strategy.setSerializer((b, f, v) -> b.field(f, v.getStrategyName()), SpatialStrategy::getStrategyName);
+            // serialize treeLevels if treeLevels is configured, OR if defaults are requested and precision is not configured
+            treeLevels.setSerializerCheck((id, ic, v) -> ic || (id && precision.get() == null));
+            treeLevels.setSerializer((b, f, v) -> {
+                if (v != null && v != 0) {
+                    b.field(f, v);
+                } else {
+                    b.field(f, Defaults.defaultTreeLevel(tree.get()));
+                }
+            }, Objects::toString);
+            // serialize precision if precision is configured, OR if defaults are requested and treeLevels is not configured
+            precision.setSerializerCheck((id, ic, v) -> ic || (id && treeLevels.get() == null));
+            precision.setSerializer((b, f, v) -> {
+                if (v == null) {
+                    b.field(f, "50.0m");
+                } else {
+                    b.field(f, v.toString());
+                }
+            }, Objects::toString);
+            pointsOnly.setSerializer((b, f, v) -> {
+                if (v == null) {
+                    b.field(f, strategy.get() == SpatialStrategy.TERM);
+                } else {
+                    b.field(f, v);
+                }
+            }, Objects::toString);
+        }
 
-        public Builder(String name) {
-            this(name, new DeprecatedParameters());
+        @Override
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(indexed, ignoreMalformed, ignoreZValue, coerce, orientation,
+                strategy, tree, treeLevels, precision, distanceErrorPct, pointsOnly, meta);
         }
 
-        public Builder(String name, DeprecatedParameters deprecatedParameters) {
-            super(name, Defaults.FIELD_TYPE);
-            this.deprecatedParameters = deprecatedParameters;
+        public Builder coerce(boolean coerce) {
+            this.coerce.setValue(new Explicit<>(coerce, true));
+            return this;
         }
 
         private void setupFieldTypeDeprecatedParameters(GeoShapeFieldType ft) {
-            if (deprecatedParameters.strategy != null) {
-                ft.setStrategy(deprecatedParameters.strategy);
-            }
-            if (deprecatedParameters.tree != null) {
-                ft.setTree(deprecatedParameters.tree);
+            ft.setStrategy(strategy.get());
+            ft.setTree(tree.get());
+            if (treeLevels.get() != null) {
+                ft.setTreeLevels(treeLevels.get());
             }
-            if (deprecatedParameters.treeLevels != null) {
-                ft.setTreeLevels(deprecatedParameters.treeLevels);
+            if (precision.get() != null) {
+                ft.setPrecisionInMeters(precision.get().value);
             }
-            if (deprecatedParameters.precision != null) {
-                // precision is only set iff: a. treeLevel is not explicitly set, b. its explicitly set
-                ft.setPrecisionInMeters(DistanceUnit.parse(deprecatedParameters.precision,
-                    DistanceUnit.DEFAULT, DistanceUnit.DEFAULT));
+            if (pointsOnly.get() != null) {
+                ft.setPointsOnly(pointsOnly.get());
             }
-            if (deprecatedParameters.distanceErrorPct != null) {
-                ft.setDistanceErrorPct(deprecatedParameters.distanceErrorPct);
+            if (distanceErrorPct.get() != null) {
+                ft.setDistanceErrorPct(distanceErrorPct.get());
             }
-            if (deprecatedParameters.pointsOnly != null) {
-                ft.setPointsOnly(deprecatedParameters.pointsOnly);
-            }
-
             if (ft.treeLevels() == 0 && ft.precisionInMeters() < 0) {
-                ft.setDefaultDistanceErrorPct(DeprecatedParameters.Defaults.DISTANCE_ERROR_PCT);
+                ft.setDefaultDistanceErrorPct(Defaults.DISTANCE_ERROR_PCT);
             }
         }
 
         private void setupPrefixTrees(GeoShapeFieldType ft) {
             SpatialPrefixTree prefixTree;
-            if (ft.tree().equals(DeprecatedParameters.PrefixTrees.GEOHASH)) {
+            if (ft.tree().equals(PrefixTrees.GEOHASH)) {
                 prefixTree = new GeohashPrefixTree(ShapeBuilder.SPATIAL_CONTEXT,
-                    getLevels(ft.treeLevels(), ft.precisionInMeters(), DeprecatedParameters.Defaults.GEOHASH_TREE_LEVELS, true));
-            } else if (ft.tree().equals(DeprecatedParameters.PrefixTrees.LEGACY_QUADTREE)) {
+                    getLevels(ft.treeLevels(), ft.precisionInMeters(), Defaults.GEOHASH_TREE_LEVELS, true));
+            } else if (ft.tree().equals(PrefixTrees.LEGACY_QUADTREE)) {
                 prefixTree = new QuadPrefixTree(ShapeBuilder.SPATIAL_CONTEXT,
-                    getLevels(ft.treeLevels(), ft.precisionInMeters(), DeprecatedParameters.Defaults.QUADTREE_LEVELS, false));
-            } else if (ft.tree().equals(DeprecatedParameters.PrefixTrees.QUADTREE)) {
+                    getLevels(ft.treeLevels(), ft.precisionInMeters(), Defaults.QUADTREE_LEVELS, false));
+            } else if (ft.tree().equals(PrefixTrees.QUADTREE)) {
                 prefixTree = new PackedQuadPrefixTree(ShapeBuilder.SPATIAL_CONTEXT,
-                    getLevels(ft.treeLevels(), ft.precisionInMeters(), DeprecatedParameters.Defaults.QUADTREE_LEVELS, false));
+                    getLevels(ft.treeLevels(), ft.precisionInMeters(), Defaults.QUADTREE_LEVELS, false));
             } else {
                 throw new IllegalArgumentException("Unknown prefix tree type [" + ft.tree() + "]");
             }
@@ -258,10 +286,9 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
         private GeoShapeFieldType buildFieldType(LegacyGeoShapeParser parser, BuilderContext context) {
             GeoShapeFieldType ft =
-                new GeoShapeFieldType(buildFullName(context), indexed, fieldType.stored(), false, parser, meta);
+                new GeoShapeFieldType(buildFullName(context), indexed.get(), orientation.get().value(), parser, meta.get());
             setupFieldTypeDeprecatedParameters(ft);
             setupPrefixTrees(ft);
-            ft.setOrientation(orientation == null ? Defaults.ORIENTATION.value() : orientation);
             return ft;
         }
 
@@ -281,10 +308,9 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
             }
             LegacyGeoShapeParser parser = new LegacyGeoShapeParser();
             GeoShapeFieldType ft = buildFieldType(parser, context);
-            return new LegacyGeoShapeFieldMapper(name, fieldType, ft, ignoreMalformed(context),
-                coerce(context), orientation(), ignoreZValue(), context.indexSettings(),
-                multiFieldsBuilder.build(this, context), copyTo,
-                new LegacyGeoShapeIndexer(ft), parser);
+            return new LegacyGeoShapeFieldMapper(name, ft,
+                multiFieldsBuilder.build(this, context), copyTo.build(),
+                new LegacyGeoShapeIndexer(ft), parser, this);
         }
     }
 
@@ -312,9 +338,9 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
     public static final class GeoShapeFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable {
 
-        private String tree = DeprecatedParameters.Defaults.TREE;
-        private SpatialStrategy strategy = DeprecatedParameters.Defaults.STRATEGY;
-        private boolean pointsOnly = DeprecatedParameters.Defaults.POINTS_ONLY;
+        private String tree = Defaults.TREE;
+        private SpatialStrategy strategy = Defaults.STRATEGY;
+        private boolean pointsOnly = Defaults.POINTS_ONLY;
         private int treeLevels = 0;
         private double precisionInMeters = -1;
         private Double distanceErrorPct;
@@ -327,14 +353,14 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
 
         private final LegacyGeoShapeQueryProcessor queryProcessor;
 
-        private GeoShapeFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
+        private GeoShapeFieldType(String name, boolean indexed, Orientation orientation,
                                   LegacyGeoShapeParser parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, false, parser, meta);
+            super(name, indexed, false, false, false, parser, orientation, meta);
             this.queryProcessor = new LegacyGeoShapeQueryProcessor(this);
         }
 
         public GeoShapeFieldType(String name) {
-            this(name, true, false, true, null, Collections.emptyMap());
+            this(name, true, Orientation.RIGHT, null, Collections.emptyMap());
         }
 
         @Override
@@ -427,15 +453,17 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
     }
 
     private final Version indexCreatedVersion;
+    private final Builder builder;
 
-    public LegacyGeoShapeFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                                     Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce, Explicit<Orientation> orientation,
-                                     Explicit<Boolean> ignoreZValue, Settings indexSettings,
+    public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                      MultiFields multiFields, CopyTo copyTo,
-                                     LegacyGeoShapeIndexer indexer, LegacyGeoShapeParser parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, coerce, ignoreZValue, orientation,
+                                     LegacyGeoShapeIndexer indexer, LegacyGeoShapeParser parser,
+                                     Builder builder) {
+        super(simpleName, mappedFieldType,
+            builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(),
             multiFields, copyTo, indexer, parser);
-        this.indexCreatedVersion = Version.indexCreated(indexSettings);
+        this.indexCreatedVersion = builder.indexCreatedVersion;
+        this.builder = builder;
     }
 
     @Override
@@ -443,6 +471,10 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
         return (GeoShapeFieldType) super.fieldType();
     }
 
+    String strategy() {
+        return fieldType().strategy().getStrategyName();
+    }
+
     @Override
     protected void addStoredFields(ParseContext context, Shape geometry) {
         // noop: we do not store geo_shapes; and will not store legacy geo_shape types
@@ -459,101 +491,22 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
     }
 
     @Override
-    protected boolean docValuesByDefault() {
-        return false;
+    protected String contentType() {
+        return CONTENT_TYPE;
     }
 
     @Override
-    public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
-        super.doXContentBody(builder, includeDefaults, params);
-
-        if (includeDefaults
-            || (fieldType().tree().equals(DeprecatedParameters.Defaults.TREE)) == false) {
-            builder.field(DeprecatedParameters.Names.TREE.getPreferredName(), fieldType().tree());
-        }
-
-        if (fieldType().treeLevels() != 0) {
-            builder.field(DeprecatedParameters.Names.TREE_LEVELS.getPreferredName(), fieldType().treeLevels());
-        } else if(includeDefaults && fieldType().precisionInMeters() == -1) { // defaults only make sense if precision is not specified
-            if (DeprecatedParameters.PrefixTrees.GEOHASH.equals(fieldType().tree())) {
-                builder.field(DeprecatedParameters.Names.TREE_LEVELS.getPreferredName(),
-                    DeprecatedParameters.Defaults.GEOHASH_TREE_LEVELS);
-            } else if (DeprecatedParameters.PrefixTrees.LEGACY_QUADTREE.equals(fieldType().tree())) {
-                builder.field(DeprecatedParameters.Names.TREE_LEVELS.getPreferredName(),
-                    DeprecatedParameters.Defaults.QUADTREE_LEVELS);
-            } else if (DeprecatedParameters.PrefixTrees.QUADTREE.equals(fieldType().tree())) {
-                builder.field(DeprecatedParameters.Names.TREE_LEVELS.getPreferredName(),
-                    DeprecatedParameters.Defaults.QUADTREE_LEVELS);
-            } else {
-                throw new IllegalArgumentException("Unknown prefix tree type [" + fieldType().tree() + "]");
-            }
-        }
-        if (fieldType().precisionInMeters() != -1) {
-            builder.field(DeprecatedParameters.Names.PRECISION.getPreferredName(),
-                DistanceUnit.METERS.toString(fieldType().precisionInMeters()));
-        } else if (includeDefaults && fieldType().treeLevels() == 0) { // defaults only make sense if tree levels are not specified
-            builder.field(DeprecatedParameters.Names.PRECISION.getPreferredName(),
-                DistanceUnit.METERS.toString(50));
-        }
-
-        if (indexCreatedVersion.onOrAfter(Version.V_7_0_0)) {
-            builder.field(DeprecatedParameters.Names.STRATEGY.getPreferredName(), fieldType().strategy().getStrategyName());
-        }
-
-        if (includeDefaults || fieldType().distanceErrorPct() != fieldType().defaultDistanceErrorPct) {
-            builder.field(DeprecatedParameters.Names.DISTANCE_ERROR_PCT.getPreferredName(), fieldType().distanceErrorPct());
-        }
-        if (fieldType().strategy() == SpatialStrategy.TERM) {
-            // For TERMs strategy the defaults for points only change to true
-            if (includeDefaults || fieldType().pointsOnly() != true) {
-                builder.field(DeprecatedParameters.Names.POINTS_ONLY.getPreferredName(), fieldType().pointsOnly());
-            }
-        } else {
-            if (includeDefaults || fieldType().pointsOnly() != DeprecatedParameters.Defaults.POINTS_ONLY) {
-                builder.field(DeprecatedParameters.Names.POINTS_ONLY.getPreferredName(), fieldType().pointsOnly());
-            }
-        }
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(simpleName(), indexCreatedVersion,
+            builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value()).init(this);
     }
 
     @Override
-    protected void mergeGeoOptions(AbstractShapeGeometryFieldMapper<?,?> mergeWith, List<String> conflicts) {
-
+    protected void checkIncomingMergeType(FieldMapper mergeWith) {
         if (mergeWith instanceof GeoShapeFieldMapper) {
-            GeoShapeFieldMapper fieldMapper = (GeoShapeFieldMapper) mergeWith;
-            throw new IllegalArgumentException("[" + fieldType().name() + "] with field mapper [" + fieldType().typeName() + "] " +
-                "using [" + fieldType().strategy() + "] strategy cannot be merged with " + "[" + fieldMapper.typeName() +
-                "] with [BKD] strategy");
-        }
-
-        GeoShapeFieldType g = (GeoShapeFieldType)mergeWith.fieldType();
-        // prevent user from changing strategies
-        if (fieldType().strategy() != g.strategy()) {
-            conflicts.add("mapper [" + name() + "] has different [strategy]");
-        }
-
-        // prevent user from changing trees (changes encoding)
-        if (fieldType().tree().equals(g.tree()) == false) {
-            conflicts.add("mapper [" + name() + "] has different [tree]");
-        }
-
-        if (fieldType().pointsOnly() != g.pointsOnly()) {
-            conflicts.add("mapper [" + name() + "] has different points_only");
-        }
-
-        // TODO we should allow this, but at the moment levels is used to build bookkeeping variables
-        // in lucene's SpatialPrefixTree implementations, need a patch to correct that first
-        if (fieldType().treeLevels() != g.treeLevels()) {
-            conflicts.add("mapper [" + name() + "] has different [tree_levels]");
-        }
-        if (fieldType().precisionInMeters() != g.precisionInMeters()) {
-            conflicts.add("mapper [" + name() + "] has different [precision]");
+            throw new IllegalArgumentException("mapper [" + name()
+                + "] of type [geo_shape] cannot change strategy from [" + strategy() + "] to [BKD]");
         }
-
-        this.orientation = mergeWith.orientation;
-    }
-
-    @Override
-    protected String contentType() {
-        return CONTENT_TYPE;
+        super.checkIncomingMergeType(mergeWith);
     }
 }

+ 29 - 9
server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java

@@ -82,15 +82,7 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
             throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type ["
                 + contentType() + "] to [" + mergeWith.getClass().getSimpleName() + "]");
         }
-        String mergeWithContentType = ((FieldMapper)mergeWith).contentType();
-        if (Objects.equals(this.getClass(), mergeWith.getClass()) == false) {
-            throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type ["
-                + contentType() + "] to [" + mergeWithContentType + "]");
-        }
-        if (Objects.equals(contentType(), mergeWithContentType) == false) {
-            throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type ["
-                + contentType() + "] to [" + mergeWithContentType + "]");
-        }
+        checkIncomingMergeType((FieldMapper)mergeWith);
 
         ParametrizedFieldMapper.Builder builder = getMergeBuilder();
         if (builder == null) {
@@ -102,6 +94,17 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
         return builder.build(new BuilderContext(Settings.EMPTY, parentPath(name())));
     }
 
+    protected void checkIncomingMergeType(FieldMapper mergeWith) {
+        if (Objects.equals(this.getClass(), mergeWith.getClass()) == false) {
+            throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type ["
+                + contentType() + "] to [" + mergeWith.contentType() + "]");
+        }
+        if (Objects.equals(contentType(), mergeWith.contentType()) == false) {
+            throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type ["
+                + contentType() + "] to [" + mergeWith.contentType() + "]");
+        }
+    }
+
     private static ContentPath parentPath(String name) {
         int endPos = name.lastIndexOf(".");
         if (endPos == -1) {
@@ -164,6 +167,7 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
         private Serializer<T> serializer = XContentBuilder::field;
         private SerializerCheck<T> serializerCheck = (includeDefaults, isConfigured, value) -> includeDefaults || isConfigured;
         private Function<T, String> conflictSerializer = Objects::toString;
+        private boolean deprecated;
         private MergeValidator<T> mergeValidator;
         private T value;
         private boolean isSet;
@@ -236,6 +240,17 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
             return this;
         }
 
+        /**
+         * Deprecates the entire parameter.
+         *
+         * If this parameter is encountered during parsing, a deprecation warning will
+         * be emitted.
+         */
+        public Parameter<T> deprecated() {
+            this.deprecated = true;
+            return this;
+        }
+
         /**
          * Adds validation to a parameter, called after parsing and merging
          */
@@ -628,6 +643,11 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
                     throw new MapperParsingException("unknown parameter [" + propName
                         + "] on mapper [" + name + "] of type [" + type + "]");
                 }
+                if (parameter.deprecated) {
+                    deprecationLogger.deprecate(propName,
+                        "Parameter [{}] is deprecated and will be removed in a future version",
+                        propName);
+                }
                 if (propNode == null && parameter.acceptsNull == false) {
                     throw new MapperParsingException("[" + propName + "] on mapper [" + name
                         + "] of type [" + type + "] must not have a [null] value");

+ 1 - 1
server/src/main/java/org/elasticsearch/indices/IndicesModule.java

@@ -121,7 +121,7 @@ public class IndicesModule extends AbstractModule {
         mappers.put(ObjectMapper.NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser());
         mappers.put(CompletionFieldMapper.CONTENT_TYPE, CompletionFieldMapper.PARSER);
         mappers.put(FieldAliasMapper.CONTENT_TYPE, new FieldAliasMapper.TypeParser());
-        mappers.put(GeoPointFieldMapper.CONTENT_TYPE, new GeoPointFieldMapper.TypeParser());
+        mappers.put(GeoPointFieldMapper.CONTENT_TYPE, GeoPointFieldMapper.PARSER);
 
         for (MapperPlugin mapperPlugin : mapperPlugins) {
             for (Map.Entry<String, Mapper.TypeParser> entry : mapperPlugin.getMappers().entrySet()) {

+ 1 - 1
server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java

@@ -305,7 +305,7 @@ public class GeoJsonShapeParserTests extends BaseGeoParsingTestCase {
         Polygon expected = GEOMETRY_FACTORY.createPolygon(shell, null);
         Mapper.BuilderContext mockBuilderContext = new Mapper.BuilderContext(indexSettings, new ContentPath());
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            (LegacyGeoShapeFieldMapper) (new LegacyGeoShapeFieldMapper.Builder("test").ignoreZValue(true).build(mockBuilderContext));
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(mockBuilderContext);
         try (XContentParser parser = createParser(polygonGeoJson)) {
             parser.nextToken();
             ElasticsearchGeoAssertions.assertEquals(jtsGeom(expected), ShapeParser.parse(parser, mapperBuilder).buildS4J());

+ 5 - 5
server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java

@@ -316,7 +316,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
 
         Mapper.BuilderContext mockBuilderContext = new Mapper.BuilderContext(indexSettings, new ContentPath());
         final GeoShapeFieldMapper mapperBuilder =
-            (GeoShapeFieldMapper) (new GeoShapeFieldMapper.Builder("test").ignoreZValue(false).build(mockBuilderContext));
+            new GeoShapeFieldMapper.Builder("test", false, true).ignoreZValue(false).build(mockBuilderContext);
 
         // test store z disabled
         ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class,
@@ -355,7 +355,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
 
         Mapper.BuilderContext mockBuilderContext = new Mapper.BuilderContext(indexSettings, new ContentPath());
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            (LegacyGeoShapeFieldMapper)(new LegacyGeoShapeFieldMapper.Builder("test").ignoreZValue(true).build(mockBuilderContext));
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(mockBuilderContext);
 
         // test store z disabled
         ElasticsearchException e = expectThrows(ElasticsearchException.class,
@@ -385,7 +385,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
 
         Mapper.BuilderContext mockBuilderContext = new Mapper.BuilderContext(indexSettings, new ContentPath());
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            (LegacyGeoShapeFieldMapper)(new LegacyGeoShapeFieldMapper.Builder("test").ignoreZValue(true).build(mockBuilderContext));
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(mockBuilderContext);
 
         ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, mapperBuilder);
         assertEquals(shapeBuilder.numDimensions(), 3);
@@ -406,13 +406,13 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
 
         Mapper.BuilderContext mockBuilderContext = new Mapper.BuilderContext(indexSettings, new ContentPath());
         final LegacyGeoShapeFieldMapper defaultMapperBuilder =
-            (LegacyGeoShapeFieldMapper)(new LegacyGeoShapeFieldMapper.Builder("test").coerce(false).build(mockBuilderContext));
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).coerce(false).build(mockBuilderContext);
         ElasticsearchParseException exception = expectThrows(ElasticsearchParseException.class,
             () -> ShapeParser.parse(parser, defaultMapperBuilder));
         assertEquals("invalid LinearRing found (coordinates are not closed)", exception.getMessage());
 
         final LegacyGeoShapeFieldMapper coercingMapperBuilder =
-            (LegacyGeoShapeFieldMapper)(new LegacyGeoShapeFieldMapper.Builder("test").coerce(true).build(mockBuilderContext));
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).coerce(true).build(mockBuilderContext);
         ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, coercingMapperBuilder);
         assertNotNull(shapeBuilder);
         assertEquals("polygon ((100.0 5.0, 100.0 10.0, 90.0 10.0, 90.0 5.0, 100.0 5.0))", shapeBuilder.toWKT());

+ 1 - 1
server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java

@@ -122,7 +122,7 @@ public abstract class AbstractFieldDataTestCase extends ESSingleNodeTestCase {
             fieldType = new NumberFieldMapper.Builder(fieldName, NumberFieldMapper.NumberType.BYTE, false, true)
                     .docValues(docValues).build(context).fieldType();
         } else if (type.equals("geo_point")) {
-            fieldType = new GeoPointFieldMapper.Builder(fieldName).docValues(docValues).build(context).fieldType();
+            fieldType = new GeoPointFieldMapper.Builder(fieldName, false).docValues(docValues).build(context).fieldType();
         } else if (type.equals("binary")) {
             fieldType = new BinaryFieldMapper.Builder(fieldName, docValues).build(context).fieldType();
         } else {

+ 2 - 2
server/src/test/java/org/elasticsearch/index/mapper/ExternalMapper.java

@@ -54,8 +54,8 @@ public class ExternalMapper extends ParametrizedFieldMapper {
 
         private final BinaryFieldMapper.Builder binBuilder = new BinaryFieldMapper.Builder(Names.FIELD_BIN);
         private final BooleanFieldMapper.Builder boolBuilder = new BooleanFieldMapper.Builder(Names.FIELD_BOOL);
-        private final GeoPointFieldMapper.Builder latLonPointBuilder = new GeoPointFieldMapper.Builder(Names.FIELD_POINT);
-        private final GeoShapeFieldMapper.Builder shapeBuilder = new GeoShapeFieldMapper.Builder(Names.FIELD_SHAPE);
+        private final GeoPointFieldMapper.Builder latLonPointBuilder = new GeoPointFieldMapper.Builder(Names.FIELD_POINT, false);
+        private final GeoShapeFieldMapper.Builder shapeBuilder = new GeoShapeFieldMapper.Builder(Names.FIELD_SHAPE, false, true);
         private final Mapper.Builder stringBuilder;
         private final String generatedValue;
         private final String mapperName;

+ 8 - 23
server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java

@@ -20,13 +20,11 @@ package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.geo.GeoPoint;
-import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.hamcrest.CoreMatchers;
 
 import java.io.IOException;
-import java.util.Set;
 
 import static org.elasticsearch.geometry.utils.Geohash.stringEncode;
 import static org.hamcrest.Matchers.arrayWithSize;
@@ -39,12 +37,7 @@ import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
-public class GeoPointFieldMapperTests extends FieldMapperTestCase2<GeoPointFieldMapper.Builder> {
-
-    @Override
-    protected Set<String> unsupportedProperties() {
-        return Set.of("analyzer", "similarity", "doc_values");
-    }
+public class GeoPointFieldMapperTests extends MapperTestCase {
 
     @Override
     protected void minimalMapping(XContentBuilder b) throws IOException {
@@ -55,18 +48,15 @@ public class GeoPointFieldMapperTests extends FieldMapperTestCase2<GeoPointField
     protected void registerParameters(ParameterChecker checker) throws IOException {
         checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> {
             GeoPointFieldMapper gpfm = (GeoPointFieldMapper) m;
-            assertTrue(gpfm.ignoreMalformed.value());
+            assertTrue(gpfm.ignoreMalformed());
         });
         checker.registerUpdateCheck(b -> b.field("ignore_z_value", false), m -> {
             GeoPointFieldMapper gpfm = (GeoPointFieldMapper) m;
-            assertFalse(gpfm.ignoreZValue.value());
-        });
-        GeoPoint point = GeoUtils.parseFromString("41.12,-71.34");
-        // TODO this should not be updateable!
-        checker.registerUpdateCheck(b -> b.field("null_value", "41.12,-71.34"), m -> {
-            GeoPointFieldMapper gpfm = (GeoPointFieldMapper) m;
-            assertEquals(gpfm.nullValue, point);
+            assertFalse(gpfm.ignoreZValue());
         });
+        checker.registerConflictCheck("null_value", b -> b.field("null_value", "41.12,-71.34"));
+        checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false));
+        checker.registerConflictCheck("store", b -> b.field("store", true));
     }
 
     @Override
@@ -202,14 +192,14 @@ public class GeoPointFieldMapperTests extends FieldMapperTestCase2<GeoPointField
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_point").field("ignore_z_value", true)));
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));
-        boolean ignoreZValue = ((GeoPointFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((GeoPointFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
         mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_point").field("ignore_z_value", false)));
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));
-        ignoreZValue = ((GeoPointFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((GeoPointFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
     }
 
@@ -326,11 +316,6 @@ public class GeoPointFieldMapperTests extends FieldMapperTestCase2<GeoPointField
         );
     }
 
-    @Override
-    protected GeoPointFieldMapper.Builder newBuilder() {
-        return new GeoPointFieldMapper.Builder("geo");
-    }
-
     protected void assertSearchable(MappedFieldType fieldType) {
         //always searchable even if it uses TextSearchInfo.NONE
         assertTrue(fieldType.isSearchable());

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java

@@ -33,7 +33,7 @@ public class GeoPointFieldTypeTests extends FieldTypeTestCase {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
 
-        MappedFieldType mapper = new GeoPointFieldMapper.Builder("field").build(context).fieldType();
+        MappedFieldType mapper = new GeoPointFieldMapper.Builder("field", false).build(context).fieldType();
 
         Map<String, Object> jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.1));
         Map<String, Object> otherJsonPoint = Map.of("type", "Point", "coordinates", List.of(30.0, 50.0));

+ 32 - 37
server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java

@@ -18,37 +18,24 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 
-public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeFieldMapper.Builder> {
-
-    @Override
-    protected Set<String> unsupportedProperties() {
-        return Set.of("analyzer", "similarity", "doc_values", "store");
-    }
-
-    @Override
-    protected GeoShapeFieldMapper.Builder newBuilder() {
-        return new GeoShapeFieldMapper.Builder("geoshape");
-    }
+public class GeoShapeFieldMapperTests extends MapperTestCase {
 
     @Override
     protected void registerParameters(ParameterChecker checker) throws IOException {
@@ -58,11 +45,11 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         });
         checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> {
             GeoShapeFieldMapper gpfm = (GeoShapeFieldMapper) m;
-            assertTrue(gpfm.ignoreMalformed.value());
+            assertTrue(gpfm.ignoreMalformed());
         });
         checker.registerUpdateCheck(b -> b.field("ignore_z_value", false), m -> {
             GeoShapeFieldMapper gpfm = (GeoShapeFieldMapper) m;
-            assertFalse(gpfm.ignoreZValue.value());
+            assertFalse(gpfm.ignoreZValue());
         });
         checker.registerUpdateCheck(b -> b.field("coerce", true), m -> {
             GeoShapeFieldMapper gpfm = (GeoShapeFieldMapper) m;
@@ -70,14 +57,6 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         });
     }
 
-    @Before
-    public void addModifiers() {
-        addModifier("orientation", true, (a, b) -> {
-            a.orientation(ShapeBuilder.Orientation.LEFT);
-            b.orientation(ShapeBuilder.Orientation.RIGHT);
-        });
-    }
-
     @Override
     protected Collection<? extends Plugin> getPlugins() {
         return List.of(new TestGeoShapeFieldMapperPlugin());
@@ -99,7 +78,7 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
         GeoShapeFieldMapper geoShapeFieldMapper = (GeoShapeFieldMapper) fieldMapper;
         assertThat(geoShapeFieldMapper.fieldType().orientation(),
-            equalTo(GeoShapeFieldMapper.Defaults.ORIENTATION.value()));
+            equalTo(ShapeBuilder.Orientation.RIGHT));
         assertThat(geoShapeFieldMapper.fieldType().hasDocValues(), equalTo(false));
     }
 
@@ -134,16 +113,15 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_shape").field("coerce", true)));
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
-        boolean coerce = ((GeoShapeFieldMapper)fieldMapper).coerce().value();
+        boolean coerce = ((GeoShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(true));
 
         // explicit false coerce test
         mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_shape").field("coerce", false)));
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
-        coerce = ((GeoShapeFieldMapper)fieldMapper).coerce().value();
+        coerce = ((GeoShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(false));
-        assertFieldWarnings("tree");
     }
 
     /**
@@ -154,7 +132,7 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
 
-        boolean ignoreZValue = ((GeoShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((GeoShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
@@ -162,7 +140,7 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
 
-        ignoreZValue = ((GeoShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((GeoShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
     }
 
@@ -173,24 +151,24 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_shape").field("ignore_malformed", true)));
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
-        Explicit<Boolean> ignoreMalformed = ((GeoShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.value(), equalTo(true));
+        boolean ignoreMalformed = ((GeoShapeFieldMapper)fieldMapper).ignoreMalformed();
+        assertThat(ignoreMalformed, equalTo(true));
 
         // explicit false ignore_malformed test
         mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_shape").field("ignore_malformed", false)));
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class));
         ignoreMalformed = ((GeoShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.explicit(), equalTo(true));
-        assertThat(ignoreMalformed.value(), equalTo(false));
+        assertThat(ignoreMalformed, equalTo(false));
     }
 
     private void assertFieldWarnings(String... fieldNames) {
         String[] warnings = new String[fieldNames.length];
         for (int i = 0; i < fieldNames.length; ++i) {
-            warnings[i] = "Field parameter [" + fieldNames[i] + "] "
-                + "is deprecated and will be removed in a future version.";
+            warnings[i] = "Parameter [" + fieldNames[i] + "] "
+                + "is deprecated and will be removed in a future version";
         }
+        assertWarnings(warnings);
     }
 
     public void testGeoShapeMapperMerge() throws Exception {
@@ -208,6 +186,23 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
         assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(ShapeBuilder.Orientation.CW));
     }
 
+    public void testGeoShapeLegacyMerge() throws Exception {
+        MapperService m = createMapperService(fieldMapping(b -> b.field("type", "geo_shape")));
+        Exception e = expectThrows(IllegalArgumentException.class,
+            () -> merge(m, fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive"))));
+
+        assertThat(e.getMessage(),
+            containsString("mapper [field] of type [geo_shape] cannot change strategy from [BKD] to [recursive]"));
+        assertFieldWarnings("strategy");
+
+        MapperService lm = createMapperService(fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive")));
+        e = expectThrows(IllegalArgumentException.class,
+            () -> merge(lm, fieldMapping(b -> b.field("type", "geo_shape"))));
+        assertThat(e.getMessage(),
+            containsString("mapper [field] of type [geo_shape] cannot change strategy from [recursive] to [BKD]"));
+        assertFieldWarnings("strategy");
+    }
+
     public void testSerializeDefaults() throws Exception {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
         assertThat(
@@ -215,7 +210,7 @@ public class GeoShapeFieldMapperTests extends FieldMapperTestCase2<GeoShapeField
                 mapper.mappers().getMapper("field"),
                 new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"))
             ),
-            containsString("\"orientation\":\"" + AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value() + "\"")
+            containsString("\"orientation\":\"" + ShapeBuilder.Orientation.RIGHT + "\"")
         );
     }
 

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldTypeTests.java

@@ -33,7 +33,7 @@ public class GeoShapeFieldTypeTests extends FieldTypeTestCase {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
 
-        MappedFieldType mapper = new GeoShapeFieldMapper.Builder("field").build(context).fieldType();
+        MappedFieldType mapper = new GeoShapeFieldMapper.Builder("field", false, true).build(context).fieldType();
 
         Map<String, Object> jsonLineString = Map.of("type", "LineString", "coordinates",
             List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)));

+ 31 - 47
server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java

@@ -24,7 +24,6 @@ import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy;
 import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.geo.ShapeRelation;
@@ -40,7 +39,6 @@ import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
-import java.util.Set;
 
 import static java.util.Collections.singletonMap;
 import static org.hamcrest.Matchers.containsString;
@@ -52,23 +50,13 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 @SuppressWarnings("deprecation")
-public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyGeoShapeFieldMapper.Builder> {
+public class LegacyGeoShapeFieldMapperTests extends MapperTestCase {
 
     @Override
     protected Object getSampleValueForDocument() {
         return "POINT (14.0 15.0)";
     }
 
-    @Override
-    protected LegacyGeoShapeFieldMapper.Builder newBuilder() {
-        return new LegacyGeoShapeFieldMapper.Builder("geoshape");
-    }
-
-    @Override
-    protected Set<String> unsupportedProperties() {
-        return Set.of("analyzer", "similarity", "doc_values", "store");
-    }
-
     @Override
     protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "geo_shape").field("strategy", "recursive");
@@ -87,17 +75,18 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         checker.registerConflictCheck("tree", b -> b.field("tree", "geohash"));
         checker.registerConflictCheck("tree_levels", b -> b.field("tree_levels", 5));
         checker.registerConflictCheck("precision", b -> b.field("precision", 10));
+        checker.registerConflictCheck("points_only", b -> b.field("points_only", true));
         checker.registerUpdateCheck(b -> b.field("orientation", "right"), m -> {
             LegacyGeoShapeFieldMapper gsfm = (LegacyGeoShapeFieldMapper) m;
             assertEquals(ShapeBuilder.Orientation.RIGHT, gsfm.orientation());
         });
         checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> {
             LegacyGeoShapeFieldMapper gpfm = (LegacyGeoShapeFieldMapper) m;
-            assertTrue(gpfm.ignoreMalformed.value());
+            assertTrue(gpfm.ignoreMalformed());
         });
         checker.registerUpdateCheck(b -> b.field("ignore_z_value", false), m -> {
             LegacyGeoShapeFieldMapper gpfm = (LegacyGeoShapeFieldMapper) m;
-            assertFalse(gpfm.ignoreZValue.value());
+            assertFalse(gpfm.ignoreZValue());
         });
         checker.registerUpdateCheck(b -> b.field("coerce", true), m -> {
             LegacyGeoShapeFieldMapper gpfm = (LegacyGeoShapeFieldMapper) m;
@@ -126,14 +115,14 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
 
         LegacyGeoShapeFieldMapper geoShapeFieldMapper = (LegacyGeoShapeFieldMapper) fieldMapper;
         assertThat(geoShapeFieldMapper.fieldType().tree(),
-            equalTo(LegacyGeoShapeFieldMapper.DeprecatedParameters.Defaults.TREE));
+            equalTo(LegacyGeoShapeFieldMapper.Defaults.TREE));
         assertThat(geoShapeFieldMapper.fieldType().treeLevels(), equalTo(0));
         assertThat(geoShapeFieldMapper.fieldType().pointsOnly(),
-            equalTo(LegacyGeoShapeFieldMapper.DeprecatedParameters.Defaults.POINTS_ONLY));
+            equalTo(LegacyGeoShapeFieldMapper.Defaults.POINTS_ONLY));
         assertThat(geoShapeFieldMapper.fieldType().distanceErrorPct(),
-            equalTo(LegacyGeoShapeFieldMapper.DeprecatedParameters.Defaults.DISTANCE_ERROR_PCT));
+            equalTo(LegacyGeoShapeFieldMapper.Defaults.DISTANCE_ERROR_PCT));
         assertThat(geoShapeFieldMapper.fieldType().orientation(),
-            equalTo(LegacyGeoShapeFieldMapper.Defaults.ORIENTATION.value()));
+            equalTo(ShapeBuilder.Orientation.RIGHT));
         assertFieldWarnings("strategy");
     }
 
@@ -173,7 +162,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         );
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
-        boolean coerce = ((LegacyGeoShapeFieldMapper)fieldMapper).coerce().value();
+        boolean coerce = ((LegacyGeoShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(true));
 
         // explicit false coerce test
@@ -182,7 +171,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         );
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
-        coerce = ((LegacyGeoShapeFieldMapper)fieldMapper).coerce().value();
+        coerce = ((LegacyGeoShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(false));
         assertFieldWarnings("tree", "strategy");
     }
@@ -196,7 +185,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         );
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
-        boolean ignoreZValue = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
@@ -205,7 +194,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         );
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
-        ignoreZValue = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
         assertFieldWarnings("strategy", "tree");
     }
@@ -219,8 +208,8 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         );
         Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
-        Explicit<Boolean> ignoreMalformed = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.value(), equalTo(true));
+        boolean ignoreMalformed = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreMalformed();
+        assertThat(ignoreMalformed, equalTo(true));
 
         // explicit false ignore_malformed test
         mapper = createDocumentMapper(
@@ -229,8 +218,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
         fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(LegacyGeoShapeFieldMapper.class));
         ignoreMalformed = ((LegacyGeoShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.explicit(), equalTo(true));
-        assertThat(ignoreMalformed.value(), equalTo(false));
+        assertThat(ignoreMalformed, equalTo(false));
         assertFieldWarnings("tree", "strategy");
     }
 
@@ -278,8 +266,7 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
     private void assertFieldWarnings(String... fieldNames) {
         String[] warnings = new String[fieldNames.length];
         for (int i = 0; i < fieldNames.length; ++i) {
-            warnings[i] = "Field parameter [" + fieldNames[i] + "] "
-                + "is deprecated and will be removed in a future version.";
+            warnings[i] = "Parameter [" + fieldNames[i] + "] is deprecated and will be removed in a future version";
         }
         assertWarnings(warnings);
     }
@@ -464,10 +451,10 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
             .field("strategy", "term").field("precision", "1km")
             .field("tree_levels", 26).field("distance_error_pct", 26)
             .field("orientation", "cw"))));
-        assertThat(e.getMessage(), containsString("mapper [field] has different [strategy]"));
-        assertThat(e.getMessage(), containsString("mapper [field] has different [tree]"));
-        assertThat(e.getMessage(), containsString("mapper [field] has different [tree_levels]"));
-        assertThat(e.getMessage(), containsString("mapper [field] has different [precision]"));
+        assertThat(e.getMessage(), containsString("Cannot update parameter [strategy] from [recursive] to [term]"));
+        assertThat(e.getMessage(), containsString("Cannot update parameter [tree] from [geohash] to [quadtree]"));
+        assertThat(e.getMessage(), containsString("Cannot update parameter [tree_levels] from [8] to [26]"));
+        assertThat(e.getMessage(), containsString("Cannot update parameter [precision] from [1.0m] to [1.0km]"));
 
         // verify nothing changed
         fieldMapper = mapperService.documentMapper().mappers().getMapper("field");
@@ -597,23 +584,20 @@ public class LegacyGeoShapeFieldMapperTests extends FieldMapperTestCase2<LegacyG
     }
 
     @Override
-    protected void assertParseMinimalWarnings() {
-        assertWarnings("Field parameter [strategy] is deprecated and will be removed in a future version.");
-    }
-
-    @Override
-    protected void assertParseMaximalWarnings() {
-        assertWarnings("Field parameter [strategy] is deprecated and will be removed in a future version.",
-            "Field parameter [tree] is deprecated and will be removed in a future version.",
-            "Field parameter [tree_levels] is deprecated and will be removed in a future version.",
-            "Field parameter [precision] is deprecated and will be removed in a future version.",
-            "Field parameter [distance_error_pct] is deprecated and will be removed in a future version."
-        );
+    protected String[] getParseMinimalWarnings() {
+        return new String[]{"Parameter [strategy] is deprecated and will be removed in a future version"};
     }
 
     @Override
-    protected void assertSerializationWarnings() {
-        assertParseMinimalWarnings();
+    protected String[] getParseMaximalWarnings() {
+        return new String[]{
+            "Parameter [strategy] is deprecated and will be removed in a future version",
+            "Parameter [tree] is deprecated and will be removed in a future version",
+            "Parameter [tree_levels] is deprecated and will be removed in a future version",
+            "Parameter [precision] is deprecated and will be removed in a future version",
+            "Parameter [distance_error_pct] is deprecated and will be removed in a future version",
+            "Parameter [points_only] is deprecated and will be removed in a future version"
+        };
     }
 
     public void testGeoShapeArrayParsing() throws Exception {

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java

@@ -47,7 +47,7 @@ public class LegacyGeoShapeFieldTypeTests extends FieldTypeTestCase {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
 
-        MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field").build(context).fieldType();
+        MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", Version.CURRENT, false, true).build(context).fieldType();
 
         Map<String, Object> jsonLineString = Map.of("type", "LineString", "coordinates",
             List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)));

+ 4 - 6
server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java

@@ -26,7 +26,6 @@ import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.SpatialStrategy;
 import org.elasticsearch.common.geo.builders.CircleBuilder;
 import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
 import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
@@ -70,8 +69,8 @@ import static org.hamcrest.Matchers.greaterThan;
 
 public class GeoShapeQueryTests extends GeoQueryTests {
     protected static final String[] PREFIX_TREES = new String[] {
-        LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.GEOHASH,
-        LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE
+        LegacyGeoShapeFieldMapper.PrefixTrees.GEOHASH,
+        LegacyGeoShapeFieldMapper.PrefixTrees.QUADTREE
     };
 
     @Override
@@ -102,8 +101,7 @@ public class GeoShapeQueryTests extends GeoQueryTests {
             .startObject("properties").startObject("geo")
             .field("type", "geo_shape");
         if (randomBoolean()) {
-            xcb = xcb.field("tree", randomFrom(PREFIX_TREES))
-                .field("strategy", randomFrom(SpatialStrategy.RECURSIVE, SpatialStrategy.TERM));
+            xcb = xcb.field("tree", randomFrom(PREFIX_TREES));
         }
         xcb = xcb.endObject().endObject().endObject();
 
@@ -477,7 +475,7 @@ public class GeoShapeQueryTests extends GeoQueryTests {
         // don't use random mapping as permits quadtree
         String mapping = Strings.toString(
             usePrefixTrees ?
-                createPrefixTreeMapping(LegacyGeoShapeFieldMapper.DeprecatedParameters.PrefixTrees.QUADTREE) :
+                createPrefixTreeMapping(LegacyGeoShapeFieldMapper.PrefixTrees.QUADTREE) :
                 createDefaultMapping());
         client().admin().indices().prepareCreate("test").setMapping(mapping).get();
         ensureGreen();

+ 21 - 3
test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

@@ -205,12 +205,28 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
         assertParseMaximalWarnings();
     }
 
-    protected void assertParseMinimalWarnings() {
+    protected final void assertParseMinimalWarnings() {
+        String[] warnings = getParseMinimalWarnings();
+        if (warnings.length > 0) {
+            assertWarnings(warnings);
+        }
+    }
+
+    protected final void assertParseMaximalWarnings() {
+        String[] warnings = getParseMaximalWarnings();
+        if (warnings.length > 0) {
+            assertWarnings(warnings);
+        }
+    }
+
+    protected String[] getParseMinimalWarnings() {
         // Most mappers don't emit any warnings
+        return Strings.EMPTY_ARRAY;
     }
 
-    protected void assertParseMaximalWarnings() {
+    protected String[] getParseMaximalWarnings() {
         // Most mappers don't emit any warnings
+        return Strings.EMPTY_ARRAY;
     }
 
     /**
@@ -262,7 +278,9 @@ public abstract class MapperTestCase extends MapperServiceTestCase {
                 minimalMapping(b);
                 b.field("boost", 2.0);
             }));
-            assertWarnings("Parameter [boost] on field [field] is deprecated and has no effect");
+            String[] warnings = Strings.concatStringArrays(getParseMinimalWarnings(),
+                new String[]{"Parameter [boost] on field [field] is deprecated and has no effect"});
+            assertWarnings(warnings);
         } catch (MapperParsingException e) {
             assertThat(e.getMessage(), anyOf(
                 containsString("Unknown parameter [boost]"),

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/test/TestGeoShapeFieldMapperPlugin.java

@@ -39,7 +39,7 @@ public class TestGeoShapeFieldMapperPlugin extends Plugin implements MapperPlugi
     @Override
     public Map<String, Mapper.TypeParser> getMappers() {
         Map<String, Mapper.TypeParser> mappers = new LinkedHashMap<>();
-        mappers.put(GeoShapeFieldMapper.CONTENT_TYPE, new GeoShapeFieldMapper.TypeParser());
+        mappers.put(GeoShapeFieldMapper.CONTENT_TYPE, GeoShapeFieldMapper.PARSER);
         return Collections.unmodifiableMap(mappers);
     }
 }

+ 125 - 226
x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java

@@ -26,98 +26,87 @@ package org.elasticsearch.xpack.spatial.index.mapper;
 
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.Version;
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
 import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.FieldMapperTestCase;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.mapper.SourceToParse;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.test.InternalSettingsPlugin;
 import org.elasticsearch.test.VersionUtils;
-import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin;
 import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Set;
 
-import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 
-public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<GeoShapeWithDocValuesFieldMapper.Builder> {
+public class GeoShapeWithDocValuesFieldMapperTests extends MapperTestCase {
 
     @Override
-    protected GeoShapeWithDocValuesFieldMapper.Builder newBuilder() {
-        return new GeoShapeWithDocValuesFieldMapper.Builder("geoshape");
+    protected void minimalMapping(XContentBuilder b) throws IOException {
+        b.field("type", "geo_shape");
     }
 
     @Override
-    protected Set<String> unsupportedProperties() {
-        return Set.of("analyzer", "similarity", "store");
-    }
-
-    @Before
-    public void addModifiers() {
-        addModifier("orientation", true, (a, b) -> {
-            a.orientation(ShapeBuilder.Orientation.RIGHT);
-            b.orientation(ShapeBuilder.Orientation.LEFT);
-        });
+    protected Object getSampleValueForDocument() {
+        return "POINT (14.0 15.0)";
     }
 
     @Override
-    protected boolean forbidPrivateIndexSettings() {
-        return false;
+    protected void registerParameters(ParameterChecker checker) throws IOException {
+        checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false));
+        checker.registerConflictCheck("index", b -> b.field("index", false));
+        checker.registerUpdateCheck(b -> b.field("orientation", "right"), m -> {
+            GeoShapeWithDocValuesFieldMapper gsfm = (GeoShapeWithDocValuesFieldMapper) m;
+            assertEquals(ShapeBuilder.Orientation.RIGHT, gsfm.orientation());
+        });
+        checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> {
+            GeoShapeWithDocValuesFieldMapper gpfm = (GeoShapeWithDocValuesFieldMapper) m;
+            assertTrue(gpfm.ignoreMalformed());
+        });
+        checker.registerUpdateCheck(b -> b.field("ignore_z_value", false), m -> {
+            GeoShapeWithDocValuesFieldMapper gpfm = (GeoShapeWithDocValuesFieldMapper) m;
+            assertFalse(gpfm.ignoreZValue());
+        });
+        checker.registerUpdateCheck(b -> b.field("coerce", true), m -> {
+            GeoShapeWithDocValuesFieldMapper gpfm = (GeoShapeWithDocValuesFieldMapper) m;
+            assertTrue(gpfm.coerce());
+        });
     }
 
     @Override
-    protected Collection<Class<? extends Plugin>> getPlugins() {
-        return pluginList(InternalSettingsPlugin.class, LocalStateCompositeXPackPlugin.class, LocalStateSpatialPlugin.class);
+    protected Collection<Plugin> getPlugins() {
+        return Collections.singletonList(new LocalStateSpatialPlugin());
     }
 
     public void testDefaultConfiguration() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
+        Mapper fieldMapper = mapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         GeoShapeWithDocValuesFieldMapper geoShapeFieldMapper = (GeoShapeWithDocValuesFieldMapper) fieldMapper;
-        assertThat(geoShapeFieldMapper.fieldType().orientation(),
-            equalTo(org.elasticsearch.index.mapper.GeoShapeFieldMapper.Defaults.ORIENTATION.value()));
+        assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(ShapeBuilder.Orientation.RIGHT));
         assertTrue(geoShapeFieldMapper.fieldType().hasDocValues());
     }
 
     public void testDefaultDocValueConfigurationOnPre7_8() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .endObject().endObject()
-            .endObject().endObject());
 
         Version oldVersion = VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_7_0);
-        DocumentMapper defaultMapper = createIndex("test", settings(oldVersion).build()).mapperService()
-            .parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+        DocumentMapper defaultMapper = createDocumentMapper(oldVersion, fieldMapping(this::minimalMapping));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         GeoShapeWithDocValuesFieldMapper geoShapeFieldMapper = (GeoShapeWithDocValuesFieldMapper) fieldMapper;
@@ -128,15 +117,12 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
      * Test that orientation parameter correctly parses
      */
     public void testOrientationParsing() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("orientation", "left")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("orientation", "left");
+        }));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         ShapeBuilder.Orientation orientation = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).fieldType().orientation();
@@ -145,15 +131,11 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
         assertThat(orientation, equalTo(ShapeBuilder.Orientation.CW));
 
         // explicit right orientation test
-        mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("orientation", "right")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        defaultMapper = createIndex("test2").mapperService().parse("type1", new CompressedXContent(mapping));
-        fieldMapper = defaultMapper.mappers().getMapper("location");
+        defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("orientation", "right");
+        }));
+        fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         orientation = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).fieldType().orientation();
@@ -166,35 +148,27 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
      * Test that coerce parameter correctly parses
      */
     public void testCoerceParsing() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("coerce", "true")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("coerce", true);
+        }));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        boolean coerce = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).coerce().value();
+        boolean coerce = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(true));
 
-        // explicit false coerce test
-        mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("coerce", "false")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        defaultMapper = createIndex("test2").mapperService().parse("type1", new CompressedXContent(mapping));
-        fieldMapper = defaultMapper.mappers().getMapper("location");
+        defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("coerce", false);
+        }));
+        fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        coerce = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).coerce().value();
+        coerce = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(false));
-        assertFieldWarnings("tree");
+
     }
 
 
@@ -202,33 +176,25 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
      * Test that accept_z_value parameter correctly parses
      */
     public void testIgnoreZValue() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "true")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_z_value", true);
+        }));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        boolean ignoreZValue = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
-        mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "false")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        defaultMapper = createIndex("test2").mapperService().parse("type1", new CompressedXContent(mapping));
-        fieldMapper = defaultMapper.mappers().getMapper("location");
+        defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_z_value", false);
+        }));
+        fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        ignoreZValue = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
     }
 
@@ -236,166 +202,97 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
      * Test that ignore_malformed parameter correctly parses
      */
     public void testIgnoreMalformedParsing() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("ignore_malformed", "true")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_malformed", true);
+        }));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        Explicit<Boolean> ignoreMalformed = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.value(), equalTo(true));
+        boolean ignoreMalformed = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreMalformed();
+        assertThat(ignoreMalformed, equalTo(true));
 
         // explicit false ignore_malformed test
-        mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("ignore_malformed", "false")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        defaultMapper = createIndex("test2").mapperService().parse("type1", new CompressedXContent(mapping));
-        fieldMapper = defaultMapper.mappers().getMapper("location");
+        defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_malformed", false);
+        }));
+        fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         ignoreMalformed = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.explicit(), equalTo(true));
-        assertThat(ignoreMalformed.value(), equalTo(false));
+        assertThat(ignoreMalformed, equalTo(false));
     }
 
     /**
      * Test that doc_values parameter correctly parses
      */
     public void testDocValues() throws IOException {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("doc_values", true)
-            .endObject().endObject()
-            .endObject().endObject());
-
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
+
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("doc_values", true);
+        }));
+        Mapper fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         boolean hasDocValues = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).fieldType().hasDocValues();
         assertTrue(hasDocValues);
 
         // explicit false doc_values
-        mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("doc_values", "false")
-            .endObject().endObject()
-            .endObject().endObject());
-
-        defaultMapper = createIndex("test2").mapperService().parse("type1", new CompressedXContent(mapping));
-        fieldMapper = defaultMapper.mappers().getMapper("location");
+        defaultMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("doc_values", false);
+        }));
+        fieldMapper = defaultMapper.mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
         hasDocValues = ((GeoShapeWithDocValuesFieldMapper)fieldMapper).fieldType().hasDocValues();
         assertFalse(hasDocValues);
     }
 
-    private void assertFieldWarnings(String... fieldNames) {
-        String[] warnings = new String[fieldNames.length];
-        for (int i = 0; i < fieldNames.length; ++i) {
-            warnings[i] = "Field parameter [" + fieldNames[i] + "] "
-                + "is deprecated and will be removed in a future version.";
-        }
-    }
-
     public void testGeoShapeMapperMerge() throws Exception {
-        String stage1Mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
-            .startObject("shape").field("type", "geo_shape")
-            .field("orientation", "ccw")
-            .endObject().endObject().endObject().endObject());
-        MapperService mapperService = createIndex("test").mapperService();
-        DocumentMapper docMapper = mapperService.merge("type", new CompressedXContent(stage1Mapping),
-            MapperService.MergeReason.MAPPING_UPDATE);
-        String stage2Mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
-            .startObject("properties").startObject("shape").field("type", "geo_shape")
-            .field("orientation", "cw").endObject().endObject().endObject().endObject());
-        mapperService.merge("type", new CompressedXContent(stage2Mapping), MapperService.MergeReason.MAPPING_UPDATE);
-
-        // verify nothing changed
-        Mapper fieldMapper = docMapper.mappers().getMapper("shape");
-        assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
-
-        GeoShapeWithDocValuesFieldMapper geoShapeFieldMapper = (GeoShapeWithDocValuesFieldMapper) fieldMapper;
-        assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(ShapeBuilder.Orientation.CCW));
+        MapperService mapperService = createMapperService(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("orientation", "ccw");
+        }));
 
-        // change mapping; orientation
-        stage2Mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
-            .startObject("properties").startObject("shape").field("type", "geo_shape")
-            .field("orientation", "cw").endObject().endObject().endObject().endObject());
-        docMapper = mapperService.merge("type", new CompressedXContent(stage2Mapping), MapperService.MergeReason.MAPPING_UPDATE);
+        merge(mapperService, fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("orientation", "cw");
+        }));
 
-        fieldMapper = docMapper.mappers().getMapper("shape");
+        Mapper fieldMapper = mapperService.documentMapper().mappers().getMapper("field");
         assertThat(fieldMapper, instanceOf(GeoShapeWithDocValuesFieldMapper.class));
 
-        geoShapeFieldMapper = (GeoShapeWithDocValuesFieldMapper) fieldMapper;
+        GeoShapeWithDocValuesFieldMapper geoShapeFieldMapper = (GeoShapeWithDocValuesFieldMapper) fieldMapper;
         assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(ShapeBuilder.Orientation.CW));
     }
 
-    public void testEmptyName() throws Exception {
-        // after 5.x
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("")
-            .field("type", "geo_shape")
-            .endObject().endObject()
-            .endObject().endObject());
-        MapperService mapperService = createIndex("test").mapperService();
-
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-            () -> mapperService.parse("type1", new CompressedXContent(mapping))
-        );
-        assertThat(e.getMessage(), containsString("name cannot be empty string"));
-    }
-
     public void testSerializeDefaults() throws Exception {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .endObject().endObject()
-            .endObject().endObject());
-        DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) defaultMapper.mappers().getMapper("location"));
+        DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping));
+        String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) defaultMapper.mappers().getMapper("field"));
         assertTrue(serialized, serialized.contains("\"orientation\":\"" +
-            AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value() + "\""));
+            ShapeBuilder.Orientation.RIGHT + "\""));
         assertTrue(serialized, serialized.contains("\"doc_values\":true"));
     }
 
     public void testSerializeDocValues() throws IOException {
         boolean docValues = randomBoolean();
-        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
-            .startObject("properties").startObject("location")
-            .field("type", "geo_shape")
-            .field("doc_values", docValues)
-            .endObject().endObject()
-            .endObject().endObject());
-        DocumentMapper mapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
-        String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) mapper.mappers().getMapper("location"));
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("doc_values", docValues);
+        }));
+        String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) mapper.mappers().getMapper("field"));
         assertTrue(serialized, serialized.contains("\"orientation\":\"" +
-            AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value() + "\""));
+            ShapeBuilder.Orientation.RIGHT + "\""));
         assertTrue(serialized, serialized.contains("\"doc_values\":" + docValues));
     }
 
     public void testGeoShapeArrayParsing() throws Exception {
-        String mapping = Strings.toString(XContentFactory.jsonBuilder()
-            .startObject()
-            .startObject("properties")
-            .startObject("location")
-            .field("type", "geo_shape")
-            .endObject()
-            .endObject()
-            .endObject());
 
-        DocumentMapper mapper = createIndex("test").mapperService().parse("_doc", new CompressedXContent(mapping));
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
 
         BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
             .startObject()
@@ -419,19 +316,21 @@ public class GeoShapeWithDocValuesFieldMapperTests extends FieldMapperTestCase<G
         assertThat(fields.length, equalTo(2));
     }
 
-    public String toXContentString(GeoShapeWithDocValuesFieldMapper mapper, boolean includeDefaults) throws IOException {
-        XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
-        ToXContent.Params params;
+    public String toXContentString(GeoShapeWithDocValuesFieldMapper mapper, boolean includeDefaults) {
         if (includeDefaults) {
-            params = new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
+            ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
+            return Strings.toString(mapper, params);
         } else {
-            params = ToXContent.EMPTY_PARAMS;
+            return Strings.toString(mapper);
         }
-        mapper.doXContentBody(builder, includeDefaults, params);
-        return Strings.toString(builder.endObject());
     }
 
     public String toXContentString(GeoShapeWithDocValuesFieldMapper mapper) throws IOException {
         return toXContentString(mapper, true);
     }
+
+    @Override
+    protected void assertSearchable(MappedFieldType fieldType) {
+
+    }
 }

+ 7 - 9
x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapperTests.java

@@ -21,8 +21,6 @@ import org.hamcrest.CoreMatchers;
 
 import java.io.IOException;
 
-import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
-import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.NULL_VALUE;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
@@ -37,10 +35,10 @@ public class PointFieldMapperTests extends CartesianFieldMapperTests {
         XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("properties").startObject(fieldName).field("type", "point");
         if (ignored_malformed || randomBoolean()) {
-            xContentBuilder.field(PointFieldMapper.Names.IGNORE_MALFORMED.getPreferredName(), ignored_malformed);
+            xContentBuilder.field("ignore_malformed", ignored_malformed);
         }
         if (ignoreZValue == false || randomBoolean()) {
-            xContentBuilder.field(PointFieldMapper.Names.IGNORE_Z_VALUE.getPreferredName(), ignoreZValue);
+            xContentBuilder.field("ignore_z_value", ignoreZValue);
         }
         return xContentBuilder.endObject().endObject().endObject().endObject();
     }
@@ -210,7 +208,7 @@ public class PointFieldMapperTests extends CartesianFieldMapperTests {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("properties").startObject("location")
             .field("type", "point")
-            .field(NULL_VALUE.getPreferredName(), "1,2")
+            .field("null_value", "1,2")
             .endObject().endObject()
             .endObject().endObject());
 
@@ -257,7 +255,7 @@ public class PointFieldMapperTests extends CartesianFieldMapperTests {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
             .startObject("properties").startObject("location")
             .field("type", "point")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "true")
+            .field("ignore_z_value", "true")
             .endObject().endObject()
             .endObject().endObject());
 
@@ -265,14 +263,14 @@ public class PointFieldMapperTests extends CartesianFieldMapperTests {
         Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(PointFieldMapper.class));
 
-        boolean ignoreZValue = ((PointFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((PointFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
         mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
             .startObject("properties").startObject("location")
             .field("type", "point")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "false")
+            .field("ignore_z_value", "false")
             .endObject().endObject()
             .endObject().endObject());
 
@@ -280,7 +278,7 @@ public class PointFieldMapperTests extends CartesianFieldMapperTests {
         fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(PointFieldMapper.class));
 
-        ignoreZValue = ((PointFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((PointFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
     }
 

+ 1 - 1
x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldTypeTests.java

@@ -24,7 +24,7 @@ public class PointFieldTypeTests extends FieldTypeTestCase {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
 
-        MappedFieldType mapper = new PointFieldMapper.Builder("field").build(context).fieldType();
+        MappedFieldType mapper = new PointFieldMapper.Builder("field", false).build(context).fieldType();
 
         Map<String, Object> jsonPoint = Map.of("type", "Point", "coordinates", List.of(42.0, 27.1));
         String wktPoint = "POINT (42.0 27.1)";

+ 17 - 24
x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapperTests.java

@@ -6,7 +6,6 @@
 package org.elasticsearch.xpack.spatial.index.mapper;
 
 import org.apache.lucene.index.IndexableField;
-import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
@@ -15,7 +14,6 @@ import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.MapperService;
@@ -25,7 +23,6 @@ import org.elasticsearch.index.mapper.SourceToParse;
 import java.io.IOException;
 import java.util.Collections;
 
-import static org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper.Names.IGNORE_Z_VALUE;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
@@ -43,7 +40,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
             xContentBuilder.field("ignore_malformed", ignored_malformed);
         }
         if (ignoreZValue == false || randomBoolean()) {
-            xContentBuilder.field(PointFieldMapper.Names.IGNORE_Z_VALUE.getPreferredName(), ignoreZValue);
+            xContentBuilder.field("ignore_z_value", ignoreZValue);
         }
         return xContentBuilder.endObject().endObject().endObject().endObject();
     }
@@ -61,7 +58,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
 
         ShapeFieldMapper shapeFieldMapper = (ShapeFieldMapper) fieldMapper;
         assertThat(shapeFieldMapper.fieldType().orientation(),
-            equalTo(ShapeFieldMapper.Defaults.ORIENTATION.value()));
+            equalTo(ShapeBuilder.Orientation.RIGHT));
     }
 
     /**
@@ -117,7 +114,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
-        boolean coerce = ((ShapeFieldMapper)fieldMapper).coerce().value();
+        boolean coerce = ((ShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(true));
 
         // explicit false coerce test
@@ -132,7 +129,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
-        coerce = ((ShapeFieldMapper)fieldMapper).coerce().value();
+        coerce = ((ShapeFieldMapper)fieldMapper).coerce();
         assertThat(coerce, equalTo(false));
     }
 
@@ -144,7 +141,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
             .startObject("properties").startObject("location")
             .field("type", "shape")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "true")
+            .field("ignore_z_value", "true")
             .endObject().endObject()
             .endObject().endObject());
 
@@ -152,14 +149,14 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
-        boolean ignoreZValue = ((ShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        boolean ignoreZValue = ((ShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(true));
 
         // explicit false accept_z_value test
         mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
             .startObject("properties").startObject("location")
             .field("type", "shape")
-            .field(IGNORE_Z_VALUE.getPreferredName(), "false")
+            .field("ignore_z_value", "false")
             .endObject().endObject()
             .endObject().endObject());
 
@@ -167,7 +164,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
-        ignoreZValue = ((ShapeFieldMapper)fieldMapper).ignoreZValue().value();
+        ignoreZValue = ((ShapeFieldMapper)fieldMapper).ignoreZValue();
         assertThat(ignoreZValue, equalTo(false));
     }
 
@@ -186,8 +183,8 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         Mapper fieldMapper = defaultMapper.mappers().getMapper("location");
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
-        Explicit<Boolean> ignoreMalformed = ((ShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.value(), equalTo(true));
+        boolean ignoreMalformed = ((ShapeFieldMapper)fieldMapper).ignoreMalformed();
+        assertThat(ignoreMalformed, equalTo(true));
 
         // explicit false ignore_malformed test
         mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
@@ -202,8 +199,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         assertThat(fieldMapper, instanceOf(ShapeFieldMapper.class));
 
         ignoreMalformed = ((ShapeFieldMapper)fieldMapper).ignoreMalformed();
-        assertThat(ignoreMalformed.explicit(), equalTo(true));
-        assertThat(ignoreMalformed.value(), equalTo(false));
+        assertThat(ignoreMalformed, equalTo(false));
     }
 
     public void testShapeMapperMerge() throws Exception {
@@ -248,7 +244,7 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         DocumentMapper defaultMapper = createIndex("test").mapperService().parse("type1", new CompressedXContent(mapping));
         String serialized = toXContentString((ShapeFieldMapper) defaultMapper.mappers().getMapper("location"));
         assertTrue(serialized, serialized.contains("\"orientation\":\"" +
-            AbstractShapeGeometryFieldMapper.Defaults.ORIENTATION.value() + "\""));
+            ShapeBuilder.Orientation.RIGHT + "\""));
     }
 
     public void testShapeArrayParsing() throws Exception {
@@ -285,19 +281,16 @@ public class ShapeFieldMapperTests extends CartesianFieldMapperTests {
         assertThat(fields.length, equalTo(2));
     }
 
-    public String toXContentString(ShapeFieldMapper mapper, boolean includeDefaults) throws IOException {
-        XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
-        ToXContent.Params params;
+    public String toXContentString(ShapeFieldMapper mapper, boolean includeDefaults) {
         if (includeDefaults) {
-            params = new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
+            ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
+            return Strings.toString(mapper, params);
         } else {
-            params = ToXContent.EMPTY_PARAMS;
+            return Strings.toString(mapper);
         }
-        mapper.doXContentBody(builder, includeDefaults, params);
-        return Strings.toString(builder.endObject());
     }
 
-    public String toXContentString(ShapeFieldMapper mapper) throws IOException {
+    public String toXContentString(ShapeFieldMapper mapper) {
         return toXContentString(mapper, true);
     }
 }

+ 1 - 1
x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldTypeTests.java

@@ -24,7 +24,7 @@ public class ShapeFieldTypeTests extends FieldTypeTestCase {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
 
-        MappedFieldType mapper = new ShapeFieldMapper.Builder("field").build(context).fieldType();
+        MappedFieldType mapper = new ShapeFieldMapper.Builder("field", false, true).build(context).fieldType();
 
         Map<String, Object> jsonLineString = Map.of("type", "LineString", "coordinates",
             List.of(List.of(42.0, 27.1), List.of(30.0, 50.0)));

+ 3 - 3
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java

@@ -77,9 +77,9 @@ public class SpatialPlugin extends GeoPlugin implements ActionPlugin, MapperPlug
     @Override
     public Map<String, Mapper.TypeParser> getMappers() {
         Map<String, Mapper.TypeParser> mappers = new HashMap<>(super.getMappers());
-        mappers.put(ShapeFieldMapper.CONTENT_TYPE, new ShapeFieldMapper.TypeParser());
-        mappers.put(PointFieldMapper.CONTENT_TYPE, new PointFieldMapper.TypeParser());
-        mappers.put(GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE, new GeoShapeWithDocValuesFieldMapper.TypeParser());
+        mappers.put(ShapeFieldMapper.CONTENT_TYPE, ShapeFieldMapper.PARSER);
+        mappers.put(PointFieldMapper.CONTENT_TYPE, PointFieldMapper.PARSER);
+        mappers.put(GeoShapeWithDocValuesFieldMapper.CONTENT_TYPE, GeoShapeWithDocValuesFieldMapper.PARSER);
         return Collections.unmodifiableMap(mappers);
     }
 

+ 2 - 4
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/common/CartesianPoint.java

@@ -26,8 +26,6 @@ import java.util.Collections;
 import java.util.Locale;
 import java.util.Objects;
 
-import static org.elasticsearch.index.mapper.AbstractGeometryFieldMapper.Names.IGNORE_Z_VALUE;
-
 /**
  * Represents a point in the cartesian space.
  */
@@ -279,8 +277,8 @@ public class CartesianPoint implements ToXContentFragment {
 
     public static double assertZValue(final boolean ignoreZValue, double zValue) {
         if (ignoreZValue == false) {
-            throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [{}] "
-                + "parameter is [{}]", zValue, IGNORE_Z_VALUE, ignoreZValue);
+            throw new ElasticsearchParseException("Exception parsing coordinates: found Z value [{}] but [ignore_z_value] "
+                + "parameter is [{}]", zValue, ignoreZValue);
         }
         if (Double.isFinite(zValue) == false) {
             throw new ElasticsearchParseException("invalid [{}] value [{}]; " +

+ 106 - 79
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java

@@ -6,36 +6,36 @@
 
 package org.elasticsearch.xpack.spatial.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.LatLonShape;
 import org.apache.lucene.document.ShapeField;
-import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.search.Query;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryParser;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.elasticsearch.common.xcontent.support.XContentMapValues;
+import org.elasticsearch.common.geo.ShapeRelation;
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
 import org.elasticsearch.index.mapper.FieldMapper;
-import org.elasticsearch.index.mapper.GeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.index.mapper.GeoShapeParser;
+import org.elasticsearch.index.mapper.GeoShapeQueryable;
 import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.Mapper;
+import org.elasticsearch.index.mapper.ParametrizedFieldMapper;
 import org.elasticsearch.index.mapper.ParseContext;
-import org.elasticsearch.index.mapper.TypeParsers;
+import org.elasticsearch.index.query.QueryShardContext;
+import org.elasticsearch.index.query.VectorGeoShapeQueryProcessor;
 import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.xpack.spatial.index.fielddata.AbstractLatLonShapeIndexFieldData;
 import org.elasticsearch.xpack.spatial.index.fielddata.CentroidCalculator;
 import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType;
 
-import java.util.HashMap;
-import java.util.Iterator;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Supplier;
@@ -62,44 +62,58 @@ import java.util.function.Supplier;
  * <p>
  * "field" : "POLYGON ((100.0 0.0, 101.0 0.0, 101.0 1.0, 100.0 1.0, 100.0 0.0))
  */
-public class GeoShapeWithDocValuesFieldMapper extends GeoShapeFieldMapper {
+public class GeoShapeWithDocValuesFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
     public static final String CONTENT_TYPE = "geo_shape";
-    public static final FieldType FIELD_TYPE = new FieldType();
-    static {
-        FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
+
+    private static Builder builder(FieldMapper in) {
+        return ((GeoShapeWithDocValuesFieldMapper)in).builder;
     }
 
-    public static class Builder extends AbstractShapeGeometryFieldMapper.Builder {
+    public static class Builder extends ParametrizedFieldMapper.Builder {
+
+        final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+        final Parameter<Boolean> hasDocValues;
+
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<Explicit<Boolean>> coerce;
+        final Parameter<Explicit<Orientation>> orientation = orientationParam(m -> builder(m).orientation.get());
 
-        private boolean docValuesSet = false;
+        final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
-        public Builder(String name) {
-            super (name, FIELD_TYPE);
-            this.hasDocValues = true;
+        private final Version version;
+
+        public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault) {
+            super(name);
+            this.version = version;
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
+            this.hasDocValues
+                = Parameter.docValuesParam(m -> builder(m).hasDocValues.get(), Version.V_7_8_0.onOrBefore(version));
         }
 
         @Override
-        public FieldMapper.Builder docValues(boolean docValues) {
-            docValuesSet = true;
-            return super.docValues(docValues);
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(indexed, hasDocValues, ignoreMalformed, ignoreZValue, coerce, orientation, meta);
         }
 
         @Override
         public GeoShapeWithDocValuesFieldMapper build(BuilderContext context) {
-            if (docValuesSet == false) {
-                hasDocValues = Version.V_7_8_0.onOrBefore(context.indexCreatedVersion());
-            }
-            GeometryParser geometryParser = new GeometryParser(orientation().value().getAsBoolean(), coerce().value(),
-                ignoreZValue().value());
+            GeometryParser geometryParser = new GeometryParser(
+                orientation.get().value().getAsBoolean(),
+                coerce.get().value(),
+                ignoreZValue.get().value());
             GeoShapeParser parser = new GeoShapeParser(geometryParser);
-            GeoShapeWithDocValuesFieldType ft
-                = new GeoShapeWithDocValuesFieldType(buildFullName(context), indexed, fieldType.stored(), hasDocValues, parser, meta);
-            // @todo check coerce
-            ft.setOrientation(orientation().value());
-            return new GeoShapeWithDocValuesFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context),
-                ignoreZValue(), orientation(), Version.V_7_8_0.onOrBefore(context.indexCreatedVersion()),
-                multiFieldsBuilder.build(this, context), copyTo,
-                new GeoShapeIndexer(orientation().value().getAsBoolean(), ft.name()), parser);
+            GeoShapeWithDocValuesFieldType ft = new GeoShapeWithDocValuesFieldType(
+                buildFullName(context),
+                indexed.get(),
+                hasDocValues.get(),
+                orientation.get().value(),
+                parser,
+                meta.get());
+            return new GeoShapeWithDocValuesFieldMapper(name, ft,
+                multiFieldsBuilder.build(this, context), copyTo.build(),
+                new GeoShapeIndexer(orientation.get().value().getAsBoolean(), ft.name()), parser, this);
         }
 
     }
@@ -128,68 +142,77 @@ public class GeoShapeWithDocValuesFieldMapper extends GeoShapeFieldMapper {
         }
     }
 
-    public static final class GeoShapeWithDocValuesFieldType extends GeoShapeFieldMapper.GeoShapeFieldType {
-        public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
-                                              GeoShapeParser parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, parser, meta);
+    public static final class GeoShapeWithDocValuesFieldType extends AbstractShapeGeometryFieldType implements GeoShapeQueryable {
+
+        private final VectorGeoShapeQueryProcessor queryProcessor = new VectorGeoShapeQueryProcessor();
+
+        public GeoShapeWithDocValuesFieldType(String name, boolean indexed, boolean hasDocValues,
+                                              Orientation orientation, GeoShapeParser parser, Map<String, String> meta) {
+            super(name, indexed, false, hasDocValues, false, parser, orientation, meta);
         }
 
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
             failIfNoDocValues();
             return new AbstractLatLonShapeIndexFieldData.Builder(name(), GeoShapeValuesSourceType.instance());
         }
-    }
-
-    public static final class TypeParser extends AbstractShapeGeometryFieldMapper.TypeParser {
 
         @Override
-        @SuppressWarnings("rawtypes")
-        protected AbstractShapeGeometryFieldMapper.Builder newBuilder(String name, Map<String, Object> params) {
-            if (params.containsKey(DEPRECATED_PARAMETERS_KEY)) {
-                return new LegacyGeoShapeFieldMapper.Builder(name,
-                    (LegacyGeoShapeFieldMapper.DeprecatedParameters)params.get(DEPRECATED_PARAMETERS_KEY));
-            }
-            return new GeoShapeWithDocValuesFieldMapper.Builder(name);
+        public String typeName() {
+            return CONTENT_TYPE;
         }
 
         @Override
-        @SuppressWarnings("rawtypes")
-        public AbstractShapeGeometryFieldMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
-                throws MapperParsingException {
-            AbstractShapeGeometryFieldMapper.Builder builder = super.parse(name, node, parserContext);
-            Map<String, Object> params = new HashMap<>();
-            for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
-                Map.Entry<String, Object> entry = iterator.next();
-                String fieldName = entry.getKey();
-                Object fieldNode = entry.getValue();
-                if (TypeParsers.DOC_VALUES.equals(fieldName)) {
-                    params.put(TypeParsers.DOC_VALUES, XContentMapValues.nodeBooleanValue(fieldNode, name + "." + TypeParsers.DOC_VALUES));
-                    iterator.remove();
-                }
-            }
-
-            if (params.containsKey(TypeParsers.DOC_VALUES)) {
-                builder.docValues((Boolean) params.get(TypeParsers.DOC_VALUES));
-            }
-            return builder;
+        public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relation, QueryShardContext context) {
+            return queryProcessor.geoShapeQuery(shape, fieldName, relation, context);
         }
     }
 
-    private final boolean defaultDocValues;
+    @SuppressWarnings("deprecation")
+    public static Mapper.TypeParser PARSER = (name, node, parserContext) -> {
+        ParametrizedFieldMapper.Builder builder;
+        boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(parserContext.getSettings());
+        boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings());
+        if (LegacyGeoShapeFieldMapper.containsDeprecatedParameter(node.keySet())) {
+            builder = new LegacyGeoShapeFieldMapper.Builder(
+                name,
+                parserContext.indexVersionCreated(),
+                ignoreMalformedByDefault,
+                coerceByDefault);
+        } else {
+            builder = new GeoShapeWithDocValuesFieldMapper.Builder(
+                name,
+                parserContext.indexVersionCreated(),
+                ignoreMalformedByDefault,
+                coerceByDefault);
+        }
+        builder.parse(name, parserContext, node);
+        return builder;
+    };
+
+    private final Builder builder;
 
-    public GeoShapeWithDocValuesFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                                            Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
-                                            Explicit<Boolean> ignoreZValue, Explicit<ShapeBuilder.Orientation> orientation,
-                                            boolean defaultDocValues, MultiFields multiFields, CopyTo copyTo,
-                                            GeoShapeIndexer indexer, GeoShapeParser parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, coerce, ignoreZValue, orientation,
+    public GeoShapeWithDocValuesFieldMapper(String simpleName, MappedFieldType mappedFieldType,
+                                            MultiFields multiFields, CopyTo copyTo,
+                                            GeoShapeIndexer indexer, GeoShapeParser parser, Builder builder) {
+        super(simpleName, mappedFieldType, builder.ignoreMalformed.get(), builder.coerce.get(),
+            builder.ignoreZValue.get(), builder.orientation.get(),
             multiFields, copyTo, indexer, parser);
-        this.defaultDocValues = defaultDocValues;
+        this.builder = builder;
+    }
+
+    @Override
+    protected String contentType() {
+        return CONTENT_TYPE;
     }
 
     @Override
-    protected boolean docValuesByDefault() {
-        return defaultDocValues;
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(
+            simpleName(),
+            builder.version,
+            builder.ignoreMalformed.getDefaultValue().value(),
+            builder.coerce.getDefaultValue().value()
+        ).init(this);
     }
 
     @Override
@@ -198,8 +221,12 @@ public class GeoShapeWithDocValuesFieldMapper extends GeoShapeFieldMapper {
     }
 
     @Override
-    protected String contentType() {
-        return CONTENT_TYPE;
+    protected void addStoredFields(ParseContext context, Geometry geometry) {
+        // noop (stored fields not available for geo_shape fields)
     }
 
+    @Override
+    protected void addMultiFields(ParseContext context, Geometry geometry) {
+        // noop (completion suggester currently not compatible with geo_shape)
+    }
 }

+ 55 - 33
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/PointFieldMapper.java

@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.spatial.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.StoredField;
 import org.apache.lucene.document.XYDocValuesField;
 import org.apache.lucene.document.XYPointField;
@@ -16,7 +15,9 @@ import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.index.mapper.AbstractPointGeometryFieldMapper;
+import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.ParametrizedFieldMapper;
 import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.xpack.spatial.common.CartesianPoint;
@@ -24,6 +25,7 @@ import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper.ParsedCarte
 import org.elasticsearch.xpack.spatial.index.query.ShapeQueryPointProcessor;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -46,32 +48,36 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
         }
     }
 
-    public static class Builder extends AbstractPointGeometryFieldMapper.Builder {
+    private static Builder builder(FieldMapper in) {
+        return ((PointFieldMapper)in).builder;
+    }
 
-        public Builder(String name) {
-            super(name, new FieldType());
-        }
+    public static class Builder extends ParametrizedFieldMapper.Builder {
 
-        @Override
-        public PointFieldMapper build(BuilderContext context, String simpleName, FieldType fieldType,
-                                      MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
-                                      Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo) {
-            CartesianPointParser parser = new CartesianPointParser(name, nullValue, ignoreZValue.value(), ignoreMalformed.value());
-            PointFieldType ft = new PointFieldType(buildFullName(context), indexed, fieldType.stored(), hasDocValues, parser, meta);
-            return new PointFieldMapper(simpleName, fieldType, ft, multiFields,
-                ignoreMalformed, ignoreZValue(context), nullValue, copyTo, new PointIndexer(ft), parser);
-        }
+        final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+        final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> builder(m).hasDocValues.get(), true);
+        final Parameter<Boolean> stored = Parameter.storeParam(m -> builder(m).stored.get(), false);
 
-    }
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<ParsedPoint> nullValue;
+        final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
-    public static class TypeParser extends AbstractPointGeometryFieldMapper.TypeParser<Builder> {
-        @Override
-        protected Builder newBuilder(String name, Map<String, Object> params) {
-            return new PointFieldMapper.Builder(name);
+        public Builder(String name, boolean ignoreMalformedByDefault) {
+            super(name);
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.nullValue = nullValueParam(
+                m -> builder(m).nullValue.get(),
+                (n, c, o) -> o == null ? null : parseNullValue(o, ignoreZValue.get().value(), ignoreMalformed.get().value()),
+                () -> null).acceptsNull();
         }
 
         @Override
-        protected ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) {
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(indexed, hasDocValues, stored, ignoreMalformed, ignoreZValue, nullValue, meta);
+        }
+
+        private static ParsedPoint parseNullValue(Object nullValue, boolean ignoreZValue, boolean ignoreMalformed) {
             ParsedCartesianPoint point = new ParsedCartesianPoint();
             CartesianPoint.parsePoint(nullValue, point, ignoreZValue);
             if (ignoreMalformed == false) {
@@ -84,14 +90,29 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
             }
             return point;
         }
+
+        @Override
+        public ParametrizedFieldMapper build(BuilderContext context) {
+            CartesianPointParser parser
+                = new CartesianPointParser(name, nullValue.get(), ignoreZValue.get().value(), ignoreMalformed.get().value());
+            PointFieldType ft
+                = new PointFieldType(buildFullName(context), indexed.get(), stored.get(), hasDocValues.get(), parser, meta.get());
+            return new PointFieldMapper(name, ft, multiFieldsBuilder.build(this, context),
+                copyTo.build(), new PointIndexer(ft), parser, this);
+        }
+
     }
 
-    public PointFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                            MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
-                            Explicit<Boolean> ignoreZValue, ParsedPoint nullValue, CopyTo copyTo,
-                            PointIndexer pointIndexer, CartesianPointParser parser) {
-        super(simpleName, fieldType, mappedFieldType, multiFields,
-            ignoreMalformed, ignoreZValue, nullValue, copyTo, pointIndexer, parser);
+    public static TypeParser PARSER = new TypeParser((n, c) -> new Builder(n, IGNORE_MALFORMED_SETTING.get(c.getSettings())));
+
+    private final Builder builder;
+
+    public PointFieldMapper(String simpleName, MappedFieldType mappedFieldType,
+                            MultiFields multiFields, CopyTo copyTo,
+                            PointIndexer pointIndexer, CartesianPointParser parser, Builder builder) {
+        super(simpleName, mappedFieldType, multiFields,
+            builder.ignoreMalformed.get(), builder.ignoreZValue.get(), builder.nullValue.get(), copyTo, pointIndexer, parser);
+        this.builder = builder;
     }
 
     @Override
@@ -124,13 +145,18 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
         return (PointFieldType) mappedFieldType;
     }
 
-    public static class PointFieldType extends AbstractPointGeometryFieldType implements ShapeQueryable {
+    @Override
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(simpleName(), builder.ignoreMalformed.getDefaultValue().value()).init(this);
+    }
+
+    public static class PointFieldType extends AbstractGeometryFieldType implements ShapeQueryable {
 
         private final ShapeQueryPointProcessor queryProcessor;
 
         private PointFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
                                CartesianPointParser parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, parser, meta);
+            super(name, indexed, stored, hasDocValues, true, parser, meta);
             this.queryProcessor = new ShapeQueryPointProcessor();
         }
 
@@ -186,12 +212,8 @@ public class PointFieldMapper extends AbstractPointGeometryFieldMapper<List<Pars
                 CartesianPoint o = (CartesianPoint)other;
                 oX = o.getX();
                 oY = o.getY();
-            } else if (other instanceof ParsedCartesianPoint == false) {
-                return false;
             } else {
-                ParsedCartesianPoint o = (ParsedCartesianPoint)other;
-                oX = o.getX();
-                oY = o.getY();
+                return false;
             }
             if (Double.compare(oX, x) != 0) return false;
             if (Double.compare(oY, y) != 0) return false;

+ 52 - 38
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/ShapeFieldMapper.java

@@ -5,8 +5,8 @@
  */
 package org.elasticsearch.xpack.spatial.index.mapper;
 
-import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.XYShape;
+import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryParser;
@@ -14,13 +14,15 @@ import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapper;
+import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.GeoShapeParser;
 import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.ParametrizedFieldMapper;
 import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.xpack.spatial.index.query.ShapeQueryProcessor;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -43,47 +45,57 @@ import java.util.Map;
 public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry, Geometry> {
     public static final String CONTENT_TYPE = "shape";
 
-    public static class Builder extends AbstractShapeGeometryFieldMapper.Builder {
+    private static Builder builder(FieldMapper in) {
+        return ((ShapeFieldMapper)in).builder;
+    }
 
-        public Builder(String name) {
-            super(name, new FieldType());
-            fieldType.setDimensions(7, 4, Integer.BYTES);
-        }
+    public static class Builder extends ParametrizedFieldMapper.Builder {
 
-        @Override
-        public ShapeFieldMapper build(BuilderContext context) {
-            GeometryParser geometryParser
-                = new GeometryParser(orientation().value().getAsBoolean(), coerce().value(), ignoreZValue().value());
-            Parser<Geometry> parser = new GeoShapeParser(geometryParser);
-            ShapeFieldType ft = new ShapeFieldType(buildFullName(context), indexed, fieldType.stored(), hasDocValues, parser, meta);
-            ft.setOrientation(orientation().value());
-            return new ShapeFieldMapper(name, fieldType, ft, ignoreMalformed(context), coerce(context),
-                ignoreZValue(), orientation(), multiFieldsBuilder.build(this, context), copyTo,
-                new ShapeIndexer(ft.name()), parser);
+        final Parameter<Boolean> indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true);
+
+        final Parameter<Explicit<Boolean>> ignoreMalformed;
+        final Parameter<Explicit<Boolean>> ignoreZValue = ignoreZValueParam(m -> builder(m).ignoreZValue.get());
+        final Parameter<Explicit<Boolean>> coerce;
+        final Parameter<Explicit<Orientation>> orientation = orientationParam(m -> builder(m).orientation.get());
+
+        final Parameter<Map<String, String>> meta = Parameter.metaParam();
+
+        public Builder(String name, boolean ignoreMalformedByDefault, boolean coerceByDefault) {
+            super(name);
+            this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
+            this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
         }
-    }
 
-    public static class TypeParser extends AbstractShapeGeometryFieldMapper.TypeParser {
         @Override
-        protected boolean parseXContentParameters(String name, Map.Entry<String, Object> entry,
-                                                  Map<String, Object> params) throws MapperParsingException {
-            return false;
+        protected List<Parameter<?>> getParameters() {
+            return Arrays.asList(indexed, ignoreMalformed, ignoreZValue, coerce, orientation, meta);
         }
 
         @Override
-        public Builder newBuilder(String name, Map<String, Object> params) {
-            return new Builder(name);
+        public ShapeFieldMapper build(BuilderContext context) {
+            GeometryParser geometryParser
+                = new GeometryParser(orientation.get().value().getAsBoolean(), coerce.get().value(), ignoreZValue.get().value());
+            Parser<Geometry> parser = new GeoShapeParser(geometryParser);
+            ShapeFieldType ft
+                = new ShapeFieldType(buildFullName(context), indexed.get(), orientation.get().value(), parser, meta.get());
+            return new ShapeFieldMapper(name, ft,
+                multiFieldsBuilder.build(this, context), copyTo.build(),
+                new ShapeIndexer(ft.name()), parser, this);
         }
     }
 
+    public static TypeParser PARSER = new TypeParser((n, c) -> new Builder(n,
+        IGNORE_MALFORMED_SETTING.get(c.getSettings()),
+        COERCE_SETTING.get(c.getSettings())));
+
     public static final class ShapeFieldType extends AbstractShapeGeometryFieldType
         implements ShapeQueryable {
 
         private final ShapeQueryProcessor queryProcessor;
 
-        public ShapeFieldType(String name, boolean indexed, boolean stored, boolean hasDocValues,
+        public ShapeFieldType(String name, boolean indexed, Orientation orientation,
                               Parser<Geometry> parser, Map<String, String> meta) {
-            super(name, indexed, stored, hasDocValues, false, parser, meta);
+            super(name, indexed, false, false, false, parser, orientation, meta);
             this.queryProcessor = new ShapeQueryProcessor();
         }
 
@@ -98,18 +110,15 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry,
         }
     }
 
-    public ShapeFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
-                            Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
-                            Explicit<Boolean> ignoreZValue, Explicit<Orientation> orientation,
+    private final Builder builder;
+
+    public ShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                             MultiFields multiFields, CopyTo copyTo,
-                            Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser) {
-        super(simpleName, fieldType, mappedFieldType, ignoreMalformed, coerce, ignoreZValue, orientation,
+                            Indexer<Geometry, Geometry> indexer, Parser<Geometry> parser, Builder builder) {
+        super(simpleName, mappedFieldType, builder.ignoreMalformed.get(),
+            builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(),
             multiFields, copyTo, indexer, parser);
-    }
-
-    @Override
-    protected void mergeGeoOptions(AbstractShapeGeometryFieldMapper<?,?> mergeWith, List<String> conflicts) {
-
+        this.builder = builder;
     }
 
     @Override
@@ -119,8 +128,7 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry,
     }
 
     @Override
-    @SuppressWarnings("rawtypes")
-    protected void addDocValuesFields(String name, Geometry geometry, List fields, ParseContext context) {
+    protected void addDocValuesFields(String name, Geometry geometry, List<IndexableField> fields, ParseContext context) {
         // we should throw a mapping exception before we get here
     }
 
@@ -134,6 +142,12 @@ public class ShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geometry,
         return CONTENT_TYPE;
     }
 
+    @Override
+    public ParametrizedFieldMapper.Builder getMergeBuilder() {
+        return new Builder(simpleName(), builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value())
+            .init(this);
+    }
+
     @Override
     public ShapeFieldType fieldType() {
         return (ShapeFieldType) super.fieldType();

+ 6 - 5
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeCentroidAggregatorTests.java

@@ -15,6 +15,7 @@ import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.store.Directory;
 import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.mapper.GeoShapeIndexer;
@@ -62,7 +63,7 @@ public class GeoShapeCentroidAggregatorTests extends AggregatorTestCase {
                 .field("field");
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 InternalGeoCentroid result = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
@@ -85,12 +86,12 @@ public class GeoShapeCentroidAggregatorTests extends AggregatorTestCase {
                 IndexSearcher searcher = new IndexSearcher(reader);
 
                 MappedFieldType fieldType = new GeoShapeWithDocValuesFieldType("another_field",
-                    true, false, true, null, Collections.emptyMap());
+                    true, true, Orientation.RIGHT, null, Collections.emptyMap());
                 InternalGeoCentroid result = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
                 assertNull(result.centroid());
 
                 fieldType = new GeoShapeWithDocValuesFieldType("field",
-                    true, false, true, null, Collections.emptyMap());
+                    true, true, Orientation.RIGHT, null, Collections.emptyMap());
                 result = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
                 assertNull(result.centroid());
                 assertFalse(AggregationInspectionHelper.hasValue(result));
@@ -115,7 +116,7 @@ public class GeoShapeCentroidAggregatorTests extends AggregatorTestCase {
                 IndexSearcher searcher = new IndexSearcher(reader);
 
                 MappedFieldType fieldType = new GeoShapeWithDocValuesFieldType("another_field",
-                    true, false, true, null, Collections.emptyMap());
+                    true, true, Orientation.RIGHT, null, Collections.emptyMap());
                 InternalGeoCentroid result = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
                 assertThat(result.centroid(), equalTo(expectedCentroid));
                 assertTrue(AggregationInspectionHelper.hasValue(result));
@@ -176,7 +177,7 @@ public class GeoShapeCentroidAggregatorTests extends AggregatorTestCase {
 
     private void assertCentroid(RandomIndexWriter w, GeoPoint expectedCentroid) throws IOException {
         MappedFieldType fieldType = new GeoShapeWithDocValuesFieldType("field",
-            true, false, true, null, Collections.emptyMap());
+            true, true, Orientation.RIGHT, null, Collections.emptyMap());
         GeoCentroidAggregationBuilder aggBuilder = new GeoCentroidAggregationBuilder("my_agg")
             .field("field");
         try (IndexReader reader = w.getReader()) {

+ 3 - 2
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/ingest/CircleProcessorTests.java

@@ -16,6 +16,7 @@ import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.geo.GeoJson;
 import org.elasticsearch.common.geo.ShapeRelation;
+import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -214,7 +215,7 @@ public class CircleProcessorTests extends ESTestCase {
         Geometry geometry = SpatialUtils.createRegularGeoShapePolygon(circle, numSides);
 
         MappedFieldType shapeType
-            = new GeoShapeWithDocValuesFieldType(fieldName, true, false, false, null, Collections.emptyMap());
+            = new GeoShapeWithDocValuesFieldType(fieldName, true, false, ShapeBuilder.Orientation.RIGHT, null, Collections.emptyMap());
 
         VectorGeoShapeQueryProcessor processor = new VectorGeoShapeQueryProcessor();
         QueryShardContext mockedContext = mock(QueryShardContext.class);
@@ -246,7 +247,7 @@ public class CircleProcessorTests extends ESTestCase {
         int numSides = randomIntBetween(4, 1000);
         Geometry geometry = SpatialUtils.createRegularShapePolygon(circle, numSides);
 
-        MappedFieldType shapeType = new ShapeFieldType(fieldName, true, false, false, null, Collections.emptyMap());
+        MappedFieldType shapeType = new ShapeFieldType(fieldName, true, ShapeBuilder.Orientation.RIGHT, null, Collections.emptyMap());
 
         ShapeQueryProcessor processor = new ShapeQueryProcessor();
         QueryShardContext mockedContext = mock(QueryShardContext.class);

+ 2 - 1
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java

@@ -19,6 +19,7 @@ import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.MultiPoint;
 import org.elasticsearch.geometry.Point;
@@ -299,7 +300,7 @@ public abstract class GeoShapeGeoGridTestCase<T extends InternalGeoGridBucket<T>
         }
 
         MappedFieldType fieldType
-            = new GeoShapeWithDocValuesFieldType(FIELD_NAME, true, false, true, null, Collections.emptyMap());
+            = new GeoShapeWithDocValuesFieldType(FIELD_NAME, true, true, Orientation.RIGHT, null, Collections.emptyMap());
 
         Aggregator aggregator = createAggregator(aggregationBuilder, indexSearcher, fieldType);
         aggregator.preCollection();

+ 6 - 5
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java

@@ -15,6 +15,7 @@ import org.apache.lucene.index.RandomIndexWriter;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.store.Directory;
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
 import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.MultiPoint;
@@ -58,7 +59,7 @@ public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase {
                 .wrapLongitude(false);
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 InternalGeoBounds bounds = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
@@ -86,7 +87,7 @@ public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase {
                 .wrapLongitude(false);
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 InternalGeoBounds bounds = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
@@ -108,7 +109,7 @@ public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase {
             w.addDocument(doc);
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
 
             Point point = GeometryTestUtils.randomPoint(false);
             double lon = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(point.getX()));
@@ -140,7 +141,7 @@ public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase {
             w.addDocument(doc);
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
 
             GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg")
                 .field("field")
@@ -201,7 +202,7 @@ public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase {
                 .wrapLongitude(false);
 
             MappedFieldType fieldType
-                = new GeoShapeWithDocValuesFieldType("field", true, false, true, null, Collections.emptyMap());
+                = new GeoShapeWithDocValuesFieldType("field", true, true, Orientation.RIGHT, null, Collections.emptyMap());
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 InternalGeoBounds bounds = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);