From c866b4202527b3acc64e7cc205a98a8aa754cb59 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 27 Oct 2021 12:19:15 +0200 Subject: [PATCH] Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A few further tweaks: ClassTypeInformation now never unwraps the user type to consistently work with the type originally presented in the lookup. That allows us to get rid of the custom equals(…)/hashCode() methods and removes the need to deal with original and user type in the same CTI instance. The augmented caching of PersistentEntities in AbstractMappingContext now primarily happens in doAddPersistentEntity(…). We still need to implement a cache manipulation for the case that a user type has caused the creation of a PersistentEntity first and we see a lookup for the proxy type later. Before actually processing the TypeInformation we now try to lookup an already existing PersistentEntity for the user type and augment the cache if one was found. See #2485 Original pull request: #2486. --- .../context/AbstractMappingContext.java | 34 +++++++++++++------ .../data/util/ClassTypeInformation.java | 25 +------------- .../AbstractMappingContextUnitTests.java | 3 +- 3 files changed, 27 insertions(+), 35 deletions(-) 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 6300e3a82..5a1d93290 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -354,12 +354,12 @@ public abstract class AbstractMappingContext persistentEntity = persistentEntities.computeIfAbsent(typeInformation, - it -> persistentEntities.get(typeInformation.getUserTypeInformation())); + Optional persistentEntity = persistentEntities.get(typeInformation); if (persistentEntity != null) { return persistentEntity; } + } finally { read.unlock(); } @@ -370,12 +370,15 @@ public abstract class AbstractMappingContext typeInformationToUse = typeInformation.getUserTypeInformation(); - entity = doAddPersistentEntity(typeInformation.getUserTypeInformation()); + Optional userTypeEntity = persistentEntities.get(typeInformation.getUserTypeInformation()); - if (!typeInformationToUse.equals(typeInformation)) { - persistentEntities.put(typeInformation, Optional.of(entity)); + if (userTypeEntity != null) { + persistentEntities.put(typeInformation, userTypeEntity); + return userTypeEntity; } + + entity = doAddPersistentEntity(typeInformation); + } catch (BeansException e) { throw new MappingException(e.getMessage(), e); } finally { @@ -392,19 +395,26 @@ public abstract class AbstractMappingContext typeInformation) { + TypeInformation userTypeInformation = typeInformation.getUserTypeInformation(); + try { - Class type = typeInformation.getType(); + Class type = userTypeInformation.getType(); - E entity = createPersistentEntity(typeInformation); + E entity = createPersistentEntity(userTypeInformation); entity.setEvaluationContextProvider(evaluationContextProvider); // Eagerly cache the entity as we might have to find it during recursive lookups. - persistentEntities.put(typeInformation, Optional.of(entity)); + persistentEntities.put(userTypeInformation, Optional.of(entity)); + + // Cache original TypeInformation as well. + if (!userTypeInformation.equals(typeInformation)) { + persistentEntities.put(typeInformation, Optional.of(entity)); + } PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type); - Map descriptors = new HashMap<>(); + for (PropertyDescriptor descriptor : pds) { descriptors.put(descriptor.getName(), descriptor); } @@ -420,8 +430,12 @@ public abstract class AbstractMappingContext extends TypeDiscoverer { * @param type */ ClassTypeInformation(Class type) { - super(ProxyUtils.getUserClass(type), getTypeVariableMap(type)); + super(type, getTypeVariableMap(type)); this.type = type; } @@ -178,26 +177,4 @@ public class ClassTypeInformation 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/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java index 7b4f59614..905d25020 100755 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -39,7 +39,6 @@ import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; - import org.springframework.aop.SpringProxy; import org.springframework.aop.framework.Advised; import org.springframework.context.ApplicationContext; @@ -343,6 +342,7 @@ class AbstractMappingContextUnitTests { assertThat(context.hasPersistentEntityFor(Base$$SpringProxy$873fa2e.class)).isTrue(); assertThat(context.getPersistentEntities()).hasSize(1); // only one distinct instance + assertThat(persistentEntity).isSameAs(persistentEntityForProxy); } @Test // GH-2485 @@ -362,6 +362,7 @@ class AbstractMappingContextUnitTests { persistentEntity.getTypeInformation().getType().equals(Base.class); assertThat(context.getPersistentEntities()).hasSize(1); // only one distinct instance + assertThat(persistentEntity).isSameAs(persistentEntityForProxy); } private static void assertHasEntityFor(Class type, SampleMappingContext context, boolean expected) {