From 673b4b534d83b85fd3e06e10e7428196861f7a97 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 22 Jun 2021 10:55:45 +0200 Subject: [PATCH 1/3] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9001e043d4..17ce3e93dc 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.6.0-SNAPSHOT + 2.6.0-GH-2390-SNAPSHOT Spring Data Core From 282e33bf1602e5e9929372deae995aa8ec3094aa Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 23 Jun 2021 15:29:15 +0200 Subject: [PATCH 2/3] Don't create PersistentEntity for Nullable wrapper type. --- .../context/AbstractMappingContext.java | 19 ++++++++++++++++++- .../data/util/TypeDiscoverer.java | 8 ++++++++ .../data/util/TypeInformation.java | 5 +++++ 3 files changed, 31 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 63f01a3552..352bcd4148 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -30,6 +30,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,6 +62,7 @@ import org.springframework.data.spel.ExtensionAwareEvaluationContextProvider; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.KotlinReflectionUtils; +import org.springframework.data.util.NullableWrapperConverters; import org.springframework.data.util.Optionals; import org.springframework.data.util.Streamable; import org.springframework.data.util.TypeInformation; @@ -483,6 +485,9 @@ protected boolean shouldCreatePersistentEntityFor(TypeInformation type) { if (simpleTypeHolder.isSimpleType(type.getType())) { return false; } + if(NullableWrapperConverters.supports(type.getType())) { + return false; + } return !KotlinDetector.isKotlinType(type.getType()) || KotlinReflectionUtils.isSupportedKotlinClass(type.getType()); } @@ -569,7 +574,19 @@ private void createAndRegisterProperty(Property input) { return; } - property.getPersistentEntityTypes().forEach(AbstractMappingContext.this::addPersistentEntity); + StreamSupport.stream(property.getPersistentEntityTypes().spliterator(), false) + .map(it -> { + if(it.isNullableWrapper()) { + return it.getActualType(); + } + return it; + }) + .filter(it -> { + + boolean shouldCreate = AbstractMappingContext.this.shouldCreatePersistentEntityFor(it); + return shouldCreate; + }) + .forEach(AbstractMappingContext.this::addPersistentEntity); } protected boolean shouldSkipOverrideProperty(P property) { diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index 6ce897ac4e..141249f423 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -309,6 +309,10 @@ public TypeInformation getActualType() { return getComponentType(); } + if (isNullableWrapper()) { + return getComponentType(); + } + return this; } @@ -384,6 +388,10 @@ protected TypeInformation doGetComponentType() { return getTypeArgument(Iterable.class, 0); } + if(isNullableWrapper()) { + return getTypeArgument(rawType, 0); + } + List> arguments = getTypeArguments(); return arguments.size() > 0 ? arguments.get(0) : null; diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java index 014ffad0ea..2db2b5c91b 100644 --- a/src/main/java/org/springframework/data/util/TypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeInformation.java @@ -273,4 +273,9 @@ default TypeInformation getRequiredSuperTypeInformation(Class superType) { default boolean isSubTypeOf(Class type) { return !type.equals(getType()) && type.isAssignableFrom(getType()); } + + default boolean isNullableWrapper() { + return NullableWrapperConverters.supports(getType()); + } + } From cedafdceca9e1d5d46f86c95da7440780b8b84b0 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 25 Jun 2021 13:29:37 +0200 Subject: [PATCH 3/3] Resolve persistent entities for a property fully. --- .../context/AbstractMappingContext.java | 23 +++----- .../model/AbstractPersistentProperty.java | 51 ++++++++++++++++- .../data/util/TypeInformation.java | 5 ++ .../AbstractMappingContextUnitTests.java | 55 +++++++++++++++++++ 4 files changed, 115 insertions(+), 19 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 352bcd4148..625bc7dab0 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -34,7 +34,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.InitializingBean; @@ -116,7 +115,8 @@ protected AbstractMappingContext() { this.persistentPropertyPathFactory = new PersistentPropertyPathFactory<>(this); EntityInstantiators instantiators = new EntityInstantiators(); - PersistentPropertyAccessorFactory accessorFactory = NativeDetector.inNativeImage() ? BeanWrapperPropertyAccessorFactory.INSTANCE + PersistentPropertyAccessorFactory accessorFactory = NativeDetector.inNativeImage() + ? BeanWrapperPropertyAccessorFactory.INSTANCE : new ClassGeneratingPropertyAccessorFactory(); this.persistentPropertyAccessorFactory = new InstantiationAwarePropertyAccessorFactory(accessorFactory, @@ -485,7 +485,8 @@ protected boolean shouldCreatePersistentEntityFor(TypeInformation type) { if (simpleTypeHolder.isSimpleType(type.getType())) { return false; } - if(NullableWrapperConverters.supports(type.getType())) { + + if (NullableWrapperConverters.supports(type.getType())) { return false; } @@ -564,6 +565,7 @@ private void createAndRegisterProperty(Property input) { if (shouldSkipOverrideProperty(property)) { return; } + entity.addPersistentProperty(property); if (property.isAssociation()) { @@ -574,18 +576,8 @@ private void createAndRegisterProperty(Property input) { return; } - StreamSupport.stream(property.getPersistentEntityTypes().spliterator(), false) - .map(it -> { - if(it.isNullableWrapper()) { - return it.getActualType(); - } - return it; - }) - .filter(it -> { - - boolean shouldCreate = AbstractMappingContext.this.shouldCreatePersistentEntityFor(it); - return shouldCreate; - }) + StreamSupport.stream(property.getPersistentEntityTypes().spliterator(), false) // + .filter(AbstractMappingContext.this::shouldCreatePersistentEntityFor) // .forEach(AbstractMappingContext.this::addPersistentEntity); } @@ -668,7 +660,6 @@ private Class getPropertyType(PersistentProperty persistentProperty) { } } - /** * Filter rejecting static fields as well as artificially introduced ones. See * {@link PersistentPropertyFilter#UNMAPPED_PROPERTIES} for details. 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 836984db31..346137f999 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -18,11 +18,16 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; +import org.springframework.core.GenericTypeResolver; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -64,6 +69,7 @@ public abstract class AbstractPersistentProperty

private final Lazy isAssociation; private final Lazy> associationTargetType; + private final Lazy>> entityTypes; private final Method getter; private final Method setter; @@ -111,6 +117,43 @@ public AbstractPersistentProperty(Property property, PersistentEntity owne } else { this.immutable = false; } + + this.entityTypes = Lazy.of(() -> collectEntityTypes(simpleTypeHolder, information, new LinkedHashSet<>())); + } + + protected Set> collectEntityTypes(SimpleTypeHolder simpleTypeHolder, @Nullable TypeInformation typeInformation, Set> entityTypes) { + + if(typeInformation == null || entityTypes.contains(typeInformation) || simpleTypeHolder.isSimpleType(typeInformation.getType())) { + return entityTypes; + } + + if(typeInformation.isMap()) { + + collectEntityTypes(simpleTypeHolder, typeInformation.getComponentType(), entityTypes); + collectEntityTypes(simpleTypeHolder, typeInformation.getMapValueType(), entityTypes); + return entityTypes; + } + + if(typeInformation.isCollectionLike()) { + + collectEntityTypes(simpleTypeHolder, typeInformation.getComponentType(), entityTypes); + return entityTypes; + } + + if(typeInformation.isNullableWrapper()) { + + collectEntityTypes(simpleTypeHolder, typeInformation.getActualType(), entityTypes); + return entityTypes; + } + + if(ASSOCIATION_TYPE != null && ASSOCIATION_TYPE.isAssignableFrom(typeInformation.getType())) { + + entityTypes.add(getAssociationOrActualType()); + return entityTypes; + } + + entityTypes.add(typeInformation); + return entityTypes; } protected abstract Association

createAssociation(); @@ -167,13 +210,15 @@ public TypeInformation getTypeInformation() { @Override public Iterable> getPersistentEntityTypes() { + if(isMap() || isCollectionLike()) { + return entityTypes.get(); + } + if (!isEntity()) { return Collections.emptySet(); } - TypeInformation result = getAssociationTypeOr(() -> entityTypeInformation.getNullable()); - - return result != null ? Collections.singleton(result) : Collections.emptySet(); + return entityTypes.get(); } /* diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java index 2db2b5c91b..17ec425d36 100644 --- a/src/main/java/org/springframework/data/util/TypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeInformation.java @@ -274,6 +274,11 @@ default boolean isSubTypeOf(Class type) { return !type.equals(getType()) && type.isAssignableFrom(getType()); } + /** + * Returns whether the current type is considered a {@literal null} value wrapper or not. + * + * @return {@literal true} if the type is considered to be a {@literal null} value wrapper such as {@link java.util.Optional}. + */ default boolean isNullableWrapper() { return NullableWrapperConverters.supports(getType()); } 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 650afde73c..4e4de0a7d2 100755 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -26,11 +26,14 @@ import lombok.Value; import java.time.LocalDateTime; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import java.util.function.Supplier; @@ -272,6 +275,36 @@ void shouldIgnoreNonAssignableOverridePropertyInSuperClass() { }); } + @Test // GH-2390 + void shouldNotCreatePersistentEntityForOptionalButItsGenericTypeArgument() { + + context.getPersistentEntity(WithOptionals.class); + + assertThat(context.getPersistentEntities()).map(it -> (Class) it.getType()) + .contains(WithOptionals.class, Person.class, Base.class) + .doesNotContain(Optional.class, List.class, ArrayList.class); + } + + @Test // GH-2390 + void shouldNotCreatePersistentEntityForListButItsGenericTypeArgument() { + + context.getPersistentEntity(WithNestedLists.class); + + assertThat(context.getPersistentEntities()).map(it -> (Class) it.getType()) + .contains(Base.class) + .doesNotContain(List.class, ArrayList.class); + } + + @Test // GH-2390 + void shouldNotCreatePersistentEntityForMapButItsGenericTypeArguments() { + + context.getPersistentEntity(WithMap.class); + + assertThat(context.getPersistentEntities()).map(it -> (Class) it.getType()) + .contains(Base.class, Person.class, MapKey.class) + .doesNotContain(List.class, Map.class, String.class, Integer.class); + } + private static void assertHasEntityFor(Class type, SampleMappingContext context, boolean expected) { boolean found = false; @@ -432,4 +465,26 @@ static class ShadowingPropertyAssignable extends ShadowedPropertyAssignable { } } + class WithOptionals { + + Optional optionalOfString; + Optional optionalOfPerson; + List> listOfOptionalOfBase; + } + + class WithNestedLists { + ArrayList> arrayListOfOptionalOfBase; + } + + class MapKey { + + } + + class WithMap { + + Map> mapOfStringToList; + Map mapOfStringToPerson; + Map mapOfKeyToPerson; + } + }