From 90c86a533a6c174fde8fb72b1c55b9706e6b8585 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 16 Feb 2018 08:38:31 +0100 Subject: [PATCH] DATAREDIS-768 - Polishing. Added test with decimal and date map keys to verify those are flattened out correctly using the converters registered and updated documentation to point out limitation of map keys to simple types. Original Pull Request: #312 --- .../reference/redis-repositories.adoc | 2 + .../core/convert/MappingRedisConverter.java | 62 ++++++++------- .../core/convert/ConversionTestEntities.java | 9 ++- .../MappingRedisConverterUnitTests.java | 79 ++++++++++++++++--- 4 files changed, 109 insertions(+), 43 deletions(-) diff --git a/src/main/asciidoc/reference/redis-repositories.adoc b/src/main/asciidoc/reference/redis-repositories.adoc index 6db528106..5166f72ce 100644 --- a/src/main/asciidoc/reference/redis-repositories.adoc +++ b/src/main/asciidoc/reference/redis-repositories.adoc @@ -158,6 +158,8 @@ of Complex Type addresses.[work].city = "... |=== +WARNING: Due to the flat representation structure Map keys need to be simple types such as ``String``s or ``Number``s. + Mapping behavior can be customized by registering the according `Converter` in `CustomConversions`. Those converters can take care of converting from/to a single `byte[]` as well as `Map` whereas the first one is suitable for eg. converting one complex type to eg. a binary JSON representation that still uses the default mappings hash structure. The second option offers full control over the resulting hash. Writing objects to a Redis hash will delete the content from the hash and re-create the whole hash, so not mapped data will be lost. .Sample byte[] Converters 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 4fb199c7e..b23b5a444 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 @@ -788,7 +788,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { continue; } - String currentPath = path + ".[" + entry.getKey() + "]"; + String currentPath = path + ".[" + mapMapKey(entry.getKey()) + "]"; if (!ClassUtils.isAssignable(mapValueType, entry.getValue().getClass())) { throw new MappingException( @@ -803,6 +803,15 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } } + private String mapMapKey(Object key) { + + if (conversionService.canConvert(key.getClass(), byte[].class)) { + return new String(conversionService.convert(key, byte[].class)); + } + + return conversionService.convert(key, String.class); + } + /** * @param path * @param mapType @@ -824,22 +833,8 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { continue; } - String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])"; - Pattern pattern = Pattern.compile(regex); - - Matcher matcher = pattern.matcher(entry.getKey()); - if (!matcher.find()) { - throw new IllegalArgumentException( - String.format("Cannot extract map value for key '%s' in path '%s'.", entry.getKey(), path)); - } - Object key = matcher.group(2); - + Object key = extractMapKeyForPath(path, entry.getKey(), keyType); Class typeToUse = getTypeHint(path + ".[" + key + "]", source.getBucket(), valueType); - - if (!keyType.isAssignableFrom(key.getClass())) { - key = conversionService.convert(key, keyType); - } - target.put(key, fromBytes(entry.getValue(), typeToUse)); } @@ -863,16 +858,6 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { for (String key : keys) { - String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])"; - Pattern pattern = Pattern.compile(regex); - - Matcher matcher = pattern.matcher(key); - if (!matcher.find()) { - throw new IllegalArgumentException( - String.format("Cannot extract map value for key '%s' in path '%s'.", key, path)); - } - Object mapKey = matcher.group(2); - Bucket partial = source.getBucket().extract(key); byte[] typeInfo = partial.get(key + "." + TYPE_HINT_ALIAS); @@ -880,18 +865,35 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { partial.put(TYPE_HINT_ALIAS, typeInfo); } - if (!keyType.isAssignableFrom(mapKey.getClass())) { - mapKey = conversionService.convert(mapKey, keyType); - } - Object value = readInternal(key, valueType, new RedisData(partial)); + Object mapKey = extractMapKeyForPath(path, key, keyType); target.put(mapKey, value); } return target.isEmpty() ? null : target; } + private Object extractMapKeyForPath(String path, String key, Class targetType) { + + String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])"; + Pattern pattern = Pattern.compile(regex); + + Matcher matcher = pattern.matcher(key); + if (!matcher.find()) { + throw new IllegalArgumentException( + String.format("Cannot extract map value for key '%s' in path '%s'.", key, path)); + } + + Object mapKey = matcher.group(2); + + if (ClassUtils.isAssignable(targetType, mapKey.getClass())) { + return mapKey; + } + + return conversionService.convert(toBytes(mapKey), targetType); + } + private Class getTypeHint(String path, Bucket bucket, Class fallback) { byte[] typeInfo = bucket.get(path + "." + TYPE_HINT_ALIAS); 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 e466d0795..855da99a4 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 @@ -74,7 +74,6 @@ public class ConversionTestEntities { Address address; Map physicalAttributes; - Map numberMapping; Map relatives; Map favoredRelatives; @@ -181,8 +180,16 @@ public class ConversionTestEntities { } static class TypeWithObjectValueTypes { + Object object; Map map = new HashMap(); List list = new ArrayList(); } + + static class TypeWithMaps { + + Map integerMapKeyMapping; + Map decimalMapKeyMapping; + Map dateMapKeyMapping; + } } 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 43009fc22..db593e417 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 @@ -418,16 +418,74 @@ public class MappingRedisConverterUnitTests { public void readSimpleIntegerMapValuesCorrectly() { Map map = new LinkedHashMap(); - map.put("numberMapping.[1]", "2"); - map.put("numberMapping.[3]", "4"); + map.put("integerMapKeyMapping.[1]", "2"); + map.put("integerMapKeyMapping.[3]", "4"); RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map)); - Person target = converter.read(Person.class, rdo); + TypeWithMaps target = converter.read(TypeWithMaps.class, rdo); - assertThat(target.numberMapping, notNullValue()); - assertThat(target.numberMapping.get(1), is(2)); - assertThat(target.numberMapping.get(3), is(4)); + assertThat(target.integerMapKeyMapping, notNullValue()); + assertThat(target.integerMapKeyMapping.get(1), is(2)); + assertThat(target.integerMapKeyMapping.get(3), is(4)); + } + + @Test // DATAREDIS-768 + public void readMapWithDecimalMapKeyCorrectly() { + + Map map = new LinkedHashMap(); + map.put("decimalMapKeyMapping.[1.7]", "2"); + map.put("decimalMapKeyMapping.[3.1]", "4"); + + RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map)); + + TypeWithMaps target = converter.read(TypeWithMaps.class, rdo); + + assertThat(target.decimalMapKeyMapping, notNullValue()); + assertThat(target.decimalMapKeyMapping.get(1.7D), is("2")); + assertThat(target.decimalMapKeyMapping.get(3.1D), is("4")); + } + + @Test // DATAREDIS-768 + public void writeMapWithDecimalMapKeyCorrectly() { + + TypeWithMaps source = new TypeWithMaps(); + source.decimalMapKeyMapping = new LinkedHashMap(); + source.decimalMapKeyMapping.put(1.7D, "2"); + source.decimalMapKeyMapping.put(3.1D, "4"); + + RedisData target = write(source); + + assertThat(target.getBucket(), isBucket().containingUtf8String("decimalMapKeyMapping.[1.7]", "2") // + .containingUtf8String("decimalMapKeyMapping.[3.1]", "4")); + } + + @Test // DATAREDIS-768 + public void readMapWithDateMapKeyCorrectly() { + + Date judgmentDay = Date.from(Instant.parse("1979-08-29T12:00:00Z")); + + Map map = new LinkedHashMap(); + map.put("dateMapKeyMapping.[" + judgmentDay.getTime() + "]", "skynet"); + + RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map)); + + TypeWithMaps target = converter.read(TypeWithMaps.class, rdo); + + assertThat(target.dateMapKeyMapping, notNullValue()); + assertThat(target.dateMapKeyMapping.get(judgmentDay), is("skynet")); + } + + @Test // DATAREDIS-768 + public void writeMapWithDateMapKeyCorrectly() { + + Date judgmentDay = Date.from(Instant.parse("1979-08-29T12:00:00Z")); + + TypeWithMaps source = new TypeWithMaps(); + source.dateMapKeyMapping = Collections.singletonMap(judgmentDay, "skynet"); + + assertThat(write(source).getBucket(), + isBucket().containingUtf8String("dateMapKeyMapping.[" + judgmentDay.getTime() + "]", "skynet")); } @Test // DATAREDIS-425 @@ -1404,8 +1462,7 @@ public class MappingRedisConverterUnitTests { @Test // DATAREDIS-471 public void writeShouldWritePartialUpdatePathWithSimpleValueCorrectly() { - PartialUpdate update = new PartialUpdate("123", Person.class).set("firstname", "rand").set("age", - 24); + PartialUpdate update = new PartialUpdate("123", Person.class).set("firstname", "rand").set("age", 24); assertThat(write(update).getBucket(), isBucket().containingUtf8String("firstname", "rand").containingUtf8String("age", "24")); @@ -1559,8 +1616,7 @@ public class MappingRedisConverterUnitTests { @Test // DATAREDIS-471 public void writeShouldWritePartialUpdatePathWithSimpleMapValueOnNestedElementCorrectly() { - PartialUpdate update = new PartialUpdate("123", Person.class).set("relatives.[father].firstname", - "tam"); + PartialUpdate update = new PartialUpdate("123", Person.class).set("relatives.[father].firstname", "tam"); assertThat(write(update).getBucket(), isBucket().containingUtf8String("relatives.[father].firstname", "tam")); } @@ -1602,8 +1658,7 @@ public class MappingRedisConverterUnitTests { tear.id = "2"; tear.name = "city of tear"; - PartialUpdate update = new PartialUpdate("123", Person.class).set("visited", - Arrays.asList(tar, tear)); + PartialUpdate update = new PartialUpdate("123", Person.class).set("visited", Arrays.asList(tar, tear)); assertThat(write(update).getBucket(), isBucket().containingUtf8String("visited.[0]", "locations:1").containingUtf8String("visited.[1]", "locations:2") //