From d0ba1252687acf416c838cef840b4145ab859814 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 12 Jul 2023 12:22:59 +0200 Subject: [PATCH] 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 Original pull request #2874 --- .../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 be839a899..d885d0fc9 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 @@ final class AnnotationAuditingMetadata { 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 020cbdc9a..c427db182 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory private final Class type; @SuppressWarnings("unchecked") - public AuditableInterfaceBeanWrapper(ConversionService conversionService, Auditable auditable) { + public AuditableInterfaceBeanWrapper(ConversionService conversionService, + Auditable auditable) { super(conversionService); @@ -151,8 +153,8 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory } /** - * 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory /** * 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory @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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return conversionService.convert(date, targetType); } - throw rejectUnsupportedType(source); + throw rejectUnsupportedType(value.getClass(), targetType); } /** @@ -217,19 +219,20 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory } 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 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory @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 424de0631..4691245cd 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 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap /** * 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 a1b08119b..5569cfe6c 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.time.format.DateTimeFormatter; 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 0e7b4d739..cc3d84d33 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 static org.assertj.core.api.Assertions.*; 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 @@ import org.springframework.data.auditing.DefaultAuditableBeanWrapperFactory.Refl * @author Oliver Gierke * @author Christoph Strobl * @author Jens Schauder - * @since 1.5 + * @author Mark Paluch */ class DefaultAuditableBeanWrapperFactoryUnitTests { @@ -135,10 +137,56 @@ class DefaultAuditableBeanWrapperFactoryUnitTests { 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 1d08aea50..9b161561d 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 static org.mockito.Mockito.*; 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 @@ class MappingAuditableBeanWrapperFactoryUnitTests { }); } + @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 @@ class MappingAuditableBeanWrapperFactoryUnitTests { @LastModifiedBy String modifier; } + static class WithZonedDateTime { + + @CreatedDate ZonedDateTime created; + } + static class WithEmbedded { Embedded embedded; Collection embeddeds;