From c72f33d8c596f610c5fdc49e0800c144be89080e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 16 Sep 2024 13:52:44 +0200 Subject: [PATCH 1/3] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 09bebeb5bd..6c0e634480 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 3.4.0-SNAPSHOT + 3.4.0-GH-2981-SNAPSHOT Spring Data Redis Spring Data module for Redis From 41c512b4ec97331c9541a2100ad743df40ecc0a8 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 16 Sep 2024 14:35:10 +0200 Subject: [PATCH 2/3] Support deserialization in `GenericJackson2JsonRedisSerializer` when using custom `JsonFactory`. --- pom.xml | 7 +++ .../GenericJackson2JsonRedisSerializer.java | 53 ++++++++++++++++--- ...cJackson2JsonRedisSerializerUnitTests.java | 27 ++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 6c0e634480..bab11bf0cb 100644 --- a/pom.xml +++ b/pom.xml @@ -276,6 +276,13 @@ test + + org.msgpack + jackson-dataformat-msgpack + 0.9.8 + test + + edu.umd.cs.mtc multithreadedtc diff --git a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java index 7ecc89d18c..9358f36a7e 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java @@ -32,13 +32,19 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeInfo.As; import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.TreeNode; import com.fasterxml.jackson.databind.DeserializationConfig; import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping; import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.deser.BeanDeserializerFactory; +import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext; +import com.fasterxml.jackson.databind.deser.std.JsonNodeDeserializer; import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; import com.fasterxml.jackson.databind.jsontype.TypeSerializer; @@ -179,7 +185,7 @@ private static TypeResolver newTypeResolver(ObjectMapper mapper, @Nullable Strin Lazy lazyTypeHintPropertyName = typeHintPropertyName != null ? Lazy.of(typeHintPropertyName) : newLazyTypeHintPropertyName(mapper, defaultTypingEnabled); - return new TypeResolver(lazyTypeFactory, lazyTypeHintPropertyName); + return new TypeResolver(mapper, lazyTypeFactory, lazyTypeHintPropertyName); } private static Lazy newLazyTypeHintPropertyName(ObjectMapper mapper, Lazy defaultTypingEnabled) { @@ -340,14 +346,13 @@ protected JavaType resolveType(byte[] source, Class type) throws IOException */ static class TypeResolver { - // need a separate instance to bypass class hint checks - private final ObjectMapper mapper = new ObjectMapper(); - + private final ObjectMapper mapper; private final Supplier typeFactory; private final Supplier hintName; - TypeResolver(Supplier typeFactory, Supplier hintName) { + TypeResolver(ObjectMapper mapper, Supplier typeFactory, Supplier hintName) { + this.mapper = mapper; this.typeFactory = typeFactory; this.hintName = hintName; } @@ -358,7 +363,7 @@ protected JavaType constructType(Class type) { protected JavaType resolveType(byte[] source, Class type) throws IOException { - JsonNode root = mapper.readTree(source); + JsonNode root = readTree(source); JsonNode jsonNode = root.get(hintName.get()); if (jsonNode instanceof TextNode && jsonNode.asText() != null) { @@ -367,6 +372,42 @@ protected JavaType resolveType(byte[] source, Class type) throws IOException return constructType(type); } + + /** + * Lenient variant of ObjectMapper._readTreeAndClose using a strict {@link JsonNodeDeserializer}. + * + * @param source + * @return + * @throws IOException + */ + private JsonNode readTree(byte[] source) throws IOException { + + JsonDeserializer deserializer = JsonNodeDeserializer.getDeserializer(JsonNode.class); + DeserializationConfig cfg = mapper.getDeserializationConfig(); + + try (JsonParser parser = mapper.createParser(source)) { + + cfg.initialize(parser); + JsonToken t = parser.currentToken(); + if (t == null) { + t = parser.nextToken(); + if (t == null) { + return cfg.getNodeFactory().missingNode(); + } + } + + /* + * Hokey pokey! Oh my. + */ + DefaultDeserializationContext ctxt = new DefaultDeserializationContext.Impl(BeanDeserializerFactory.instance) + .createInstance(cfg, parser, mapper.getInjectableValues()); + if (t == JsonToken.VALUE_NULL) { + return cfg.getNodeFactory().nullNode(); + } else { + return deserializer.deserialize(parser, ctxt); + } + } + } } /** diff --git a/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java b/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java index 16daa677c8..c91e6b46f7 100644 --- a/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java @@ -32,6 +32,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import org.msgpack.jackson.dataformat.MessagePackFactory; import org.springframework.beans.BeanUtils; import org.springframework.cache.support.NullValue; @@ -449,6 +450,7 @@ void configureWithNullConsumerThrowsIllegalArgumentException() { @Test void defaultSerializeAndDeserializeNullValueWithBuilderClass() { + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() .objectMapper(new ObjectMapper().enableDefaultTyping(DefaultTyping.EVERYTHING, As.PROPERTY)) .build(); @@ -487,6 +489,31 @@ public void serializeWithType(NullValue value, JsonGenerator jsonGenerator, Seri assertThat(deserializedValue).isNull(); } + @Test // GH-2981 + void defaultSerializeAndDeserializeWithCustomJsonFactory() { + + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .objectMapper( + new ObjectMapper(new MessagePackFactory()).enableDefaultTyping(DefaultTyping.EVERYTHING, As.PROPERTY)) + .build(); + + byte[] serializedValue = serializer.serialize(COMPLEX_OBJECT); + + Object deserializedValue = serializer.deserialize(serializedValue, Object.class); + assertThat(deserializedValue).isEqualTo(COMPLEX_OBJECT); + } + + @Test // GH-2981 + void defaultSerializeAndDeserializeNullValueWithBuilderClassAndCustomJsonFactory() { + + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .objectMapper( + new ObjectMapper(new MessagePackFactory()).enableDefaultTyping(DefaultTyping.EVERYTHING, As.PROPERTY)) + .build(); + + serializeAndDeserializeNullValue(serializer); + } + private static void serializeAndDeserializeNullValue(GenericJackson2JsonRedisSerializer serializer) { NullValue nv = BeanUtils.instantiateClass(NullValue.class); From 8aaf501b73a8c74df7db839793d55b6e83720835 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 17 Sep 2024 08:51:18 +0200 Subject: [PATCH 3/3] Spike on parsed JsonNode reuse. --- .../GenericJackson2JsonRedisSerializer.java | 28 ++++++++++++++----- .../Jackson2JsonRedisSerializer.java | 2 +- .../redis/serializer/JacksonObjectReader.java | 18 ++++++++++-- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java index 9358f36a7e..dab4e7716f 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java @@ -51,6 +51,7 @@ import com.fasterxml.jackson.databind.jsontype.impl.StdTypeResolverBuilder; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.node.TextNode; +import com.fasterxml.jackson.databind.node.TreeTraversingParser; import com.fasterxml.jackson.databind.ser.SerializerFactory; import com.fasterxml.jackson.databind.ser.std.StdSerializer; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -306,12 +307,21 @@ public T deserialize(@Nullable byte[] source, Class type) throws Serializ } try { - return (T) reader.read(mapper, source, resolveType(source, type)); + + TypeTuple typeTuple = resolveType(source, type); + try (JsonParser parser = createParser(source, typeTuple)) { + return (T) reader.read(mapper, parser, typeTuple.type()); + } + } catch (Exception ex) { - throw new SerializationException("Could not read JSON:%s ".formatted(ex.getMessage()), ex); + throw new SerializationException("Could not read JSON: %s ".formatted(ex.getMessage()), ex); } } + private JsonParser createParser(byte[] source, TypeTuple typeTuple) throws IOException { + return typeTuple.node() == null ? mapper.createParser(source) : new TreeTraversingParser(typeTuple.node(), mapper); + } + /** * Builder method used to configure and customize the internal Jackson {@link ObjectMapper} created by this * {@link GenericJackson2JsonRedisSerializer} and used to de/serialize {@link Object objects} as {@literal JSON}. @@ -332,15 +342,19 @@ public GenericJackson2JsonRedisSerializer configure(Consumer objec return this; } - protected JavaType resolveType(byte[] source, Class type) throws IOException { + protected TypeTuple resolveType(byte[] source, Class type) throws IOException { if (!type.equals(Object.class) || !defaultTypingEnabled.get()) { - return typeResolver.constructType(type); + return new TypeTuple(typeResolver.constructType(type), null); } return typeResolver.resolveType(source, type); } + protected record TypeTuple(JavaType type, @Nullable JsonNode node) { + + } + /** * @since 3.0 */ @@ -361,16 +375,16 @@ protected JavaType constructType(Class type) { return typeFactory.get().constructType(type); } - protected JavaType resolveType(byte[] source, Class type) throws IOException { + protected TypeTuple resolveType(byte[] source, Class type) throws IOException { JsonNode root = readTree(source); JsonNode jsonNode = root.get(hintName.get()); if (jsonNode instanceof TextNode && jsonNode.asText() != null) { - return typeFactory.get().constructFromCanonical(jsonNode.asText()); + return new TypeTuple(typeFactory.get().constructFromCanonical(jsonNode.asText()), root); } - return constructType(type); + return new TypeTuple(constructType(type), root); } /** diff --git a/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java index 76238bc60d..2d68ea5aed 100644 --- a/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java @@ -166,7 +166,7 @@ public T deserialize(@Nullable byte[] bytes) throws SerializationException { return null; } try { - return (T) this.reader.read(this.mapper, bytes, javaType); + return (T) this.reader.read(this.mapper, this.mapper.createParser(bytes), javaType); } catch (Exception ex) { throw new SerializationException("Could not read JSON: " + ex.getMessage(), ex); } diff --git a/src/main/java/org/springframework/data/redis/serializer/JacksonObjectReader.java b/src/main/java/org/springframework/data/redis/serializer/JacksonObjectReader.java index 4e76cd4ba7..5070d5c4b5 100644 --- a/src/main/java/org/springframework/data/redis/serializer/JacksonObjectReader.java +++ b/src/main/java/org/springframework/data/redis/serializer/JacksonObjectReader.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.io.InputStream; +import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; @@ -43,7 +44,20 @@ public interface JacksonObjectReader { * @return the deserialized Java object. * @throws IOException if an I/O error or JSON deserialization error occurs. */ - Object read(ObjectMapper mapper, byte[] source, JavaType type) throws IOException; + default Object read(ObjectMapper mapper, byte[] source, JavaType type) throws IOException { + return read(mapper, mapper.createParser(source), type); + } + + /** + * Read an object graph from the given root JSON into a Java object considering the {@link JavaType}. + * + * @param mapper the object mapper to use. + * @param parser the JSON parser to use. + * @param type the Java target type + * @return the deserialized Java object. + * @throws IOException if an I/O error or JSON deserialization error occurs. + */ + Object read(ObjectMapper mapper, JsonParser parser, JavaType type) throws IOException; /** * Create a default {@link JacksonObjectReader} delegating to {@link ObjectMapper#readValue(InputStream, JavaType)}. @@ -51,7 +65,7 @@ public interface JacksonObjectReader { * @return the default {@link JacksonObjectReader}. */ static JacksonObjectReader create() { - return (mapper, source, type) -> mapper.readValue(source, 0, source.length, type); + return ObjectMapper::readValue; } }