From d4d66db847829f538ad3c79323f1d39596c47b91 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 7 Mar 2024 09:31:55 +0100 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 2ca5f056c7..9ab21ce869 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.2.4-SNAPSHOT + 3.2.4-GH-3056-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. From 5c4a186f169625513be7ba93780fee004ad8d730 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 7 Mar 2024 09:32:56 +0100 Subject: [PATCH 2/3] Do not create persistent properties for Map and Collection-like entities. These types are expected to behave like maps and collections and should not carry properties. The only exception are types implementing Streamable as Streamable can be used in domain types. --- .../context/AbstractMappingContext.java | 50 +++++++++++++++---- .../AbstractMappingContextUnitTests.java | 48 +++++++++++++++--- 2 files changed, 81 insertions(+), 17 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 4c2ecc88f2..1fbc89791b 100644 --- a/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -59,6 +59,7 @@ import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.spel.EvaluationContextProvider; import org.springframework.data.spel.ExtensionAwareEvaluationContextProvider; +import org.springframework.data.util.CustomCollections; import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.data.util.NullableWrapperConverters; import org.springframework.data.util.Optionals; @@ -143,7 +144,6 @@ public void setApplicationContext(ApplicationContext applicationContext) throws * * @param initialEntitySet * @see #setManagedTypes(ManagedTypes) - * */ public void setInitialEntitySet(Set> initialEntitySet) { setManagedTypes(ManagedTypes.fromIterable(initialEntitySet)); @@ -415,16 +415,18 @@ private E doAddPersistentEntity(TypeInformation typeInformation) { persistentEntities.put(typeInformation, Optional.of(entity)); } - PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type); - Map descriptors = new HashMap<>(); + if (shouldCreateProperties(userTypeInformation)) { + PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type); + Map descriptors = new HashMap<>(); - for (PropertyDescriptor descriptor : pds) { - descriptors.put(descriptor.getName(), descriptor); - } + for (PropertyDescriptor descriptor : pds) { + descriptors.put(descriptor.getName(), descriptor); + } - PersistentPropertyCreator persistentPropertyCreator = new PersistentPropertyCreator(entity, descriptors); - ReflectionUtils.doWithFields(type, persistentPropertyCreator, PersistentPropertyFilter.INSTANCE); - persistentPropertyCreator.addPropertiesForRemainingDescriptors(); + PersistentPropertyCreator persistentPropertyCreator = new PersistentPropertyCreator(entity, descriptors); + ReflectionUtils.doWithFields(type, persistentPropertyCreator, PersistentPropertyFilter.INSTANCE); + persistentPropertyCreator.addPropertiesForRemainingDescriptors(); + } entity.verify(); @@ -475,6 +477,35 @@ public Collection> getManagedTypes() { */ protected abstract P createPersistentProperty(Property property, E owner, SimpleTypeHolder simpleTypeHolder); + /** + * Whether to create the {@link PersistentProperty}s for the given {@link TypeInformation}. + * + * @param typeInformation must not be {@literal null}. + * @return {@literal true} properties should be created, {@literal false} otherwise + */ + protected boolean shouldCreateProperties(TypeInformation typeInformation) { + + Class type = typeInformation.getType(); + + return !typeInformation.isMap() && !isCollectionLike(type); + } + + /** + * In contrast to TypeInformation, we do not consider types implementing Streamable collection-like as domain types + * can implement that type. + * + * @param type must not be {@literal null}. + * @return + * @see TypeInformation#isCollectionLike() + */ + private static boolean isCollectionLike(Class type) { + + return type.isArray() // + || Iterable.class.equals(type) // + || Streamable.class.equals(type) // + || Collection.class.isAssignableFrom(type) || CustomCollections.isCollection(type); + } + @Override public void afterPropertiesSet() { initialize(); @@ -532,6 +563,7 @@ private PersistentPropertyCreator(E entity, Map desc this.remainingDescriptors = remainingDescriptors; } + @Override public void doWith(Field field) { String fieldName = field.getName(); 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 e864b5a2f9..9395ef92cc 100755 --- a/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -22,14 +22,7 @@ import groovy.lang.MetaClass; import java.time.LocalDateTime; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.TreeMap; +import java.util.*; import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; @@ -49,6 +42,7 @@ import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.StreamUtils; +import org.springframework.data.util.Streamable; import org.springframework.data.util.TypeInformation; /** @@ -285,6 +279,20 @@ void shouldNotCreatePersistentEntityForListButItsGenericTypeArgument() { .doesNotContain(List.class, ArrayList.class); } + @Test // GH-2390 + void doesNotCreatePropertiesForMapAndCollectionTypes() { + + assertThat(context.getPersistentEntity(HashSet.class)).isEmpty(); + assertThat(context.getPersistentEntity(HashMap.class)).isEmpty(); + } + + @Test // GH-2390 + void createsPropertiesForStreamableAndIterableTypes() { + + assertThat(context.getPersistentEntity(MyStreamable.class)).hasSize(1); + assertThat(context.getPersistentEntity(MyIterable.class)).hasSize(1); + } + @Test // GH-2390 void detectsEntityTypeEvenIfSimpleTypeHolderConsidersCollectionsSimple() { @@ -534,4 +542,28 @@ static abstract class Base$$SpringProxy$873fa2e extends Base implements SpringPr } + static class MyIterable implements Iterable { + + String name; + + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } + } + + static class MyStreamable implements Streamable { + + String name; + + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } + } + + record StreamComponent(String name) { + + } + } From c8cb32cf65818b020603efbbc81c65fb1919bed9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 7 Mar 2024 09:33:04 +0100 Subject: [PATCH 3/3] Polishing. Simplify code. --- .../data/mapping/model/AbstractPersistentProperty.java | 10 +--------- 1 file changed, 1 insertion(+), 9 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 7ccdff4a5c..c8af5bd5a3 100644 --- a/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AbstractPersistentProperty.java @@ -157,15 +157,6 @@ public TypeInformation getTypeInformation() { @Override public Iterable> getPersistentEntityTypeInformation() { - - if (isMap() || isCollectionLike()) { - return entityTypeInformation.get(); - } - - if (!isEntity()) { - return Collections.emptySet(); - } - return entityTypeInformation.get(); } @@ -292,6 +283,7 @@ public Class getActualType() { return getActualTypeInformation().getType(); } + @Override public boolean usePropertyAccess() { return usePropertyAccess.get(); }