DATAREDIS-489 - Add type hints for Object types.

We now store the type hint for simple types when the declaring bean property does not match the actual value type.

Author: christophstrobl

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

@@ -45,6 +45,7 @@
 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;
@@ -103,6 +104,8 @@
  */
 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;
@@ -247,9 +250,9 @@ else if (persistentProperty.isCollectionLike()) {
 
 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));
@@ -265,8 +268,8 @@ else if (persistentProperty.isCollectionLike()) {
 }
 }
 
-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));
 }
 }
 
@@ -376,16 +379,17 @@ private void writeInternal(final String keyspace, final String path, final Objec
 
 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());
@@ -416,7 +420,7 @@ public void doWithPersistentProperty(KeyValuePersistentProperty persistentProper
 } else {
 
 Object propertyValue = accessor.getProperty(persistentProperty);
-sink.getBucket().put(propertyStringPath, toBytes(propertyValue));
+writeToBucket(propertyStringPath, propertyValue, sink, persistentProperty.getType());
 }
 }
 });
@@ -494,15 +498,15 @@ private void writeCollection(String keyspace, String path, Collection<?> values,
 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);
 }
 i++;
 }
 }
 
-private void writeToBucket(String path, Object value, RedisData sink) {
+private void writeToBucket(String path, Object value, RedisData sink, Class<?> propertyType) {
 
 if (value == null) {
 return;
@@ -512,6 +516,12 @@ private void writeToBucket(String path, Object value, RedisData sink) {
 
 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);
@@ -547,7 +557,13 @@ private Collection<?> readCollectionOfSimpleTypes(String path, Class<?> collecti
 Collection<Object> target = CollectionFactory.createCollection(collectionType, valueType, partial.size());
 
 for (String key : keys) {
-target.add(fromBytes(partial.get(key), valueType));
+
+if (key.endsWith(TYPE_HINT_ALIAS)) {
+continue;
+}
+
+Class<?> typeToUse = getTypeHint(key, source.getBucket(), valueType);
+target.add(fromBytes(partial.get(key), typeToUse));
 }
 
 return target;
@@ -572,9 +588,9 @@ private Collection<?> readCollectionOfComplexTypes(String path, Class<?> collect
 
 Bucket elementData = source.extract(key);
 
-byte[] typeInfo = elementData.get(key + "._class");
+byte[] typeInfo = elementData.get(key + "." + TYPE_HINT_ALIAS);
 if (typeInfo != null && typeInfo.length > 0) {
-elementData.put("_class", typeInfo);
+elementData.put(TYPE_HINT_ALIAS, typeInfo);
 }
 
 Object o = readInternal(key, valueType, new RedisData(elementData));
@@ -606,7 +622,7 @@ private void writeMap(String keyspace, String path, Class<?> mapValueType, Map<?
 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);
 }
@@ -630,6 +646,10 @@ private void writeMap(String keyspace, String path, Class<?> mapValueType, Map<?
 
 for (Entry<String, byte[]> entry : partial.entrySet()) {
 
+if (entry.getKey().endsWith(TYPE_HINT_ALIAS)) {
+continue;
+}
+
 String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])";
 Pattern pattern = Pattern.compile(regex);
 
@@ -639,7 +659,9 @@ private void writeMap(String keyspace, String path, Class<?> mapValueType, Map<?
 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;
@@ -674,9 +696,9 @@ private void writeMap(String keyspace, String path, Class<?> mapValueType, Map<?
 
 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));
@@ -686,6 +708,24 @@ private void writeMap(String keyspace, String path, Class<?> mapValueType, Map<?
 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}.
  * 
@@ -789,7 +829,7 @@ private static class RedisTypeAliasAccessor implements TypeAliasAccessor<RedisDa
 private final ConversionService conversionService;
 
 RedisTypeAliasAccessor(ConversionService conversionService) {
-this(conversionService, "_class");
+this(conversionService, TYPE_HINT_ALIAS);
 }
 
 RedisTypeAliasAccessor(ConversionService conversionService, String typeKey) {

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

@@ -23,7 +23,9 @@
 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;
@@ -160,4 +162,9 @@ public static class Size {
 int length;
 }
 
+static class TypeWithObjectValueTypes {
+Object object;
+Map<String, Object> map = new HashMap<String, Object>();
+List<Object> list = new ArrayList<Object>();
+}
 }

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

@@ -65,6 +65,7 @@
 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.KeyspaceConfiguration.KeyspaceSettings;
 import org.springframework.data.redis.core.mapping.RedisMappingContext;
 import org.springframework.data.redis.serializer.Jackson2JsonRedisSerializer;
@@ -1325,6 +1326,114 @@ public void readShouldConsiderMapConvertersForValuesInList() {
 assertThat(target.species.get(0).name, is("trolloc"));
 }
 
+/**
+ * @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();