From 060e3fdc76082cecd80b0778ed08ae9d682d739e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 21 Sep 2023 14:46:14 +0200 Subject: [PATCH] Refine `RedisSerializer` implementations. This commit polishes up method ordering, introduces Javadoc where missing and updates nullability annotations and argument names. Closes #1097 --- .../serializer/ByteArrayRedisSerializer.java | 4 +- .../serializer/DefaultRedisElementReader.java | 2 +- .../serializer/DefaultRedisElementWriter.java | 3 +- .../GenericJackson2JsonRedisSerializer.java | 6 +- .../serializer/GenericToStringSerializer.java | 65 ++++++++++--------- .../Jackson2JsonRedisSerializer.java | 53 +++++++-------- .../JdkSerializationRedisSerializer.java | 25 +++---- .../data/redis/serializer/OxmSerializer.java | 28 ++++---- .../redis/serializer/RedisElementWriter.java | 3 +- .../redis/serializer/RedisSerializer.java | 58 ++++++++++------- .../serializer/StringRedisSerializer.java | 8 +-- .../data/redis/cache/CacheTestParams.java | 4 +- .../adapter/MessageListenerUnitTests.java | 3 +- 13 files changed, 142 insertions(+), 120 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/serializer/ByteArrayRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/ByteArrayRedisSerializer.java index 17e09c3495..d8f9a4a7e6 100644 --- a/src/main/java/org/springframework/data/redis/serializer/ByteArrayRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/ByteArrayRedisSerializer.java @@ -29,8 +29,8 @@ enum ByteArrayRedisSerializer implements RedisSerializer { @Nullable @Override - public byte[] serialize(@Nullable byte[] bytes) throws SerializationException { - return bytes; + public byte[] serialize(@Nullable byte[] value) throws SerializationException { + return value; } @Nullable diff --git a/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementReader.java b/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementReader.java index dc2cd06a57..bff07ac46a 100644 --- a/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementReader.java +++ b/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementReader.java @@ -31,7 +31,7 @@ class DefaultRedisElementReader implements RedisElementReader { private final @Nullable RedisSerializer serializer; - DefaultRedisElementReader(RedisSerializer serializer) { + DefaultRedisElementReader(@Nullable RedisSerializer serializer) { this.serializer = serializer; } diff --git a/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementWriter.java b/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementWriter.java index f59f338796..4d4cb3b707 100644 --- a/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementWriter.java +++ b/src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementWriter.java @@ -35,7 +35,7 @@ class DefaultRedisElementWriter implements RedisElementWriter { } @Override - public ByteBuffer write(T value) { + public ByteBuffer write(@Nullable T value) { if (serializer != null && (value == null || serializer.canSerialize(value.getClass()))) { return ByteBuffer.wrap(serializer.serialize(value)); @@ -51,6 +51,5 @@ public ByteBuffer write(T value) { throw new IllegalStateException( String.format("Cannot serialize value of type %s without a serializer", value.getClass())); - } } 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 c3922d5f7d..44625809b3 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java @@ -198,14 +198,14 @@ public static void registerNullValueSerializer(ObjectMapper objectMapper, @Nulla } @Override - public byte[] serialize(@Nullable Object source) throws SerializationException { + public byte[] serialize(@Nullable Object value) throws SerializationException { - if (source == null) { + if (value == null) { return SerializationUtils.EMPTY_ARRAY; } try { - return writer.write(mapper, source); + return writer.write(mapper, value); } catch (IOException e) { throw new SerializationException("Could not write JSON: " + e.getMessage(), e); } diff --git a/src/main/java/org/springframework/data/redis/serializer/GenericToStringSerializer.java b/src/main/java/org/springframework/data/redis/serializer/GenericToStringSerializer.java index df8ae97621..67e88fe564 100644 --- a/src/main/java/org/springframework/data/redis/serializer/GenericToStringSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/GenericToStringSerializer.java @@ -22,7 +22,6 @@ import org.springframework.beans.TypeConverter; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.lang.Nullable; @@ -42,8 +41,7 @@ public class GenericToStringSerializer implements RedisSerializer, BeanFac private final Class type; private final Charset charset; - - private Converter converter = new Converter(new DefaultConversionService()); + private Converter converter; public GenericToStringSerializer(Class type) { this(type, StandardCharsets.UTF_8); @@ -55,55 +53,64 @@ public GenericToStringSerializer(Class type, Charset charset) { this.type = type; this.charset = charset; + this.converter = new Converter(DefaultConversionService.getSharedInstance()); } + /** + * Set the {@link ConversionService} to be used. + * + * @param conversionService the conversion service to be used, must not be {@literal null}. + */ public void setConversionService(ConversionService conversionService) { - Assert.notNull(conversionService, "non null conversion service required"); + Assert.notNull(conversionService, "ConversionService must not be null"); + converter = new Converter(conversionService); } + /** + * Set the {@link TypeConverter} to be used. + * + * @param typeConverter the conversion service to be used, must not be {@literal null}. + */ public void setTypeConverter(TypeConverter typeConverter) { - Assert.notNull(typeConverter, "non null type converter required"); + Assert.notNull(typeConverter, "TypeConverter must not be null"); + converter = new Converter(typeConverter); } @Override - public T deserialize(@Nullable byte[] bytes) { + public byte[] serialize(@Nullable T value) { - if (bytes == null) { + if (value == null) { return null; } - String string = new String(bytes, charset); - return converter.convert(string, type); + String string = converter.convert(value, String.class); + return string.getBytes(charset); } @Override - public byte[] serialize(@Nullable T object) { - if (object == null) { + public T deserialize(@Nullable byte[] bytes) { + + if (bytes == null) { return null; } - String string = converter.convert(object, String.class); - return string.getBytes(charset); + + String string = new String(bytes, charset); + return converter.convert(string, type); } + @Override public void setBeanFactory(BeanFactory beanFactory) throws BeansException { - - // TODO: This code can never happen... - if (converter == null && beanFactory instanceof ConfigurableBeanFactory) { - ConfigurableBeanFactory cFB = (ConfigurableBeanFactory) beanFactory; - ConversionService conversionService = cFB.getConversionService(); - - converter = (conversionService != null ? new Converter(conversionService) - : new Converter(cFB.getTypeConverter())); - } + // no-op } - private class Converter { - private final ConversionService conversionService; - private final TypeConverter typeConverter; + private final static class Converter { + + private final @Nullable ConversionService conversionService; + private final @Nullable TypeConverter typeConverter; public Converter(ConversionService conversionService) { this.conversionService = conversionService; @@ -115,11 +122,11 @@ public Converter(TypeConverter typeConverter) { this.typeConverter = typeConverter; } + @Nullable E convert(Object value, Class targetType) { - if (conversionService != null) { - return conversionService.convert(value, targetType); - } - return typeConverter.convertIfNecessary(value, targetType); + + return conversionService != null ? conversionService.convert(value, targetType) + : typeConverter.convertIfNecessary(value, targetType); } } } 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 9541fe5177..85eac9da7f 100644 --- a/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializer.java @@ -126,32 +126,6 @@ public Jackson2JsonRedisSerializer(ObjectMapper mapper, JavaType javaType, Jacks this.javaType = javaType; } - @SuppressWarnings("unchecked") - public T deserialize(@Nullable byte[] bytes) throws SerializationException { - - if (SerializationUtils.isEmpty(bytes)) { - return null; - } - try { - return (T) this.reader.read(this.mapper, bytes, javaType); - } catch (Exception ex) { - throw new SerializationException("Could not read JSON: " + ex.getMessage(), ex); - } - } - - @Override - public byte[] serialize(@Nullable Object t) throws SerializationException { - - if (t == null) { - return SerializationUtils.EMPTY_ARRAY; - } - try { - return this.writer.write(this.mapper, t); - } catch (Exception ex) { - throw new SerializationException("Could not write JSON: " + ex.getMessage(), ex); - } - } - /** * Sets the {@code ObjectMapper} for this view. If not set, a default {@link ObjectMapper#ObjectMapper() ObjectMapper} * is used. @@ -171,6 +145,33 @@ public void setObjectMapper(ObjectMapper mapper) { this.mapper = mapper; } + @Override + public byte[] serialize(@Nullable T value) throws SerializationException { + + if (value == null) { + return SerializationUtils.EMPTY_ARRAY; + } + try { + return this.writer.write(this.mapper, value); + } catch (Exception ex) { + throw new SerializationException("Could not write JSON: " + ex.getMessage(), ex); + } + } + + @Override + @SuppressWarnings("unchecked") + public T deserialize(@Nullable byte[] bytes) throws SerializationException { + + if (SerializationUtils.isEmpty(bytes)) { + return null; + } + try { + return (T) this.reader.read(this.mapper, bytes, javaType); + } catch (Exception ex) { + throw new SerializationException("Could not read JSON: " + ex.getMessage(), ex); + } + } + /** * Returns the Jackson {@link JavaType} for the specific class. *

diff --git a/src/main/java/org/springframework/data/redis/serializer/JdkSerializationRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/JdkSerializationRedisSerializer.java index 5beada15b4..24f9ac89c1 100644 --- a/src/main/java/org/springframework/data/redis/serializer/JdkSerializationRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/JdkSerializationRedisSerializer.java @@ -72,28 +72,31 @@ public JdkSerializationRedisSerializer(Converter serializer, Con this.deserializer = deserializer; } - public Object deserialize(@Nullable byte[] bytes) { + @Override + public byte[] serialize(@Nullable Object value) { - if (SerializationUtils.isEmpty(bytes)) { - return null; + if (value == null) { + return SerializationUtils.EMPTY_ARRAY; } try { - return deserializer.convert(bytes); - } catch (Exception ex) { - throw new SerializationException("Cannot deserialize", ex); + return serializer.convert(value); + } catch (Exception cause) { + throw new SerializationException("Cannot serialize", cause); } } @Override - public byte[] serialize(@Nullable Object object) { - if (object == null) { - return SerializationUtils.EMPTY_ARRAY; + public Object deserialize(@Nullable byte[] bytes) { + + if (SerializationUtils.isEmpty(bytes)) { + return null; } + try { - return serializer.convert(object); + return deserializer.convert(bytes); } catch (Exception ex) { - throw new SerializationException("Cannot serialize", ex); + throw new SerializationException("Cannot deserialize", ex); } } } diff --git a/src/main/java/org/springframework/data/redis/serializer/OxmSerializer.java b/src/main/java/org/springframework/data/redis/serializer/OxmSerializer.java index b195e98b4f..8d9fbb5d52 100644 --- a/src/main/java/org/springframework/data/redis/serializer/OxmSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/OxmSerializer.java @@ -85,34 +85,34 @@ public void afterPropertiesSet() { } @Override - public Object deserialize(@Nullable byte[] bytes) throws SerializationException { + public byte[] serialize(@Nullable Object value) throws SerializationException { - if (SerializationUtils.isEmpty(bytes)) { - return null; + if (value == null) { + return SerializationUtils.EMPTY_ARRAY; } + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + StreamResult result = new StreamResult(stream); + try { - return unmarshaller.unmarshal(new StreamSource(new ByteArrayInputStream(bytes))); + marshaller.marshal(value, result); } catch (Exception ex) { - throw new SerializationException("Cannot deserialize bytes", ex); + throw new SerializationException("Cannot serialize object", ex); } + return stream.toByteArray(); } @Override - public byte[] serialize(@Nullable Object t) throws SerializationException { + public Object deserialize(@Nullable byte[] bytes) throws SerializationException { - if (t == null) { - return SerializationUtils.EMPTY_ARRAY; + if (SerializationUtils.isEmpty(bytes)) { + return null; } - ByteArrayOutputStream stream = new ByteArrayOutputStream(); - StreamResult result = new StreamResult(stream); - try { - marshaller.marshal(t, result); + return unmarshaller.unmarshal(new StreamSource(new ByteArrayInputStream(bytes))); } catch (Exception ex) { - throw new SerializationException("Cannot serialize object", ex); + throw new SerializationException("Cannot deserialize bytes", ex); } - return stream.toByteArray(); } } diff --git a/src/main/java/org/springframework/data/redis/serializer/RedisElementWriter.java b/src/main/java/org/springframework/data/redis/serializer/RedisElementWriter.java index 49c1d61bef..31b09b0001 100644 --- a/src/main/java/org/springframework/data/redis/serializer/RedisElementWriter.java +++ b/src/main/java/org/springframework/data/redis/serializer/RedisElementWriter.java @@ -17,6 +17,7 @@ import java.nio.ByteBuffer; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** @@ -36,7 +37,7 @@ public interface RedisElementWriter { * @param element can be {@literal null}. * @return the {@link ByteBuffer} representing {@code element} in its binary form. */ - ByteBuffer write(T element); + ByteBuffer write(@Nullable T element); /** * Create new {@link RedisElementWriter} using given {@link RedisSerializer}. diff --git a/src/main/java/org/springframework/data/redis/serializer/RedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/RedisSerializer.java index 3f28d20125..6da9eafa19 100644 --- a/src/main/java/org/springframework/data/redis/serializer/RedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/RedisSerializer.java @@ -20,8 +20,9 @@ /** * Basic interface serialization and deserialization of Objects to byte arrays (binary data). It is recommended that - * implementations are designed to handle null objects/empty arrays on serialization and deserialization side. Note that - * Redis does not accept null keys or values but can return null replies (for non existing keys). + * implementations are designed to handle {@literal null} objects/empty arrays on serialization and deserialization + * side. Note that Redis does not accept {@literal null} keys or values but can return null replies (for non-existing + * keys). * * @author Mark Pollack * @author Costin Leau @@ -30,26 +31,8 @@ public interface RedisSerializer { /** - * Serialize the given object to binary data. - * - * @param t object to serialize. Can be {@literal null}. - * @return the equivalent binary data. Can be {@literal null}. - */ - @Nullable - byte[] serialize(@Nullable T t) throws SerializationException; - - /** - * Deserialize an object from the given binary data. - * - * @param bytes object binary representation. Can be {@literal null}. - * @return the equivalent object instance. Can be {@literal null}. - */ - @Nullable - T deserialize(@Nullable byte[] bytes) throws SerializationException; - - /** - * Obtain a {@link RedisSerializer} using java serialization.
- * Note: Ensure that your domain objects are actually {@link java.io.Serializable serializable}. + * Obtain a {@link RedisSerializer} using java serialization. Note: Ensure that your domain objects + * are actually {@link java.io.Serializable serializable}. * * @return never {@literal null}. * @since 2.1 @@ -59,7 +42,7 @@ static RedisSerializer java() { } /** - * Obtain a {@link RedisSerializer} using java serialization with the given {@link ClassLoader}.
+ * Obtain a {@link RedisSerializer} using java serialization with the given {@link ClassLoader}. * Note: Ensure that your domain objects are actually {@link java.io.Serializable serializable}. * * @param classLoader the {@link ClassLoader} to use for deserialization. Can be {@literal null}. @@ -102,10 +85,39 @@ static RedisSerializer byteArray() { return ByteArrayRedisSerializer.INSTANCE; } + /** + * Serialize the given object to binary data. + * + * @param value object to serialize. Can be {@literal null}. + * @return the equivalent binary data. Can be {@literal null}. + */ + @Nullable + byte[] serialize(@Nullable T value) throws SerializationException; + + /** + * Deserialize an object from the given binary data. + * + * @param bytes object binary representation. Can be {@literal null}. + * @return the equivalent object instance. Can be {@literal null}. + */ + @Nullable + T deserialize(@Nullable byte[] bytes) throws SerializationException; + + /** + * Check whether the given value {@code type} can be serialized by this serializer. + * + * @param type the value type. + * @return {@code true} if the value type can be serialized; {@code false} otherwise. + */ default boolean canSerialize(Class type) { return ClassUtils.isAssignable(getTargetType(), type); } + /** + * Return the serializer target type. + * + * @return the serializer target type. + */ default Class getTargetType() { return Object.class; } diff --git a/src/main/java/org/springframework/data/redis/serializer/StringRedisSerializer.java b/src/main/java/org/springframework/data/redis/serializer/StringRedisSerializer.java index 72d7c8ac67..c0b1f088d4 100644 --- a/src/main/java/org/springframework/data/redis/serializer/StringRedisSerializer.java +++ b/src/main/java/org/springframework/data/redis/serializer/StringRedisSerializer.java @@ -81,13 +81,13 @@ public StringRedisSerializer(Charset charset) { } @Override - public String deserialize(@Nullable byte[] bytes) { - return (bytes == null ? null : new String(bytes, charset)); + public byte[] serialize(@Nullable String value) { + return (value == null ? null : value.getBytes(charset)); } @Override - public byte[] serialize(@Nullable String string) { - return (string == null ? null : string.getBytes(charset)); + public String deserialize(@Nullable byte[] bytes) { + return (bytes == null ? null : new String(bytes, charset)); } @Override diff --git a/src/test/java/org/springframework/data/redis/cache/CacheTestParams.java b/src/test/java/org/springframework/data/redis/cache/CacheTestParams.java index 8bbfaa7b31..cafdd6c5dd 100644 --- a/src/test/java/org/springframework/data/redis/cache/CacheTestParams.java +++ b/src/test/java/org/springframework/data/redis/cache/CacheTestParams.java @@ -108,8 +108,8 @@ static class FixDamnedJunitParameterizedNameForRedisSerializer/* ¯\_(ツ)_/¯ * @Override @Nullable - public byte[] serialize(@Nullable Object o) throws SerializationException { - return serializer.serialize(o); + public byte[] serialize(@Nullable Object value) throws SerializationException { + return serializer.serialize(value); } @Override diff --git a/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerUnitTests.java b/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerUnitTests.java index 482064d248..ae27fae60d 100644 --- a/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerUnitTests.java @@ -23,7 +23,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - import org.springframework.data.redis.connection.DefaultMessage; import org.springframework.data.redis.connection.Message; import org.springframework.data.redis.connection.MessageListener; @@ -311,7 +310,7 @@ static class Pojo {} static class PojoRedisSerializer implements RedisSerializer { @Override - public byte[] serialize(Pojo t) throws SerializationException { + public byte[] serialize(Pojo value) throws SerializationException { return new byte[0]; }