From 902be2b6f383c9049f3924ac822efd888f8714ce Mon Sep 17 00:00:00 2001 From: Mikita Karaliou Date: Sat, 6 Jan 2018 18:26:34 +0300 Subject: [PATCH 1/3] support only string format for date, root object & date range --- .../index/mapper/TypeParsers.java | 5 +++- .../index/mapper/DateFieldMapperTests.java | 18 +++++++++++++ .../index/mapper/RangeFieldMapperTests.java | 18 +++++++++++++ .../index/mapper/RootObjectMapperTests.java | 27 +++++++++++++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 37fd1203622b1..916c3824cb345 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -297,7 +297,10 @@ private static IndexOptions nodeIndexOptionValue(final Object propNode) { } public static FormatDateTimeFormatter parseDateTimeFormatter(Object node) { - return Joda.forPattern(node.toString()); + if (node instanceof String) { + return Joda.forPattern((String) node); + } + throw new IllegalArgumentException("Invalid format: [" + node.toString() + "]: expected string value"); } public static void parseTermVector(String fieldName, String termVector, FieldMapper.Builder builder) throws MapperParsingException { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index 5776e9d618e3b..600aa7cfe258a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -411,4 +411,22 @@ public void testMergeText() throws Exception { () -> mapper.merge(update.mapping(), randomBoolean())); assertEquals("mapper [date] of different type, current_type [date], merged_type [text]", e.getMessage()); } + + public void testIllegalFormatField() throws Exception { + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field") + .field("type", "date") + .array("format", "test_format") + .endObject() + .endObject() + .endObject() + .endObject().string(); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> parser.parse("type", new CompressedXContent(mapping))); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index eea71525c705c..2e972eaf4e816 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.InternalSettingsPlugin; +import org.junit.Test; import java.io.IOException; import java.net.InetAddress; @@ -432,4 +433,21 @@ public void testSerializeDefaults() throws Exception { } } + public void testIllegalFormatField() throws Exception { + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field") + .field("type", "date_range") + .array("format", "test_format") + .endObject() + .endObject() + .endObject() + .endObject().string(); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> parser.parse("type", new CompressedXContent(mapping))); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java index a76d5d01316fb..dc8ab3fbed2e3 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.MapperService.MergeReason; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.junit.Before; import java.util.Arrays; @@ -158,4 +159,30 @@ public void testDynamicTemplates() throws Exception { mapper = mapperService.merge("type", new CompressedXContent(mapping3), MergeReason.MAPPING_UPDATE, false); assertEquals(mapping3, mapper.mappingSource().toString()); } + + public void testIllegalFormatField() throws Exception { + String dynamicMapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startArray("dynamic_date_formats") + .startArray().value("test_format").endArray() + .endArray() + .endObject() + .endObject().string(); + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startArray("date_formats") + .startArray().value("test_format").endArray() + .endArray() + .endObject() + .endObject().string(); + + DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + for (String m : Arrays.asList(mapping, dynamicMapping)) { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> parser.parse("type", new CompressedXContent(m))); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + } + } } From 4d41cf83955f96ef499f9b13ea002ffb0b0b281b Mon Sep 17 00:00:00 2001 From: Mikita Karaliou Date: Sun, 29 Jul 2018 17:42:56 +0300 Subject: [PATCH 2/3] change proto of parseDateTimeFormatter to support only Strings --- .../org/elasticsearch/index/mapper/DateFieldMapper.java | 2 +- .../org/elasticsearch/index/mapper/RangeFieldMapper.java | 2 +- .../org/elasticsearch/index/mapper/RootObjectMapper.java | 7 +++++-- .../java/org/elasticsearch/index/mapper/TypeParsers.java | 7 ++----- .../elasticsearch/index/mapper/DateFieldMapperTests.java | 2 +- .../elasticsearch/index/mapper/RangeFieldMapperTests.java | 2 +- .../elasticsearch/index/mapper/RootObjectMapperTests.java | 2 +- 7 files changed, 12 insertions(+), 12 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 c8360e468d725..e3bc2ce8bb552 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -162,7 +162,7 @@ public Mapper.Builder parse(String name, Map node, ParserCo builder.locale(LocaleUtils.parse(propNode.toString())); iterator.remove(); } else if (propName.equals("format")) { - builder.dateTimeFormatter(parseDateTimeFormatter(propNode)); + builder.dateTimeFormatter(parseDateTimeFormatter(XContentMapValues.nodeStringValue(propNode, null))); iterator.remove(); } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { iterator.remove(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java index 4c356c3a5592d..131b8cc402647 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java @@ -189,7 +189,7 @@ public Mapper.Builder parse(String name, Map node, builder.locale(LocaleUtils.parse(propNode.toString())); iterator.remove(); } else if (propName.equals("format")) { - builder.dateTimeFormatter(parseDateTimeFormatter(propNode)); + builder.dateTimeFormatter(parseDateTimeFormatter(XContentMapValues.nodeStringValue(propNode, null))); iterator.remove(); } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { iterator.remove(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java index 009caf2b8e814..2d61e5e796ab1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType; import java.io.IOException; @@ -145,13 +146,15 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam if (formatter.toString().startsWith("epoch_")) { throw new MapperParsingException("Epoch ["+ formatter +"] is not supported as dynamic date format"); } - formatters.add(parseDateTimeFormatter(formatter)); + formatters.add(parseDateTimeFormatter(XContentMapValues + .nodeStringValue(formatter, null))); } builder.dynamicDateTimeFormatter(formatters); } else if ("none".equals(fieldNode.toString())) { builder.dynamicDateTimeFormatter(Collections.emptyList()); } else { - builder.dynamicDateTimeFormatter(Collections.singleton(parseDateTimeFormatter(fieldNode))); + builder.dynamicDateTimeFormatter(Collections.singleton(parseDateTimeFormatter(XContentMapValues + .nodeStringValue(fieldNode, null)))); } return true; } else if (fieldName.equals("dynamic_templates")) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 5354ac14c40dd..54af864e9b5ba 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -264,11 +264,8 @@ private static IndexOptions nodeIndexOptionValue(final Object propNode) { } } - public static FormatDateTimeFormatter parseDateTimeFormatter(Object node) { - if (node instanceof String) { - return Joda.forPattern((String) node); - } - throw new IllegalArgumentException("Invalid format: [" + node.toString() + "]: expected string value"); + public static FormatDateTimeFormatter parseDateTimeFormatter(String node) { + return Joda.forPattern(node); } public static void parseTermVector(String fieldName, String termVector, FieldMapper.Builder builder) throws MapperParsingException { 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 d16bdc444e6e7..97a2312663178 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -430,6 +430,6 @@ public void testIllegalFormatField() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(mapping))); - assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index 00068f76e753d..d31d94d509186 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -458,7 +458,7 @@ public void testIllegalFormatField() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(mapping))); - assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java index 574d4eee70a00..c2d73cdfec593 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -182,7 +182,7 @@ public void testIllegalFormatField() throws Exception { for (String m : Arrays.asList(mapping, dynamicMapping)) { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(m))); - assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); } } } From cac0999e427e17b878e64d837b8d645bf8f86808 Mon Sep 17 00:00:00 2001 From: Mikita Karaliou Date: Sun, 29 Jul 2018 17:52:27 +0300 Subject: [PATCH 3/3] Revert "change proto of parseDateTimeFormatter to support only Strings" This reverts commit 4d41cf83955f96ef499f9b13ea002ffb0b0b281b. --- .../org/elasticsearch/index/mapper/DateFieldMapper.java | 2 +- .../org/elasticsearch/index/mapper/RangeFieldMapper.java | 2 +- .../org/elasticsearch/index/mapper/RootObjectMapper.java | 7 ++----- .../java/org/elasticsearch/index/mapper/TypeParsers.java | 7 +++++-- .../elasticsearch/index/mapper/DateFieldMapperTests.java | 2 +- .../elasticsearch/index/mapper/RangeFieldMapperTests.java | 2 +- .../elasticsearch/index/mapper/RootObjectMapperTests.java | 2 +- 7 files changed, 12 insertions(+), 12 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 e3bc2ce8bb552..c8360e468d725 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -162,7 +162,7 @@ public Mapper.Builder parse(String name, Map node, ParserCo builder.locale(LocaleUtils.parse(propNode.toString())); iterator.remove(); } else if (propName.equals("format")) { - builder.dateTimeFormatter(parseDateTimeFormatter(XContentMapValues.nodeStringValue(propNode, null))); + builder.dateTimeFormatter(parseDateTimeFormatter(propNode)); iterator.remove(); } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { iterator.remove(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java index 131b8cc402647..4c356c3a5592d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java @@ -189,7 +189,7 @@ public Mapper.Builder parse(String name, Map node, builder.locale(LocaleUtils.parse(propNode.toString())); iterator.remove(); } else if (propName.equals("format")) { - builder.dateTimeFormatter(parseDateTimeFormatter(XContentMapValues.nodeStringValue(propNode, null))); + builder.dateTimeFormatter(parseDateTimeFormatter(propNode)); iterator.remove(); } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { iterator.remove(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java index 2d61e5e796ab1..009caf2b8e814 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType; import java.io.IOException; @@ -146,15 +145,13 @@ protected boolean processField(RootObjectMapper.Builder builder, String fieldNam if (formatter.toString().startsWith("epoch_")) { throw new MapperParsingException("Epoch ["+ formatter +"] is not supported as dynamic date format"); } - formatters.add(parseDateTimeFormatter(XContentMapValues - .nodeStringValue(formatter, null))); + formatters.add(parseDateTimeFormatter(formatter)); } builder.dynamicDateTimeFormatter(formatters); } else if ("none".equals(fieldNode.toString())) { builder.dynamicDateTimeFormatter(Collections.emptyList()); } else { - builder.dynamicDateTimeFormatter(Collections.singleton(parseDateTimeFormatter(XContentMapValues - .nodeStringValue(fieldNode, null)))); + builder.dynamicDateTimeFormatter(Collections.singleton(parseDateTimeFormatter(fieldNode))); } return true; } else if (fieldName.equals("dynamic_templates")) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 54af864e9b5ba..5354ac14c40dd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -264,8 +264,11 @@ private static IndexOptions nodeIndexOptionValue(final Object propNode) { } } - public static FormatDateTimeFormatter parseDateTimeFormatter(String node) { - return Joda.forPattern(node); + public static FormatDateTimeFormatter parseDateTimeFormatter(Object node) { + if (node instanceof String) { + return Joda.forPattern((String) node); + } + throw new IllegalArgumentException("Invalid format: [" + node.toString() + "]: expected string value"); } public static void parseTermVector(String fieldName, String termVector, FieldMapper.Builder builder) throws MapperParsingException { 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 97a2312663178..d16bdc444e6e7 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -430,6 +430,6 @@ public void testIllegalFormatField() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(mapping))); - assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index d31d94d509186..00068f76e753d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -458,7 +458,7 @@ public void testIllegalFormatField() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(mapping))); - assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java index c2d73cdfec593..574d4eee70a00 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -182,7 +182,7 @@ public void testIllegalFormatField() throws Exception { for (String m : Arrays.asList(mapping, dynamicMapping)) { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parser.parse("type", new CompressedXContent(m))); - assertEquals("Invalid format: [[test_format]]: Illegal pattern component: t", e.getMessage()); + assertEquals("Invalid format: [[test_format]]: expected string value", e.getMessage()); } } }