From b3b590dcf0f98730fb3528081235912ae9f9c35b Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 8 Jul 2021 18:36:16 +0200 Subject: [PATCH] Unify entity type detection. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unified the calculations made for entityTypeInformation and entityTypes in AbstractPersistentProperty. This avoids both calculations getting out of sync. Also we avoid premature calculation abortions if SimpleTypeHolder.isSimpleType(…) returns true for the raw property type. The latter has caused issues for collection properties in Spring Data KeyValue which considers everything in java.util a simple type. Related ticket: #2390. --- .../model/AbstractPersistentProperty.java | 108 +++++++----------- .../AbstractMappingContextUnitTests.java | 18 +++ 2 files changed, 60 insertions(+), 66 deletions(-) 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 5afb34d5e..fb30c1387 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -18,19 +18,17 @@ package org.springframework.data.mapping.model; 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.HashSet; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; +import java.util.stream.Collectors; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.util.Lazy; -import org.springframework.data.util.NullableWrapperConverters; import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; @@ -64,11 +62,10 @@ public abstract class AbstractPersistentProperty

private final Property property; private final Lazy hashCode; private final Lazy usePropertyAccess; - private final Lazy> entityTypeInformation; + private final Lazy>> entityTypeInformation; private final Lazy isAssociation; private final Lazy> associationTargetType; - private final Lazy>> entityTypes; private final Method getter; private final Method setter; @@ -100,10 +97,7 @@ public abstract class AbstractPersistentProperty

.map(TypeInformation::getComponentType) // .orElse(null)); - this.entityTypeInformation = Lazy.of(() -> Optional.ofNullable(getAssociationOrActualType()) - .filter(it -> !simpleTypeHolder.isSimpleType(it.getType())) // - .filter(it -> !it.isCollectionLike()) // - .filter(it -> !it.isMap()).orElse(null)); + this.entityTypeInformation = Lazy.of(() -> detectEntityTypes(simpleTypeHolder)); this.getter = property.getGetter().orElse(null); this.setter = property.getSetter().orElse(null); @@ -115,45 +109,6 @@ public abstract class AbstractPersistentProperty

} 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 (NullableWrapperConverters.supports(typeInformation.getType())) { - - 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(); @@ -211,14 +166,14 @@ public abstract class AbstractPersistentProperty

public Iterable> getPersistentEntityTypes() { if (isMap() || isCollectionLike()) { - return entityTypes.get(); + return entityTypeInformation.get(); } if (!isEntity()) { return Collections.emptySet(); } - return entityTypes.get(); + return entityTypeInformation.get(); } /* @@ -362,7 +317,7 @@ public abstract class AbstractPersistentProperty

*/ @Override public boolean isEntity() { - return !isTransient() && entityTypeInformation.getNullable() != null; + return !isTransient() && !entityTypeInformation.get().isEmpty(); } /* @@ -401,7 +356,11 @@ public abstract class AbstractPersistentProperty

*/ @Override public Class getActualType() { - return getRequiredAssociationOrActualType().getType(); + + TypeInformation targetType = associationTargetType.getNullable(); + TypeInformation result = targetType == null ? information.getRequiredActualType() : targetType; + + return result.getType(); } /* @@ -454,23 +413,40 @@ public abstract class AbstractPersistentProperty

return property.toString(); } - @Nullable - private TypeInformation getAssociationOrActualType() { - return getAssociationTypeOr(() -> information.getActualType()); + private Set> detectEntityTypes(SimpleTypeHolder simpleTypes) { + + TypeInformation typeToStartWith = ASSOCIATION_TYPE != null && ASSOCIATION_TYPE.isAssignableFrom(rawType) + ? information.getComponentType() + : information; + + Set> result = detectEntityTypes(typeToStartWith); + + return result.stream() + .filter(it -> !simpleTypes.isSimpleType(it.getType())) + .filter(it -> !it.getType().equals(ASSOCIATION_TYPE)) + .collect(Collectors.toSet()); } - private TypeInformation getRequiredAssociationOrActualType() { - return getAssociationTypeOr(() -> information.getRequiredActualType()); - } + private Set> detectEntityTypes(@Nullable TypeInformation source) { - private TypeInformation getAssociationTypeOr(Supplier> fallback) { - - TypeInformation result = associationTargetType.getNullable(); - - if (result != null) { - return result; + if (source == null) { + return Collections.emptySet(); } - return fallback.get(); + Set> result = new HashSet<>(); + + if (source.isMap()) { + result.addAll(detectEntityTypes(source.getComponentType())); + } + + TypeInformation actualType = source.getActualType(); + + if (source.equals(actualType)) { + result.add(source); + } else { + result.addAll(detectEntityTypes(actualType)); + } + + 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 4e4de0a7d..3be499745 100755 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -295,6 +295,24 @@ class AbstractMappingContextUnitTests { .doesNotContain(List.class, ArrayList.class); } + @Test // GH-2390 + void detectsEntityTypeEveneIfSimpleTypeHolderConsidersCollectionsSimple() { + + context.setSimpleTypeHolder(new SimpleTypeHolder(Collections.emptySet(), true) { + + @Override + public boolean isSimpleType(Class type) { + return type.getName().startsWith("java.util."); + } + }); + + context.getPersistentEntity(WithNestedLists.class); + + assertThat(context.getPersistentEntities()) // + .map(it -> (Class) it.getType()) // + .contains(Base.class); + } + @Test // GH-2390 void shouldNotCreatePersistentEntityForMapButItsGenericTypeArguments() {