From 4d7becad79422be6a541111d68d6e43284d33364 Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 8 May 2020 01:29:26 -0700 Subject: [PATCH] Simplify ObjectMapper configuration. Remove activation of default typing configuration. Change target Class type for ObjectTypeMetadataMixin (Mixin) to the (converted) source object's Class type. Remove unnecessary SimpleModule registration including the addition of the custom BigDecimalSerializer, BigIntegerSerializer and TypelessCollectionSerializer objects. Resolves gh-67. --- .../support/JacksonObjectToJsonConverter.java | 49 ++++--------- ...JacksonObjectToJsonConverterUnitTests.java | 68 +++++++++++++++++-- 2 files changed, 76 insertions(+), 41 deletions(-) diff --git a/spring-geode/src/main/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverter.java b/spring-geode/src/main/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverter.java index 24d8935f..87c03a71 100644 --- a/spring-geode/src/main/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverter.java +++ b/spring-geode/src/main/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverter.java @@ -18,56 +18,46 @@ package org.springframework.geode.data.json.converter.support; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.Version; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.TypeDescriptor; import org.springframework.geode.data.json.converter.ObjectToJsonConverter; -import org.springframework.geode.jackson.databind.serializer.BigDecimalSerializer; -import org.springframework.geode.jackson.databind.serializer.BigIntegerSerializer; -import org.springframework.geode.jackson.databind.serializer.TypelessCollectionSerializer; import org.springframework.lang.NonNull; -import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * A {@link ObjectToJsonConverter} implementation using Jackson's {@link ObjectMapper} to convert * from an {@link Object} to a {@literal JSON} {@link String}. * * @author John Blum + * @see com.fasterxml.jackson.annotation.JsonTypeInfo + * @see com.fasterxml.jackson.core.JsonGenerator * @see com.fasterxml.jackson.databind.ObjectMapper + * @see com.fasterxml.jackson.databind.MapperFeature * @see org.springframework.geode.data.json.converter.ObjectToJsonConverter * @since 1.3.0 */ public class JacksonObjectToJsonConverter implements ObjectToJsonConverter { - protected static final ObjectMapper.DefaultTyping DEFAULT_TYPING = ObjectMapper.DefaultTyping.NON_FINAL; - protected static final String AT_TYPE_METADATA_PROPERTY_NAME = "@type"; - private static final String SPRING_BOOT_DATA_GEODE_JACKSON_MODULE_NAME = "spring.boot.data.geode.module"; - - // Since SBDG Version - // Only change version when the Module definition changes since SimpleModule implements java.io.Serializable; - // Although, as of Jackson 2.5.0, SimpleModule defines a fixed serialVersionUID, so... - private static final Version VERSION = new Version(1, 3, 0, null, - "org.springframework.geode", "spring-geode-starter"); - /** * Converts the given {@link Object} into {@link String JSON}. * * @param source {@link Object} to convert into {@link String JSON}. * @return {@link String JSON} generated from the given {@link Object} using Jackson's {@link ObjectMapper}. * @see com.fasterxml.jackson.databind.ObjectMapper - * @see #newObjectMapper() + * @see #newObjectMapper(Object) */ - @Nullable @Override - public String convert(Object source) { + @Override + public @NonNull String convert(@NonNull Object source) { + + Assert.notNull(source, "Source object to convert must not be null"); try { - return newObjectMapper().writeValueAsString(source); + return newObjectMapper(source).writeValueAsString(source); } catch (JsonProcessingException cause) { throw new ConversionFailedException(TypeDescriptor.forObject(source), TypeDescriptor.valueOf(String.class), @@ -81,25 +71,16 @@ public class JacksonObjectToJsonConverter implements ObjectToJsonConverter { * @return a new instance of the Jackson {@link ObjectMapper} class. * @see com.fasterxml.jackson.databind.ObjectMapper */ - protected @NonNull ObjectMapper newObjectMapper() { + protected @NonNull ObjectMapper newObjectMapper(@NonNull Object target) { - ObjectMapper objectMapper = new ObjectMapper() - .activateDefaultTypingAsProperty(null, DEFAULT_TYPING, AT_TYPE_METADATA_PROPERTY_NAME) - .addMixIn(NonAccessibleType.class, ObjectTypeMetadataMixin.class) + Assert.notNull(target, "Target object must not be null"); + + return new ObjectMapper() + .addMixIn(target.getClass(), ObjectTypeMetadataMixin.class) .configure(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN, true) .configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true); - - objectMapper.registerModule(new SimpleModule(SPRING_BOOT_DATA_GEODE_JACKSON_MODULE_NAME, VERSION) - .addSerializer(BigDecimalSerializer.INSTANCE) - .addSerializer(BigIntegerSerializer.INSTANCE) - .addSerializer(new TypelessCollectionSerializer(objectMapper))); - - return objectMapper; } - @SuppressWarnings("unused") - private interface NonAccessibleType { } - @JsonTypeInfo( use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, diff --git a/spring-geode/src/test/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverterUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverterUnitTests.java index 02d32c6d..6abf87ee 100644 --- a/spring-geode/src/test/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverterUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/data/json/converter/support/JacksonObjectToJsonConverterUnitTests.java @@ -18,10 +18,10 @@ package org.springframework.geode.data.json.converter.support; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -29,12 +29,15 @@ import static org.mockito.Mockito.verify; import com.fasterxml.jackson.core.JsonGenerationException; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Test; import org.springframework.core.convert.ConversionFailedException; +import example.app.crm.model.Customer; + /** * Unit Tests for {@link JacksonObjectToJsonConverter}. * @@ -58,27 +61,49 @@ public class JacksonObjectToJsonConverterUnitTests { JacksonObjectToJsonConverter converter = spy(new JacksonObjectToJsonConverter()); doReturn(json).when(mockObjectMapper).writeValueAsString(eq(source)); - doReturn(mockObjectMapper).when(converter).newObjectMapper(); + doReturn(mockObjectMapper).when(converter).newObjectMapper(any()); assertThat(converter.convert(source)).isEqualTo(json); - verify(converter, times(1)).newObjectMapper(); + verify(converter, times(1)).newObjectMapper(eq(source)); verify(mockObjectMapper, times(1)).writeValueAsString(eq(source)); } + @Test(expected = IllegalArgumentException.class) + public void convertNullThrowsIllegalArgumentException() { + + JacksonObjectToJsonConverter converter = spy(new JacksonObjectToJsonConverter()); + + try { + converter.convert(null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("Source object to convert must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } + finally { + verify(converter, never()).newObjectMapper(any()); + } + } + @Test(expected = ConversionFailedException.class) public void convertHandlesJsonProcessingException() throws JsonProcessingException { + Object source = new Object(); + ObjectMapper mockObjectMapper = mock(ObjectMapper.class); JacksonObjectToJsonConverter converter = spy(new JacksonObjectToJsonConverter()); - doReturn(mockObjectMapper).when(converter).newObjectMapper(); + doReturn(mockObjectMapper).when(converter).newObjectMapper(any()); doThrow(new JsonGenerationException("TEST", (JsonGenerator) null)) .when(mockObjectMapper).writeValueAsString(any()); try { - converter.convert(null); + converter.convert(source); } catch (ConversionFailedException expected) { @@ -89,8 +114,37 @@ public class JacksonObjectToJsonConverterUnitTests { throw expected; } finally { - verify(converter, times(1)).newObjectMapper(); - verify(mockObjectMapper, times(1)).writeValueAsString(isNull()); + verify(converter, times(1)).newObjectMapper(eq(source)); + verify(mockObjectMapper, times(1)).writeValueAsString(eq(source)); } } + + @Test(expected = IllegalArgumentException.class) + public void newObjectMapperWithNullTarget() { + + try { + new JacksonObjectToJsonConverter().newObjectMapper(null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("Target object must not be null"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + + @Test + public void newObjectMapperIsConfiguredCorrectly() { + + Object target = Customer.newCustomer(1L, "Jon Doe"); + + ObjectMapper objectMapper = new JacksonObjectToJsonConverter().newObjectMapper(target); + + assertThat(objectMapper).isNotNull(); + assertThat(objectMapper.isEnabled(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)).isTrue(); + assertThat(objectMapper.isEnabled(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)).isTrue(); + assertThat(objectMapper.getSerializationConfig().findMixInClassFor(target.getClass())) + .isEqualTo(JacksonObjectToJsonConverter.ObjectTypeMetadataMixin.class); + } }