From 23ebf3164f1ec33d4052743451a6e908d1993956 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 31 Mar 2021 07:51:02 +0200 Subject: [PATCH] Support for association target type detection for jMolecules. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now detect the component type of jMolecules' Association as association target type in AbstractPersistentProperty. AnnotationBasedPersistentProperty keeps the lookup of the explicit @Reference around but falls back to the Association analysis for that. @Reference is now only considered by AnnotationBasedPersistentProperty. It was previously also (erroneously) considered by AbstractPersistentProperty. It was also changed to return `null` in case no association target type could be looked up instead of returning the sole property's type in case that is an entity. This was done to satisfy the behavior documented on the interface but also to avoid the call to isEntity() during the calculation which might use association information in turn and thus lead to a stack overflow. Reworded the contract of PersistentProperty.getAssociationTargetType() to emphasize the symmetry with ….isAssociation() in implementation classes. Closes: #2344. --- .../data/mapping/PersistentProperty.java | 10 +++-- .../model/AbstractPersistentProperty.java | 29 +++++++++++++-- .../AnnotationBasedPersistentProperty.java | 25 +++++++------ .../AbstractPersistentPropertyUnitTests.java | 14 ++++--- ...ationBasedPersistentPropertyUnitTests.java | 37 +++++++++++++++---- 5 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 8b4c8ad7a..150ba628f 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -383,11 +383,13 @@ public interface PersistentProperty

> { } /** - * Return the type the property refers to in case it's an association. + * Return the type the property refers to in case it's an association, i.e. {@link #isAssociation()} returns + * {@literal true}. That means, that implementations must return a non-{@literal null} value from this method + * in that case. We also recommend to return {@literal null} for non-associations right away to establish symmetry + * between this method and {@link #isAssociation()}. * - * @return the type the property refers to in case it's an association or {@literal null} in case it's not an - * association, the target entity type is not explicitly defined (either explicitly or through the property - * type itself). + * @return the type the property refers to in case it's an association, i.e. {@link #isAssociation()} returns + * {@literal true}. * @since 2.1 */ @Nullable diff --git a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java index 321503ad9..49618a8a2 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.Map; import java.util.Optional; -import org.springframework.data.annotation.Reference; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -64,6 +63,9 @@ public abstract class AbstractPersistentProperty

private final Lazy usePropertyAccess; private final Lazy>> entityTypeInformation; + private final Lazy isAssociation; + private final Lazy> associationTargetType; + private final Method getter; private final Method setter; private final Field field; @@ -86,6 +88,15 @@ public abstract class AbstractPersistentProperty

this.hashCode = Lazy.of(property::hashCode); this.usePropertyAccess = Lazy.of(() -> owner.getType().isInterface() || CAUSE_FIELD.equals(getField())); + this.isAssociation = Lazy.of(() -> ASSOCIATION_TYPE != null && ASSOCIATION_TYPE.isAssignableFrom(rawType)); + this.associationTargetType = ASSOCIATION_TYPE == null + ? Lazy.empty() + : Lazy.of(() -> Optional.of(getTypeInformation()) + .map(it -> it.getSuperTypeInformation(ASSOCIATION_TYPE)) + .map(TypeInformation::getComponentType) + .map(TypeInformation::getType) + .orElse(null)); + this.entityTypeInformation = Lazy.of(() -> Optional.ofNullable(information.getActualType())// .filter(it -> !simpleTypeHolder.isSimpleType(it.getType()))// .filter(it -> !it.isCollectionLike())// @@ -170,6 +181,7 @@ public abstract class AbstractPersistentProperty

* (non-Javadoc) * @see org.springframework.data.mapping.PersistentProperty#getGetter() */ + @Nullable @Override public Method getGetter() { return this.getter; @@ -179,6 +191,7 @@ public abstract class AbstractPersistentProperty

* (non-Javadoc) * @see org.springframework.data.mapping.PersistentProperty#getSetter() */ + @Nullable @Override public Method getSetter() { return this.setter; @@ -188,6 +201,7 @@ public abstract class AbstractPersistentProperty

* (non-Javadoc) * @see org.springframework.data.mapping.PersistentProperty#getWither() */ + @Nullable @Override public Method getWither() { return this.wither; @@ -245,8 +259,7 @@ public abstract class AbstractPersistentProperty

*/ @Override public boolean isAssociation() { - return isAnnotationPresent(Reference.class) // - || ASSOCIATION_TYPE != null && ASSOCIATION_TYPE.isAssignableFrom(rawType); + return isAssociation.get(); } /* @@ -259,6 +272,16 @@ public abstract class AbstractPersistentProperty

return association.orElse(null); } + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.PersistentProperty#getAssociationTargetType() + */ + @Nullable + @Override + public Class getAssociationTargetType() { + return associationTargetType.getNullable(); + } + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentProperty#isCollectionLike() diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index b7eb902e8..ffca2689e 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -74,6 +74,18 @@ public abstract class AnnotationBasedPersistentProperty

isId = Lazy.of(() -> isAnnotationPresent(Id.class)); private final Lazy isVersion = Lazy.of(() -> isAnnotationPresent(Version.class)); + private final Lazy> associationTargetType = Lazy.of(() -> { + + if (!isAssociation()) { + return null; + } + + return Optional.of(Reference.class) // + .map(this::findAnnotation) // + .map(Reference::to) // + .map(it -> !Class.class.equals(it) ? it : getActualType()) // + .orElseGet(() -> super.getAssociationTargetType()); + }); /** * Creates a new {@link AnnotationBasedPersistentProperty}. @@ -285,18 +297,7 @@ public abstract class AnnotationBasedPersistentProperty

getAssociationTargetType() { - - Reference reference = findAnnotation(Reference.class); - - if (reference == null) { - return isEntity() ? getActualType() : null; - } - - Class targetType = reference.to(); - - return Class.class.equals(targetType) // - ? isEntity() ? getActualType() : null // - : targetType; + return associationTargetType.getNullable(); } /* diff --git a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java index e17b8ec7b..06cd5300b 100755 --- a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java @@ -32,6 +32,8 @@ import java.util.Optional; import java.util.TreeMap; import java.util.TreeSet; +import org.jmolecules.ddd.types.AggregateRoot; +import org.jmolecules.ddd.types.Identifier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.data.mapping.Association; @@ -230,6 +232,7 @@ public class AbstractPersistentPropertyUnitTests { SamplePersistentProperty property = getProperty(JMolecules.class, "association"); assertThat(property.isAssociation()).isTrue(); + assertThat(property.getAssociationTargetType()).isEqualTo(JMoleculesAggregate.class); } private BasicPersistentEntity getEntity(Class type) { @@ -371,11 +374,6 @@ public class AbstractPersistentPropertyUnitTests { public A findPropertyOrOwnerAnnotation(Class annotationType) { return null; } - - @Override - public Class getAssociationTargetType() { - return null; - } } static class Sample { @@ -392,6 +390,10 @@ public class AbstractPersistentPropertyUnitTests { } class JMolecules { - org.jmolecules.ddd.types.Association association; + org.jmolecules.ddd.types.Association association; + } + + interface JMoleculesAggregate extends AggregateRoot { + } } diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 35c8db2a9..badaf11e5 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -24,11 +24,17 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.util.Map; import java.util.Optional; +import java.util.function.Function; import java.util.stream.Stream; +import org.assertj.core.api.ClassAssert; +import org.jmolecules.ddd.types.AggregateRoot; import org.jmolecules.ddd.types.Association; +import org.jmolecules.ddd.types.Identifier; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestFactory; import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType; @@ -242,12 +248,21 @@ public class AnnotationBasedPersistentPropertyUnitTests

property.getRequiredAnnotation(Transient.class)).isInstanceOf(IllegalStateException.class); } - @Test // DATACMNS-1318 - public void detectsUltimateAssociationTargetClass() { + @TestFactory // DATACMNS-1318 + public Stream detectsUltimateAssociationTargetClass() { - Stream.of("toSample", "toSample2", "sample", "withoutAnnotation").forEach(it -> { - assertThat(getProperty(WithReferences.class, it).getAssociationTargetType()).isEqualTo(Sample.class); - }); + Function verifier = it -> assertThat( + getProperty(WithReferences.class, it).getAssociationTargetType()); + + Stream positives = DynamicTest.stream(Stream.of("toSample", "toSample2", "sample"), // + it -> String.format("Property %s resolves to %s.", it, Sample.class.getSimpleName()), // + it -> verifier.apply(it).isEqualTo(Sample.class)); + + Stream negatives = DynamicTest.stream(Stream.of("withoutAnnotation"), // + it -> String.format("Property %s resolves to null.", it), // + it -> verifier.apply(it).isNull()); + + return Stream.concat(positives, negatives); } @Test // DATACMNS-1359 @@ -295,8 +310,12 @@ public class AnnotationBasedPersistentPropertyUnitTests

association; } + + interface JMoleculesAggregate extends AggregateRoot {} }