From c30f3fc8af6ccea493f0cfd396d5f9c1b3475ad8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 21 Oct 2021 16:00:25 +0200 Subject: [PATCH] Create PersistentEntities only for user class in case of proxy instance. This commit makes sure to only create a PersistentEntity for an actual user type. Therefore ClassTypeInformation now considers the given type and not only the user one for both equals and hashcode. This makes sure we can distinguish TypeInformation for a proxy from the one of a user class. The AbstractMapping context will take care of unpacking proxied types and registering the created entity for both the user as well as the proxy type information. Prior to this commit build time generated proxy instances would have been able to pollute the context depending on the order they had been served by the initial entity set. Closes #2485 Original pull request: #2486. --- .../context/AbstractMappingContext.java | 16 +- .../data/util/ClassTypeInformation.java | 23 +++ .../data/util/TypeInformation.java | 23 +++ .../AbstractMappingContextUnitTests.java | 160 ++++++++++++++++++ .../util/ClassTypeInformationUnitTests.java | 136 +++++++++++++++ 5 files changed, 357 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index a04306336..83ed7b9d6 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -360,6 +360,17 @@ public abstract class AbstractMappingContext userTypeInformation = typeInformation.getUserTypeInformation(); + persistentEntity = persistentEntities.get(userTypeInformation); + + if (persistentEntity != null) { + persistentEntities.put(typeInformation, persistentEntity); + return persistentEntity; + } + } + } finally { read.unlock(); } @@ -369,7 +380,10 @@ public abstract class AbstractMappingContext extends TypeDiscoverer { public String toString() { return type.getName(); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + + if (!super.equals(o)) { + return false; + } + + ClassTypeInformation that = (ClassTypeInformation) o; + return ObjectUtils.nullSafeEquals(type, that.type); + } + + @Override + public int hashCode() { + + int result = super.hashCode(); + result = 31 * result + ObjectUtils.nullSafeHashCode(type); + return result; + } } diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java index d5a2f2642..25615c6d9 100644 --- a/src/main/java/org/springframework/data/util/TypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeInformation.java @@ -149,6 +149,29 @@ public interface TypeInformation { */ Class getType(); + /** + * Returns the user type of the property if proxied. + * + * @return the unpacked (if proxied) type of the property. + * @see ProxyUtils#getUserClass(Class) + * @since 2.6 + */ + default TypeInformation getUserTypeInformation() { + + Class userType = ProxyUtils.getUserClass(getType()); + return userType.equals(getType()) ? this : ClassTypeInformation.from(userType); + } + + /** + * Returns if {@link #getType()} refers to a proxy or user class. + * + * @return true if type is a proxy. + * @since 2.6 + */ + default boolean isProxyTypeInformation() { + return !this.equals(getUserTypeInformation()); + } + /** * Returns a {@link ClassTypeInformation} to represent the {@link TypeInformation} of the raw type of the current * instance. diff --git a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java index f492abf81..ca79972f5 100755 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -37,8 +37,14 @@ import java.util.Optional; import java.util.TreeMap; import java.util.function.Supplier; +import org.aopalliance.aop.Advice; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.aop.Advisor; +import org.springframework.aop.SpringProxy; +import org.springframework.aop.TargetSource; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.framework.AopConfigException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.data.annotation.Id; @@ -54,6 +60,7 @@ import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.StreamUtils; import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; /** * Unit test for {@link AbstractMappingContext}. @@ -323,6 +330,40 @@ class AbstractMappingContextUnitTests { .doesNotContain(List.class, Map.class, String.class, Integer.class); } + @Test // GH-2485 + void contextSeesUserTypeBeforeProxy() { + + SampleMappingContext context = new SampleMappingContext(); + BasicPersistentEntity persistentEntity = context.getPersistentEntity(Base.class); + persistentEntity.getTypeInformation().getType().equals(Base.class); + + assertThat(context.hasPersistentEntityFor(Base.class)).isTrue(); + assertThat(context.hasPersistentEntityFor(Base$$SpringProxy$873fa2e.class)).isFalse(); + + BasicPersistentEntity persistentEntityForProxy = context.getPersistentEntity(Base$$SpringProxy$873fa2e.class); + persistentEntityForProxy.getTypeInformation().getType().equals(Base.class); + assertThat(context.hasPersistentEntityFor(Base$$SpringProxy$873fa2e.class)).isTrue(); + + assertThat(context.getPersistentEntities()).hasSize(1); // only one distinct instance + } + + @Test // GH-2485 + void contextSeesProxyBeforeUserType() { + + SampleMappingContext context = new SampleMappingContext(); + + BasicPersistentEntity persistentEntityForProxy = context.getPersistentEntity(Base$$SpringProxy$873fa2e.class); + persistentEntityForProxy.getTypeInformation().getType().equals(Base.class); + + assertThat(context.hasPersistentEntityFor(Base$$SpringProxy$873fa2e.class)).isTrue(); + assertThat(context.hasPersistentEntityFor(Base.class)).isTrue(); + + BasicPersistentEntity persistentEntity = context.getPersistentEntity(Base.class); + persistentEntity.getTypeInformation().getType().equals(Base.class); + + assertThat(context.getPersistentEntities()).hasSize(1); // only one distinct instance + } + private static void assertHasEntityFor(Class type, SampleMappingContext context, boolean expected) { boolean found = false; @@ -505,4 +546,123 @@ class AbstractMappingContextUnitTests { Map mapOfKeyToPerson; } + static class Base$$SpringProxy$873fa2e extends Base implements SpringProxy, Advised { + + @Override + public boolean isFrozen() { + return false; + } + + @Override + public boolean isProxyTargetClass() { + return false; + } + + @Override + public Class[] getProxiedInterfaces() { + return new Class[0]; + } + + @Override + public boolean isInterfaceProxied(Class intf) { + return false; + } + + @Override + public void setTargetSource(TargetSource targetSource) { + + } + + @Override + public TargetSource getTargetSource() { + return null; + } + + @Override + public void setExposeProxy(boolean exposeProxy) { + + } + + @Override + public boolean isExposeProxy() { + return false; + } + + @Override + public void setPreFiltered(boolean preFiltered) { + + } + + @Override + public boolean isPreFiltered() { + return false; + } + + @Override + public Advisor[] getAdvisors() { + return new Advisor[0]; + } + + @Override + public void addAdvisor(Advisor advisor) throws AopConfigException { + + } + + @Override + public void addAdvisor(int pos, Advisor advisor) throws AopConfigException { + + } + + @Override + public boolean removeAdvisor(Advisor advisor) { + return false; + } + + @Override + public void removeAdvisor(int index) throws AopConfigException { + + } + + @Override + public int indexOf(Advisor advisor) { + return 0; + } + + @Override + public boolean replaceAdvisor(Advisor a, Advisor b) throws AopConfigException { + return false; + } + + @Override + public void addAdvice(Advice advice) throws AopConfigException { + + } + + @Override + public void addAdvice(int pos, Advice advice) throws AopConfigException { + + } + + @Override + public boolean removeAdvice(Advice advice) { + return false; + } + + @Override + public int indexOf(Advice advice) { + return 0; + } + + @Override + public String toProxyConfigString() { + return null; + } + + @Nullable + @Override + public Class getTargetClass() { + return null; + } + } + } diff --git a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java index a327cf28a..f8487b778 100755 --- a/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java +++ b/src/test/java/org/springframework/data/util/ClassTypeInformationUnitTests.java @@ -28,8 +28,16 @@ import java.util.Map; import java.util.Set; import java.util.SortedMap; +import org.aopalliance.aop.Advice; import org.junit.jupiter.api.Test; +import org.springframework.aop.Advisor; +import org.springframework.aop.SpringProxy; +import org.springframework.aop.TargetSource; +import org.springframework.aop.framework.Advised; +import org.springframework.aop.framework.AopConfigException; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.data.mapping.Person; +import org.springframework.lang.Nullable; /** * Unit tests for {@link ClassTypeInformation}. @@ -469,6 +477,15 @@ public class ClassTypeInformationUnitTests { assertThat(justMap.getMapValueType().getType()).isEqualTo(Long.class); } + @Test // GH-2485 + public void proxyTypeInformationShouldNotEqualUserClassTypeInfo () { + + ClassTypeInformation typeInfoLeaf = from(Leaf.class); + ClassTypeInformation typeInformationLeafProxy = from(Leaf$$SpringProxy$873fa2e.class); + + assertThat(typeInfoLeaf).isNotEqualTo(typeInformationLeafProxy); + } + static class StringMapContainer extends MapContainer { } @@ -718,4 +735,123 @@ public class ClassTypeInformationUnitTests { MultiValueMap longMultiValueMap; Map justMap; } + + static class Leaf$$SpringProxy$873fa2e extends Leaf implements SpringProxy, Advised { + + @Override + public boolean isFrozen() { + return false; + } + + @Override + public boolean isProxyTargetClass() { + return false; + } + + @Override + public Class[] getProxiedInterfaces() { + return new Class[0]; + } + + @Override + public boolean isInterfaceProxied(Class intf) { + return false; + } + + @Override + public void setTargetSource(TargetSource targetSource) { + + } + + @Override + public TargetSource getTargetSource() { + return null; + } + + @Override + public void setExposeProxy(boolean exposeProxy) { + + } + + @Override + public boolean isExposeProxy() { + return false; + } + + @Override + public void setPreFiltered(boolean preFiltered) { + + } + + @Override + public boolean isPreFiltered() { + return false; + } + + @Override + public Advisor[] getAdvisors() { + return new Advisor[0]; + } + + @Override + public void addAdvisor(Advisor advisor) throws AopConfigException { + + } + + @Override + public void addAdvisor(int pos, Advisor advisor) throws AopConfigException { + + } + + @Override + public boolean removeAdvisor(Advisor advisor) { + return false; + } + + @Override + public void removeAdvisor(int index) throws AopConfigException { + + } + + @Override + public int indexOf(Advisor advisor) { + return 0; + } + + @Override + public boolean replaceAdvisor(Advisor a, Advisor b) throws AopConfigException { + return false; + } + + @Override + public void addAdvice(Advice advice) throws AopConfigException { + + } + + @Override + public void addAdvice(int pos, Advice advice) throws AopConfigException { + + } + + @Override + public boolean removeAdvice(Advice advice) { + return false; + } + + @Override + public int indexOf(Advice advice) { + return 0; + } + + @Override + public String toProxyConfigString() { + return null; + } + + @Nullable + @Override + public Class getTargetClass() { + return null; + } + } }