From ad65dddfc41ef3ef590b210c3ac90d6eb5a5b34d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 12 Jul 2023 12:20:36 +0200 Subject: [PATCH 1/3] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8174e38303..329d09c475 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.2.0-SNAPSHOT + 3.2.0-GH-2719-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. From 8dd8fdecb12c6a1fc791c949840c636230161f0d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 12 Jul 2023 12:22:59 +0200 Subject: [PATCH 2/3] Introduce support to pass-thru TemporalAccessor auditing values. We now allow passing-thru TemporalAccessor auditing values, bypassing conversion if the target value type matches the value provided from e.g. DateTimeProvider. Refined the error messages and listing all commonly supported types for which we provide converters. Closes #2719 --- .../auditing/AnnotationAuditingMetadata.java | 10 +++- .../DefaultAuditableBeanWrapperFactory.java | 30 +++++------ .../MappingAuditableBeanWrapperFactory.java | 3 +- .../data/convert/Jsr310Converters.java | 10 +++- ...tAuditableBeanWrapperFactoryUnitTests.java | 50 ++++++++++++++++++- ...gAuditableBeanWrapperFactoryUnitTests.java | 29 +++++++++++ 6 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java index be839a8990..d885d0fc93 100644 --- a/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java +++ b/src/main/java/org/springframework/data/auditing/AnnotationAuditingMetadata.java @@ -16,6 +16,7 @@ package org.springframework.data.auditing; import java.lang.reflect.Field; +import java.time.temporal.TemporalAccessor; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -58,7 +59,12 @@ final class AnnotationAuditingMetadata { static { - List types = new ArrayList<>(3); + List types = new ArrayList<>(Jsr310Converters.getSupportedClasses() // + .stream() // + .filter(TemporalAccessor.class::isAssignableFrom) // + .map(Class::getName) // + .toList()); + types.add(Date.class.getName()); types.add(Long.class.getName()); types.add(long.class.getName()); @@ -104,7 +110,7 @@ private void assertValidDateFieldType(Optional field) { Class type = it.getType(); - if (Jsr310Converters.supports(type)) { + if (TemporalAccessor.class.isAssignableFrom(type)) { return; } diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index 020cbdc9a3..c427db1828 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -73,7 +73,8 @@ public Optional> getBeanWrapperFor(T source) { return Optional.of(source).map(it -> { if (it instanceof Auditable) { - return (AuditableBeanWrapper) new AuditableInterfaceBeanWrapper(conversionService, (Auditable) it); + return (AuditableBeanWrapper) new AuditableInterfaceBeanWrapper(conversionService, + (Auditable) it); } AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass()); @@ -98,7 +99,8 @@ static class AuditableInterfaceBeanWrapper private final Class type; @SuppressWarnings("unchecked") - public AuditableInterfaceBeanWrapper(ConversionService conversionService, Auditable auditable) { + public AuditableInterfaceBeanWrapper(ConversionService conversionService, + Auditable auditable) { super(conversionService); @@ -151,8 +153,8 @@ public TemporalAccessor setLastModifiedDate(TemporalAccessor value) { } /** - * Base class for {@link AuditableBeanWrapper} implementations that might need to convert {@link TemporalAccessor} values into - * compatible types when setting date/time information. + * Base class for {@link AuditableBeanWrapper} implementations that might need to convert {@link TemporalAccessor} + * values into compatible types when setting date/time information. * * @author Oliver Gierke * @since 1.8 @@ -168,7 +170,7 @@ abstract static class DateConvertingAuditableBeanWrapper implements Auditable /** * Returns the {@link TemporalAccessor} in a type, compatible to the given field. * - * @param value can be {@literal null}. + * @param value must not be {@literal null}. * @param targetType must not be {@literal null}. * @param source must not be {@literal null}. * @return @@ -176,7 +178,7 @@ abstract static class DateConvertingAuditableBeanWrapper implements Auditable @Nullable protected Object getDateValueToSet(TemporalAccessor value, Class targetType, Object source) { - if (TemporalAccessor.class.equals(targetType)) { + if (targetType.isInstance(value)) { return value; } @@ -188,7 +190,7 @@ protected Object getDateValueToSet(TemporalAccessor value, Class targetType, if (!conversionService.canConvert(value.getClass(), Date.class)) { throw new IllegalArgumentException( - String.format("Cannot convert date type for member %s; From %s to java.util.Date to %s", source, + String.format("Cannot convert date type for %s; From %s to java.util.Date to %s", source, value.getClass(), targetType)); } @@ -196,7 +198,7 @@ protected Object getDateValueToSet(TemporalAccessor value, Class targetType, return conversionService.convert(date, targetType); } - throw rejectUnsupportedType(source); + throw rejectUnsupportedType(value.getClass(), targetType); } /** @@ -217,19 +219,20 @@ protected Optional getAsTemporalAccessor(Optiona } Class typeToConvertTo = Stream.of(target, Instant.class)// - .filter(type -> target.isAssignableFrom(type))// + .filter(target::isAssignableFrom)// .filter(type -> conversionService.canConvert(it.getClass(), type))// .findFirst() // - .orElseThrow(() -> rejectUnsupportedType(source.map(Object.class::cast).orElseGet(() -> source))); + .orElseThrow(() -> rejectUnsupportedType(it.getClass(), target)); return (S) conversionService.convert(it, typeToConvertTo); }); } } - private static IllegalArgumentException rejectUnsupportedType(Object source) { - return new IllegalArgumentException(String.format("Invalid date type %s for member %s; Supported types are %s", - source.getClass(), source, AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES)); + private static IllegalArgumentException rejectUnsupportedType(Class sourceType, Class targetType) { + return new IllegalArgumentException( + String.format("Cannot convert unsupported date type %s to %s; Supported types are %s", sourceType.getName(), + targetType.getName(), AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES)); } /** @@ -264,7 +267,6 @@ public Object setCreatedBy(Object value) { @Override public TemporalAccessor setCreatedDate(TemporalAccessor value) { - return setDateField(metadata.getCreatedDateField(), value); } diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index 424de0631b..4691245cd4 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -114,7 +114,8 @@ static class MappingAuditingMetadata { /** * Creates a new {@link MappingAuditingMetadata} instance from the given {@link PersistentEntity}. * - * @param entity must not be {@literal null}. + * @param context must not be {@literal null}. + * @param type must not be {@literal null}. */ public

MappingAuditingMetadata(MappingContext> context, Class type) { diff --git a/src/main/java/org/springframework/data/convert/Jsr310Converters.java b/src/main/java/org/springframework/data/convert/Jsr310Converters.java index a1b08119b8..5569cfe6cc 100644 --- a/src/main/java/org/springframework/data/convert/Jsr310Converters.java +++ b/src/main/java/org/springframework/data/convert/Jsr310Converters.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.List; @@ -82,10 +83,17 @@ public abstract class Jsr310Converters { } public static boolean supports(Class type) { - return CLASSES.contains(type); } + /** + * @return the collection of supported temporal classes. + * @since 3.2 + */ + public static Collection> getSupportedClasses() { + return Collections.unmodifiableList(CLASSES); + } + @ReadingConverter public enum DateToLocalDateTimeConverter implements Converter { diff --git a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java index 0e7b4d7395..cc3d84d33b 100755 --- a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java @@ -19,7 +19,9 @@ import java.time.Instant; import java.time.LocalDateTime; +import java.time.OffsetDateTime; import java.time.ZoneOffset; +import java.time.ZonedDateTime; import java.time.temporal.ChronoField; import org.junit.jupiter.api.Test; @@ -34,7 +36,7 @@ * @author Oliver Gierke * @author Christoph Strobl * @author Jens Schauder - * @since 1.5 + * @author Mark Paluch */ class DefaultAuditableBeanWrapperFactoryUnitTests { @@ -135,10 +137,56 @@ void lastModifiedAsLocalDateTimeDateIsAvailableViaWrapperAsLocalDateTime() { assertThat(result).hasValue(now); } + @Test + void shouldRejectUnsupportedTemporalConversion() { + + var source = new WithZonedDateTime(); + AuditableBeanWrapper wrapper = factory.getBeanWrapperFor(source).get(); + + assertThatIllegalArgumentException().isThrownBy(() -> wrapper.setCreatedDate(LocalDateTime.now())) + .withMessageContaining( + "Cannot convert unsupported date type java.time.LocalDateTime to java.time.ZonedDateTime"); + } + + @Test // GH-2719 + void shouldPassthruZonedDateTimeValue() { + + var source = new WithZonedDateTime(); + var now = ZonedDateTime.now(); + AuditableBeanWrapper wrapper = factory.getBeanWrapperFor(source).get(); + + wrapper.setCreatedDate(now); + + assertThat(source.created).isEqualTo(now); + } + + @Test // GH-2719 + void shouldPassthruOffsetDatetimeValue() { + + var source = new WithOffsetDateTime(); + var now = OffsetDateTime.now(); + AuditableBeanWrapper wrapper = factory.getBeanWrapperFor(source).get(); + + wrapper.setCreatedDate(now); + + assertThat(source.created).isEqualTo(now); + } + public static class LongBasedAuditable { @CreatedDate public Long dateCreated; @LastModifiedDate public Long dateModified; } + + static class WithZonedDateTime { + + @CreatedDate ZonedDateTime created; + } + + static class WithOffsetDateTime { + + @CreatedDate OffsetDateTime created; + } + } diff --git a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java index 1d08aea50e..9b161561d4 100755 --- a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java @@ -21,6 +21,7 @@ import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneOffset; +import java.time.ZonedDateTime; import java.time.temporal.ChronoField; import java.time.temporal.TemporalAccessor; import java.util.Arrays; @@ -242,6 +243,29 @@ void skipsCollectionPropertiesWhenSettingProperties() { }); } + @Test // GH-2719 + void shouldRejectUnsupportedTemporalConversion() { + + var source = new WithZonedDateTime(); + AuditableBeanWrapper wrapper = factory.getBeanWrapperFor(source).get(); + + assertThatIllegalArgumentException().isThrownBy(() -> wrapper.setCreatedDate(LocalDateTime.now())) + .withMessageContaining( + "Cannot convert unsupported date type java.time.LocalDateTime to java.time.ZonedDateTime"); + } + + @Test // GH-2719 + void shouldPassthruTemporalValue() { + + var source = new WithZonedDateTime(); + var now = ZonedDateTime.now(); + AuditableBeanWrapper wrapper = factory.getBeanWrapperFor(source).get(); + + wrapper.setCreatedDate(now); + + assertThat(source.created).isEqualTo(now); + } + private void assertLastModificationDate(Object source, TemporalAccessor expected) { var sample = new Sample(); @@ -302,6 +326,11 @@ static class Embedded { @LastModifiedBy String modifier; } + static class WithZonedDateTime { + + @CreatedDate ZonedDateTime created; + } + static class WithEmbedded { Embedded embedded; Collection embeddeds; From ec0a7176d9183c8ab74c87e5b3eaa68e90ac7561 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 12 Jul 2023 12:29:01 +0200 Subject: [PATCH 3/3] Polishing. Remove outdated Javadoc. --- .../org/springframework/data/convert/Jsr310Converters.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/Jsr310Converters.java b/src/main/java/org/springframework/data/convert/Jsr310Converters.java index 5569cfe6cc..5e4f1afe64 100644 --- a/src/main/java/org/springframework/data/convert/Jsr310Converters.java +++ b/src/main/java/org/springframework/data/convert/Jsr310Converters.java @@ -38,7 +38,7 @@ import org.springframework.lang.NonNull; /** - * Helper class to register JSR-310 specific {@link Converter} implementations in case the we're running on Java 8. + * Helper class to register JSR-310 specific {@link Converter} implementations. * * @author Oliver Gierke * @author Barak Schoster @@ -52,9 +52,9 @@ public abstract class Jsr310Converters { Instant.class, ZoneId.class, Duration.class, Period.class); /** - * Returns the converters to be registered. Will only return converters in case we're running on Java 8. + * Returns the converters to be registered. * - * @return + * @return the converters to be registered. */ public static Collection> getConvertersToRegister() {