Refine RedisSerializer implementations.

This commit polishes up method ordering, introduces Javadoc where missing and updates nullability annotations and argument names.

Closes #1097

Author: mp911de

src/main/java/org/springframework/data/redis/serializer/ByteArrayRedisSerializer.java

@@ -29,8 +29,8 @@ enum ByteArrayRedisSerializer implements RedisSerializer<byte[]> {
 
 	@Nullable
 	@Override
-	public byte[] serialize(@Nullable byte[] bytes) throws SerializationException {
-		return bytes;
+	public byte[] serialize(@Nullable byte[] value) throws SerializationException {
+		return value;
 	}
 
 	@Nullable

src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementReader.java

@@ -31,7 +31,7 @@ class DefaultRedisElementReader<T> implements RedisElementReader<T> {
 
 	private final @Nullable RedisSerializer<T> serializer;
 
-	DefaultRedisElementReader(RedisSerializer<T> serializer) {
+	DefaultRedisElementReader(@Nullable RedisSerializer<T> serializer) {
 		this.serializer = serializer;
 	}
 

src/main/java/org/springframework/data/redis/serializer/DefaultRedisElementWriter.java

@@ -35,7 +35,7 @@ class DefaultRedisElementWriter<T> implements RedisElementWriter<T> {
 	}
 
 	@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()));
-
 	}
 }

src/main/java/org/springframework/data/redis/serializer/GenericJackson2JsonRedisSerializer.java

@@ -66,21 +66,6 @@
  */
 public class GenericJackson2JsonRedisSerializer implements RedisSerializer<Object> {
 
-	/**
-	 * Register {@link NullValueSerializer} in the given {@link ObjectMapper} with an optional
-	 * {@code classPropertyTypeName}. This method should be called by code that customizes
-	 * {@link GenericJackson2JsonRedisSerializer} by providing an external {@link ObjectMapper}.
-	 *
-	 * @param objectMapper the object mapper to customize.
-	 * @param classPropertyTypeName name of the type property. Defaults to {@code @class} if {@literal null}/empty.
-	 * @since 2.2
-	 */
-	public static void registerNullValueSerializer(ObjectMapper objectMapper, @Nullable String classPropertyTypeName) {
-
-		// Simply setting {@code mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)} does not help here
-		// since we need the type hint embedded for deserialization using the default typing feature.
-		objectMapper.registerModule(new SimpleModule().addSerializer(new NullValueSerializer(classPropertyTypeName)));
-	}
 
 	private final JacksonObjectReader reader;
 
@@ -203,6 +188,22 @@ private Supplier<String> newTypeHintPropertyNameSupplier(ObjectMapper mapper, @N
 					.or("@class");
 	}
 
+	/**
+	 * Register {@link NullValueSerializer} in the given {@link ObjectMapper} with an optional
+	 * {@code classPropertyTypeName}. This method should be called by code that customizes
+	 * {@link GenericJackson2JsonRedisSerializer} by providing an external {@link ObjectMapper}.
+	 *
+	 * @param objectMapper the object mapper to customize.
+	 * @param classPropertyTypeName name of the type property. Defaults to {@code @class} if {@literal null}/empty.
+	 * @since 2.2
+	 */
+	public static void registerNullValueSerializer(ObjectMapper objectMapper, @Nullable String classPropertyTypeName) {
+
+		// Simply setting {@code mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)} does not help here
+		// since we need the type hint embedded for deserialization using the default typing feature.
+		objectMapper.registerModule(new SimpleModule().addSerializer(new NullValueSerializer(classPropertyTypeName)));
+	}
+
 	/**
 	 * Gets the configured {@link ObjectMapper} used internally by this {@link GenericJackson2JsonRedisSerializer}
 	 * to de/serialize {@link Object objects} as {@literal JSON}.
@@ -214,14 +215,14 @@ protected ObjectMapper getObjectMapper() {
 	}
 
 	@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 cause) {
 			String message = String.format("Could not write JSON: %s", cause.getMessage());
 			throw new SerializationException(message, cause);

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<T> implements RedisSerializer<T>, BeanFac
 
 	private final Class<T> type;
 	private final Charset charset;
-
-	private Converter converter = new Converter(new DefaultConversionService());
+	private Converter converter;
 
 	public GenericToStringSerializer(Class<T> type) {
 		this(type, StandardCharsets.UTF_8);
@@ -55,55 +53,64 @@ public GenericToStringSerializer(Class<T> 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> E convert(Object value, Class<E> 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);
 		}
 	}
 }

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.
 	 * <p>

src/main/java/org/springframework/data/redis/serializer/JdkSerializationRedisSerializer.java

@@ -82,30 +82,30 @@ public JdkSerializationRedisSerializer(Converter<Object, byte[]> serializer,
 	}
 
 	@Override
-	public Object deserialize(@Nullable byte[] bytes) {
+	public byte[] serialize(@Nullable Object value) {
 
-		if (SerializationUtils.isEmpty(bytes)) {
-			return null;
+		if (value == null) {
+			return SerializationUtils.EMPTY_ARRAY;
 		}
 
 		try {
-			return deserializer.convert(bytes);
+			return serializer.convert(value);
 		} catch (Exception cause) {
-			throw new SerializationException("Cannot deserialize", cause);
+			throw new SerializationException("Cannot serialize", cause);
 		}
 	}
 
 	@Override
-	public byte[] serialize(@Nullable Object object) {
+	public Object deserialize(@Nullable byte[] bytes) {
 
-		if (object == null) {
-			return SerializationUtils.EMPTY_ARRAY;
+		if (SerializationUtils.isEmpty(bytes)) {
+			return null;
 		}
 
 		try {
-			return serializer.convert(object);
+			return deserializer.convert(bytes);
 		} catch (Exception cause) {
-			throw new SerializationException("Cannot serialize", cause);
+			throw new SerializationException("Cannot deserialize", cause);
 		}
 	}
 }

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();
 	}
 }

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<T> {
 	 * @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}.