From 3db2669512467ab7dab89f8e74d6142a68f751d5 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mark.paluch@broadcom.com>
Date: Thu, 7 Mar 2024 09:32:56 +0100
Subject: [PATCH] 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.

Resolves: #3056
Original Pull Request: #3059
---
 .../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 4c2ecc88f..1fbc89791 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.Property;
 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 abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 	 *
 	 * @param initialEntitySet
 	 * @see #setManagedTypes(ManagedTypes)
-	 *
 	 */
 	public void setInitialEntitySet(Set<? extends Class<?>> initialEntitySet) {
 		setManagedTypes(ManagedTypes.fromIterable(initialEntitySet));
@@ -415,17 +415,19 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 				persistentEntities.put(typeInformation, Optional.of(entity));
 			}
 
-			PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type);
-			Map<String, PropertyDescriptor> descriptors = new HashMap<>();
+			if (shouldCreateProperties(userTypeInformation)) {
+				PropertyDescriptor[] pds = BeanUtils.getPropertyDescriptors(type);
+				Map<String, PropertyDescriptor> 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();
 
 			if (persistentPropertyAccessorFactory.isSupported(entity)) {
@@ -475,6 +477,35 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 	 */
 	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 @@ public abstract class AbstractMappingContext<E extends MutablePersistentEntity<?
 			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 e864b5a2f..9395ef92c 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 static org.mockito.Mockito.*;
 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.ShadowingPropertyTypeWithCtor;
 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 @@ class AbstractMappingContextUnitTests {
 				.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 @@ class AbstractMappingContextUnitTests {
 
 	}
 
+	static class MyIterable implements Iterable<StreamComponent> {
+
+		String name;
+
+		@Override
+		public Iterator<StreamComponent> iterator() {
+			return Collections.emptyIterator();
+		}
+	}
+
+	static class MyStreamable implements Streamable<StreamComponent> {
+
+		String name;
+
+		@Override
+		public Iterator<StreamComponent> iterator() {
+			return Collections.emptyIterator();
+		}
+	}
+
+	record StreamComponent(String name) {
+
+	}
+
 }