diff --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java index 8a4d4f43f..e5cd5760b 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java @@ -16,18 +16,10 @@ package org.springframework.data.elasticsearch.core.convert; import java.time.temporal.TemporalAccessor; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -281,8 +273,8 @@ public class MappingElasticsearchConverter Class rawType = targetType.getType(); - if (property.hasPropertyConverter() && String.class.isAssignableFrom(source.getClass())) { - source = property.getPropertyConverter().read((String) source); + if (property.hasPropertyConverter()) { + source = propertyConverterRead(property, source); } else if (TemporalAccessor.class.isAssignableFrom(property.getType()) && !conversions.hasCustomReadTarget(source.getClass(), rawType)) { @@ -310,6 +302,32 @@ public class MappingElasticsearchConverter return (R) readSimpleValue(source, targetType); } + private Object propertyConverterRead(ElasticsearchPersistentProperty property, Object source) { + ElasticsearchPersistentPropertyConverter propertyConverter = Objects + .requireNonNull(property.getPropertyConverter()); + + if (source instanceof String[]) { + // convert to a List + source = Arrays.asList((String[]) source); + } + + if (source instanceof List) { + source = ((List) source).stream().map(it -> convertOnRead(propertyConverter, it)).collect(Collectors.toList()); + } else if (source instanceof Set) { + source = ((Set) source).stream().map(it -> convertOnRead(propertyConverter, it)).collect(Collectors.toSet()); + } else { + source = convertOnRead(propertyConverter, source); + } + return source; + } + + private Object convertOnRead(ElasticsearchPersistentPropertyConverter propertyConverter, Object source) { + if (String.class.isAssignableFrom(source.getClass())) { + source = propertyConverter.read((String) source); + } + return source; + } + @SuppressWarnings("unchecked") @Nullable private R readCollectionValue(@Nullable List source, ElasticsearchPersistentProperty property, @@ -506,9 +524,8 @@ public class MappingElasticsearchConverter } if (property.hasPropertyConverter()) { - ElasticsearchPersistentPropertyConverter propertyConverter = property.getPropertyConverter(); - value = propertyConverter.write(value); - } else if (TemporalAccessor.class.isAssignableFrom(property.getType()) + value = propertyConverterWrite(property, value); + } else if (TemporalAccessor.class.isAssignableFrom(property.getActualType()) && !conversions.hasCustomWriteTarget(value.getClass())) { // log at most 5 times @@ -535,6 +552,20 @@ public class MappingElasticsearchConverter } } + private Object propertyConverterWrite(ElasticsearchPersistentProperty property, Object value) { + ElasticsearchPersistentPropertyConverter propertyConverter = Objects + .requireNonNull(property.getPropertyConverter()); + + if (value instanceof List) { + value = ((List) value).stream().map(propertyConverter::write).collect(Collectors.toList()); + } else if (value instanceof Set) { + value = ((Set) value).stream().map(propertyConverter::write).collect(Collectors.toSet()); + } else { + value = propertyConverter.write(value); + } + return value; + } + protected void writeProperty(ElasticsearchPersistentProperty property, Object value, MapValueAccessor sink) { Optional> customWriteTarget = conversions.getCustomWriteTarget(value.getClass()); 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 6adf747e7..50665af2a 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 @@ -64,7 +64,7 @@ public class SimpleElasticsearchPersistentProperty extends private final boolean isSeqNoPrimaryTerm; private final @Nullable String annotatedFieldName; @Nullable private ElasticsearchPersistentPropertyConverter propertyConverter; - private boolean storeNullValue; + private final boolean storeNullValue; public SimpleElasticsearchPersistentProperty(Property property, PersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { @@ -143,8 +143,9 @@ public class SimpleElasticsearchPersistentProperty extends */ private void initDateConverter() { Field field = findAnnotation(Field.class); - boolean isTemporalAccessor = TemporalAccessor.class.isAssignableFrom(getType()); - boolean isDate = Date.class.isAssignableFrom(getType()); + Class actualType = getActualType(); + boolean isTemporalAccessor = TemporalAccessor.class.isAssignableFrom(actualType); + boolean isDate = Date.class.isAssignableFrom(actualType); if (field != null && (field.type() == FieldType.Date || field.type() == FieldType.Date_Nanos) && (isTemporalAccessor || isDate)) { @@ -156,44 +157,50 @@ public class SimpleElasticsearchPersistentProperty extends getOwner().getType().getSimpleName() + "." + getName(), field.type().name())); } - ElasticsearchDateConverter converter = null; + ElasticsearchDateConverter converter; if (dateFormat == DateFormat.custom) { String pattern = field.pattern(); - if (StringUtils.hasLength(pattern)) { - converter = ElasticsearchDateConverter.of(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", + getOwner().getType().getSimpleName() + "." + getName(), field.type().name())); } - } else if (dateFormat != DateFormat.none) { + + converter = ElasticsearchDateConverter.of(pattern); + } else { converter = ElasticsearchDateConverter.of(dateFormat); } - if (converter != null) { - ElasticsearchDateConverter dateConverter = converter; - propertyConverter = new ElasticsearchPersistentPropertyConverter() { - @Override - public String write(Object property) { - if (isTemporalAccessor) { - return dateConverter.format((TemporalAccessor) property); - } else { // must be Date - return dateConverter.format((Date) property); - } - } + propertyConverter = new ElasticsearchPersistentPropertyConverter() { + final ElasticsearchDateConverter dateConverter = converter; - @SuppressWarnings("unchecked") - @Override - public Object read(String s) { - if (isTemporalAccessor) { - return dateConverter.parse(s, (Class) getType()); - } else { // must be date - return dateConverter.parse(s); - } + @Override + public String write(Object property) { + if (isTemporalAccessor && TemporalAccessor.class.isAssignableFrom(property.getClass())) { + return dateConverter.format((TemporalAccessor) property); + } else if (isDate && Date.class.isAssignableFrom(property.getClass())) { + return dateConverter.format((Date) property); + } else { + 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); + } + } + }; } } + @SuppressWarnings("ConstantConditions") @Nullable private String getAnnotatedFieldName() { 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 71e710445..c66e65cfb 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 @@ -26,6 +26,7 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.Setter; import java.time.LocalDate; import java.util.ArrayList; @@ -617,6 +618,25 @@ public class MappingElasticsearchConverterUnitTests { assertEquals(expected, json, false); } + @Test // DATAES-924 + @DisplayName("should write list of LocalDate") + void shouldWriteListOfLocalDate() throws JSONException { + + LocalDatesEntity entity = new LocalDatesEntity(); + entity.setId("4711"); + entity.setDates(Arrays.asList(LocalDate.of(2020, 9, 15), LocalDate.of(2019, 5, 1))); + String expected = "{\n" + // + " \"id\": \"4711\",\n" + // + " \"dates\": [\"15.09.2020\", \"01.05.2019\"]\n" + // + "}\n"; // + + Document document = Document.create(); + mappingElasticsearchConverter.write(entity, document); + String json = document.toJson(); + + assertEquals(expected, json, false); + } + @Test // DATAES-716 void shouldReadLocalDate() { Document document = Document.create(); @@ -633,6 +653,20 @@ public class MappingElasticsearchConverterUnitTests { assertThat(person.getGender()).isEqualTo(Gender.MAN); } + @Test // DATAES-924 + @DisplayName("should read list of LocalDate") + void shouldReadListOfLocalDate() { + + Document document = Document.create(); + document.put("id", "4711"); + document.put("dates", new String[] { "15.09.2020", "01.05.2019" }); + + LocalDatesEntity entity = mappingElasticsearchConverter.read(LocalDatesEntity.class, document); + + assertThat(entity.getId()).isEqualTo("4711"); + assertThat(entity.getDates()).hasSize(2).containsExactly(LocalDate.of(2020, 9, 15), LocalDate.of(2019, 5, 1)); + } + @Test // DATAES-763 void writeEntityWithMapDataType() { @@ -870,6 +904,15 @@ public class MappingElasticsearchConverterUnitTests { Map inventoryMap; } + @Data + @Getter + @Setter + static class LocalDatesEntity { + @Id private String id; + @Field(name = "dates", type = FieldType.Date, format = DateFormat.custom, + pattern = "dd.MM.uuuu") private List dates; + } + enum Gender { MAN("1"), MACHINE("0"); 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 186f3792b..1512f430c 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 @@ -17,13 +17,17 @@ package org.springframework.data.elasticsearch.core.mapping; import static org.assertj.core.api.Assertions.*; +import lombok.Data; + import java.time.LocalDate; import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.Date; import java.util.GregorianCalendar; +import java.util.List; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.data.elasticsearch.annotations.DateFormat; import org.springframework.data.elasticsearch.annotations.Field; @@ -86,7 +90,7 @@ public class SimpleElasticsearchPersistentPropertyUnitTests { assertThat(persistentProperty.getFieldName()).isEqualTo("mainfield"); } - @Test // DATAES-716, DATAES-792 + @Test // DATAES-716, DATAES-792, DATAES-924 void shouldSetPropertyConverters() { SimpleElasticsearchPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DatesProperty.class); @@ -102,6 +106,9 @@ public class SimpleElasticsearchPersistentPropertyUnitTests { assertThat(persistentProperty.hasPropertyConverter()).isTrue(); assertThat(persistentProperty.getPropertyConverter()).isNotNull(); + persistentProperty = persistentEntity.getRequiredPersistentProperty("localDateList"); + assertThat(persistentProperty.hasPropertyConverter()).isTrue(); + assertThat(persistentProperty.getPropertyConverter()).isNotNull(); } @Test // DATAES-716 @@ -199,6 +206,14 @@ public class SimpleElasticsearchPersistentPropertyUnitTests { .withMessageContaining("date"); } + @Test // DATAES-924 + @DisplayName("should require pattern for custom date format") + void shouldRequirePatternForCustomDateFormat() { + assertThatExceptionOfType(MappingException.class) // + .isThrownBy(() -> context.getRequiredPersistentEntity(DateFieldWithCustomFormatAndNoPattern.class)) // + .withMessageContaining("pattern"); + } + static class InvalidScoreProperty { @Nullable @Score String scoreProperty; } @@ -220,17 +235,27 @@ public class SimpleElasticsearchPersistentPropertyUnitTests { @Nullable @Field(type = FieldType.Date, format = DateFormat.custom, 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; } + @Data static class SeqNoPrimaryTermProperty { SeqNoPrimaryTerm seqNoPrimaryTerm; String string; } + @Data static class DateFieldWithNoFormat { @Field(type = FieldType.Date) LocalDateTime datetime; } + @Data + static class DateFieldWithCustomFormatAndNoPattern { + @Field(type = FieldType.Date, format = DateFormat.custom, pattern = "") LocalDateTime datetime; + } + + @Data static class DateNanosFieldWithNoFormat { @Field(type = FieldType.Date_Nanos) LocalDateTime datetime; }