diff --git a/pom.xml b/pom.xml index 4c38669587..2da31004b5 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 2.7.0-SNAPSHOT + 2.7.0-GH-2198-SNAPSHOT Spring Data Redis diff --git a/src/main/java/org/springframework/data/redis/core/StreamObjectMapper.java b/src/main/java/org/springframework/data/redis/core/StreamObjectMapper.java index 59fb015ad7..6239364fe9 100644 --- a/src/main/java/org/springframework/data/redis/core/StreamObjectMapper.java +++ b/src/main/java/org/springframework/data/redis/core/StreamObjectMapper.java @@ -17,9 +17,9 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; @@ -29,6 +29,7 @@ import org.springframework.data.redis.connection.stream.StreamRecords; import org.springframework.data.redis.core.convert.RedisCustomConversions; import org.springframework.data.redis.hash.HashMapper; +import org.springframework.data.redis.hash.HashObjectReader; import org.springframework.data.redis.hash.ObjectHashMapper; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -72,25 +73,7 @@ class StreamObjectMapper { this.mapper = (HashMapper) mapper; if (mapper instanceof ObjectHashMapper) { - - ObjectHashMapper ohm = (ObjectHashMapper) mapper; - this.objectHashMapper = new HashMapper() { - - @Override - public Map toHash(Object object) { - return (Map) ohm.toHash(object); - } - - @Override - public Object fromHash(Map hash) { - - Map map = hash.entrySet().stream() - .collect(Collectors.toMap(e -> conversionService.convert(e.getKey(), byte[].class), - e -> conversionService.convert(e.getValue(), byte[].class))); - - return ohm.fromHash(map); - } - }; + this.objectHashMapper = new BinaryObjectHashMapperAdapter((ObjectHashMapper) mapper); } else { this.objectHashMapper = null; } @@ -174,9 +157,27 @@ static List> toObjectRecords(@Nullable List HashMapper getHashMapper(Class targetType) { - return (HashMapper) doGetHashMapper(conversionService, targetType); + + HashMapper hashMapper = doGetHashMapper(conversionService, targetType); + + if (hashMapper instanceof HashObjectReader) { + + return new HashMapper() { + @Override + public Map toHash(V object) { + return hashMapper.toHash(object); + } + + @Override + public V fromHash(Map hash) { + return ((HashObjectReader) hashMapper).fromHash(targetType, hash); + } + }; + } + + return hashMapper; } /** @@ -208,4 +209,46 @@ boolean isSimpleType(Class targetType) { ConversionService getConversionService() { return conversionService; } + + private static class BinaryObjectHashMapperAdapter + implements HashMapper, HashObjectReader { + + private final ObjectHashMapper ohm; + + public BinaryObjectHashMapperAdapter(ObjectHashMapper ohm) { + this.ohm = ohm; + } + + @Override + @SuppressWarnings({ "unchecked", "rawtypes" }) + public Map toHash(Object object) { + return (Map) ohm.toHash(object); + } + + @Override + public Object fromHash(Map hash) { + return ohm.fromHash(toMap(hash)); + } + + @Override + public R fromHash(Class type, Map hash) { + return ohm.fromHash(type, toMap(hash)); + } + + private static Map toMap(Map hash) { + + Map target = new LinkedHashMap<>(hash.size()); + + for (Map.Entry entry : hash.entrySet()) { + target.put(toBytes(entry.getKey()), toBytes(entry.getValue())); + } + + return target; + } + + @Nullable + private static byte[] toBytes(Object value) { + return value instanceof byte[] ? (byte[]) value : conversionService.convert(value, byte[].class); + } + } } diff --git a/src/main/java/org/springframework/data/redis/hash/BeanUtilsHashMapper.java b/src/main/java/org/springframework/data/redis/hash/BeanUtilsHashMapper.java index 17c936240c..e9b105d5d6 100644 --- a/src/main/java/org/springframework/data/redis/hash/BeanUtilsHashMapper.java +++ b/src/main/java/org/springframework/data/redis/hash/BeanUtilsHashMapper.java @@ -21,6 +21,8 @@ import org.apache.commons.beanutils.BeanUtils; +import org.springframework.util.Assert; + /** * HashMapper based on Apache Commons BeanUtils project. Does NOT supports nested properties. * @@ -28,7 +30,7 @@ * @author Christoph Strobl * @author Mark Paluch */ -public class BeanUtilsHashMapper implements HashMapper { +public class BeanUtilsHashMapper implements HashMapper, HashObjectReader { private final Class type; @@ -47,8 +49,20 @@ public BeanUtilsHashMapper(Class type) { */ @Override public T fromHash(Map hash) { + return fromHash(type, hash); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.hash.HashMapper#fromHash(java.lang.Class, java.util.Map) + */ + @Override + public R fromHash(Class type, Map hash) { + + Assert.notNull(type, "Type must not be null"); + Assert.notNull(hash, "Hash must not be null"); - T instance = org.springframework.beans.BeanUtils.instantiateClass(type); + R instance = org.springframework.beans.BeanUtils.instantiateClass(type); try { diff --git a/src/main/java/org/springframework/data/redis/hash/HashMapper.java b/src/main/java/org/springframework/data/redis/hash/HashMapper.java index f95dd56e65..be04f69b56 100644 --- a/src/main/java/org/springframework/data/redis/hash/HashMapper.java +++ b/src/main/java/org/springframework/data/redis/hash/HashMapper.java @@ -26,6 +26,7 @@ * @param Redis Hash value type * @author Costin Leau * @author Mark Paluch + * @see HashObjectReader */ public interface HashMapper { diff --git a/src/main/java/org/springframework/data/redis/hash/HashObjectReader.java b/src/main/java/org/springframework/data/redis/hash/HashObjectReader.java new file mode 100644 index 0000000000..a31d00865b --- /dev/null +++ b/src/main/java/org/springframework/data/redis/hash/HashObjectReader.java @@ -0,0 +1,38 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.hash; + +import java.util.Map; + +/** + * Core mapping contract to materialize an object using particular Java class from a Redis Hash. + * + * @param Redis Hash field type + * @param Redis Hash value type + * @author Mark Paluch + * @since 2.7 + * @see HashMapper + */ +public interface HashObjectReader { + + /** + * Materialize an object of the {@link Class type} from a {@code hash}. + * + * @param hash must not be {@literal null}. + * @return the materialized object from the given {@code hash}. + */ + R fromHash(Class type, Map hash); +} diff --git a/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java b/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java index 44a5382c4f..40d0ec27d5 100644 --- a/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java +++ b/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 the original author or authors. + * Copyright 2016-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -48,10 +48,13 @@ import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping; import com.fasterxml.jackson.databind.SerializationFeature; import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.deser.BeanDeserializerFactory; +import com.fasterxml.jackson.databind.deser.Deserializers; import com.fasterxml.jackson.databind.deser.std.UntypedObjectDeserializer; import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; @@ -148,7 +151,7 @@ * @author Mark Paluch * @since 1.8 */ -public class Jackson2HashMapper implements HashMapper { +public class Jackson2HashMapper implements HashMapper, HashObjectReader { private final HashMapperModule HASH_MAPPER_MODULE = new HashMapperModule(); @@ -168,7 +171,11 @@ public Jackson2HashMapper(boolean flatten) { @Override protected TypeResolverBuilder _constructDefaultTypeResolverBuilder(DefaultTyping applicability, PolymorphicTypeValidator ptv) { + return new DefaultTypeResolverBuilder(applicability, ptv) { + + Map serializerPresentCache = new HashMap<>(); + public boolean useForType(JavaType t) { if (t.isPrimitive()) { @@ -176,11 +183,43 @@ public boolean useForType(JavaType t) { } if (EVERYTHING.equals(_appliesFor)) { + + while (t.isArrayType()) { + t = t.getContentType(); + } + while (t.isReferenceType()) { + t = t.getReferencedType(); + } + + /* + * check for registered serializers and make uses of those. + */ + if (serializerPresentCache.computeIfAbsent(t.getRawClass(), this::hasConfiguredSerializer)) { + return false; + } + return !TreeNode.class.isAssignableFrom(t.getRawClass()); } return super.useForType(t); } + + private Boolean hasConfiguredSerializer(Class key) { + + if (!(_deserializationContext.getFactory() instanceof BeanDeserializerFactory)) { + return false; + } + + Iterator deserializers = ((BeanDeserializerFactory) _deserializationContext.getFactory()) + .getFactoryConfig().deserializers().iterator(); + while (deserializers.hasNext()) { + Deserializers next = deserializers.next(); + if (next.hasDeserializerFor(_deserializationConfig, key)) { + return true; + } + } + return false; + } }; } }.findAndRegisterModules(), flatten); @@ -188,12 +227,14 @@ public boolean useForType(JavaType t) { typingMapper.activateDefaultTyping(typingMapper.getPolymorphicTypeValidator(), DefaultTyping.EVERYTHING, As.PROPERTY); typingMapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false); + typingMapper.configure(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL, true); // Prevent splitting time types into arrays. E typingMapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false); typingMapper.setSerializationInclusion(Include.NON_NULL); typingMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); typingMapper.registerModule(HASH_MAPPER_MODULE); + typingMapper.findAndRegisterModules(); } /** @@ -209,7 +250,7 @@ public Jackson2HashMapper(ObjectMapper mapper, boolean flatten) { this.flatten = flatten; this.untypedMapper = new ObjectMapper(); - untypedMapper.findAndRegisterModules(); + this.untypedMapper.findAndRegisterModules(); this.untypedMapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false); this.untypedMapper.setSerializationInclusion(Include.NON_NULL); } @@ -232,16 +273,25 @@ public Map toHash(Object source) { */ @Override public Object fromHash(Map hash) { + return fromHash(Object.class, hash); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.hash.HashMapper#fromHash(Class, java.util.Map) + */ + @Override + public R fromHash(Class type, Map hash) { try { if (flatten) { - return typingMapper.reader().forType(Object.class) + return typingMapper.reader().forType(type) .readValue(untypedMapper.writeValueAsBytes(doUnflatten(hash))); } - return typingMapper.treeToValue(untypedMapper.valueToTree(hash), Object.class); + return typingMapper.treeToValue(untypedMapper.valueToTree(hash), type); } catch (IOException e) { throw new MappingException(e.getMessage(), e); diff --git a/src/main/java/org/springframework/data/redis/hash/ObjectHashMapper.java b/src/main/java/org/springframework/data/redis/hash/ObjectHashMapper.java index 72a2812304..09f20c6d1c 100644 --- a/src/main/java/org/springframework/data/redis/hash/ObjectHashMapper.java +++ b/src/main/java/org/springframework/data/redis/hash/ObjectHashMapper.java @@ -69,7 +69,7 @@ * @author Mark Paluch * @since 1.8 */ -public class ObjectHashMapper implements HashMapper { +public class ObjectHashMapper implements HashMapper, HashObjectReader { @Nullable private volatile static ObjectHashMapper sharedInstance; @@ -169,12 +169,20 @@ public Map toHash(Object source) { */ @Override public Object fromHash(Map hash) { + return fromHash(Object.class, hash); + } - if (hash == null || hash.isEmpty()) { - return null; - } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.hash.HashMapper#fromHash(java.lang.Class, java.util.Map) + */ + @Override + public R fromHash(Class type, Map hash) { + + Assert.notNull(type, "Type must not be null"); + Assert.notNull(hash, "Hash must not be null"); - return converter.read(Object.class, new RedisData(hash)); + return converter.read(type, new RedisData(hash)); } /** diff --git a/src/test/java/org/springframework/data/redis/core/StreamObjectMapperUnitTests.java b/src/test/java/org/springframework/data/redis/core/StreamObjectMapperUnitTests.java new file mode 100644 index 0000000000..d4f61b777f --- /dev/null +++ b/src/test/java/org/springframework/data/redis/core/StreamObjectMapperUnitTests.java @@ -0,0 +1,62 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.core; + +import static org.assertj.core.api.Assertions.*; + +import lombok.Data; + +import java.util.Collections; + +import org.junit.jupiter.api.Test; + +import org.springframework.data.redis.hash.Jackson2HashMapper; +import org.springframework.data.redis.hash.ObjectHashMapper; + +/** + * Unit tests for {@link StreamObjectMapper}. + * + * @author Mark Paluch + */ +class StreamObjectMapperUnitTests { + + @Test // GH-2198 + void shouldRetainTypeHintUsingObjectHashMapper() { + + StreamObjectMapper mapper = new StreamObjectMapper(ObjectHashMapper.getSharedInstance()); + + MyType result = mapper.getHashMapper(MyType.class) + .fromHash(Collections.singletonMap("value".getBytes(), "hello".getBytes())); + + assertThat(result.value).isEqualTo("hello"); + } + + @Test // GH-2198 + void shouldRetainTypeHintUsingJackson() { + + StreamObjectMapper mapper = new StreamObjectMapper(new Jackson2HashMapper(true)); + + MyType result = mapper.getHashMapper(MyType.class).fromHash(Collections.singletonMap("value", "hello")); + + assertThat(result.value).isEqualTo("hello"); + } + + @Data + static class MyType { + String value; + } + +} diff --git a/src/test/java/org/springframework/data/redis/mapping/FlatteningJackson2HashMapperUnitTests.java b/src/test/java/org/springframework/data/redis/mapping/FlatteningJackson2HashMapperUnitTests.java new file mode 100644 index 0000000000..e2b980d84b --- /dev/null +++ b/src/test/java/org/springframework/data/redis/mapping/FlatteningJackson2HashMapperUnitTests.java @@ -0,0 +1,27 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.mapping; + +import org.springframework.data.redis.hash.Jackson2HashMapper; + +/** + * @author Mark Paluch + */ +public class FlatteningJackson2HashMapperUnitTests extends Jackson2HashMapperUnitTests { + FlatteningJackson2HashMapperUnitTests() { + super(new Jackson2HashMapper(true)); + } +} diff --git a/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperUnitTests.java b/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperUnitTests.java index 7553c4621a..280e6b78aa 100644 --- a/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperUnitTests.java +++ b/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2021 the original author or authors. + * Copyright 2016-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.springframework.data.redis.mapping; +import static org.assertj.core.api.Assertions.*; + import lombok.Data; import java.time.LocalDate; @@ -40,7 +42,7 @@ * @author Christoph Strobl * @author Mark Paluch */ -public abstract class Jackson2HashMapperUnitTests extends AbstractHashMapperTests { +abstract class Jackson2HashMapperUnitTests extends AbstractHashMapperTests { private final Jackson2HashMapper mapper; @@ -48,19 +50,6 @@ public abstract class Jackson2HashMapperUnitTests extends AbstractHashMapperTest this.mapper = mapper; } - static class FlatteningJackson2HashMapperUnitTests extends Jackson2HashMapperUnitTests { - FlatteningJackson2HashMapperUnitTests() { - super(new Jackson2HashMapper(true)); - } - } - - static class NonFlatteningJackson2HashMapperUnitTests extends Jackson2HashMapperUnitTests { - - NonFlatteningJackson2HashMapperUnitTests() { - super(new Jackson2HashMapper(false)); - } - } - @Override @SuppressWarnings("rawtypes") protected HashMapper mapperFor(Class t) { @@ -183,6 +172,24 @@ void dateValueShouldBeTreatedCorrectly() { assertBackAndForwardMapping(source); } + @Test // GH-2198 + void shouldDeserializeObjectWithoutClassHint() { + + WithDates source = new WithDates(); + source.string = "id-1"; + source.date = new Date(1561543964015L); + source.calendar = Calendar.getInstance(); + source.localDate = LocalDate.parse("2018-01-02"); + source.localDateTime = LocalDateTime.parse("2018-01-02T12:13:14"); + + Map map = mapper.toHash(source); + + // ensure that we remove the correct type hint + assertThat(map.remove("@class")).isNotNull(); + + assertThat(mapper.fromHash(WithDates.class, map)).isEqualTo(source); + } + @Test // GH-1566 void mapFinalClass() { diff --git a/src/test/java/org/springframework/data/redis/mapping/NonFlatteningJackson2HashMapperUnitTests.java b/src/test/java/org/springframework/data/redis/mapping/NonFlatteningJackson2HashMapperUnitTests.java new file mode 100644 index 0000000000..96e67c6b53 --- /dev/null +++ b/src/test/java/org/springframework/data/redis/mapping/NonFlatteningJackson2HashMapperUnitTests.java @@ -0,0 +1,28 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.mapping; + +import org.springframework.data.redis.hash.Jackson2HashMapper; + +/** + * @author Mark Paluch + */ +public class NonFlatteningJackson2HashMapperUnitTests extends Jackson2HashMapperUnitTests { + + NonFlatteningJackson2HashMapperUnitTests() { + super(new Jackson2HashMapper(false)); + } +} diff --git a/src/test/java/org/springframework/data/redis/mapping/ObjectHashMapperTests.java b/src/test/java/org/springframework/data/redis/mapping/ObjectHashMapperTests.java index ed30bb8f63..626119b079 100644 --- a/src/test/java/org/springframework/data/redis/mapping/ObjectHashMapperTests.java +++ b/src/test/java/org/springframework/data/redis/mapping/ObjectHashMapperTests.java @@ -20,6 +20,7 @@ import lombok.Data; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; import org.junit.jupiter.api.Test; @@ -83,6 +84,18 @@ void fromHashShouldFailIfTypeDoesNotMatch() { assertThat(objectHashMapper.fromHash(hash)).isEqualTo(source); } + @Test // GH-2198 + void readHashConsidersTypeHint() { + + Map hash = new LinkedHashMap<>(); + hash.put("value".getBytes(), "hello".getBytes()); + + ObjectHashMapper objectHashMapper = ObjectHashMapper.getSharedInstance(); + WithTypeAlias withTypeAlias = objectHashMapper.fromHash(WithTypeAlias.class, hash); + + assertThat(withTypeAlias.value).isEqualTo("hello"); + } + @TypeAlias("_42_") @Data static class WithTypeAlias {