From 5251f72e69919aea49f3f8ee78c6b6556963e9f5 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 13 Feb 2018 14:34:58 +0100 Subject: [PATCH] DATACMNS-1259 - Fixed support for Long values in Auditables. Using Instant as internal data type since it's a point in time without time zone which LocalDateTime isn't. Added necessary converters. Fixed one JodaTime converter that used UTC to use SystemDefault like other similar converters. In case of a conversion failure the error message now contains the source type. Original pull request: #273. --- .../DefaultAuditableBeanWrapperFactory.java | 16 ++-- .../data/convert/JodaTimeConverters.java | 30 +++++++- .../convert/ThreeTenBackPortConverters.java | 28 ++++++- ...tAuditableBeanWrapperFactoryUnitTests.java | 75 +++++++++++++++++++ ...gAuditableBeanWrapperFactoryUnitTests.java | 34 ++++++++- 5 files changed, 175 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index ad58e5828..3d631e34d 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -19,6 +19,7 @@ import lombok.NonNull; import lombok.RequiredArgsConstructor; import java.lang.reflect.Field; +import java.time.Instant; import java.time.LocalDateTime; import java.time.temporal.TemporalAccessor; import java.util.Calendar; @@ -42,6 +43,7 @@ import org.springframework.util.Assert; * * @author Oliver Gierke * @author Christoph Strobl + * @author Jens Schauder * @since 1.5 */ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory { @@ -207,8 +209,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return conversionService.convert(date, targetType); } - throw new IllegalArgumentException(String.format("Invalid date type for member %s! Supported types are %s.", - source, AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES)); + throw new IllegalArgumentException(createUnsupportedTypeErrorMessage(source)); } /** @@ -228,19 +229,24 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return (T) it; } - Class typeToConvertTo = Stream.of(target, LocalDateTime.class)// + Class typeToConvertTo = Stream.of(target, Instant.class)// .filter(type -> target.isAssignableFrom(type))// .filter(type -> conversionService.canConvert(it.getClass(), type))// .findFirst() .orElseThrow(() -> new IllegalArgumentException( - String.format("Invalid date type for member %s! Supported types are %s.", source, - AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES))); + createUnsupportedTypeErrorMessage(((Optional)source).orElseGet(() -> source)))); return (T) conversionService.convert(it, typeToConvertTo); }); } } + private static String createUnsupportedTypeErrorMessage(Object source) { + + return String.format("Invalid date type %s for member %s! Supported types are %s.", source.getClass(), source, + AnnotationAuditingMetadata.SUPPORTED_DATE_TYPES); + } + /** * An {@link AuditableBeanWrapper} implementation that sets values on the target object using reflection. * diff --git a/src/main/java/org/springframework/data/convert/JodaTimeConverters.java b/src/main/java/org/springframework/data/convert/JodaTimeConverters.java index bd41a7433..090f04258 100644 --- a/src/main/java/org/springframework/data/convert/JodaTimeConverters.java +++ b/src/main/java/org/springframework/data/convert/JodaTimeConverters.java @@ -15,16 +15,19 @@ */ package org.springframework.data.convert; +import java.time.Instant; import java.time.ZoneId; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; +import java.util.TimeZone; import javax.annotation.Nonnull; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.joda.time.LocalDate; import org.joda.time.LocalDateTime; import org.springframework.core.convert.converter.Converter; @@ -65,6 +68,9 @@ public abstract class JodaTimeConverters { converters.add(LocalDateTimeToJodaLocalDateTime.INSTANCE); converters.add(LocalDateTimeToJodaDateTime.INSTANCE); + converters.add(InstantToJodaLocalDateTime.INSTANCE); + converters.add(JodaLocalDateTimeToInstant.INSTANCE); + converters.add(LocalDateTimeToJsr310Converter.INSTANCE); return converters; @@ -77,7 +83,7 @@ public abstract class JodaTimeConverters { @Nonnull @Override public java.time.LocalDateTime convert(LocalDateTime source) { - return java.time.LocalDateTime.ofInstant(source.toDate().toInstant(), ZoneId.of("UTC")); + return java.time.LocalDateTime.ofInstant(source.toDate().toInstant(), ZoneId.systemDefault()); } } @@ -158,6 +164,28 @@ public abstract class JodaTimeConverters { } } + public enum InstantToJodaLocalDateTime implements Converter { + + INSTANCE; + + @Nonnull + @Override + public LocalDateTime convert(java.time.Instant source) { + return LocalDateTime.fromDateFields(new Date(source.toEpochMilli())); + } + } + + public enum JodaLocalDateTimeToInstant implements Converter { + + INSTANCE; + + @Nonnull + @Override + public Instant convert(LocalDateTime source) { + return Instant.ofEpochMilli(source.toDateTime().getMillis()); + } + } + public enum LocalDateTimeToJodaDateTime implements Converter { INSTANCE; diff --git a/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java b/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java index d2ff2cd1a..a794f3c74 100644 --- a/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java +++ b/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java @@ -36,6 +36,7 @@ import org.threeten.bp.LocalDate; import org.threeten.bp.LocalDateTime; import org.threeten.bp.LocalTime; import org.threeten.bp.ZoneId; +import org.threeten.bp.ZoneOffset; /** * Helper class to register {@link Converter} implementations for the ThreeTen Backport project in case it's present on @@ -43,6 +44,7 @@ import org.threeten.bp.ZoneId; * * @author Oliver Gierke * @author Christoph Strobl + * @author Jens Schauder * @see http://www.threeten.org/threetenbp * @since 1.10 */ @@ -74,6 +76,8 @@ public abstract class ThreeTenBackPortConverters { converters.add(ZoneIdToStringConverter.INSTANCE); converters.add(StringToZoneIdConverter.INSTANCE); converters.add(LocalDateTimeToJsr310LocalDateTimeConverter.INSTANCE); + converters.add(LocalDateTimeToJavaTimeInstantConverter.INSTANCE); + converters.add(JavaTimeInstantToLocalDateTimeConverter.INSTANCE); return converters; } @@ -84,7 +88,7 @@ public abstract class ThreeTenBackPortConverters { return false; } - return Arrays.> asList(LocalDateTime.class, LocalDate.class, LocalTime.class, Instant.class) + return Arrays.> asList(LocalDateTime.class, LocalDate.class, LocalTime.class, Instant.class, java.time.Instant.class) .contains(type); } @@ -195,6 +199,28 @@ public abstract class ThreeTenBackPortConverters { } } + public static enum LocalDateTimeToJavaTimeInstantConverter implements Converter { + + INSTANCE; + + @Nonnull + @Override + public java.time.Instant convert(LocalDateTime source) { + return java.time.Instant.ofEpochMilli(source.atZone(ZoneOffset.systemDefault()).toInstant().toEpochMilli()); + } + } + + public static enum JavaTimeInstantToLocalDateTimeConverter implements Converter { + + INSTANCE; + + @Nonnull + @Override + public LocalDateTime convert(java.time.Instant source) { + return LocalDateTime.ofInstant(Instant.ofEpochMilli(source.toEpochMilli()), ZoneOffset.systemDefault()); + } + } + @WritingConverter public static enum ZoneIdToStringConverter 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 3879817b7..e987f797d 100755 --- a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java @@ -18,18 +18,27 @@ package org.springframework.data.auditing; import static org.assertj.core.api.Assertions.*; 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.time.temporal.TemporalField; import java.util.Optional; import org.junit.Test; +import org.springframework.data.annotation.CreatedDate; +import org.springframework.data.annotation.LastModifiedDate; import org.springframework.data.auditing.DefaultAuditableBeanWrapperFactory.AuditableInterfaceBeanWrapper; import org.springframework.data.auditing.DefaultAuditableBeanWrapperFactory.ReflectionAuditingBeanWrapper; +import org.springframework.data.domain.Auditable; /** * Unit tests for {@link DefaultAuditableBeanWrapperFactory}. * * @author Oliver Gierke * @author Christoph Strobl + * @author Jens Schauder * @since 1.5 */ public class DefaultAuditableBeanWrapperFactoryUnitTests { @@ -88,4 +97,70 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { assertThat(wrapper).hasValueSatisfying(it -> it.setLastModifiedDate(zonedDateTime)); } + + @Test // DATACMNS-1259 + public void lastModifiedDateAsLongIsAvailableViaWrapper() { + + LongBasedAuditable source = new LongBasedAuditable(); + source.dateModified = 42000L; + + Optional beanWrapper = factory.getBeanWrapperFor(source); + assertThat(beanWrapper).isPresent(); + + assertThat(beanWrapper.flatMap(AuditableBeanWrapper::getLastModifiedDate).get()) // + .extracting(ta -> ta.getLong(ChronoField.INSTANT_SECONDS)) // + .containsExactly(42L); + } + + + @Test // DATACMNS-1259 + public void canSetLastModifiedDateAsInstantViaWrapperOnLongField() { + + LongBasedAuditable source = new LongBasedAuditable(); + + Optional beanWrapper = factory.getBeanWrapperFor(source); + assertThat(beanWrapper).isPresent(); + + beanWrapper.get().setLastModifiedDate(Instant.ofEpochMilli(42L)); + + assertThat(source.dateModified).isEqualTo(42L); + } + + @Test // DATACMNS-1259 + public void canSetLastModifiedDateAsLocalDateTimeViaWrapperOnLongField() { + + LongBasedAuditable source = new LongBasedAuditable(); + + Optional beanWrapper = factory.getBeanWrapperFor(source); + assertThat(beanWrapper).isPresent(); + + beanWrapper.get().setLastModifiedDate(LocalDateTime.ofInstant(Instant.ofEpochMilli(42L), ZoneOffset.systemDefault())); + + assertThat(source.dateModified).isEqualTo(42L); + } + + + @Test // DATACMNS-1259 + public void lastModifiedAsLocalDateTimeDateIsAvailableViaWrapperAsLocalDateTime() { + + LocalDateTime now = LocalDateTime.now(); + + AuditedUser source = new AuditedUser(); + source.setLastModifiedDate(now); + + Optional beanWrapper = factory.getBeanWrapperFor(source); + assertThat(beanWrapper).isPresent(); + + assertThat(beanWrapper.flatMap(AuditableBeanWrapper::getLastModifiedDate).get()) // + .isEqualTo(now); + } + + public static class LongBasedAuditable { + + @CreatedDate + public Long dateCreated; + + @LastModifiedDate + public Long dateModified; + } } diff --git a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java index c72ea24ec..a60982de0 100755 --- a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java @@ -20,6 +20,8 @@ import static org.mockito.Mockito.*; import java.time.Instant; import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.temporal.ChronoField; import java.time.temporal.TemporalAccessor; import java.util.Calendar; import java.util.Collections; @@ -27,6 +29,7 @@ import java.util.Date; import java.util.GregorianCalendar; import java.util.Optional; +import org.assertj.core.api.AbstractLongAssert; import org.junit.Before; import org.junit.Test; import org.springframework.data.annotation.CreatedBy; @@ -45,6 +48,7 @@ import org.springframework.data.mapping.context.SampleMappingContext; * Unit tests for {@link MappingAuditableBeanWrapperFactory}. * * @author Oliver Gierke + * @author Jens Schauder * @since 1.8 */ public class MappingAuditableBeanWrapperFactoryUnitTests { @@ -169,6 +173,14 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValue(sample.modified); } + @Test // DATACMNS-1259 + public void exposesLongAsModificationDate() { + + Long reference = new Date().getTime(); + + assertLastModificationDate(reference, Instant.ofEpochMilli(reference)); + } + private void assertLastModificationDate(Object source, TemporalAccessor expected) { Sample sample = new Sample(); @@ -176,7 +188,27 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Optional wrapper = factory.getBeanWrapperFor(sample); - assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValue(expected); + assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValueSatisfying(ta -> { + compareTemporalAccessors(expected, ta); + }); + } + + private AbstractLongAssert compareTemporalAccessors(TemporalAccessor expected, TemporalAccessor actual) { + + long actualSeconds = getInstantSeconds(actual); + long expectedSeconds = getInstantSeconds(expected); + + return assertThat(actualSeconds).describedAs("Difference is %s", actualSeconds - expectedSeconds) + .isEqualTo(expectedSeconds); + } + + private long getInstantSeconds(TemporalAccessor actual) { + + if (actual instanceof LocalDateTime) { + return getInstantSeconds(((LocalDateTime) actual).atZone(ZoneOffset.systemDefault())); + } + + return actual.getLong(ChronoField.INSTANT_SECONDS); } static class Sample {