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 de32a69f3..549135621 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 @@ -48,6 +48,7 @@ import org.springframework.data.mapping.AssociationHandler; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.PropertyHandler; +import org.springframework.data.mapping.model.MappingException; import org.springframework.data.mapping.model.PersistentEntityParameterValueProvider; import org.springframework.data.mapping.model.PropertyValueProvider; import org.springframework.data.redis.core.index.Indexed; @@ -107,6 +108,8 @@ import org.springframework.util.comparator.NullSafeComparator; */ public class MappingRedisConverter implements RedisConverter, InitializingBean { + private static final String TYPE_HINT_ALIAS = "_class"; + private final RedisMappingContext mappingContext; private final GenericConversionService conversionService; private final EntityInstantiators entityInstantiators; @@ -231,41 +234,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { else if (persistentProperty.isCollectionLike()) { - if (conversionService.canConvert(byte[].class, persistentProperty.getComponentType())) { - - Object targetValue = null; - if (persistentProperty.getType().isArray()) { - - List list = (List) readCollectionOfSimpleTypes(currentPath, ArrayList.class, - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), source); - - targetValue = list.toArray((Object[]) Array.newInstance( - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), list.size())); - } else { - targetValue = readCollectionOfSimpleTypes(currentPath, persistentProperty.getType(), - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), source); - } - - accessor.setProperty(persistentProperty, targetValue); - } else { - - Object targetValue = null; - if (persistentProperty.getType().isArray()) { - - List list = (List) readCollectionOfComplexTypes(currentPath, ArrayList.class, - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), - source.getBucket()); - - targetValue = list.toArray((Object[]) Array.newInstance( - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), list.size())); - } else { - - targetValue = readCollectionOfComplexTypes(currentPath, persistentProperty.getType(), - persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), - source.getBucket()); - } - accessor.setProperty(persistentProperty, targetValue); - } + Object targetValue = readCollectionOrArray(currentPath, persistentProperty.getType(), + persistentProperty.getTypeInformation().getComponentType().getActualType().getType(), source.getBucket()); + accessor.setProperty(persistentProperty, targetValue); } else if (persistentProperty.isEntity() && !conversionService.canConvert(byte[].class, persistentProperty.getTypeInformation().getActualType().getType())) { @@ -276,9 +247,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { RedisData source = new RedisData(bucket); - byte[] type = bucket.get(currentPath + "._class"); + byte[] type = bucket.get(currentPath + "." + TYPE_HINT_ALIAS); if (type != null && type.length > 0) { - source.getBucket().put("_class", type); + source.getBucket().put(TYPE_HINT_ALIAS, type); } accessor.setProperty(persistentProperty, readInternal(currentPath, targetType, source)); @@ -294,8 +265,8 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } } - accessor.setProperty(persistentProperty, - fromBytes(source.getBucket().get(currentPath), persistentProperty.getActualType())); + Class typeToUse = getTypeHint(currentPath, source.getBucket(), persistentProperty.getActualType()); + accessor.setProperty(persistentProperty, fromBytes(source.getBucket().get(currentPath), typeToUse)); } } @@ -405,16 +376,17 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { if (customConversions.hasCustomWriteTarget(value.getClass())) { - if (customConversions.getCustomWriteTarget(value.getClass()).equals(byte[].class)) { + if (!StringUtils.hasText(path) && customConversions.getCustomWriteTarget(value.getClass()).equals(byte[].class)) { sink.getBucket().put(StringUtils.hasText(path) ? path : "_raw", conversionService.convert(value, byte[].class)); } else { - writeToBucket(path, value, sink); + writeToBucket(path, value, sink, typeHint.getType()); } return; } if (value.getClass() != typeHint.getType()) { - sink.getBucket().put((!path.isEmpty() ? path + "._class" : "_class"), toBytes(value.getClass().getName())); + sink.getBucket().put((!path.isEmpty() ? path + "." + TYPE_HINT_ALIAS : TYPE_HINT_ALIAS), + toBytes(value.getClass().getName())); } final KeyValuePersistentEntity entity = mappingContext.getPersistentEntity(value.getClass()); @@ -459,7 +431,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } else { Object propertyValue = accessor.getProperty(persistentProperty); - sink.getBucket().put(propertyStringPath, toBytes(propertyValue)); + writeToBucket(propertyStringPath, propertyValue, sink, persistentProperty.getType()); } } }); @@ -541,7 +513,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { String currentPath = path + ".[" + i + "]"; if (customConversions.hasCustomWriteTarget(value.getClass())) { - writeToBucket(currentPath, value, sink); + writeToBucket(currentPath, value, sink, typeHint.getType()); } else { writeInternal(keyspace, currentPath, value, typeHint, sink); } @@ -549,7 +521,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } } - private void writeToBucket(String path, Object value, RedisData sink) { + private void writeToBucket(String path, Object value, RedisData sink, Class propertyType) { if (value == null) { return; @@ -559,6 +531,12 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { Class targetType = customConversions.getCustomWriteTarget(value.getClass()); + if (!ClassUtils.isAssignable(Map.class, targetType) && customConversions.isSimpleType(value.getClass()) + && value.getClass() != propertyType) { + sink.getBucket().put((!path.isEmpty() ? path + "." + TYPE_HINT_ALIAS : TYPE_HINT_ALIAS), + toBytes(value.getClass().getName())); + } + if (ClassUtils.isAssignable(Map.class, targetType)) { Map map = (Map) conversionService.convert(value, targetType); @@ -576,59 +554,37 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } - /** - * @param path - * @param collectionType - * @param valueType - * @param source - * @return - */ - private Collection readCollectionOfSimpleTypes(String path, Class collectionType, Class valueType, - RedisData source) { + private Object readCollectionOrArray(String path, Class collectionType, Class valueType, Bucket bucket) { - Bucket partial = source.getBucket().extract(path + ".["); - - List keys = new ArrayList(partial.keySet()); + List keys = new ArrayList(bucket.extractAllKeysFor(path)); Collections.sort(keys, listKeyComparator); - Collection target = CollectionFactory.createCollection(collectionType, valueType, partial.size()); - - for (String key : keys) { - target.add(fromBytes(partial.get(key), valueType)); - } - - return target; - } - - /** - * @param path - * @param collectionType - * @param valueType - * @param source - * @return - */ - private Collection readCollectionOfComplexTypes(String path, Class collectionType, Class valueType, - Bucket source) { - - List keys = new ArrayList(source.extractAllKeysFor(path)); - Collections.sort(keys, listKeyComparator); - - Collection target = CollectionFactory.createCollection(collectionType, valueType, keys.size()); + boolean isArray = collectionType.isArray(); + Class collectionTypeToUse = isArray ? ArrayList.class : collectionType; + Collection target = CollectionFactory.createCollection(collectionTypeToUse, valueType, keys.size()); for (String key : keys) { - Bucket elementData = source.extract(key); - - byte[] typeInfo = elementData.get(key + "._class"); - if (typeInfo != null && typeInfo.length > 0) { - elementData.put("_class", typeInfo); + if (key.endsWith(TYPE_HINT_ALIAS)) { + continue; } - Object o = readInternal(key, valueType, new RedisData(elementData)); - target.add(o); + Bucket elementData = bucket.extract(key); + + byte[] typeInfo = elementData.get(key + "." + TYPE_HINT_ALIAS); + if (typeInfo != null && typeInfo.length > 0) { + elementData.put(TYPE_HINT_ALIAS, typeInfo); + } + + Class typeToUse = getTypeHint(key, elementData, valueType); + if (conversionService.canConvert(byte[].class, typeToUse)) { + target.add(fromBytes(elementData.get(key), typeToUse)); + } else { + target.add(readInternal(key, valueType, new RedisData(elementData))); + } } - return target; + return isArray ? target.toArray((Object[]) Array.newInstance(valueType, target.size())) : target; } /** @@ -653,7 +609,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { String currentPath = path + ".[" + entry.getKey() + "]"; if (customConversions.hasCustomWriteTarget(entry.getValue().getClass())) { - writeToBucket(currentPath, entry.getValue(), sink); + writeToBucket(currentPath, entry.getValue(), sink, mapValueType); } else { writeInternal(keyspace, currentPath, entry.getValue(), ClassTypeInformation.from(mapValueType), sink); } @@ -677,6 +633,10 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { for (Entry entry : partial.entrySet()) { + if (entry.getKey().endsWith(TYPE_HINT_ALIAS)) { + continue; + } + String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])"; Pattern pattern = Pattern.compile(regex); @@ -686,7 +646,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { String.format("Cannot extract map value for key '%s' in path '%s'.", entry.getKey(), path)); } String key = matcher.group(2); - target.put(key, fromBytes(entry.getValue(), valueType)); + + Class typeToUse = getTypeHint(path + ".[" + key + "]", source.getBucket(), valueType); + target.put(key, fromBytes(entry.getValue(), typeToUse)); } return target; @@ -721,9 +683,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { Bucket partial = source.getBucket().extract(key); - byte[] typeInfo = partial.get(key + "._class"); + byte[] typeInfo = partial.get(key + "." + TYPE_HINT_ALIAS); if (typeInfo != null && typeInfo.length > 0) { - partial.put("_class", typeInfo); + partial.put(TYPE_HINT_ALIAS, typeInfo); } Object o = readInternal(key, valueType, new RedisData(partial)); @@ -733,6 +695,24 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { return target; } + private Class getTypeHint(String path, Bucket bucket, Class fallback) { + + byte[] typeInfo = bucket.get(path + "." + TYPE_HINT_ALIAS); + + if (typeInfo == null || typeInfo.length < 1) { + return fallback; + } + + String typeName = fromBytes(typeInfo, String.class); + try { + return ClassUtils.forName(typeName, this.getClass().getClassLoader()); + } catch (ClassNotFoundException e) { + throw new MappingException(String.format("Cannot find class for type %s. ", typeName), e); + } catch (LinkageError e) { + throw new MappingException(String.format("Cannot find class for type %s. ", typeName), e); + } + } + /** * Convert given source to binary representation using the underlying {@link ConversionService}. * @@ -836,7 +816,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { private final ConversionService conversionService; RedisTypeAliasAccessor(ConversionService conversionService) { - this(conversionService, "_class"); + this(conversionService, TYPE_HINT_ALIAS); } RedisTypeAliasAccessor(ConversionService conversionService, String typeKey) { 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 e75cfee33..ecd55cc97 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 @@ -23,7 +23,9 @@ import java.time.LocalTime; import java.time.Period; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.util.ArrayList; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -167,4 +169,9 @@ public class ConversionTestEntities { Species[] arrayOfCompexTypes; } + static class TypeWithObjectValueTypes { + Object object; + Map map = new HashMap(); + List list = new ArrayList(); + } } 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 1d3b5bfc1..5a7a1fae4 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 @@ -67,6 +67,7 @@ import org.springframework.data.redis.core.convert.ConversionTestEntities.Person import org.springframework.data.redis.core.convert.ConversionTestEntities.Species; import org.springframework.data.redis.core.convert.ConversionTestEntities.TaVeren; import org.springframework.data.redis.core.convert.ConversionTestEntities.TheWheelOfTime; +import org.springframework.data.redis.core.convert.ConversionTestEntities.TypeWithObjectValueTypes; import org.springframework.data.redis.core.convert.ConversionTestEntities.WithArrays; import org.springframework.data.redis.core.convert.KeyspaceConfiguration.KeyspaceSettings; import org.springframework.data.redis.core.mapping.RedisMappingContext; @@ -1427,6 +1428,159 @@ public class MappingRedisConverterUnitTests { assertThat(target.arrayOfCompexTypes[1].alsoKnownAs, contains("halfmen", "fades", "neverborn")); } + /** + * @see DATAREDIS-489 + */ + @Test + public void writeHandlesArraysOfObjectTypeProperly() { + + Species trolloc = new Species(); + trolloc.name = "trolloc"; + + WithArrays source = new WithArrays(); + source.arrayOfObject = new Object[] { "rand", trolloc, 100L }; + + assertThat(write(source).getBucket(), + isBucket().containingUtf8String("arrayOfObject.[0]", "rand") // + .containingUtf8String("arrayOfObject.[0]._class", "java.lang.String") + .containingUtf8String("arrayOfObject.[1]._class", Species.class.getName()) // + .containingUtf8String("arrayOfObject.[1].name", "trolloc") // + .containingUtf8String("arrayOfObject.[2]._class", "java.lang.Long") // + .containingUtf8String("arrayOfObject.[2]", "100")); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void readHandlesArraysOfObjectTypeProperly() { + + Map source = new LinkedHashMap(); + source.put("arrayOfObject.[0]", "rand"); + source.put("arrayOfObject.[0]._class", "java.lang.String"); + source.put("arrayOfObject.[1]._class", Species.class.getName()); + source.put("arrayOfObject.[1].name", "trolloc"); + source.put("arrayOfObject.[2]._class", "java.lang.Long"); + source.put("arrayOfObject.[2]", "100"); + + WithArrays target = read(WithArrays.class, source); + + assertThat(target.arrayOfObject[0], notNullValue()); + assertThat(target.arrayOfObject[0], instanceOf(String.class)); + assertThat(target.arrayOfObject[1], notNullValue()); + assertThat(target.arrayOfObject[1], instanceOf(Species.class)); + assertThat(target.arrayOfObject[2], notNullValue()); + assertThat(target.arrayOfObject[2], instanceOf(Long.class)); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void writeShouldAppendTyeHintToObjectPropertyValueTypesCorrectly() { + + TypeWithObjectValueTypes sample = new TypeWithObjectValueTypes(); + sample.object = "bar"; + + Bucket bucket = write(sample).getBucket(); + + assertThat(bucket, + isBucket().containingUtf8String("object", "bar").containingUtf8String("object._class", "java.lang.String")); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void shouldWriteReadObjectPropertyValueTypeCorrectly() { + + TypeWithObjectValueTypes di = new TypeWithObjectValueTypes(); + di.object = "foo"; + + RedisData rd = write(di); + + TypeWithObjectValueTypes result = converter.read(TypeWithObjectValueTypes.class, rd); + assertThat(result.object, instanceOf(String.class)); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void writeShouldAppendTyeHintToObjectMapValueTypesCorrectly() { + + TypeWithObjectValueTypes sample = new TypeWithObjectValueTypes(); + sample.map.put("string", "bar"); + sample.map.put("long", new Long(1L)); + sample.map.put("date", new Date()); + + Bucket bucket = write(sample).getBucket(); + + assertThat(bucket, isBucket().containingUtf8String("map.[string]", "bar") + .containingUtf8String("map.[string]._class", "java.lang.String")); + assertThat(bucket, + isBucket().containingUtf8String("map.[long]", "1").containingUtf8String("map.[long]._class", "java.lang.Long")); + assertThat(bucket, isBucket().containingUtf8String("map.[date]._class", "java.util.Date")); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void shouldWriteReadObjectMapValueTypeCorrectly() { + + TypeWithObjectValueTypes sample = new TypeWithObjectValueTypes(); + sample.map.put("string", "bar"); + sample.map.put("long", new Long(1L)); + sample.map.put("date", new Date()); + + RedisData rd = write(sample); + + TypeWithObjectValueTypes result = converter.read(TypeWithObjectValueTypes.class, rd); + assertThat(result.map.get("string"), instanceOf(String.class)); + assertThat(result.map.get("long"), instanceOf(Long.class)); + assertThat(result.map.get("date"), instanceOf(Date.class)); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void writeShouldAppendTyeHintToObjectListValueTypesCorrectly() { + + TypeWithObjectValueTypes sample = new TypeWithObjectValueTypes(); + sample.list.add("string"); + sample.list.add(new Long(1L)); + sample.list.add(new Date()); + + Bucket bucket = write(sample).getBucket(); + + assertThat(bucket, isBucket().containingUtf8String("list.[0]", "string").containingUtf8String("list.[0]._class", + "java.lang.String")); + assertThat(bucket, + isBucket().containingUtf8String("list.[1]", "1").containingUtf8String("list.[1]._class", "java.lang.Long")); + assertThat(bucket, isBucket().containingUtf8String("list.[2]._class", "java.util.Date")); + } + + /** + * @see DATAREDIS-489 + */ + @Test + public void shouldWriteReadObjectListValueTypeCorrectly() { + + TypeWithObjectValueTypes sample = new TypeWithObjectValueTypes(); + sample.list.add("string"); + sample.list.add(new Long(1L)); + sample.list.add(new Date()); + + RedisData rd = write(sample); + + TypeWithObjectValueTypes result = converter.read(TypeWithObjectValueTypes.class, rd); + assertThat(result.list.get(0), instanceOf(String.class)); + assertThat(result.list.get(1), instanceOf(Long.class)); + assertThat(result.list.get(2), instanceOf(Date.class)); + } + private RedisData write(Object source) { RedisData rdo = new RedisData();