Refine Jackson2JsonRedisSerializer design.

Deprecate ObjectMapper setter. Introduce constructor accepting the ObjectMapper.

Original Pull Request: #2332

Author: mp911de

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

@@ -32,7 +32,6 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping;
-import com.fasterxml.jackson.databind.ObjectReader;
 import com.fasterxml.jackson.databind.SerializerProvider;
 import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
 import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
@@ -55,37 +54,16 @@
  */
 public class GenericJackson2JsonRedisSerializer implements RedisSerializer<Object> {
 
-	private ObjectMapper mapper;
+	private final ObjectMapper mapper;
 
 	private final JacksonObjectReader reader;
 
 	private final JacksonObjectWriter writer;
 
-	private boolean internalReader = false;
+	private final Lazy<Boolean> defaultTypingEnabled;
 
 	private final TypeResolver typeResolver;
 
-	private Lazy<Boolean> defaultTypingEnabled = Lazy
-			.of(() -> mapper.getSerializationConfig().getDefaultTyper(null) != null);
-
-	private Lazy<String> typeHintPropertyName;
-
-	{
-		typeHintPropertyName = Lazy.of(() -> {
-			if (defaultTypingEnabled.get()) {
-				return null;
-			}
-
-			return mapper.getDeserializationConfig().getDefaultTyper(null)
-					.buildTypeDeserializer(mapper.getDeserializationConfig(), mapper.getTypeFactory().constructType(Object.class),
-							Collections.emptyList())
-					.getPropertyName();
-
-		}).or("@class");
-
-		typeResolver = new TypeResolver(Lazy.of(() -> mapper.getTypeFactory()), typeHintPropertyName);
-	}
-
 	/**
 	 * Creates {@link GenericJackson2JsonRedisSerializer} and configures {@link ObjectMapper} for default typing.
 	 */
@@ -104,7 +82,6 @@ public GenericJackson2JsonRedisSerializer() {
 	 */
 	public GenericJackson2JsonRedisSerializer(@Nullable String classPropertyTypeName) {
 		this(classPropertyTypeName, JacksonObjectReader.create(), JacksonObjectWriter.create());
-		this.internalReader = true;
 	}
 
 	/**
@@ -122,7 +99,7 @@ public GenericJackson2JsonRedisSerializer(@Nullable String classPropertyTypeName
 	public GenericJackson2JsonRedisSerializer(@Nullable String classPropertyTypeName, JacksonObjectReader reader,
 			JacksonObjectWriter writer) {
 
-		this(new ObjectMapper(), reader, writer);
+		this(new ObjectMapper(), reader, writer, 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.
@@ -134,10 +111,6 @@ public GenericJackson2JsonRedisSerializer(@Nullable String classPropertyTypeName
 		} else {
 			mapper.activateDefaultTyping(mapper.getPolymorphicTypeValidator(), DefaultTyping.EVERYTHING, As.PROPERTY);
 		}
-
-		if (classPropertyTypeName != null) {
-			typeHintPropertyName = Lazy.of(classPropertyTypeName);
-		}
 	}
 
 	/**
@@ -149,7 +122,6 @@ public GenericJackson2JsonRedisSerializer(@Nullable String classPropertyTypeName
 	 */
 	public GenericJackson2JsonRedisSerializer(ObjectMapper mapper) {
 		this(mapper, JacksonObjectReader.create(), JacksonObjectWriter.create());
-		this.internalReader = true;
 	}
 
 	/**
@@ -164,6 +136,11 @@ public GenericJackson2JsonRedisSerializer(ObjectMapper mapper) {
 	 */
 	public GenericJackson2JsonRedisSerializer(ObjectMapper mapper, JacksonObjectReader reader,
 			JacksonObjectWriter writer) {
+		this(mapper, reader, writer, null);
+	}
+
+	private GenericJackson2JsonRedisSerializer(ObjectMapper mapper, JacksonObjectReader reader,
+			JacksonObjectWriter writer, @Nullable String typeHintPropertyName) {
 
 		Assert.notNull(mapper, "ObjectMapper must not be null");
 		Assert.notNull(reader, "Reader must not be null");
@@ -172,6 +149,29 @@ public GenericJackson2JsonRedisSerializer(ObjectMapper mapper, JacksonObjectRead
 		this.mapper = mapper;
 		this.reader = reader;
 		this.writer = writer;
+
+		this.defaultTypingEnabled = Lazy.of(() -> mapper.getSerializationConfig().getDefaultTyper(null) != null);
+
+		Supplier<String> typeHintPropertyNameSupplier;
+
+		if (typeHintPropertyName == null) {
+
+			typeHintPropertyNameSupplier = Lazy.of(() -> {
+				if (defaultTypingEnabled.get()) {
+					return null;
+				}
+
+				return mapper.getDeserializationConfig().getDefaultTyper(null)
+						.buildTypeDeserializer(mapper.getDeserializationConfig(),
+								mapper.getTypeFactory().constructType(Object.class), Collections.emptyList())
+						.getPropertyName();
+
+			}).or("@class");
+		} else {
+			typeHintPropertyNameSupplier = () -> typeHintPropertyName;
+		}
+
+		this.typeResolver = new TypeResolver(Lazy.of(mapper::getTypeFactory), typeHintPropertyNameSupplier);
 	}
 
 	/**
@@ -233,21 +233,22 @@ public <T> T deserialize(@Nullable byte[] source, Class<T> type) throws Serializ
 		}
 	}
 
-	protected JavaType resolveType(byte[] source, Class<?> type) {
+	protected JavaType resolveType(byte[] source, Class<?> type) throws IOException {
 
-		if (internalReader || !type.equals(Object.class) || !defaultTypingEnabled.get()) {
+		if (!type.equals(Object.class) || !defaultTypingEnabled.get()) {
 			return typeResolver.constructType(type);
 		}
 
 		return typeResolver.resolveType(source, type);
 	}
 
-	private static class TypeResolver {
+	static class TypeResolver {
 
-		private final ObjectReader objectReader = new ObjectMapper().reader();
+		// need a separate instance to bypass class hint checks
+		private final ObjectMapper mapper = new ObjectMapper();
 
 		private final Supplier<TypeFactory> typeFactory;
-		private Supplier<String> hintName;
+		private final Supplier<String> hintName;
 
 		public TypeResolver(Supplier<TypeFactory> typeFactory, Supplier<String> hintName) {
 
@@ -259,15 +260,13 @@ protected JavaType constructType(Class<?> type) {
 			return typeFactory.get().constructType(type);
 		}
 
-		protected JavaType resolveType(byte[] source, Class<?> type) {
+		protected JavaType resolveType(byte[] source, Class<?> type) throws IOException {
 
-			try {
-				TextNode typeName = (TextNode) objectReader.readValue(source, JsonNode.class).get(hintName.get());
-				if (typeName != null) {
-					return typeFactory.get().constructFromCanonical(typeName.textValue());
-				}
-			} catch (IOException e) {
-				// TODO: logging?
+			JsonNode root = mapper.readTree(source);
+			JsonNode jsonNode = root.get(hintName.get());
+
+			if (jsonNode instanceof TextNode && jsonNode.asText() != null) {
+				return typeFactory.get().constructFromCanonical(jsonNode.asText());
 			}
 
 			return constructType(type);

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

@@ -43,31 +43,86 @@
  */
 public class Jackson2JsonRedisSerializer<T> implements RedisSerializer<T> {
 
+	/**
+	 * @deprecated since 3.0 for removal.
+	 */
+	@Deprecated(since = "3.0", forRemoval = true) //
 	public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
 
 	private final JavaType javaType;
 
-	private ObjectMapper objectMapper = new ObjectMapper();
+	private ObjectMapper mapper;
 
-	private JacksonObjectReader reader = JacksonObjectReader.create();
+	private final JacksonObjectReader reader;
 
-	private JacksonObjectWriter writer = JacksonObjectWriter.create();
+	private final JacksonObjectWriter writer;
 
 	/**
 	 * Creates a new {@link Jackson2JsonRedisSerializer} for the given target {@link Class}.
 	 *
-	 * @param type
+	 * @param type must not be {@literal null}.
 	 */
 	public Jackson2JsonRedisSerializer(Class<T> type) {
-		this.javaType = getJavaType(type);
+		this(new ObjectMapper(), type);
 	}
 
 	/**
 	 * Creates a new {@link Jackson2JsonRedisSerializer} for the given target {@link JavaType}.
 	 *
-	 * @param javaType
+	 * @param javaType must not be {@literal null}.
 	 */
 	public Jackson2JsonRedisSerializer(JavaType javaType) {
+		this(new ObjectMapper(), javaType);
+	}
+
+	/**
+	 * Creates a new {@link Jackson2JsonRedisSerializer} for the given target {@link Class}.
+	 *
+	 * @param mapper must not be {@literal null}.
+	 * @param type must not be {@literal null}.
+	 * @since 3.0
+	 */
+	public Jackson2JsonRedisSerializer(ObjectMapper mapper, Class<T> type) {
+
+		Assert.notNull(mapper, "ObjectMapper must not be null");
+		Assert.notNull(type, "Java type must not be null");
+
+		this.javaType = getJavaType(type);
+		this.mapper = mapper;
+		this.reader = JacksonObjectReader.create();
+		this.writer = JacksonObjectWriter.create();
+	}
+
+	/**
+	 * Creates a new {@link Jackson2JsonRedisSerializer} for the given target {@link JavaType}.
+	 *
+	 * @param mapper must not be {@literal null}.
+	 * @param javaType must not be {@literal null}.
+	 * @since 3.0
+	 */
+	public Jackson2JsonRedisSerializer(ObjectMapper mapper, JavaType javaType) {
+		this(mapper, javaType, JacksonObjectReader.create(), JacksonObjectWriter.create());
+	}
+
+	/**
+	 * Creates a new {@link Jackson2JsonRedisSerializer} for the given target {@link JavaType}.
+	 *
+	 * @param mapper must not be {@literal null}.
+	 * @param javaType must not be {@literal null}.
+	 * @param reader the {@link JacksonObjectReader} function to read objects using {@link ObjectMapper}.
+	 * @param writer the {@link JacksonObjectWriter} function to write objects using {@link ObjectMapper}.
+	 * @since 3.0
+	 */
+	public Jackson2JsonRedisSerializer(ObjectMapper mapper, JavaType javaType, JacksonObjectReader reader,
+			JacksonObjectWriter writer) {
+
+		Assert.notNull(mapper, "ObjectMapper must not be null!");
+		Assert.notNull(reader, "Reader must not be null!");
+		Assert.notNull(writer, "Writer must not be null!");
+
+		this.mapper = mapper;
+		this.reader = reader;
+		this.writer = writer;
 		this.javaType = javaType;
 	}
 
@@ -78,7 +133,7 @@ public T deserialize(@Nullable byte[] bytes) throws SerializationException {
 			return null;
 		}
 		try {
-			return (T) this.reader.read(this.objectMapper, bytes, javaType);
+			return (T) this.reader.read(this.mapper, bytes, javaType);
 		} catch (Exception ex) {
 			throw new SerializationException("Could not read JSON: " + ex.getMessage(), ex);
 		}
@@ -91,7 +146,7 @@ public byte[] serialize(@Nullable Object t) throws SerializationException {
 			return SerializationUtils.EMPTY_ARRAY;
 		}
 		try {
-			return this.writer.write(this.objectMapper, t);
+			return this.writer.write(this.mapper, t);
 		} catch (Exception ex) {
 			throw new SerializationException("Could not write JSON: " + ex.getMessage(), ex);
 		}
@@ -105,31 +160,15 @@ public byte[] serialize(@Nullable Object t) throws SerializationException {
 	 * process. For example, an extended {@link SerializerFactory} can be configured that provides custom serializers for
 	 * specific types. The other option for refining the serialization process is to use Jackson's provided annotations on
 	 * the types to be serialized, in which case a custom-configured ObjectMapper is unnecessary.
-	 */
-	public void setObjectMapper(ObjectMapper objectMapper) {
-
-		Assert.notNull(objectMapper, "'objectMapper' must not be null");
-		this.objectMapper = objectMapper;
-	}
-
-	/**
-	 * Sets the {@link JacksonObjectReader} for this serializer. Setting the reader allows customization of the JSON
-	 * deserialization.
 	 *
-	 * @since 3.0
+	 * @deprecated since 3.0, use {@link #Jackson2JsonRedisSerializer(ObjectMapper, Class) constructor creation} to
+	 *             configure the object mapper.
 	 */
-	public void setReader(JacksonObjectReader reader) {
-		this.reader = reader;
-	}
+	@Deprecated(since = "3.0", forRemoval = true)
+	public void setObjectMapper(ObjectMapper mapper) {
 
-	/**
-	 * Sets the {@link JacksonObjectWriter} for this serializer. Setting the reader allows customization of the JSON
-	 * serialization.
-	 *
-	 * @since 3.0
-	 */
-	public void setWriter(JacksonObjectWriter writer) {
-		this.writer = writer;
+		Assert.notNull(mapper, "'objectMapper' must not be null");
+		this.mapper = mapper;
 	}
 
 	/**

src/test/java/org/springframework/data/redis/serializer/Jackson2JsonRedisSerializerTests.java

@@ -24,6 +24,9 @@
 import org.springframework.data.redis.Person;
 import org.springframework.data.redis.PersonObjectFactory;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.type.TypeFactory;
+
 /**
  * Unit tests for {@link Jackson2JsonRedisSerializer}.
  *
@@ -70,8 +73,10 @@ void testJackson2JsonSerilizerThrowsExceptionWhenSettingNullObjectMapper() {
 	@Test // GH-2322
 	void shouldConsiderWriter() {
 
+		serializer = new Jackson2JsonRedisSerializer<>(new ObjectMapper(),
+				TypeFactory.defaultInstance().constructType(Person.class), JacksonObjectReader.create(),
+				(mapper, source) -> "foo".getBytes());
 		Person person = new PersonObjectFactory().instance();
-		serializer.setWriter((mapper, source) -> "foo".getBytes());
 		assertThat(serializer.serialize(person)).isEqualTo("foo".getBytes());
 	}