From 6a6d995eabfc38f15347b40a5fcf2c54e7453d9a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 12 Jul 2017 18:58:00 +0200 Subject: [PATCH] DATACMNS-1109 - Reinstantiate support for all date/time APIs in auditing. Re-enabled test cases for handling of different date/time APIs in auditing. Tweaked the lookup of the last modified value to try to convert to LocalDateTime as well in case it's a subtype of the type requested. In combination with two newly introduced converters for local date type times in JodaTime and ThreeTenBP, this allows us to still convert legacy JodaTime and ThreeTenBP types to be used as well. --- .../DefaultAuditableBeanWrapperFactory.java | 20 +++++- .../MappingAuditableBeanWrapperFactory.java | 4 +- .../data/convert/JodaTimeConverters.java | 14 +++- .../convert/ThreeTenBackPortConverters.java | 19 ++++++ ...tAuditableBeanWrapperFactoryUnitTests.java | 2 +- ...gAuditableBeanWrapperFactoryUnitTests.java | 66 +++++++++++-------- 6 files changed, 94 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index 44c69d8e7..a0f50250d 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -19,10 +19,12 @@ import lombok.NonNull; import lombok.RequiredArgsConstructor; import java.lang.reflect.Field; +import java.time.LocalDateTime; import java.time.temporal.TemporalAccessor; import java.util.Calendar; import java.util.Date; import java.util.Optional; +import java.util.stream.Stream; import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; @@ -216,7 +218,23 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory @SuppressWarnings("unchecked") protected Optional getAsTemporalAccessor(Optional source, Class target) { - return source.map(it -> target.isInstance(it) ? (T) it : conversionService.convert(it, target)); + + return source.map(it -> { + + if (target.isInstance(it)) { + return (T) it; + } + + Class typeToConvertTo = Stream.of(target, LocalDateTime.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))); + + return (T) conversionService.convert(it, typeToConvertTo); + }); } } diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index cfbcd4577..ed200215b 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -15,7 +15,6 @@ */ package org.springframework.data.auditing; -import java.time.LocalDateTime; import java.time.temporal.TemporalAccessor; import java.util.HashMap; import java.util.Map; @@ -191,7 +190,8 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap */ @Override public Optional getLastModifiedDate() { - return getAsTemporalAccessor(metadata.lastModifiedDateProperty.map(accessor::getProperty), LocalDateTime.class); + return getAsTemporalAccessor(metadata.lastModifiedDateProperty.map(accessor::getProperty), + TemporalAccessor.class); } /* diff --git a/src/main/java/org/springframework/data/convert/JodaTimeConverters.java b/src/main/java/org/springframework/data/convert/JodaTimeConverters.java index 8f94d9695..a973f9a45 100644 --- a/src/main/java/org/springframework/data/convert/JodaTimeConverters.java +++ b/src/main/java/org/springframework/data/convert/JodaTimeConverters.java @@ -15,6 +15,7 @@ */ package org.springframework.data.convert; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -35,7 +36,6 @@ import org.springframework.util.ClassUtils; * @author Christoph Strobl * @author Jens Schauder */ -@SuppressWarnings("deprecation") public abstract class JodaTimeConverters { private static final boolean JODA_TIME_IS_PRESENT = ClassUtils.isPresent("org.joda.time.LocalDate", null); @@ -63,9 +63,21 @@ public abstract class JodaTimeConverters { converters.add(LocalDateTimeToJodaLocalDateTime.INSTANCE); converters.add(LocalDateTimeToJodaDateTime.INSTANCE); + converters.add(LocalDateTimeToJsr310Converter.INSTANCE); + return converters; } + public enum LocalDateTimeToJsr310Converter implements Converter { + + INSTANCE; + + @Override + public java.time.LocalDateTime convert(LocalDateTime source) { + return source == null ? null : java.time.LocalDateTime.ofInstant(source.toDate().toInstant(), ZoneId.of("UTC")); + } + } + public enum LocalDateToDateConverter 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 f51b45bbc..1bceef7e8 100644 --- a/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java +++ b/src/main/java/org/springframework/data/convert/ThreeTenBackPortConverters.java @@ -71,6 +71,7 @@ public abstract class ThreeTenBackPortConverters { converters.add(InstantToDateConverter.INSTANCE); converters.add(ZoneIdToStringConverter.INSTANCE); converters.add(StringToZoneIdConverter.INSTANCE); + converters.add(LocalDateTimeToJsr310LocalDateTimeConverter.INSTANCE); return converters; } @@ -85,6 +86,24 @@ public abstract class ThreeTenBackPortConverters { .contains(type); } + public static enum LocalDateTimeToJsr310LocalDateTimeConverter + implements Converter { + + INSTANCE; + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + */ + @Override + public java.time.LocalDateTime convert(LocalDateTime source) { + + Date date = toDate(source.atZone(ZoneId.systemDefault()).toInstant()); + + return source == null ? null : Jsr310Converters.DateToLocalDateTimeConverter.INSTANCE.convert(date); + } + } + public static enum DateToLocalDateTimeConverter implements Converter { INSTANCE; diff --git a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java index 6430248ab..7b0aff482 100755 --- a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java @@ -79,7 +79,7 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { } @Test(expected = IllegalArgumentException.class) // DATACMNS-867 - public void errorsWhenUnableToConvertDateViaIntermedeateJavaUtilDateConversion() { + public void errorsWhenUnableToConvertDateViaIntermediateJavaUtilDateConversion() { Jsr310ThreeTenBpAuditedUser user = new Jsr310ThreeTenBpAuditedUser(); ZonedDateTime zonedDateTime = ZonedDateTime.now(); diff --git a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java index 69dec3a07..27ddb0394 100755 --- a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java @@ -20,32 +20,33 @@ import static org.mockito.Mockito.*; import java.time.Instant; import java.time.LocalDateTime; +import java.time.temporal.TemporalAccessor; import java.util.Calendar; import java.util.Collections; import java.util.Date; import java.util.GregorianCalendar; import java.util.Optional; -import org.joda.time.DateTime; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.data.annotation.CreatedBy; +import org.springframework.data.annotation.CreatedDate; import org.springframework.data.annotation.LastModifiedBy; import org.springframework.data.annotation.LastModifiedDate; import org.springframework.data.auditing.DefaultAuditableBeanWrapperFactory.AuditableInterfaceBeanWrapper; -import org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConverter; +import org.springframework.data.convert.JodaTimeConverters; +import org.springframework.data.convert.Jsr310Converters; +import org.springframework.data.convert.ThreeTenBackPortConverters; import org.springframework.data.domain.Auditable; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.mapping.context.SampleMappingContext; /** - * Unit tests for {@link MappingAuditableBeanWrapperFactory}. TODO: Which date types to support? + * Unit tests for {@link MappingAuditableBeanWrapperFactory}. * * @author Oliver Gierke * @since 1.8 */ -@Ignore public class MappingAuditableBeanWrapperFactoryUnitTests { DefaultAuditableBeanWrapperFactory factory; @@ -54,8 +55,8 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { public void setUp() { SampleMappingContext context = new SampleMappingContext(); - context.setInitialEntitySet(Collections.singleton(Sample.class)); - context.afterPropertiesSet(); + context.getPersistentEntity(Sample.class); + context.getPersistentEntity(SampleWithInstant.class); PersistentEntities entities = new PersistentEntities(Collections.singleton(context)); factory = new MappingAuditableBeanWrapperFactory(entities); @@ -66,7 +67,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(Optional.of(sample)); + Optional wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> { @@ -80,7 +81,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(Optional.of(sample)); + Optional wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> { @@ -94,20 +95,20 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(Optional.of(sample)); + Optional wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> it.setLastModifiedDate(Instant.now())); } @Test // DATACMNS-365 public void doesNotReturnWrapperForEntityNotUsingAuditing() { - assertThat(factory.getBeanWrapperFor(Optional.of(new NoAuditing()))).isNotPresent(); + assertThat(factory.getBeanWrapperFor(new NoAuditing())).isNotPresent(); } @Test // DATACMNS-365 public void returnsAuditableWrapperForAuditable() { - assertThat(factory.getBeanWrapperFor(Optional.of(mock(ExtendingAuditable.class)))) + assertThat(factory.getBeanWrapperFor(mock(ExtendingAuditable.class))) .hasValueSatisfying(it -> assertThat(it).isInstanceOf(AuditableInterfaceBeanWrapper.class)); } @@ -119,15 +120,17 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Calendar calendar = new GregorianCalendar(); calendar.setTime(reference); - assertLastModificationDate(calendar, reference); + assertLastModificationDate(calendar, // + Jsr310Converters.DateToLocalDateTimeConverter.INSTANCE.convert(reference)); } @Test // DATACMNS-638 public void returnsLastModificationDateTimeAsCalendar() { - DateTime reference = new DateTime(); + org.joda.time.LocalDateTime reference = new org.joda.time.LocalDateTime(); - assertLastModificationDate(reference, reference.toDate()); + assertLastModificationDate(reference, + JodaTimeConverters.LocalDateTimeToJsr310Converter.INSTANCE.convert(reference)); } @Test // DATACMNS-638 @@ -135,7 +138,8 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Date reference = new Date(); - assertLastModificationDate(reference, reference); + assertLastModificationDate(reference, // + Jsr310Converters.DateToLocalDateTimeConverter.INSTANCE.convert(reference)); } @Test // DATACMNS-638, DATACMNS-43 @@ -143,7 +147,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { LocalDateTime reference = LocalDateTime.now(); - assertLastModificationDate(reference, LocalDateTimeToDateConverter.INSTANCE.convert(reference)); + assertLastModificationDate(reference, reference); } @Test // DATACMNS-638, DATACMNS-43 @@ -152,18 +156,27 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { org.threeten.bp.LocalDateTime reference = org.threeten.bp.LocalDateTime.now(); assertLastModificationDate(reference, - org.springframework.data.convert.ThreeTenBackPortConverters.LocalDateTimeToDateConverter.INSTANCE - .convert(reference)); + ThreeTenBackPortConverters.LocalDateTimeToJsr310LocalDateTimeConverter.INSTANCE.convert(reference)); } - private void assertLastModificationDate(Object source, Object expected) { + @Test + public void exposesInstantAsModificationDate() { + + SampleWithInstant sample = new SampleWithInstant(); + sample.modified = Instant.now(); + + Optional wrapper = factory.getBeanWrapperFor(sample); + assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValue(sample.modified); + } + + private void assertLastModificationDate(Object source, TemporalAccessor expected) { Sample sample = new Sample(); sample.lastModifiedDate = source; - Optional wrapper = factory.getBeanWrapperFor(Optional.of(sample)); + Optional wrapper = factory.getBeanWrapperFor(sample); - assertThat(wrapper).hasValueSatisfying(it -> assertThat(it.getLastModifiedDate()).isEqualTo(expected)); + assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValue(expected); } static class Sample { @@ -178,12 +191,13 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { } } - static class NoAuditing { + static class SampleWithInstant { + @CreatedDate Instant created; + @LastModifiedDate Instant modified; } - @SuppressWarnings("serial") - static abstract class ExtendingAuditable implements Auditable { + static class NoAuditing {} - } + static abstract class ExtendingAuditable implements Auditable {} }