diff --git a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java index db931aeb6..3a2bfe716 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java +++ b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java @@ -21,17 +21,8 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -232,73 +223,16 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { entity.doWithProperties((PropertyHandler) persistentProperty -> { - String currentPath = !path.isEmpty() ? path + "." + persistentProperty.getName() : persistentProperty.getName(); - PreferredConstructor constructor = entity.getPersistenceConstructor(); if (constructor.isConstructorParameter(persistentProperty)) { return; } - if (persistentProperty.isMap()) { + Object targetValue = readProperty(path, source, persistentProperty); - Map targetValue = null; - Class mapValueType = persistentProperty.getMapValueType(); - - if (mapValueType == null) { - throw new IllegalArgumentException("Unable to retrieve MapValueType!"); - } - - if (conversionService.canConvert(byte[].class, mapValueType)) { - targetValue = readMapOfSimpleTypes(currentPath, persistentProperty.getType(), - persistentProperty.getComponentType(), mapValueType, source); - } else { - targetValue = readMapOfComplexTypes(currentPath, persistentProperty.getType(), - persistentProperty.getComponentType(), mapValueType, source); - } - - if (targetValue != null) { - accessor.setProperty(persistentProperty, targetValue); - } - } - - else if (persistentProperty.isCollectionLike()) { - - Object targetValue = readCollectionOrArray(currentPath, persistentProperty.getType(), - persistentProperty.getTypeInformation().getRequiredComponentType().getActualType().getType(), - source.getBucket()); - if (targetValue != null) { - accessor.setProperty(persistentProperty, targetValue); - } - - } else if (persistentProperty.isEntity() && !conversionService.canConvert(byte[].class, - persistentProperty.getTypeInformation().getActualType().getType())) { - - Bucket bucket = source.getBucket().extract(currentPath + "."); - - RedisData newBucket = new RedisData(bucket); - TypeInformation typeInformation = typeMapper.readType(bucket.getPropertyPath(currentPath), - persistentProperty.getTypeInformation().getActualType()); - - Class targetType = (Class) typeInformation.getType(); - - R val = readInternal(currentPath, targetType, newBucket); - - accessor.setProperty(persistentProperty, val); - } else { - - if (persistentProperty.isIdProperty() && StringUtils.isEmpty(path.isEmpty())) { - - if (source.getBucket().get(currentPath) == null) { - accessor.setProperty(persistentProperty, - fromBytes(source.getBucket().get(currentPath), persistentProperty.getActualType())); - } else { - accessor.setProperty(persistentProperty, source.getId()); - } - } - - Class typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getActualType()); - accessor.setProperty(persistentProperty, fromBytes(source.getBucket().get(currentPath), typeToUse)); + if (targetValue != null) { + accessor.setProperty(persistentProperty, targetValue); } }); @@ -307,6 +241,59 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { return (R) instance; } + @SuppressWarnings("unchecked") + @Nullable + protected Object readProperty(String path, RedisData source, RedisPersistentProperty persistentProperty) { + + String currentPath = !path.isEmpty() ? path + "." + persistentProperty.getName() : persistentProperty.getName(); + + TypeInformation typeInformation = persistentProperty.getTypeInformation(); + + if (persistentProperty.isMap()) { + + Class mapValueType = persistentProperty.getMapValueType(); + + if (mapValueType == null) { + throw new IllegalArgumentException("Unable to retrieve MapValueType!"); + } + + if (conversionService.canConvert(byte[].class, mapValueType)) { + return readMapOfSimpleTypes(currentPath, typeInformation.getType(), + typeInformation.getRequiredComponentType().getType(), mapValueType, source); + } + + return readMapOfComplexTypes(currentPath, typeInformation.getType(), + typeInformation.getRequiredComponentType().getType(), mapValueType, source); + } + + if (typeInformation.isCollectionLike()) { + + return readCollectionOrArray(currentPath, typeInformation.getType(), + typeInformation.getRequiredComponentType().getActualType().getType(), source.getBucket()); + } + + if (persistentProperty.isEntity() + && !conversionService.canConvert(byte[].class, typeInformation.getActualType().getType())) { + + Bucket bucket = source.getBucket().extract(currentPath + "."); + + RedisData newBucket = new RedisData(bucket); + TypeInformation typeToRead = typeMapper.readType(bucket.getPropertyPath(currentPath), + typeInformation.getActualType()); + + return readInternal(currentPath, typeToRead.getType(), newBucket); + } + + byte[] sourceBytes = source.getBucket().get(currentPath); + + if (persistentProperty.isIdProperty() && StringUtils.isEmpty(path.isEmpty())) { + return sourceBytes == null ? fromBytes(sourceBytes, typeInformation.getActualType().getType()) : source.getId(); + } + + Class typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getActualType()); + return fromBytes(sourceBytes, typeToUse); + } + private void readAssociation(String path, RedisData source, RedisPersistentEntity entity, PersistentPropertyAccessor accessor) { @@ -463,10 +450,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { targetProperty = getTargetPropertyOrNullForPath(path.replaceAll("\\.\\[.*\\]", ""), update.getTarget()); TypeInformation ti = targetProperty == null ? ClassTypeInformation.OBJECT - : (targetProperty.isMap() - ? (targetProperty.getTypeInformation().getMapValueType() != null - ? targetProperty.getTypeInformation().getRequiredMapValueType() : ClassTypeInformation.OBJECT) - : targetProperty.getTypeInformation().getActualType()); + : (targetProperty.isMap() ? (targetProperty.getTypeInformation().getMapValueType() != null + ? targetProperty.getTypeInformation().getRequiredMapValueType() + : ClassTypeInformation.OBJECT) : targetProperty.getTypeInformation().getActualType()); writeInternal(entity.getKeySpace(), pUpdate.getPropertyPath(), pUpdate.getValue(), ti, sink); return; @@ -1037,9 +1023,10 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { /** * @author Christoph Strobl + * @author Mark Paluch */ @RequiredArgsConstructor - private static class ConverterAwareParameterValueProvider implements PropertyValueProvider { + private class ConverterAwareParameterValueProvider implements PropertyValueProvider { private final String path; private final RedisData source; @@ -1049,8 +1036,13 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { @SuppressWarnings("unchecked") public T getPropertyValue(RedisPersistentProperty property) { - String name = StringUtils.hasText(path) ? path + "." + property.getName() : property.getName(); - return (T) conversionService.convert(source.getBucket().get(name), property.getActualType()); + Object value = readProperty(path, source, property); + + if (value != null && !property.getActualType().isInstance(value)) { + return (T) conversionService.convert(value, property.getActualType()); + } + + return (T) value; } } diff --git a/src/test/java/org/springframework/data/redis/core/convert/ConversionTestEntities.java b/src/test/java/org/springframework/data/redis/core/convert/ConversionTestEntities.java index a88a7107a..98b9e98b8 100644 --- a/src/test/java/org/springframework/data/redis/core/convert/ConversionTestEntities.java +++ b/src/test/java/org/springframework/data/redis/core/convert/ConversionTestEntities.java @@ -15,6 +15,7 @@ */ package org.springframework.data.redis.core.convert; +import lombok.Data; import lombok.EqualsAndHashCode; import java.time.Duration; @@ -84,6 +85,24 @@ public class ConversionTestEntities { Species species; } + @RedisHash(KEYSPACE_PERSON) + @Data + public static class RecursiveConstructorPerson { + + final @Id String id; + final String firstname; + final RecursiveConstructorPerson father; + String lastname; + } + + @RedisHash(KEYSPACE_PERSON) + @Data + public static class PersonWithConstructorAndAddress { + + final @Id String id; + final Address address; + } + public static class PersonWithAddressReference extends Person { @Reference AddressWithId addressRef; diff --git a/src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java b/src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java index 697753a97..782bdf351 100644 --- a/src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.java @@ -231,6 +231,31 @@ public class MappingRedisConverterUnitTests { assertThat(target.address, instanceOf(AddressWithPostcode.class)); } + @Test // DATAREDIS-544 + public void readEntityViaConstructor() { + + Map map = new HashMap<>(); + map.put("id", "bart"); + map.put("firstname", "Bart"); + map.put("lastname", "Simpson"); + + map.put("father.id", "homer"); + map.put("father.firstname", "Homer"); + map.put("father.lastname", "Simpson"); + + RecursiveConstructorPerson target = converter.read(RecursiveConstructorPerson.class, + new RedisData(Bucket.newBucketFromStringMap(map))); + + assertThat(target.id, is("bart")); + assertThat(target.firstname, is("Bart")); + assertThat(target.lastname, is("Simpson")); + assertThat(target.father, is(notNullValue())); + assertThat(target.father.id, is("homer")); + assertThat(target.father.firstname, is("Homer")); + assertThat(target.father.lastname, is("Simpson")); + assertThat(target.father.father, is(nullValue())); + } + @Test // DATAREDIS-425 public void writeAddsClassTypeInformationCorrectlyForNonMatchingTypesInCollections() { @@ -1093,6 +1118,25 @@ public class MappingRedisConverterUnitTests { assertThat(target.address.country, is("Tel'aran'rhiod")); } + @Test // DATAREDIS-544 + public void readShouldHonorCustomConversionOnNestedTypeViaConstructorCreation() { + + this.converter = new MappingRedisConverter(new RedisMappingContext(), null, resolverMock); + this.converter + .setCustomConversions(new RedisCustomConversions(Collections.singletonList(new BytesToAddressConverter()))); + this.converter.afterPropertiesSet(); + + Map map = new LinkedHashMap<>(); + map.put("address", "{\"city\":\"unknown\",\"country\":\"Tel'aran'rhiod\"}"); + + PersonWithConstructorAndAddress target = converter.read(PersonWithConstructorAndAddress.class, + new RedisData(Bucket.newBucketFromStringMap(map))); + + assertThat(target.address, notNullValue()); + assertThat(target.address.city, is("unknown")); + assertThat(target.address.country, is("Tel'aran'rhiod")); + } + @Test // DATAREDIS-425 public void writeShouldPickUpTimeToLiveFromPropertyIfPresent() {