From 8357c7d88b4e7ee6d8bb73f1a38e602c11ff758c Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 21 May 2024 14:55:19 +0200 Subject: [PATCH] Polishing. Revise builder. Accept builder components in builder methods instead of the builder factory method. Enforce valid parameters instead of lenient, potentially null parameters. Introduce configuration means to control default typing. Extend tests. See #2878 Original pull request: #2905 --- .../GenericJackson2JsonRedisSerializer.java | 239 ++++++++++++------ ...cJackson2JsonRedisSerializerUnitTests.java | 101 ++++---- 2 files changed, 214 insertions(+), 126 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 6befb31e3..ae968d614 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java @@ -60,6 +60,7 @@ import com.fasterxml.jackson.databind.type.TypeFactory; * @author Mark Paluch * @author Mao Shuai * @author John Blum + * @author Anne Lee * @see org.springframework.data.redis.serializer.JacksonObjectReader * @see org.springframework.data.redis.serializer.JacksonObjectWriter * @see com.fasterxml.jackson.databind.ObjectMapper @@ -92,13 +93,13 @@ public class GenericJackson2JsonRedisSerializer implements RedisSerializer defaultTypingEnabled) { Lazy lazyTypeFactory = Lazy.of(mapper::getTypeFactory); @@ -199,19 +182,17 @@ public class GenericJackson2JsonRedisSerializer implements RedisSerializer newLazyTypeHintPropertyName(ObjectMapper mapper, Lazy defaultTypingEnabled) { + private static Lazy newLazyTypeHintPropertyName(ObjectMapper mapper, Lazy defaultTypingEnabled) { Lazy configuredTypeDeserializationPropertyName = getConfiguredTypeDeserializationPropertyName(mapper); - Lazy resolvedLazyTypeHintPropertyName = Lazy.of(() -> defaultTypingEnabled.get() ? null - : configuredTypeDeserializationPropertyName.get()); + Lazy resolvedLazyTypeHintPropertyName = Lazy + .of(() -> defaultTypingEnabled.get() ? null : configuredTypeDeserializationPropertyName.get()); - resolvedLazyTypeHintPropertyName = resolvedLazyTypeHintPropertyName.or("@class"); - - return resolvedLazyTypeHintPropertyName; + return resolvedLazyTypeHintPropertyName.or("@class"); } - private Lazy getConfiguredTypeDeserializationPropertyName(ObjectMapper mapper) { + private static Lazy getConfiguredTypeDeserializationPropertyName(ObjectMapper mapper) { return Lazy.of(() -> { @@ -226,20 +207,43 @@ public class GenericJackson2JsonRedisSerializer implements RedisSerializer { - @Serial - private static final long serialVersionUID = 1999052150548658808L; + @Serial private static final long serialVersionUID = 1999052150548658808L; private final String classIdentifier; @@ -408,65 +411,155 @@ public class GenericJackson2JsonRedisSerializer implements RedisSerializer nullValueSerializer; - private GenericJackson2JsonRedisSerializerBuilder( - ObjectMapper objectMapper, - JacksonObjectReader reader, - JacksonObjectWriter writer - ) { - this.mapper = objectMapper; - this.reader = reader; - this.writer = writer; - } + private @Nullable String typeHintPropertyName; + + private JacksonObjectReader reader = JacksonObjectReader.create(); + + private JacksonObjectWriter writer = JacksonObjectWriter.create(); + + private @Nullable ObjectMapper objectMapper; + + private @Nullable Boolean defaultTyping; + + private boolean registerNullValueSerializer = true; + + private @Nullable StdSerializer nullValueSerializer; + + private GenericJackson2JsonRedisSerializerBuilder() {} /** - * Configure a classPropertyName. + * Enable or disable default typing. Enabling default typing will override + * {@link ObjectMapper#setDefaultTyping(com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder)} for a given + * {@link ObjectMapper}. Default typing is enabled by default if no {@link ObjectMapper} is provided. * - * @param classPropertyTypeName can be {@literal null}. + * @param defaultTyping whether to enable/disable default typing. Enabled by default if the {@link ObjectMapper} is + * not provided. * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. - * @since 3.3 */ - public GenericJackson2JsonRedisSerializerBuilder classPropertyTypeName(@Nullable String classPropertyTypeName) { - this.classPropertyTypeName = classPropertyTypeName; + public GenericJackson2JsonRedisSerializerBuilder defaultTyping(boolean defaultTyping) { + this.defaultTyping = defaultTyping; return this; } /** - * Register a nullValueSerializer. + * Configure a property name to that represents the type hint. * - * @param nullValueSerializer the {@link StdSerializer} to use for {@link NullValue} serialization. Can be {@literal null}. + * @param typeHintPropertyName {@link String name} of the JSON property holding type information. * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. */ - public GenericJackson2JsonRedisSerializerBuilder registerNullValueSerializer(@Nullable StdSerializer nullValueSerializer) { + public GenericJackson2JsonRedisSerializerBuilder typeHintPropertyName(String typeHintPropertyName) { + + Assert.hasText(typeHintPropertyName, "Type hint property name must bot be null or empty"); + + this.typeHintPropertyName = typeHintPropertyName; + return this; + } + + /** + * Configure a provided {@link ObjectMapper}. Note that the provided {@link ObjectMapper} can be reconfigured with a + * {@link #nullValueSerializer} or default typing depending on the builder configuration. + * + * @param objectMapper must not be {@literal null}. + * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. + */ + public GenericJackson2JsonRedisSerializerBuilder objectMapper(ObjectMapper objectMapper) { + + Assert.notNull(objectMapper, "ObjectMapper must not be null"); + + this.objectMapper = objectMapper; + return this; + } + + /** + * Configure {@link JacksonObjectReader}. + * + * @param reader must not be {@literal null}. + * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. + */ + public GenericJackson2JsonRedisSerializerBuilder reader(JacksonObjectReader reader) { + + Assert.notNull(reader, "JacksonObjectReader must not be null"); + + this.reader = reader; + return this; + } + + /** + * Configure {@link JacksonObjectWriter}. + * + * @param writer must not be {@literal null}. + * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. + */ + public GenericJackson2JsonRedisSerializerBuilder writer(JacksonObjectWriter writer) { + + Assert.notNull(writer, "JacksonObjectWriter must not be null"); + + this.writer = writer; + return this; + } + + /** + * Register a {@link StdSerializer serializer} for {@link NullValue}. + * + * @param nullValueSerializer the {@link StdSerializer} to use for {@link NullValue} serialization, must not be + * {@literal null}. + * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. + */ + public GenericJackson2JsonRedisSerializerBuilder nullValueSerializer(StdSerializer nullValueSerializer) { + + Assert.notNull(nullValueSerializer, "Null value serializer must not be null"); + this.nullValueSerializer = nullValueSerializer; return this; } /** - * Create new instance of {@link GenericJackson2JsonRedisSerializer} with configuration options applied. + * Configure whether to register a {@link StdSerializer serializer} for {@link NullValue} serialization. The default + * serializer considers {@link #typeHintPropertyName(String)}. * - * @return new instance of {@link GenericJackson2JsonRedisSerializer}. + * @param registerNullValueSerializer {@code true} to register the default serializer; {@code false} otherwise. + * @return this {@link GenericJackson2JsonRedisSerializer.GenericJackson2JsonRedisSerializerBuilder}. + */ + public GenericJackson2JsonRedisSerializerBuilder registerNullValueSerializer(boolean registerNullValueSerializer) { + this.registerNullValueSerializer = registerNullValueSerializer; + return this; + } + + /** + * Creates a new instance of {@link GenericJackson2JsonRedisSerializer} with configuration options applied. Creates + * also a new {@link ObjectMapper} if none was provided. + * + * @return a new instance of {@link GenericJackson2JsonRedisSerializer}. */ public GenericJackson2JsonRedisSerializer build() { - Assert.notNull(this.mapper, "ObjectMapper must not be null"); - Assert.notNull(this.reader, "Reader must not be null"); - Assert.notNull(this.writer, "Writer must not be null"); - this.mapper.registerModule(new SimpleModule().addSerializer(this.nullValueSerializer != null ? this.nullValueSerializer : new NullValueSerializer(this.classPropertyTypeName))); - return new GenericJackson2JsonRedisSerializer(this.mapper, this.reader, this.writer, this.classPropertyTypeName); + ObjectMapper objectMapper = this.objectMapper; + boolean providedObjectMapper = objectMapper != null; + + if (objectMapper == null) { + objectMapper = new ObjectMapper(); + } + + if (registerNullValueSerializer) { + objectMapper.registerModule(new SimpleModule("GenericJackson2JsonRedisSerializerBuilder") + .addSerializer(this.nullValueSerializer != null ? this.nullValueSerializer + : new NullValueSerializer(this.typeHintPropertyName))); + } + + if ((!providedObjectMapper && (defaultTyping == null || defaultTyping)) + || (defaultTyping != null && defaultTyping)) { + objectMapper.setDefaultTyping(createDefaultTypeResolverBuilder(objectMapper, typeHintPropertyName)); + } + + return new GenericJackson2JsonRedisSerializer(objectMapper, this.reader, this.writer, this.typeHintPropertyName); } } 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 effbeae47..2a6011d95 100644 --- a/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializerUnitTests.java @@ -15,20 +15,10 @@ */ package org.springframework.data.redis.serializer; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; -import static org.springframework.test.util.ReflectionTestUtils.getField; -import static org.springframework.util.ObjectUtils.nullSafeEquals; -import static org.springframework.util.ObjectUtils.nullSafeHashCode; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; +import static org.springframework.test.util.ReflectionTestUtils.*; +import static org.springframework.util.ObjectUtils.*; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -40,10 +30,6 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.jsontype.TypeSerializer; -import com.fasterxml.jackson.databind.ser.std.StdSerializer; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -55,13 +41,17 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeInfo.As; import com.fasterxml.jackson.annotation.JsonView; import com.fasterxml.jackson.core.JsonGenerationException; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping; +import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder; +import com.fasterxml.jackson.databind.jsontype.TypeSerializer; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; import com.fasterxml.jackson.databind.type.TypeFactory; import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateDeserializer; import com.fasterxml.jackson.datatype.jsr310.ser.LocalDateSerializer; @@ -78,9 +68,14 @@ class GenericJackson2JsonRedisSerializerUnitTests { private static final SimpleObject SIMPLE_OBJECT = new SimpleObject(1L); private static final ComplexObject COMPLEX_OBJECT = new ComplexObject("steelheart", SIMPLE_OBJECT); - @Test // DATAREDIS-392 + @Test // DATAREDIS-392, GH-2878 void shouldUseDefaultTyping() { assertThat(extractTypeResolver(new GenericJackson2JsonRedisSerializer())).isNotNull(); + assertThat(extractTypeResolver(GenericJackson2JsonRedisSerializer.builder().build())).isNotNull(); + assertThat(extractTypeResolver(GenericJackson2JsonRedisSerializer.builder().defaultTyping(false).build())).isNull(); + assertThat( + extractTypeResolver(GenericJackson2JsonRedisSerializer.builder().objectMapper(new ObjectMapper()).build())) + .isNull(); } @Test // DATAREDIS-392 @@ -102,12 +97,13 @@ class GenericJackson2JsonRedisSerializerUnitTests { @Test // DATAREDIS-392 void shouldUseDefaultTypingWhenClassPropertyNameIsProvided() { - TypeResolverBuilder typeResolver = extractTypeResolver(new GenericJackson2JsonRedisSerializer("firefight")); + TypeResolverBuilder typeResolver = extractTypeResolver( + GenericJackson2JsonRedisSerializer.builder().typeHintPropertyName("firefight").build()); assertThat((String) getField(typeResolver, "_typeProperty")).isEqualTo("firefight"); } @Test // DATAREDIS-392 - void serializeShouldReturnEmptyByteArrayWhenSouceIsNull() { + void serializeShouldReturnEmptyByteArrayWhenSourceIsNull() { assertThat(new GenericJackson2JsonRedisSerializer().serialize(null)).isEqualTo(SerializationUtils.EMPTY_ARRAY); } @@ -231,10 +227,10 @@ class GenericJackson2JsonRedisSerializerUnitTests { user.id = 42; user.name = "Walter White"; - GenericJackson2JsonRedisSerializer serializer = new GenericJackson2JsonRedisSerializer((String) null, - JacksonObjectReader.create(), (mapper, source) -> { + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .writer((mapper, source) -> { return mapper.writerWithView(Views.Basic.class).writeValueAsBytes(source); - }); + }).build(); byte[] result = serializer.serialize(user); @@ -244,10 +240,10 @@ class GenericJackson2JsonRedisSerializerUnitTests { @Test // GH-2322 void considersWriterForCustomObjectMapper() { - GenericJackson2JsonRedisSerializer serializer = new GenericJackson2JsonRedisSerializer(new ObjectMapper(), - JacksonObjectReader.create(), (mapper, source) -> { + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .objectMapper(new ObjectMapper()).writer((mapper, source) -> { return mapper.writerWithView(Views.Basic.class).writeValueAsBytes(source); - }); + }).build(); User user = new User(); user.email = "walter@heisenberg.com"; @@ -267,13 +263,14 @@ class GenericJackson2JsonRedisSerializerUnitTests { user.id = 42; user.name = "Walter White"; - GenericJackson2JsonRedisSerializer serializer = new GenericJackson2JsonRedisSerializer((String) null, + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .reader( (mapper, source, type) -> { if (type.getRawClass() == User.class) { return mapper.readerWithView(Views.Basic.class).forType(type).readValue(source); } return mapper.readValue(source, type); - }, JacksonObjectWriter.create()); + }).build(); byte[] serializedValue = serializer.serialize(user); @@ -436,7 +433,7 @@ class GenericJackson2JsonRedisSerializerUnitTests { assertThat(serializer.configure(configurer)).isSameAs(serializer); - verify(mockObjectMapper, times(1)).registerModule(mockModule); + verify(mockObjectMapper).registerModule(mockModule); verifyNoMoreInteractions(mockObjectMapper); verifyNoInteractions(mockModule); } @@ -452,36 +449,33 @@ class GenericJackson2JsonRedisSerializerUnitTests { @Test void defaultSerializeAndDeserializeNullValueWithBuilderClass() { - GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder( - new ObjectMapper().enableDefaultTyping(DefaultTyping.EVERYTHING, As.PROPERTY), - JacksonObjectReader.create(), JacksonObjectWriter.create()) - .classPropertyTypeName(null) - .registerNullValueSerializer(null) + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .objectMapper(new ObjectMapper().enableDefaultTyping(DefaultTyping.EVERYTHING, As.PROPERTY)) .build(); serializeAndDeserializeNullValue(serializer); } - @Test - void customSerializeAndDeserializeNullValueWithBuilderClass() { - GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder( - new ObjectMapper(), JacksonObjectReader.create(), JacksonObjectWriter.create()) - .classPropertyTypeName(null) - .registerNullValueSerializer(new StdSerializer<>(NullValue.class) { - @Override - public void serialize(NullValue nullValue, - JsonGenerator jsonGenerator, - SerializerProvider serializerProvider) throws IOException { - jsonGenerator.writeNull(); - } + @Test // GH-2878 + void customNullValueSerializer() { - @Override - public void serializeWithType(NullValue value, JsonGenerator jsonGenerator, SerializerProvider serializers, - TypeSerializer typeSerializer) throws IOException { + StdSerializer nullValueSerializer = new StdSerializer<>(NullValue.class) { + @Override + public void serialize(NullValue nullValue, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) + throws IOException { + jsonGenerator.writeNull(); + } - serialize(value, jsonGenerator, serializers); - } - }) + @Override + public void serializeWithType(NullValue value, JsonGenerator jsonGenerator, SerializerProvider serializers, + TypeSerializer typeSerializer) throws IOException { + + serialize(value, jsonGenerator, serializers); + } + }; + + GenericJackson2JsonRedisSerializer serializer = GenericJackson2JsonRedisSerializer.builder() + .nullValueSerializer(nullValueSerializer) .build(); NullValue nv = BeanUtils.instantiateClass(NullValue.class); @@ -504,6 +498,7 @@ class GenericJackson2JsonRedisSerializerUnitTests { assertThat(deserializedValue).isInstanceOf(NullValue.class); } + @Nullable private TypeResolverBuilder extractTypeResolver(GenericJackson2JsonRedisSerializer serializer) { ObjectMapper mapper = (ObjectMapper) getField(serializer, "mapper");