From ced3dde2a42713c670af74ba64a6740beff33919 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 9 Jan 2019 22:48:37 +0100 Subject: [PATCH] DATACMNS-1609, DATACMNS-1438 - Skip collection path segments for auditing properties. We now skip PersistentPropertyPath instances pointing to auditing properties for which the path contains a collection or map path segment as the PersistentPropertyAccessor currently cannot handle those. A more extensive fix for that will be put in place for Moore but requires more extensive API changes which we don't want to ship in a Lovelace maintenance release. Related tickets: DATACMNS-1461. --- .../MappingAuditableBeanWrapperFactory.java | 27 ++++++++---- .../data/mapping/PersistentPropertyPaths.java | 11 +++++ .../PersistentPropertyPathFactory.java | 17 ++++++++ ...gAuditableBeanWrapperFactoryUnitTests.java | 41 +++++++++++++++++++ 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index 957fe7f4a..de02dd63f 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -99,6 +99,9 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap */ static class MappingAuditingMetadata { + private static final Predicate> HAS_COLLECTION_PROPERTY = it -> it.isCollectionLike() + || it.isMap(); + private final PersistentPropertyPaths> createdByPaths, createdDatePaths, lastModifiedByPaths, lastModifiedDatePaths; @@ -113,10 +116,10 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap Assert.notNull(type, "Type must not be null!"); - this.createdByPaths = context.findPersistentPropertyPaths(type, withAnnotation(CreatedBy.class)); - this.createdDatePaths = context.findPersistentPropertyPaths(type, withAnnotation(CreatedDate.class)); - this.lastModifiedByPaths = context.findPersistentPropertyPaths(type, withAnnotation(LastModifiedBy.class)); - this.lastModifiedDatePaths = context.findPersistentPropertyPaths(type, withAnnotation(LastModifiedDate.class)); + this.createdByPaths = findPropertyPaths(type, CreatedBy.class, context); + this.createdDatePaths = findPropertyPaths(type, CreatedDate.class, context); + this.lastModifiedByPaths = findPropertyPaths(type, LastModifiedBy.class, context); + this.lastModifiedDatePaths = findPropertyPaths(type, LastModifiedDate.class, context); this.isAuditable = Lazy.of( // () -> Arrays.asList(createdByPaths, createdDatePaths, lastModifiedByPaths, lastModifiedDatePaths) // @@ -125,10 +128,6 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap ); } - private static Predicate> withAnnotation(Class type) { - return t -> t.findAnnotation(type) != null; - } - /** * Returns whether the {@link PersistentEntity} is auditable at all (read: any of the auditing annotations is * present). @@ -138,6 +137,18 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap public boolean isAuditable() { return isAuditable.get(); } + + private PersistentPropertyPaths> findPropertyPaths(Class type, + Class annotation, MappingContext> context) { + + return context // + .findPersistentPropertyPaths(type, withAnnotation(annotation)) // + .dropPathIfSegmentMatches(HAS_COLLECTION_PROPERTY); + } + + private static Predicate> withAnnotation(Class type) { + return t -> t.findAnnotation(type) != null; + } } /** diff --git a/src/main/java/org/springframework/data/mapping/PersistentPropertyPaths.java b/src/main/java/org/springframework/data/mapping/PersistentPropertyPaths.java index 42ab35147..d05e8f9db 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentPropertyPaths.java +++ b/src/main/java/org/springframework/data/mapping/PersistentPropertyPaths.java @@ -16,6 +16,7 @@ package org.springframework.data.mapping; import java.util.Optional; +import java.util.function.Predicate; import org.springframework.data.util.Streamable; @@ -51,4 +52,14 @@ public interface PersistentPropertyPaths> * @return */ boolean contains(PropertyPath path); + + /** + * Drops {@link PersistentPropertyPath}s that contain a path segment matching the given predicate. + * + * @param predicate must not be {@literal null}. + * @return a {@link PersistentPropertyPaths} instance with all {@link PersistentPropertyPath} instances removed that + * contain path segments matching the given predicate. + * @since 2.1.4 / 2.2.2 + */ + PersistentPropertyPaths dropPathIfSegmentMatches(Predicate predicate); } diff --git a/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java b/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java index 80d303fd2..cf0d683a4 100644 --- a/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java +++ b/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java @@ -33,6 +33,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.springframework.data.mapping.AssociationHandler; @@ -356,6 +357,22 @@ class PersistentPropertyPathFactory, P extends return paths.iterator(); } + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.PersistentPropertyPaths#dropPathIfSegmentMatches(java.util.function.Predicate) + */ + @Override + public PersistentPropertyPaths dropPathIfSegmentMatches(Predicate predicate) { + + Assert.notNull(predicate, "Predicate must not be null!"); + + List> paths = this.stream() // + .filter(it -> !it.stream().anyMatch(predicate)) // + .collect(Collectors.toList()); + + return paths.equals(this.paths) ? this : new DefaultPersistentPropertyPaths<>(type, paths); + } + /** * Simple {@link Comparator} to sort {@link PersistentPropertyPath} instances by their property segment's name * length. diff --git a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java index e1564ce02..530291896 100755 --- a/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactoryUnitTests.java @@ -23,9 +23,13 @@ import java.time.LocalDateTime; import java.time.ZoneOffset; import java.time.temporal.ChronoField; import java.time.temporal.TemporalAccessor; +import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.Date; import java.util.GregorianCalendar; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import org.assertj.core.api.AbstractLongAssert; @@ -222,6 +226,41 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { }); } + @Test // DATACMNS-1438 + public void skipsCollectionPropertiesWhenSettingProperties() { + + WithEmbedded withEmbedded = new WithEmbedded(); + withEmbedded.embedded = new Embedded(); + withEmbedded.embeddeds = Arrays.asList(new Embedded()); + withEmbedded.embeddedMap = new HashMap<>(); + withEmbedded.embeddedMap.put("key", new Embedded()); + + assertThat(factory.getBeanWrapperFor(withEmbedded)).hasValueSatisfying(it -> { + + String user = "user"; + Instant now = Instant.now(); + + it.setCreatedBy(user); + it.setLastModifiedBy(user); + it.setLastModifiedDate(now); + it.setCreatedDate(now); + + Embedded embedded = withEmbedded.embeddeds.iterator().next(); + + assertThat(embedded.created).isNull(); + assertThat(embedded.creator).isNull(); + assertThat(embedded.modified).isNull(); + assertThat(embedded.modifier).isNull(); + + embedded = withEmbedded.embeddedMap.get("key"); + + assertThat(embedded.created).isNull(); + assertThat(embedded.creator).isNull(); + assertThat(embedded.modified).isNull(); + assertThat(embedded.modifier).isNull(); + }); + } + private void assertLastModificationDate(Object source, TemporalAccessor expected) { Sample sample = new Sample(); @@ -284,5 +323,7 @@ public class MappingAuditableBeanWrapperFactoryUnitTests { static class WithEmbedded { Embedded embedded; + Collection embeddeds; + Map embeddedMap; } }