From 998736b83908585b1c6cf276b90ce0dc695737de Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 16 Sep 2020 17:17:01 +0100 Subject: [PATCH 1/4] Allow empty null values for date and ip field mappers --- .../index/mapper/DateFieldMapper.java | 3 +- .../index/mapper/IpFieldMapper.java | 36 +-- .../index/mapper/DateFieldMapperTests.java | 12 +- .../index/mapper/IpFieldMapperTests.java | 214 +++++------------- 4 files changed, 91 insertions(+), 174 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index ffa8335dae125..841d9e113f1cd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -33,6 +33,7 @@ import org.apache.lucene.search.TermQuery; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.time.DateFormatter; @@ -226,7 +227,7 @@ protected List> getParameters() { } private Long parseNullValue(DateFieldType fieldType) { - if (nullValue.getValue() == null) { + if (nullValue.getValue() == null || Strings.isEmpty(nullValue.getValue())) { return null; } try { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 68d742961a7fa..e6d491554ae4b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -31,9 +31,9 @@ import org.apache.lucene.util.ArrayUtil; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.network.InetAddresses; -import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData; @@ -68,16 +68,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder { private final Parameter stored = Parameter.storeParam(m -> toType(m).stored, false); private final Parameter ignoreMalformed; - private final Parameter nullValue = new Parameter<>("null_value", false, () -> null, - (n, c, o) -> o == null ? null : InetAddresses.forString(o.toString()), m -> toType(m).nullValue) - .setSerializer((b, f, v) -> { - if (v == null) { - b.nullField(f); - } else { - b.field(f, InetAddresses.toAddrString(v)); - } - }, NetworkAddress::format) - .acceptsNull(); + private final Parameter nullValue + = Parameter.stringParam("null_value", false, m -> toType(m).nullValueAsString, null).acceptsNull(); private final Parameter> meta = Parameter.metaParam(); @@ -90,14 +82,27 @@ public Builder(String name, boolean ignoreMalformedByDefault) { = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault); } - Builder nullValue(InetAddress nullValue) { + Builder nullValue(String nullValue) { this.nullValue.setValue(nullValue); return this; } + private InetAddress parseNullValue() { + String nullValueAsString = nullValue.getValue(); + if (nullValueAsString == null || Strings.isEmpty(nullValueAsString)) { + return null; + } + try { + return InetAddresses.forString(nullValueAsString); + } + catch (Exception e) { + throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); + } + } + @Override protected List> getParameters() { - return List.of(indexed, hasDocValues, stored, ignoreMalformed, nullValue); + return List.of(indexed, hasDocValues, stored, ignoreMalformed, nullValue, meta); } @Override @@ -322,7 +327,9 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) { private final boolean hasDocValues; private final boolean stored; private final boolean ignoreMalformed; + private final InetAddress nullValue; + private final String nullValueAsString; private final boolean ignoreMalformedByDefault; @@ -338,7 +345,8 @@ private IpFieldMapper( this.hasDocValues = builder.hasDocValues.getValue(); this.stored = builder.stored.getValue(); this.ignoreMalformed = builder.ignoreMalformed.getValue(); - this.nullValue = builder.nullValue.getValue(); + this.nullValue = builder.parseNullValue(); + this.nullValueAsString = builder.nullValue.getValue(); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index f0f4c78804a38..b07e2ec9091d1 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -191,6 +191,13 @@ public void testNullValue() throws IOException { assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType()); assertEquals(1457654400000L, dvField.numericValue().longValue()); assertFalse(dvField.fieldType().stored()); + + mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "date"); + b.field("null_value", ""); + })); + doc = mapper.parse(source(b -> b.nullField("field"))); + assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); } public void testNanosNullValue() throws IOException { @@ -223,10 +230,11 @@ public void testNanosNullValue() throws IOException { public void testBadNullValue() { MapperParsingException e = expectThrows(MapperParsingException.class, - () -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("null_value", "")))); + () -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("null_value", "foo")))); assertThat(e.getMessage(), - equalTo("Failed to parse mapping: Error parsing [null_value] on field [field]: cannot parse empty date")); + equalTo("Failed to parse mapping: Error parsing [null_value] on field [field]: " + + "failed to parse date field [foo] with format [strict_date_optional_time||epoch_millis]")); } public void testNullConfigValuesFail() { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 8ba10e4044f29..3090f7c235cc3 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -28,61 +28,28 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.settings.Settings; -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.IndexService; import org.elasticsearch.index.termvectors.TermVectorsService; -import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.test.InternalSettingsPlugin; -import org.junit.Before; import java.io.IOException; import java.net.InetAddress; -import java.util.Collection; import java.util.List; -import static org.elasticsearch.index.mapper.FieldMapperTestCase.fetchSourceValue; import static org.hamcrest.Matchers.containsString; -public class IpFieldMapperTests extends ESSingleNodeTestCase { - - IndexService indexService; - DocumentMapperParser parser; - - @Before - public void setup() { - indexService = createIndex("test"); - parser = indexService.mapperService().documentMapperParser(); - } +public class IpFieldMapperTests extends MapperTestCase { @Override - protected Collection> getPlugins() { - return pluginList(InternalSettingsPlugin.class); + protected void minimalMapping(XContentBuilder b) throws IOException { + b.field("type", "ip"); } public void testDefaults() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").endObject().endObject() - .endObject().endObject()); - - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); - - assertEquals(mapping, mapper.mappingSource().toString()); + DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); - ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "::1") - .endObject()), - XContentType.JSON)); + ParsedDocument doc = mapper.parse(source(b -> b.field("field", "::1"))); IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(2, fields.length); @@ -98,20 +65,13 @@ public void testDefaults() throws Exception { } public void testNotIndexed() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").field("index", false).endObject().endObject() - .endObject().endObject()); - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("index", false); + })); - assertEquals(mapping, mapper.mappingSource().toString()); - - ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "::1") - .endObject()), - XContentType.JSON)); + ParsedDocument doc = mapper.parse(source(b -> b.field("field", "::1"))); IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(1, fields.length); @@ -120,20 +80,13 @@ public void testNotIndexed() throws Exception { } public void testNoDocValues() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").field("doc_values", false).endObject().endObject() - .endObject().endObject()); - - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); - assertEquals(mapping, mapper.mappingSource().toString()); + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("doc_values", false); + })); - ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "::1") - .endObject()), - XContentType.JSON)); + ParsedDocument doc = mapper.parse(source(b -> b.field("field", "::1"))); IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(1, fields.length); @@ -151,20 +104,13 @@ public void testNoDocValues() throws Exception { } public void testStore() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").field("store", true).endObject().endObject() - .endObject().endObject()); - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("store", true); + })); - assertEquals(mapping, mapper.mappingSource().toString()); - - ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "::1") - .endObject()), - XContentType.JSON)); + ParsedDocument doc = mapper.parse(source(b -> b.field("field", "::1"))); IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(3, fields.length); @@ -179,35 +125,19 @@ public void testStore() throws Exception { } public void testIgnoreMalformed() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").endObject().endObject() - .endObject().endObject()); - - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); - assertEquals(mapping, mapper.mappingSource().toString()); + DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); - ThrowingRunnable runnable = () -> mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", ":1") - .endObject()), - XContentType.JSON)); + ThrowingRunnable runnable = () -> mapper.parse(source(b -> b.field("field", ":1"))); MapperParsingException e = expectThrows(MapperParsingException.class, runnable); assertThat(e.getCause().getMessage(), containsString("':1' is not an IP string literal")); - mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").field("ignore_malformed", true).endObject().endObject() - .endObject().endObject()); + DocumentMapper mapper2 = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("ignore_malformed", true); + })); - DocumentMapper mapper2 = parser.parse("type", new CompressedXContent(mapping)); - - ParsedDocument doc = mapper2.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", ":1") - .endObject()), - XContentType.JSON)); + ParsedDocument doc = mapper2.parse(source(b -> b.field("field", ":1"))); IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(0, fields.length); @@ -215,45 +145,19 @@ public void testIgnoreMalformed() throws Exception { } public void testNullValue() throws IOException { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() - .startObject("type") - .startObject("properties") - .startObject("field") - .field("type", "ip") - .endObject() - .endObject() - .endObject().endObject()); - - DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); - assertEquals(mapping, mapper.mappingSource().toString()); - - ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .nullField("field") - .endObject()), - XContentType.JSON)); + + DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); + + ParsedDocument doc = mapper.parse(source(b -> b.nullField("field"))); assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() - .startObject("type") - .startObject("properties") - .startObject("field") - .field("type", "ip") - .field("null_value", "::1") - .endObject() - .endObject() - .endObject().endObject()); - - mapper = parser.parse("type", new CompressedXContent(mapping)); - assertEquals(mapping, mapper.mappingSource().toString()); - - doc = mapper.parse(new SourceToParse("test", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .nullField("field") - .endObject()), - XContentType.JSON)); + mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("null_value", "::1"); + })); + + doc = mapper.parse(source(b -> b.nullField("field"))); + IndexableField[] fields = doc.rootDoc().getFields("field"); assertEquals(2, fields.length); IndexableField pointField = fields[0]; @@ -265,33 +169,29 @@ public void testNullValue() throws IOException { assertEquals(DocValuesType.SORTED_SET, dvField.fieldType().docValuesType()); assertEquals(new BytesRef(InetAddressPoint.encode(InetAddresses.forString("::1"))), dvField.binaryValue()); assertFalse(dvField.fieldType().stored()); - } - public void testSerializeDefaults() throws Exception { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field").field("type", "ip").endObject().endObject() - .endObject().endObject()); + mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.nullField("null_value"); + })); - DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping)); - IpFieldMapper mapper = (IpFieldMapper)docMapper.root().getMapper("field"); - XContentBuilder builder = XContentFactory.jsonBuilder().startObject(); - mapper.doXContentBody(builder, true, ToXContent.EMPTY_PARAMS); - String got = Strings.toString(builder.endObject()); + doc = mapper.parse(source(b -> b.nullField("field"))); + assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - // it would be nice to check the entire serialized default mapper, but there are - // a whole lot of bogus settings right now it picks up from calling super.doXContentBody... - assertTrue(got, got.contains("\"ignore_malformed\":false")); - } + mapper = createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("null_value", ""); + })); - public void testEmptyName() throws IOException { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("").field("type", "ip").endObject().endObject() - .endObject().endObject()); + doc = mapper.parse(source(b -> b.nullField("field"))); + assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> parser.parse("type", new CompressedXContent(mapping)) - ); - assertThat(e.getMessage(), containsString("name cannot be empty string")); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "ip"); + b.field("null_value", ":1"); + }))); + assertEquals(e.getMessage(), + "Failed to parse mapping: Error parsing [null_value] on field [field]: ':1' is not an IP string literal."); } public void testFetchSourceValue() throws IOException { @@ -304,7 +204,7 @@ public void testFetchSourceValue() throws IOException { assertEquals(List.of("::1"), fetchSourceValue(mapper, "0:0:0:0:0:0:0:1")); IpFieldMapper nullValueMapper = new IpFieldMapper.Builder("field", true) - .nullValue(InetAddresses.forString("2001:db8:0:0:0:0:2:7")) + .nullValue("2001:db8:0:0:0:0:2:7") .build(context); assertEquals(List.of("2001:db8::2:7"), fetchSourceValue(nullValueMapper, null)); } From cdf283cfd93a9cb04a73e609161a2ebbce2a276c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 17 Sep 2020 13:39:15 +0100 Subject: [PATCH 2/4] Only fail null_value parsing for pre-8x indexes --- .../index/mapper/DateFieldMapper.java | 28 ++++++++++++++----- .../index/mapper/DocumentParser.java | 3 +- .../index/mapper/IpFieldMapper.java | 26 +++++++++++++---- .../MetadataRolloverServiceTests.java | 3 +- .../index/mapper/DateFieldMapperTests.java | 18 ++++++------ .../index/mapper/IpFieldMapperTests.java | 23 ++++++++------- .../index/mapper/MapperServiceTestCase.java | 14 ++++++---- 7 files changed, 73 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index 841d9e113f1cd..df145aa71ac4b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -32,9 +32,10 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.ShapeRelation; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.DateFormatters; @@ -71,6 +72,8 @@ /** A {@link FieldMapper} for dates. */ public final class DateFieldMapper extends ParametrizedFieldMapper { + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(DateFieldMapper.class); + public static final String CONTENT_TYPE = "date"; public static final String DATE_NANOS_CONTENT_TYPE = "date_nanos"; public static final DateFormatter DEFAULT_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_optional_time||epoch_millis"); @@ -201,10 +204,13 @@ public static class Builder extends ParametrizedFieldMapper.Builder { private final Parameter ignoreMalformed; private final Resolution resolution; + private final Version indexCreatedVersion; - public Builder(String name, Resolution resolution, DateFormatter dateFormatter, boolean ignoreMalformedByDefault) { + public Builder(String name, Resolution resolution, DateFormatter dateFormatter, + boolean ignoreMalformedByDefault, Version indexCreatedVersion) { super(name); this.resolution = resolution; + this.indexCreatedVersion = indexCreatedVersion; this.ignoreMalformed = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault); if (dateFormatter != null) { @@ -227,14 +233,20 @@ protected List> getParameters() { } private Long parseNullValue(DateFieldType fieldType) { - if (nullValue.getValue() == null || Strings.isEmpty(nullValue.getValue())) { + if (nullValue.getValue() == null) { return null; } try { return fieldType.parse(nullValue.getValue()); } catch (Exception e) { - throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); + if (indexCreatedVersion.onOrAfter(Version.V_8_0_0)) { + throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); + } else { + DEPRECATION_LOGGER.deprecate("date_mapper_null_field", "Error parsing [" + nullValue.getValue() + + "] as date in [null_value] on field [" + name() + "]); [null_value] will be ignored"); + return null; + } } } @@ -251,12 +263,12 @@ public DateFieldMapper build(BuilderContext context) { public static final TypeParser MILLIS_PARSER = new TypeParser((n, c) -> { boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings()); - return new Builder(n, Resolution.MILLISECONDS, c.getDateFormatter(), ignoreMalformedByDefault); + return new Builder(n, Resolution.MILLISECONDS, c.getDateFormatter(), ignoreMalformedByDefault, c.indexVersionCreated()); }); public static final TypeParser NANOS_PARSER = new TypeParser((n, c) -> { boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings()); - return new Builder(n, Resolution.NANOSECONDS, c.getDateFormatter(), ignoreMalformedByDefault); + return new Builder(n, Resolution.NANOSECONDS, c.getDateFormatter(), ignoreMalformedByDefault, c.indexVersionCreated()); }); public static final class DateFieldType extends MappedFieldType { @@ -518,6 +530,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) { private final Resolution resolution; private final boolean ignoreMalformedByDefault; + private final Version indexCreatedVersion; private DateFieldMapper( String simpleName, @@ -538,11 +551,12 @@ private DateFieldMapper( this.nullValue = nullValue; this.resolution = resolution; this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue(); + this.indexCreatedVersion = builder.indexCreatedVersion; } @Override public ParametrizedFieldMapper.Builder getMergeBuilder() { - return new Builder(simpleName(), resolution, null, ignoreMalformedByDefault).init(this); + return new Builder(simpleName(), resolution, null, ignoreMalformedByDefault, indexCreatedVersion).init(this); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 0d198e854aca6..33b2717391107 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -22,6 +22,7 @@ import org.apache.lucene.document.Field; import org.apache.lucene.index.IndexableField; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; @@ -680,7 +681,7 @@ private static Mapper.Builder createBuilderFromDynamicValue(final ParseContex if (builder == null) { boolean ignoreMalformed = IGNORE_MALFORMED_SETTING.get(context.indexSettings().getSettings()); builder = new DateFieldMapper.Builder(currentFieldName, DateFieldMapper.Resolution.MILLISECONDS, - dateTimeFormatter, ignoreMalformed); + dateTimeFormatter, ignoreMalformed, Version.indexCreated(context.indexSettings().getSettings())); } return builder; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index e6d491554ae4b..dc4e0da8af6ec 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -30,9 +30,10 @@ import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.ArrayUtil; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; @@ -55,6 +56,8 @@ /** A {@link FieldMapper} for ip addresses. */ public class IpFieldMapper extends ParametrizedFieldMapper { + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(IpFieldMapper.class); + public static final String CONTENT_TYPE = "ip"; private static IpFieldMapper toType(FieldMapper in) { @@ -74,10 +77,12 @@ public static class Builder extends ParametrizedFieldMapper.Builder { private final Parameter> meta = Parameter.metaParam(); private final boolean ignoreMalformedByDefault; + private final Version indexCreatedVersion; - public Builder(String name, boolean ignoreMalformedByDefault) { + public Builder(String name, boolean ignoreMalformedByDefault, Version indexCreatedVersion) { super(name); this.ignoreMalformedByDefault = ignoreMalformedByDefault; + this.indexCreatedVersion = indexCreatedVersion; this.ignoreMalformed = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault); } @@ -89,14 +94,21 @@ Builder nullValue(String nullValue) { private InetAddress parseNullValue() { String nullValueAsString = nullValue.getValue(); - if (nullValueAsString == null || Strings.isEmpty(nullValueAsString)) { + if (nullValueAsString == null) { return null; } try { return InetAddresses.forString(nullValueAsString); } catch (Exception e) { - throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); + if (indexCreatedVersion.onOrAfter(Version.V_8_0_0)) { + throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); + } + else { + DEPRECATION_LOGGER.deprecate("ip_mapper_null_field", "Error parsing [" + nullValue.getValue() + + "] as IP in [null_value] on field [" + name() + "]); [null_value] will be ignored"); + return null; + } } } @@ -116,7 +128,7 @@ public IpFieldMapper build(BuilderContext context) { public static final TypeParser PARSER = new TypeParser((n, c) -> { boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(c.getSettings()); - return new Builder(n, ignoreMalformedByDefault); + return new Builder(n, ignoreMalformedByDefault, c.indexVersionCreated()); }); public static final class IpFieldType extends SimpleMappedFieldType { @@ -332,6 +344,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) { private final String nullValueAsString; private final boolean ignoreMalformedByDefault; + private final Version indexCreatedVersion; private IpFieldMapper( String simpleName, @@ -347,6 +360,7 @@ private IpFieldMapper( this.ignoreMalformed = builder.ignoreMalformed.getValue(); this.nullValue = builder.parseNullValue(); this.nullValueAsString = builder.nullValue.getValue(); + this.indexCreatedVersion = builder.indexCreatedVersion; } @Override @@ -432,6 +446,6 @@ protected Object parseSourceValue(Object value) { @Override public ParametrizedFieldMapper.Builder getMergeBuilder() { - return new Builder(simpleName(), ignoreMalformedByDefault).init(this); + return new Builder(simpleName(), ignoreMalformedByDefault, indexCreatedVersion).init(this); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index a12d9d85789e0..66846af2d9096 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -553,7 +553,8 @@ public void testRolloverClusterStateForDataStream() throws Exception { try { Mapper.BuilderContext builderContext = new Mapper.BuilderContext(Settings.EMPTY, new ContentPath(0)); DateFieldMapper dateFieldMapper - = new DateFieldMapper.Builder("@timestamp", DateFieldMapper.Resolution.MILLISECONDS, null, false).build(builderContext); + = new DateFieldMapper.Builder("@timestamp", DateFieldMapper.Resolution.MILLISECONDS, null, false, Version.CURRENT) + .build(builderContext); MetadataFieldMapper mockedTimestampField = mock(MetadataFieldMapper.class); when(mockedTimestampField.name()).thenReturn("_data_stream_timestamp"); MappedFieldType mockedTimestampFieldType = mock(MappedFieldType.class); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index b07e2ec9091d1..af5ca60d31dce 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -191,13 +191,6 @@ public void testNullValue() throws IOException { assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType()); assertEquals(1457654400000L, dvField.numericValue().longValue()); assertFalse(dvField.fieldType().stored()); - - mapper = createDocumentMapper(fieldMapping(b -> { - b.field("type", "date"); - b.field("null_value", ""); - })); - doc = mapper.parse(source(b -> b.nullField("field"))); - assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); } public void testNanosNullValue() throws IOException { @@ -227,14 +220,18 @@ public void testNanosNullValue() throws IOException { assertFalse(dvField.fieldType().stored()); } - public void testBadNullValue() { + public void testBadNullValue() throws IOException { MapperParsingException e = expectThrows(MapperParsingException.class, - () -> createDocumentMapper(fieldMapping(b -> b.field("type", "date").field("null_value", "foo")))); + () -> createDocumentMapper(Version.V_8_0_0, fieldMapping(b -> b.field("type", "date").field("null_value", "foo")))); assertThat(e.getMessage(), equalTo("Failed to parse mapping: Error parsing [null_value] on field [field]: " + "failed to parse date field [foo] with format [strict_date_optional_time||epoch_millis]")); + + createDocumentMapper(Version.V_7_9_0, fieldMapping(b -> b.field("type", "date").field("null_value", "foo"))); + + assertWarnings("Error parsing [foo] as date in [null_value] on field [field]); [null_value] will be ignored"); } public void testNullConfigValuesFail() { @@ -372,7 +369,8 @@ private DateFieldMapper createMapper(Resolution resolution, String format, Strin mapping.put("null_value", nullValue); } - DateFieldMapper.Builder builder = new DateFieldMapper.Builder("field", resolution, null, false); + DateFieldMapper.Builder builder + = new DateFieldMapper.Builder("field", resolution, null, false, Version.CURRENT); builder.parse("field", null, mapping); return builder.build(context); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 3090f7c235cc3..1b170bbc2cebe 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -177,33 +177,32 @@ public void testNullValue() throws IOException { doc = mapper.parse(source(b -> b.nullField("field"))); assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - - mapper = createDocumentMapper(fieldMapping(b -> { - b.field("type", "ip"); - b.field("null_value", ""); - })); - - doc = mapper.parse(source(b -> b.nullField("field"))); - assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - - MapperParsingException e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + + MapperParsingException e = expectThrows(MapperParsingException.class, + () -> createDocumentMapper(Version.CURRENT, fieldMapping(b -> { b.field("type", "ip"); b.field("null_value", ":1"); }))); assertEquals(e.getMessage(), "Failed to parse mapping: Error parsing [null_value] on field [field]: ':1' is not an IP string literal."); + + createDocumentMapper(Version.V_7_9_0, fieldMapping(b -> { + b.field("type", "ip"); + b.field("null_value", ":1"); + })); + assertWarnings("Error parsing [:1] as IP in [null_value] on field [field]); [null_value] will be ignored"); } public void testFetchSourceValue() throws IOException { Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build(); Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath()); - IpFieldMapper mapper = new IpFieldMapper.Builder("field", true).build(context); + IpFieldMapper mapper = new IpFieldMapper.Builder("field", true, Version.CURRENT).build(context); assertEquals(List.of("2001:db8::2:1"), fetchSourceValue(mapper, "2001:db8::2:1")); assertEquals(List.of("2001:db8::2:1"), fetchSourceValue(mapper, "2001:db8:0:0:0:0:2:1")); assertEquals(List.of("::1"), fetchSourceValue(mapper, "0:0:0:0:0:0:0:1")); - IpFieldMapper nullValueMapper = new IpFieldMapper.Builder("field", true) + IpFieldMapper nullValueMapper = new IpFieldMapper.Builder("field", true, Version.CURRENT) .nullValue("2001:db8:0:0:0:0:2:7") .build(context); assertEquals(List.of("2001:db8::2:7"), fetchSourceValue(nullValueMapper, null)); diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index 1a7a29d72afcf..ff9aa896acfb1 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -73,7 +73,7 @@ protected Collection getPlugins() { } protected Settings getIndexSettings() { - return SETTINGS; + return Settings.EMPTY; } protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) { @@ -92,6 +92,10 @@ protected final DocumentMapper createDocumentMapper(XContentBuilder mappings) th return createMapperService(mappings).documentMapper(); } + protected final DocumentMapper createDocumentMapper(Version version, XContentBuilder mappings) throws IOException { + return createMapperService(version, mappings).documentMapper(); + } + protected final DocumentMapper createDocumentMapper(String mappings) throws IOException { MapperService mapperService = createMapperService(mapping(b -> {})); merge(mapperService, mappings); @@ -99,7 +103,7 @@ protected final DocumentMapper createDocumentMapper(String mappings) throws IOEx } protected final MapperService createMapperService(XContentBuilder mappings) throws IOException { - return createMapperService(getIndexSettings(), mappings); + return createMapperService(Version.CURRENT, mappings); } protected final MapperService createMapperService(String mappings) throws IOException { @@ -111,13 +115,13 @@ protected final MapperService createMapperService(String mappings) throws IOExce /** * Create a {@link MapperService} like we would for an index. */ - protected final MapperService createMapperService(Settings settings, XContentBuilder mapping) throws IOException { + protected final MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException { IndexMetadata meta = IndexMetadata.builder("index") - .settings(Settings.builder().put("index.version.created", Version.CURRENT)) + .settings(Settings.builder().put("index.version.created", version)) .numberOfReplicas(0) .numberOfShards(1) .build(); - IndexSettings indexSettings = new IndexSettings(meta, Settings.EMPTY); + IndexSettings indexSettings = new IndexSettings(meta, getIndexSettings()); MapperRegistry mapperRegistry = new IndicesModule( getPlugins().stream().filter(p -> p instanceof MapperPlugin).map(p -> (MapperPlugin) p).collect(toList()) ).getMapperRegistry(); From cc6b8e57dc82c532a5a163790a79c6d273f9a75c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 17 Sep 2020 13:41:54 +0100 Subject: [PATCH 3/4] formatting --- .../org/elasticsearch/index/mapper/DateFieldMapper.java | 3 +-- .../java/org/elasticsearch/index/mapper/IpFieldMapper.java | 6 ++---- .../org/elasticsearch/index/mapper/IpFieldMapperTests.java | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index df145aa71ac4b..d8d340d489f8f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -238,8 +238,7 @@ private Long parseNullValue(DateFieldType fieldType) { } try { return fieldType.parse(nullValue.getValue()); - } - catch (Exception e) { + } catch (Exception e) { if (indexCreatedVersion.onOrAfter(Version.V_8_0_0)) { throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); } else { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index dc4e0da8af6ec..3fdf002be374f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -99,12 +99,10 @@ private InetAddress parseNullValue() { } try { return InetAddresses.forString(nullValueAsString); - } - catch (Exception e) { + } catch (Exception e) { if (indexCreatedVersion.onOrAfter(Version.V_8_0_0)) { throw new MapperParsingException("Error parsing [null_value] on field [" + name() + "]: " + e.getMessage(), e); - } - else { + } else { DEPRECATION_LOGGER.deprecate("ip_mapper_null_field", "Error parsing [" + nullValue.getValue() + "] as IP in [null_value] on field [" + name() + "]); [null_value] will be ignored"); return null; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 1b170bbc2cebe..b8ec6745101c4 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -177,7 +177,7 @@ public void testNullValue() throws IOException { doc = mapper.parse(source(b -> b.nullField("field"))); assertArrayEquals(new IndexableField[0], doc.rootDoc().getFields("field")); - + MapperParsingException e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(Version.CURRENT, fieldMapping(b -> { b.field("type", "ip"); From 6d995ee5cb64f0e89779c2cbb62cde65826b516a Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 17 Sep 2020 14:37:14 +0100 Subject: [PATCH 4/4] fix settings for base testcase --- .../org/elasticsearch/index/mapper/MapperServiceTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index ff9aa896acfb1..f6a1768ebb603 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -73,7 +73,7 @@ protected Collection getPlugins() { } protected Settings getIndexSettings() { - return Settings.EMPTY; + return SETTINGS; } protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {