From 31b488d08fa93ba268a4406ef86ef5d2129a695d Mon Sep 17 00:00:00 2001 From: Sascha Woo Date: Wed, 17 Mar 2021 22:39:12 +0100 Subject: [PATCH] Allow multiple date formats for date fields. Original Pull Request #1728 Closes #1727 --- .../elasticsearch-object-mapping.adoc | 12 +-- .../elasticsearch/annotations/DateFormat.java | 12 +++ .../data/elasticsearch/annotations/Field.java | 5 +- .../elasticsearch/annotations/InnerField.java | 4 +- .../core/index/MappingParameters.java | 36 +++++++-- ...SimpleElasticsearchPersistentProperty.java | 80 +++++++++++-------- .../core/CriteriaQueryMappingUnitTests.java | 4 +- .../ElasticsearchDateConverterUnitTests.java | 15 +++- ...appingElasticsearchConverterUnitTests.java | 4 +- .../core/index/MappingBuilderUnitTests.java | 2 +- .../SimpleElasticsearchDateMappingTests.java | 4 +- ...sticsearchPersistentPropertyUnitTests.java | 6 +- 12 files changed, 123 insertions(+), 61 deletions(-) diff --git a/src/main/asciidoc/reference/elasticsearch-object-mapping.adoc b/src/main/asciidoc/reference/elasticsearch-object-mapping.adoc index e39f15358..f32dd1f97 100644 --- a/src/main/asciidoc/reference/elasticsearch-object-mapping.adoc +++ b/src/main/asciidoc/reference/elasticsearch-object-mapping.adoc @@ -56,16 +56,18 @@ Default value is _EXTERNAL_. Constructor arguments are mapped by name to the key values in the retrieved Document. * `@Field`: Applied at the field level and defines properties of the field, most of the attributes map to the respective https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping.html[Elasticsearch Mapping] definitions (the following list is not complete, check the annotation Javadoc for a complete reference): ** `name`: The name of the field as it will be represented in the Elasticsearch document, if not set, the Java field name is used. -** `type`: the field type, can be one of _Text, Keyword, Long, Integer, Short, Byte, Double, Float, Half_Float, Scaled_Float, Date, Date_Nanos, Boolean, Binary, Integer_Range, Float_Range, Long_Range, Double_Range, Date_Range, Ip_Range, Object, Nested, Ip, TokenCount, Percolator, Flattened, Search_As_You_Type_. +** `type`: The field type, can be one of _Text, Keyword, Long, Integer, Short, Byte, Double, Float, Half_Float, Scaled_Float, Date, Date_Nanos, Boolean, Binary, Integer_Range, Float_Range, Long_Range, Double_Range, Date_Range, Ip_Range, Object, Nested, Ip, TokenCount, Percolator, Flattened, Search_As_You_Type_. See https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html[Elasticsearch Mapping Types] -** `format` and `pattern` definitions for the _Date_ type. +** `format`: One or more built-in formats, default value is _strict_date_optional_time_ and _epoch_millis_. +See https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-date-format.html#built-in-date-formats[Elasticsearch Built In Formats] +** `pattern`: One or more custom date formats. NOTE: If you want to use only custom date formats, you must set the `format` property to empty `{}`. +See https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-date-format.html#custom-date-formats[Elasticsearch Custom Date Formats] ** `store`: Flag whether the original field value should be store in Elasticsearch, default value is _false_. ** `analyzer`, `searchAnalyzer`, `normalizer` for specifying custom analyzers and normalizer. -* `@GeoPoint`: marks a field as _geo_point_ datatype. +* `@GeoPoint`: Marks a field as _geo_point_ datatype. Can be omitted if the field is an instance of the `GeoPoint` class. -NOTE: Properties that derive from `TemporalAccessor` or are of type `java.util.Date` must either have a `@Field` annotation of type `FieldType.Date` and a -format different from `DateFormat.none` or a custom converter must be registered for this type. + +NOTE: Properties that derive from `TemporalAccessor` or are of type `java.util.Date` must either have a `@Field` annotation of type `FieldType.Date`. If you are using a custom date format, you need to use _uuuu_ for the year instead of _yyyy_. This is due to a https://www.elastic.co/guide/en/elasticsearch/reference/current/migrate-to-java-time.html#java-time-migration-incompatible-date-formats[change in Elasticsearch 7]. diff --git a/src/main/java/org/springframework/data/elasticsearch/annotations/DateFormat.java b/src/main/java/org/springframework/data/elasticsearch/annotations/DateFormat.java index 8ee7b7dcb..396dec451 100644 --- a/src/main/java/org/springframework/data/elasticsearch/annotations/DateFormat.java +++ b/src/main/java/org/springframework/data/elasticsearch/annotations/DateFormat.java @@ -23,9 +23,21 @@ package org.springframework.data.elasticsearch.annotations; * @author Jakub Vavrik * @author Tim te Beek * @author Peter-Josef Meisch + * @author Sascha Woo */ public enum DateFormat { + /** + * @deprecated since 4.2, will be removed in a future version. Use format = {} to disable built-in date + * formats in the @Field annotation. + */ + @Deprecated none(""), // + /** + * @deprecated since 4.2, will be removed in a future version.It is no longer required for using a custom date format + * pattern. If you want to use only a custom date format pattern, you must set the format + * property to empty {}. + */ + @Deprecated custom(""), // basic_date("uuuuMMdd"), // basic_date_time("uuuuMMdd'T'HHmmss.SSSXXX"), // diff --git a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java index b97b243ad..be5ae13f5 100644 --- a/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java +++ b/src/main/java/org/springframework/data/elasticsearch/annotations/Field.java @@ -36,6 +36,7 @@ import org.springframework.core.annotation.AliasFor; * @author Aleksei Arsenev * @author Brian Kimmig * @author Morgan Lutz + * @author Sascha Woo */ @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.FIELD, ElementType.ANNOTATION_TYPE }) @@ -65,9 +66,9 @@ public @interface Field { boolean index() default true; - DateFormat format() default DateFormat.none; + DateFormat[] format() default { DateFormat.date_optional_time, DateFormat.epoch_millis }; - String pattern() default ""; + String[] pattern() default {}; boolean store() default false; diff --git a/src/main/java/org/springframework/data/elasticsearch/annotations/InnerField.java b/src/main/java/org/springframework/data/elasticsearch/annotations/InnerField.java index c2d8b803f..8ba0dca61 100644 --- a/src/main/java/org/springframework/data/elasticsearch/annotations/InnerField.java +++ b/src/main/java/org/springframework/data/elasticsearch/annotations/InnerField.java @@ -40,9 +40,9 @@ public @interface InnerField { boolean index() default true; - DateFormat format() default DateFormat.none; + DateFormat[] format() default { DateFormat.date_optional_time, DateFormat.epoch_millis }; - String pattern() default ""; + String[] pattern() default {}; boolean store() default false; diff --git a/src/main/java/org/springframework/data/elasticsearch/core/index/MappingParameters.java b/src/main/java/org/springframework/data/elasticsearch/core/index/MappingParameters.java index 6d7d7104a..38bdbe7e7 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/index/MappingParameters.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/index/MappingParameters.java @@ -17,6 +17,9 @@ package org.springframework.data.elasticsearch.core.index; import java.io.IOException; import java.lang.annotation.Annotation; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -41,6 +44,7 @@ import org.springframework.util.StringUtils; * @author Aleksei Arsenev * @author Brian Kimmig * @author Morgan Lutz + * @author Sascha Woo * @since 4.0 */ public final class MappingParameters { @@ -78,12 +82,12 @@ public final class MappingParameters { private final String analyzer; private final boolean coerce; @Nullable private final String[] copyTo; - private final String datePattern; + private final DateFormat[] dateFormats; + private final String[] dateFormatPatterns; private final boolean docValues; private final boolean eagerGlobalOrdinals; private final boolean enabled; private final boolean fielddata; - private final DateFormat format; @Nullable private final Integer ignoreAbove; private final boolean ignoreMalformed; private final boolean index; @@ -129,8 +133,8 @@ public final class MappingParameters { store = field.store(); fielddata = field.fielddata(); type = field.type(); - format = field.format(); - datePattern = field.pattern(); + dateFormats = field.format(); + dateFormatPatterns = field.pattern(); analyzer = field.analyzer(); searchAnalyzer = field.searchAnalyzer(); normalizer = field.normalizer(); @@ -171,8 +175,8 @@ public final class MappingParameters { store = field.store(); fielddata = field.fielddata(); type = field.type(); - format = field.format(); - datePattern = field.pattern(); + dateFormats = field.format(); + dateFormatPatterns = field.pattern(); analyzer = field.analyzer(); searchAnalyzer = field.searchAnalyzer(); normalizer = field.normalizer(); @@ -226,8 +230,24 @@ public final class MappingParameters { if (type != FieldType.Auto) { builder.field(FIELD_PARAM_TYPE, type.name().toLowerCase()); - if (type == FieldType.Date && format != DateFormat.none) { - builder.field(FIELD_PARAM_FORMAT, format == DateFormat.custom ? datePattern : format.toString()); + + if (type == FieldType.Date) { + List formats = new ArrayList<>(); + + // built-in formats + for (DateFormat dateFormat : dateFormats) { + if (dateFormat == DateFormat.none || dateFormat == DateFormat.custom) { + continue; + } + formats.add(dateFormat.toString()); + } + + // custom date formats + Collections.addAll(formats, dateFormatPatterns); + + if (!formats.isEmpty()) { + builder.field(FIELD_PARAM_FORMAT, String.join("||", formats)); + } } } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java index 681dc05a1..0ece1ed5b 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java @@ -16,6 +16,7 @@ package org.springframework.data.elasticsearch.core.mapping; import java.time.temporal.TemporalAccessor; +import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.List; @@ -154,51 +155,74 @@ public class SimpleElasticsearchPersistentProperty extends if (field != null && (field.type() == FieldType.Date || field.type() == FieldType.Date_Nanos) && (isTemporalAccessor || isDate)) { - DateFormat dateFormat = field.format(); + + DateFormat[] dateFormats = field.format(); + String[] dateFormatPatterns = field.pattern(); String property = getOwner().getType().getSimpleName() + "." + getName(); - if (dateFormat == DateFormat.none) { + if (dateFormats.length == 0 && dateFormatPatterns.length == 0) { LOGGER.warn( - String.format("No DateFormat defined for property %s. Make sure you have a Converter registered for %s", - property, actualType.getSimpleName())); + "Property '{}' has @Field type '{}' but has no built-in format or custom date pattern defined. Make sure you have a converter registered for type {}.", + property, field.type().name(), actualType.getSimpleName()); return; } - ElasticsearchDateConverter converter = null; - - if (dateFormat == DateFormat.custom) { - String pattern = field.pattern(); - - if (!StringUtils.hasLength(pattern)) { - throw new MappingException( - String.format("Property %s is annotated with FieldType.%s and a custom format but has no pattern defined", - property, field.type().name())); - } - - converter = ElasticsearchDateConverter.of(pattern); - } else { + List converters = new ArrayList<>(); + // register converters for built-in formats + for (DateFormat dateFormat : dateFormats) { switch (dateFormat) { + case none: + case custom: + break; case weekyear: case weekyear_week: case weekyear_week_day: - LOGGER.warn("no Converter available for " + actualType.getName() + " and date format " + dateFormat.name() - + ". Use a custom converter instead"); + LOGGER.warn("No default converter available for '{}' and date format '{}'. Use a custom converter instead.", + actualType.getName(), dateFormat.name()); break; default: - converter = ElasticsearchDateConverter.of(dateFormat); + converters.add(ElasticsearchDateConverter.of(dateFormat)); break; } } - if (converter != null) { - ElasticsearchDateConverter finalConverter = converter; + // register converters for custom formats + for (String dateFormatPattern : dateFormatPatterns) { + if (!StringUtils.hasText(dateFormatPattern)) { + throw new MappingException( + String.format("Date pattern of property '%s' must not be empty", property)); + } + converters.add(ElasticsearchDateConverter.of(dateFormatPattern)); + } + + if (!converters.isEmpty()) { propertyConverter = new ElasticsearchPersistentPropertyConverter() { - final ElasticsearchDateConverter dateConverter = finalConverter; + final List dateConverters = converters; + + @SuppressWarnings("unchecked") + @Override + public Object read(String s) { + for (ElasticsearchDateConverter dateConverter : dateConverters) { + try { + if (isTemporalAccessor) { + return dateConverter.parse(s, (Class) actualType); + } else { // must be date + return dateConverter.parse(s); + } + } catch (Exception e) { + LOGGER.trace(e.getMessage(), e); + } + } + + throw new RuntimeException(String + .format("Unable to parse date value '%s' of property '%s' with configured converters", s, property)); + } @Override public String write(Object property) { + ElasticsearchDateConverter dateConverter = dateConverters.get(0); if (isTemporalAccessor && TemporalAccessor.class.isAssignableFrom(property.getClass())) { return dateConverter.format((TemporalAccessor) property); } else if (isDate && Date.class.isAssignableFrom(property.getClass())) { @@ -207,16 +231,6 @@ public class SimpleElasticsearchPersistentProperty extends return property.toString(); } } - - @SuppressWarnings("unchecked") - @Override - public Object read(String s) { - if (isTemporalAccessor) { - return dateConverter.parse(s, (Class) actualType); - } else { // must be date - return dateConverter.parse(s); - } - } }; } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java index 46ba47dba..c3e65e627 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java @@ -48,6 +48,7 @@ import org.springframework.lang.Nullable; * {@link CriteriaQueryProcessor} as this is needed to get the String representation to assert. * * @author Peter-Josef Meisch + * @author Sascha Woo */ public class CriteriaQueryMappingUnitTests { @@ -346,8 +347,7 @@ public class CriteriaQueryMappingUnitTests { @Nullable @Field(name = "first-name") String firstName; @Nullable @Field(name = "last-name") String lastName; @Nullable @Field(name = "created-date", type = FieldType.Date, format = DateFormat.epoch_millis) Date createdDate; - @Nullable @Field(name = "birth-date", type = FieldType.Date, format = DateFormat.custom, - pattern = "dd.MM.uuuu") LocalDate birthDate; + @Nullable @Field(name = "birth-date", type = FieldType.Date, format = {}, pattern = "dd.MM.uuuu") LocalDate birthDate; } static class GeoShapeEntity { diff --git a/src/test/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchDateConverterUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchDateConverterUnitTests.java index 673e3d22f..839db6cf8 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchDateConverterUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchDateConverterUnitTests.java @@ -23,6 +23,7 @@ import org.springframework.data.elasticsearch.annotations.DateFormat; /** * @author Peter-Josef Meisch * @author Tim te Beek + * @author Sascha Woo */ class ElasticsearchDateConverterUnitTests { @@ -34,16 +35,28 @@ class ElasticsearchDateConverterUnitTests { switch (dateFormat) { case none: + case custom: case weekyear: case weekyear_week: case weekyear_week_day: return; } - String pattern = (dateFormat != DateFormat.custom) ? dateFormat.name() : "dd.MM.uuuu"; + ElasticsearchDateConverter converter = ElasticsearchDateConverter.of(dateFormat.name()); + assertThat(converter).isNotNull(); + } + + @Test // DATAES-716 + void shouldCreateConvertersForDateFormatPattern() { + + // given + String pattern = "dd.MM.uuuu"; + + // when ElasticsearchDateConverter converter = ElasticsearchDateConverter.of(pattern); + // then assertThat(converter).isNotNull(); } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java index ae5822ba9..8d20208a8 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java @@ -83,6 +83,7 @@ import org.springframework.lang.Nullable; * @author Peter-Josef Meisch * @author Konrad Kurdej * @author Roman Puchkovskiy + * @author Sascha Woo */ public class MappingElasticsearchConverterUnitTests { @@ -1218,8 +1219,7 @@ public class MappingElasticsearchConverterUnitTests { String name; @Field(name = "first-name") String firstName; @Field(name = "last-name") String lastName; - @Field(name = "birth-date", type = FieldType.Date, format = DateFormat.custom, - pattern = "dd.MM.uuuu") LocalDate birthDate; + @Field(name = "birth-date", type = FieldType.Date, format = {}, pattern = "dd.MM.uuuu") LocalDate birthDate; Gender gender; Address address; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java index aa46971ca..78db40292 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/MappingBuilderUnitTests.java @@ -921,7 +921,7 @@ public class MappingBuilderUnitTests extends MappingContextBaseTests { @Nullable @Field(copyTo = { "foo", "bar" }) private String copyTo; @Nullable @Field(ignoreAbove = 42) private String ignoreAbove; @Nullable @Field(type = FieldType.Integer) private String type; - @Nullable @Field(type = FieldType.Date, format = DateFormat.custom, pattern = "YYYYMMDD") private LocalDate date; + @Nullable @Field(type = FieldType.Date, format = {}, pattern = "YYYYMMDD") private LocalDate date; @Nullable @Field(analyzer = "ana", searchAnalyzer = "sana", normalizer = "norma") private String analyzers; @Nullable @Field(type = Keyword) private String docValuesTrue; @Nullable @Field(type = Keyword, docValues = false) private String docValuesFalse; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java index 4349b8572..f8ab2aed0 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/index/SimpleElasticsearchDateMappingTests.java @@ -36,6 +36,7 @@ import org.springframework.data.elasticsearch.core.MappingContextBaseTests; * @author Mohsin Husen * @author Don Wellington * @author Peter-Josef Meisch + * @author Sascha Woo */ public class SimpleElasticsearchDateMappingTests extends MappingContextBaseTests { @@ -62,8 +63,7 @@ public class SimpleElasticsearchDateMappingTests extends MappingContextBaseTests @Field(type = Text, index = false, store = true, analyzer = "standard") private String message; - @Field(type = Date, format = DateFormat.custom, - pattern = "dd.MM.uuuu hh:mm") private LocalDateTime customFormatDate; + @Field(type = Date, format = {}, pattern = "dd.MM.uuuu hh:mm") private LocalDateTime customFormatDate; @Field(type = FieldType.Date, format = DateFormat.basic_date) private LocalDateTime basicFormatDate; } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentPropertyUnitTests.java index 698978da0..46f529aba 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentPropertyUnitTests.java @@ -50,6 +50,7 @@ import org.springframework.util.ReflectionUtils; * @author Oliver Gierke * @author Peter-Josef Meisch * @author Roman Puchkovskiy + * @author Sascha Woo */ public class SimpleElasticsearchPersistentPropertyUnitTests { @@ -257,11 +258,10 @@ public class SimpleElasticsearchPersistentPropertyUnitTests { } static class DatesProperty { - @Nullable @Field(type = FieldType.Date, format = DateFormat.custom, pattern = "dd.MM.uuuu") LocalDate localDate; + @Nullable @Field(type = FieldType.Date, format = {}, pattern = "dd.MM.uuuu") LocalDate localDate; @Nullable @Field(type = FieldType.Date, format = DateFormat.basic_date_time) LocalDateTime localDateTime; @Nullable @Field(type = FieldType.Date, format = DateFormat.basic_date_time) Date legacyDate; - @Nullable @Field(type = FieldType.Date, format = DateFormat.custom, - pattern = "dd.MM.uuuu") List localDateList; + @Nullable @Field(type = FieldType.Date, format = {}, pattern = "dd.MM.uuuu") List localDateList; } @Data