Consider type hints of the actual value in MappingRedisConverter.

We now consider the actual value type when determining whether a value should be considered an entity. Previously, we relied on PersistentProperty type information only which made it impossible to use generic-typed (or Object) properties for entities.

Closes #2349

Author: mp911de

src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java

@@ -56,6 +56,7 @@
 import org.springframework.data.redis.core.mapping.RedisPersistentProperty;
 import org.springframework.data.redis.util.ByteUtils;
 import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.ProxyUtils;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -258,12 +259,20 @@ private <R> R doReadInternal(String path, Class<R> type, RedisData source) {
 	protected Object readProperty(String path, RedisData source, RedisPersistentProperty persistentProperty) {
 
 		String currentPath = !path.isEmpty() ? path + "." + persistentProperty.getName() : persistentProperty.getName();
+		TypeInformation<?> typeInformation = typeMapper.readType(source.getBucket().getPropertyPath(currentPath),
+				persistentProperty.getTypeInformation());
 
-		TypeInformation<?> typeInformation = persistentProperty.getTypeInformation();
+		if (typeInformation.isMap()) {
 
-		if (persistentProperty.isMap()) {
+			Class<?> mapValueType = null;
 
-			Class<?> mapValueType = persistentProperty.getMapValueType();
+			if (typeInformation.getMapValueType() != null) {
+				mapValueType = typeInformation.getMapValueType().getType();
+			}
+
+			if (mapValueType == null && persistentProperty.isMap()) {
+				mapValueType = persistentProperty.getMapValueType();
+			}
 
 			if (mapValueType == null) {
 				throw new IllegalArgumentException("Unable to retrieve MapValueType");
@@ -293,16 +302,14 @@ protected Object readProperty(String path, RedisData source, RedisPersistentProp
 			}
 		}
 
-		if (persistentProperty.isEntity()
+		if (mappingContext.getPersistentEntity(typeInformation) != null
 				&& !conversionService.canConvert(byte[].class, typeInformation.getRequiredActualType().getType())) {
 
 			Bucket bucket = source.getBucket().extract(currentPath + ".");
 
 			RedisData newBucket = new RedisData(bucket);
-			TypeInformation<?> typeToRead = typeMapper.readType(bucket.getPropertyPath(currentPath),
-					typeInformation);
 
-			return readInternal(currentPath, typeToRead.getType(), newBucket);
+			return readInternal(currentPath, typeInformation.getType(), newBucket);
 		}
 
 		byte[] sourceBytes = source.getBucket().get(currentPath);
@@ -580,8 +587,7 @@ RedisPersistentProperty getTargetPropertyOrNullForPath(String path, Class<?> typ
 	 * @param sink
 	 */
 	private void writeInternal(@Nullable String keyspace, String path, @Nullable Object value,
-			TypeInformation<?> typeHint,
-			RedisData sink) {
+			TypeInformation<?> typeHint, RedisData sink) {
 
 		if (value == null) {
 			return;
@@ -656,16 +662,14 @@ private void writeInternal(@Nullable String keyspace, String path, @Nullable Obj
 					}
 				}
 
-			} else if (persistentProperty.isEntity()) {
+			} else if (propertyValue != null) {
 
-				if (propertyValue != null) {
-					writeInternal(keyspace, propertyStringPath, propertyValue,
-							persistentProperty.getTypeInformation().getRequiredActualType(), sink);
-				}
-			} else {
+				if (customConversions.isSimpleType(ProxyUtils.getUserClass(propertyValue.getClass()))) {
 
-				if (propertyValue != null) {
 					writeToBucket(propertyStringPath, propertyValue, sink, persistentProperty.getType());
+				} else {
+					writeInternal(keyspace, propertyStringPath, propertyValue,
+							persistentProperty.getTypeInformation().getRequiredActualType(), sink);
 				}
 			}
 		});

src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java

@@ -19,6 +19,8 @@
 import static org.mockito.Mockito.*;
 import static org.springframework.data.redis.core.convert.ConversionTestEntities.*;
 
+import lombok.AllArgsConstructor;
+
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
@@ -1925,6 +1927,38 @@ void readEntityWithCustomConverter() {
 		assertThat(target.getAccountName()).isEqualTo("Golam Mazid Sajib");
 	}
 
+	@Test // GH-2349
+	void writeGenericEntity() {
+
+		WithGenericEntity<User> generic = new WithGenericEntity<>();
+		generic.entity = new User("hello");
+
+		assertThat(write(generic)).hasSize(3) //
+				.containsEntry("_class",
+						"org.springframework.data.redis.core.convert.MappingRedisConverterUnitTests$WithGenericEntity")
+				.containsEntry("entity.name", "hello") //
+				.containsEntry("entity._class",
+						"org.springframework.data.redis.core.convert.MappingRedisConverterUnitTests$User");
+	}
+
+	@Test // GH-2349
+	void readGenericEntity() {
+
+		Bucket bucket = new Bucket();
+		bucket.put("entity.name", "hello".getBytes());
+		bucket.put("entity._class",
+				"org.springframework.data.redis.core.convert.MappingRedisConverterUnitTests$User".getBytes());
+
+		RedisData redisData = new RedisData(bucket);
+		redisData.setKeyspace(KEYSPACE_ACCOUNT);
+		redisData.setId("ai-id-1");
+
+		WithGenericEntity<User> generic = converter.read(WithGenericEntity.class, redisData);
+
+		assertThat(generic.entity).isNotNull();
+		assertThat(generic.entity.name).isEqualTo("hello");
+	}
+
 	@Test // DATAREDIS-1175
 	@EnabledOnJre(JRE.JAVA_8)
 	// FIXME: https://github.com/spring-projects/spring-data-redis/issues/2168
@@ -2083,4 +2117,13 @@ public AccountInfo convert(byte[] bytes) {
 		}
 	}
 
+	static class WithGenericEntity<T> {
+		T entity;
+	}
+
+	@AllArgsConstructor
+	static class User {
+		String name;
+	}
+
 }