From 6cb3587ba31deec9393a4ec2c19bc58869e65c62 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 18 Jun 2018 15:16:35 +0200 Subject: [PATCH] DATACMNS-1322 - Adapt auditing handler API to support immutable entities. We've changed the APIs in the auditing subsystem so that we support immutable entities, mostly through MappingAuditableBeanWrapperFactory that uses a PersistentPropertyAccessor. --- .../data/auditing/AuditableBeanWrapper.java | 11 ++++- .../auditing/AuditableBeanWrapperFactory.java | 2 +- .../data/auditing/AuditingHandler.java | 27 ++++++----- .../DefaultAuditableBeanWrapperFactory.java | 45 +++++++++++++------ .../auditing/IsNewAwareAuditingHandler.java | 12 ++--- .../MappingAuditableBeanWrapperFactory.java | 22 ++++++--- ...tAuditableBeanWrapperFactoryUnitTests.java | 42 +++++++---------- ...gAuditableBeanWrapperFactoryUnitTests.java | 27 +++++------ 8 files changed, 109 insertions(+), 79 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapper.java b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapper.java index 9b91a6dbd..ec8128397 100644 --- a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapper.java +++ b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapper.java @@ -24,7 +24,7 @@ import java.util.Optional; * @author Oliver Gierke * @since 1.5 */ -public interface AuditableBeanWrapper { +public interface AuditableBeanWrapper { /** * Set the creator of the object. @@ -61,4 +61,13 @@ public interface AuditableBeanWrapper { * @param value */ TemporalAccessor setLastModifiedDate(TemporalAccessor value); + + /** + * Returns the underlying bean that potentially has been modified by the setter methods exposed. Client code needs to + * make sure to call this method to see all the changes applied via this {@link AuditableBeanWrapper}. + * + * @return will never be {@literal null}. + * @since 2.1 + */ + T getBean(); } diff --git a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java index 60478f97e..c130a21f6 100644 --- a/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/AuditableBeanWrapperFactory.java @@ -31,5 +31,5 @@ public interface AuditableBeanWrapperFactory { * @param source must not be {@literal null}. * @return the {@link AuditableBeanWrapper} for the given source object if it's eligible for auditing. */ - Optional getBeanWrapperFor(Object source); + Optional> getBeanWrapperFor(T source); } diff --git a/src/main/java/org/springframework/data/auditing/AuditingHandler.java b/src/main/java/org/springframework/data/auditing/AuditingHandler.java index 19966a684..b4d5b3fbc 100644 --- a/src/main/java/org/springframework/data/auditing/AuditingHandler.java +++ b/src/main/java/org/springframework/data/auditing/AuditingHandler.java @@ -124,11 +124,11 @@ public class AuditingHandler implements InitializingBean { * * @param source */ - public void markCreated(Object source) { + public T markCreated(T source) { Assert.notNull(source, "Entity must not be null!"); - touch(source, true); + return touch(source, true); } /** @@ -136,11 +136,11 @@ public class AuditingHandler implements InitializingBean { * * @param source */ - public void markModified(Object source) { + public T markModified(T source) { Assert.notNull(source, "Entity must not be null!"); - touch(source, false); + return touch(source, false); } /** @@ -156,9 +156,11 @@ public class AuditingHandler implements InitializingBean { return factory.getBeanWrapperFor(source).isPresent(); } - private void touch(Object target, boolean isNew) { + private T touch(T target, boolean isNew) { - factory.getBeanWrapperFor(target).ifPresent(it -> { + Optional> wrapper = factory.getBeanWrapperFor(target); + + return wrapper.map(it -> { Optional auditor = touchAuditor(it, isNew); Optional now = dateTimeForNow ? touchDate(it, isNew) : Optional.empty(); @@ -168,10 +170,12 @@ public class AuditingHandler implements InitializingBean { Object defaultedNow = now.map(Object::toString).orElse("not set"); Object defaultedAuditor = auditor.map(Object::toString).orElse("unknown"); - LOGGER.debug("Touched {} - Last modification at {} by {}", - new Object[] { target, defaultedNow, defaultedAuditor }); + LOGGER.debug("Touched {} - Last modification at {} by {}", target, defaultedNow, defaultedAuditor); } - }); + + return it.getBean(); + + }).orElse(target); } /** @@ -180,7 +184,7 @@ public class AuditingHandler implements InitializingBean { * @param auditable * @return */ - private Optional touchAuditor(AuditableBeanWrapper wrapper, boolean isNew) { + private Optional touchAuditor(AuditableBeanWrapper wrapper, boolean isNew) { Assert.notNull(wrapper, "AuditableBeanWrapper must not be null!"); @@ -196,7 +200,6 @@ public class AuditingHandler implements InitializingBean { return auditor; }); - } /** @@ -205,7 +208,7 @@ public class AuditingHandler implements InitializingBean { * @param wrapper * @return */ - private Optional touchDate(AuditableBeanWrapper wrapper, boolean isNew) { + private Optional touchDate(AuditableBeanWrapper wrapper, boolean isNew) { Assert.notNull(wrapper, "AuditableBeanWrapper must not be null!"); diff --git a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java index 3efda1826..72a572c13 100644 --- a/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactory.java @@ -54,20 +54,20 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory * @return */ @SuppressWarnings("unchecked") - public Optional getBeanWrapperFor(Object source) { + public Optional> getBeanWrapperFor(T source) { Assert.notNull(source, "Source must not be null!"); return Optional.of(source).map(it -> { if (it instanceof Auditable) { - return new AuditableInterfaceBeanWrapper((Auditable) it); + return (AuditableBeanWrapper) new AuditableInterfaceBeanWrapper((Auditable) it); } AnnotationAuditingMetadata metadata = AnnotationAuditingMetadata.getMetadata(it.getClass()); if (metadata.isAuditable()) { - return new ReflectionAuditingBeanWrapper(it); + return new ReflectionAuditingBeanWrapper(it); } return null; @@ -80,7 +80,8 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory * @author Oliver Gierke */ @RequiredArgsConstructor - static class AuditableInterfaceBeanWrapper extends DateConvertingAuditableBeanWrapper { + static class AuditableInterfaceBeanWrapper + extends DateConvertingAuditableBeanWrapper> { private final @NonNull Auditable auditable; private final Class type; @@ -149,6 +150,15 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return value; } + + /* + * (non-Javadoc) + * @see org.springframework.data.auditing.AuditableBeanWrapper#getBean() + */ + @Override + public Auditable getBean() { + return auditable; + } } /** @@ -158,7 +168,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory * @author Oliver Gierke * @since 1.8 */ - abstract static class DateConvertingAuditableBeanWrapper implements AuditableBeanWrapper { + abstract static class DateConvertingAuditableBeanWrapper implements AuditableBeanWrapper { private final ConversionService conversionService; @@ -218,13 +228,13 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory * @return */ @SuppressWarnings("unchecked") - protected Optional getAsTemporalAccessor(Optional source, - Class target) { + protected Optional getAsTemporalAccessor(Optional source, + Class target) { return source.map(it -> { if (target.isInstance(it)) { - return (T) it; + return (S) it; } Class typeToConvertTo = Stream.of(target, Instant.class)// @@ -233,7 +243,7 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory .findFirst() // .orElseThrow(() -> rejectUnsupportedType(source.map(Object.class::cast).orElseGet(() -> source))); - return (T) conversionService.convert(it, typeToConvertTo); + return (S) conversionService.convert(it, typeToConvertTo); }); } } @@ -248,17 +258,17 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory * * @author Oliver Gierke */ - static class ReflectionAuditingBeanWrapper extends DateConvertingAuditableBeanWrapper { + static class ReflectionAuditingBeanWrapper extends DateConvertingAuditableBeanWrapper { private final AnnotationAuditingMetadata metadata; - private final Object target; + private final T target; /** * Creates a new {@link ReflectionAuditingBeanWrapper} to set auditing data on the given target object. * * @param target must not be {@literal null}. */ - public ReflectionAuditingBeanWrapper(Object target) { + public ReflectionAuditingBeanWrapper(T target) { Assert.notNull(target, "Target object must not be null!"); @@ -318,13 +328,22 @@ class DefaultAuditableBeanWrapperFactory implements AuditableBeanWrapperFactory return setDateField(metadata.getLastModifiedDateField(), value); } + /* + * (non-Javadoc) + * @see org.springframework.data.auditing.AuditableBeanWrapper#getBean() + */ + @Override + public T getBean() { + return target; + } + /** * Sets the given field to the given value if present. * * @param field * @param value */ - private T setField(Optional field, T value) { + private S setField(Optional field, S value) { field.ifPresent(it -> ReflectionUtils.setField(it, target, value)); diff --git a/src/main/java/org/springframework/data/auditing/IsNewAwareAuditingHandler.java b/src/main/java/org/springframework/data/auditing/IsNewAwareAuditingHandler.java index 5f88635d4..8567b4f4f 100644 --- a/src/main/java/org/springframework/data/auditing/IsNewAwareAuditingHandler.java +++ b/src/main/java/org/springframework/data/auditing/IsNewAwareAuditingHandler.java @@ -67,23 +67,19 @@ public class IsNewAwareAuditingHandler extends AuditingHandler { * Marks the given object created or modified based on {@link PersistentEntity#isNew(Object)}. Will route the calls to * {@link #markCreated(Optional)} and {@link #markModified(Optional)} accordingly. * - * @param object + * @param object must not be {@literal null}. */ - public void markAudited(Object object) { + public Object markAudited(Object object) { Assert.notNull(object, "Source object must not be null!"); if (!isAuditable(object)) { - return; + return object; } PersistentEntity> entity = entities .getRequiredPersistentEntity(object.getClass()); - if (entity.isNew(object)) { - markCreated(object); - } else { - markModified(object); - } + return entity.isNew(object) ? markCreated(object) : markModified(object); } } diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index 138189971..e202ee66c 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -69,7 +69,7 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap * @see org.springframework.data.auditing.AuditableBeanWrapperFactory#getBeanWrapperFor(java.lang.Object) */ @Override - public Optional getBeanWrapperFor(Object source) { + public Optional> getBeanWrapperFor(T source) { return Optional.of(source).flatMap(it -> { @@ -82,8 +82,8 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap MappingAuditingMetadata metadata = metadataCache.computeIfAbsent(it.getClass(), key -> new MappingAuditingMetadata(context, it.getClass())); - return Optional. ofNullable(metadata.isAuditable() // - ? new MappingMetadataAuditableBeanWrapper(entity.getPropertyAccessor(it), metadata) + return Optional.> ofNullable(metadata.isAuditable() // + ? new MappingMetadataAuditableBeanWrapper(entity.getPropertyAccessor(it), metadata) : null); }).orElseGet(() -> super.getBeanWrapperFor(source)); @@ -146,7 +146,7 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap * @author Oliver Gierke * @since 1.8 */ - static class MappingMetadataAuditableBeanWrapper extends DateConvertingAuditableBeanWrapper { + static class MappingMetadataAuditableBeanWrapper extends DateConvertingAuditableBeanWrapper { private final PersistentPropertyAccessor accessor; private final MappingAuditingMetadata metadata; @@ -219,8 +219,18 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap return setDateProperty(metadata.lastModifiedDatePaths, value); } - private > T setProperty( - PersistentPropertyPaths> paths, T value) { + /* + * (non-Javadoc) + * @see org.springframework.data.auditing.AuditableBeanWrapper#getBean() + */ + @Override + @SuppressWarnings("unchecked") + public T getBean() { + return (T) accessor.getBean(); + } + + private > S setProperty( + PersistentPropertyPaths> paths, S value) { paths.forEach(it -> this.accessor.setProperty(it, value)); diff --git a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java index e987f797d..ad8681dca 100755 --- a/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/DefaultAuditableBeanWrapperFactoryUnitTests.java @@ -23,7 +23,6 @@ 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; @@ -31,7 +30,6 @@ 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}. @@ -75,7 +73,7 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { Jsr310ThreeTenBpAuditedUser user = new Jsr310ThreeTenBpAuditedUser(); Instant instant = Instant.now(); - Optional wrapper = factory.getBeanWrapperFor(user); + Optional> wrapper = factory.getBeanWrapperFor(user); assertThat(wrapper).hasValueSatisfying(it -> { @@ -93,7 +91,7 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { Jsr310ThreeTenBpAuditedUser user = new Jsr310ThreeTenBpAuditedUser(); ZonedDateTime zonedDateTime = ZonedDateTime.now(); - Optional wrapper = factory.getBeanWrapperFor(user); + Optional> wrapper = factory.getBeanWrapperFor(user); assertThat(wrapper).hasValueSatisfying(it -> it.setLastModifiedDate(zonedDateTime)); } @@ -104,21 +102,19 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { LongBasedAuditable source = new LongBasedAuditable(); source.dateModified = 42000L; - Optional beanWrapper = factory.getBeanWrapperFor(source); - assertThat(beanWrapper).isPresent(); + Optional result = factory.getBeanWrapperFor(source) // + .flatMap(AuditableBeanWrapper::getLastModifiedDate) // + .map(ta -> ta.getLong(ChronoField.INSTANT_SECONDS)); - assertThat(beanWrapper.flatMap(AuditableBeanWrapper::getLastModifiedDate).get()) // - .extracting(ta -> ta.getLong(ChronoField.INSTANT_SECONDS)) // - .containsExactly(42L); + assertThat(result).hasValue(42L); } - @Test // DATACMNS-1259 public void canSetLastModifiedDateAsInstantViaWrapperOnLongField() { LongBasedAuditable source = new LongBasedAuditable(); - Optional beanWrapper = factory.getBeanWrapperFor(source); + Optional> beanWrapper = factory.getBeanWrapperFor(source); assertThat(beanWrapper).isPresent(); beanWrapper.get().setLastModifiedDate(Instant.ofEpochMilli(42L)); @@ -131,15 +127,14 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { LongBasedAuditable source = new LongBasedAuditable(); - Optional beanWrapper = factory.getBeanWrapperFor(source); - assertThat(beanWrapper).isPresent(); + Optional result = factory.getBeanWrapperFor(source).map(it -> { + it.setLastModifiedDate(LocalDateTime.ofInstant(Instant.ofEpochMilli(42L), ZoneOffset.systemDefault())); + return it.getBean().dateModified; + }); - beanWrapper.get().setLastModifiedDate(LocalDateTime.ofInstant(Instant.ofEpochMilli(42L), ZoneOffset.systemDefault())); - - assertThat(source.dateModified).isEqualTo(42L); + assertThat(result).hasValue(42L); } - @Test // DATACMNS-1259 public void lastModifiedAsLocalDateTimeDateIsAvailableViaWrapperAsLocalDateTime() { @@ -148,19 +143,16 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests { AuditedUser source = new AuditedUser(); source.setLastModifiedDate(now); - Optional beanWrapper = factory.getBeanWrapperFor(source); - assertThat(beanWrapper).isPresent(); + Optional result = factory.getBeanWrapperFor(source) // + .flatMap(AuditableBeanWrapper::getLastModifiedDate); - assertThat(beanWrapper.flatMap(AuditableBeanWrapper::getLastModifiedDate).get()) // - .isEqualTo(now); + assertThat(result).hasValue(now); } public static class LongBasedAuditable { - @CreatedDate - public Long dateCreated; + @CreatedDate public Long dateCreated; - @LastModifiedDate - public Long dateModified; + @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 4e4723102..dcf88c63b 100755 --- a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java @@ -71,12 +71,12 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(sample); + Optional> wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> { it.setCreatedBy(Optional.of("Me!")); - assertThat(sample.createdBy).isNotNull(); + assertThat(it.getBean().createdBy).isNotNull(); }); } @@ -85,12 +85,12 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(sample); + Optional> wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> { it.setLastModifiedBy(Optional.of("Me, too!")); - assertThat(sample.lastModifiedBy).isNotNull(); + assertThat(it.getBean().lastModifiedBy).isNotNull(); }); } @@ -99,7 +99,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); - Optional wrapper = factory.getBeanWrapperFor(sample); + Optional> wrapper = factory.getBeanWrapperFor(sample); assertThat(wrapper).hasValueSatisfying(it -> it.setLastModifiedDate(Instant.now())); } @@ -169,8 +169,10 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { SampleWithInstant sample = new SampleWithInstant(); sample.modified = Instant.now(); - Optional wrapper = factory.getBeanWrapperFor(sample); - assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValue(sample.modified); + Optional result = factory.getBeanWrapperFor(sample) // + .flatMap(it -> it.getLastModifiedDate()); + + assertThat(result).hasValue(sample.modified); } @Test // DATACMNS-1259 @@ -187,7 +189,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { WithEmbedded target = new WithEmbedded(); target.embedded = new Embedded(); - Optional wrapper = factory.getBeanWrapperFor(target); + Optional> wrapper = factory.getBeanWrapperFor(target); assertThat(wrapper).hasValueSatisfying(it -> { @@ -199,7 +201,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { it.setLastModifiedDate(now); it.setCreatedDate(now); - Embedded embedded = target.embedded; + Embedded embedded = it.getBean().embedded; assertThat(embedded.created).isEqualTo(now); assertThat(embedded.creator).isEqualTo(user); @@ -215,11 +217,10 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { Sample sample = new Sample(); sample.lastModifiedDate = source; - Optional wrapper = factory.getBeanWrapperFor(sample); + Optional result = factory.getBeanWrapperFor(sample) // + .flatMap(it -> it.getLastModifiedDate()); - assertThat(wrapper.flatMap(it -> it.getLastModifiedDate())).hasValueSatisfying(ta -> { - compareTemporalAccessors(expected, ta); - }); + assertThat(result).hasValueSatisfying(ta -> compareTemporalAccessors(expected, ta)); } private static AbstractLongAssert compareTemporalAccessors(TemporalAccessor expected, TemporalAccessor actual) {