From 906a5fdb3b3f4c8496dd2c695c065e4844c0dc46 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Sat, 14 Apr 2018 15:37:40 +0900 Subject: [PATCH 1/3] Fix NPE at ObjectMapper#copy with MessagePackFactory --- .../dataformat/MessagePackFactory.java | 4 ++- .../dataformat/MessagePackFactoryTest.java | 32 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java index a624b6b9d..225a33628 100644 --- a/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java +++ b/msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/MessagePackFactory.java @@ -58,7 +58,9 @@ public MessagePackFactory(MessagePackFactory src) this.packerConfig = src.packerConfig.clone(); this.reuseResourceInGenerator = src.reuseResourceInGenerator; this.reuseResourceInParser = src.reuseResourceInParser; - this.extTypeCustomDesers = new ExtensionTypeCustomDeserializers(src.extTypeCustomDesers); + if (src.extTypeCustomDesers != null) { + this.extTypeCustomDesers = new ExtensionTypeCustomDeserializers(src.extTypeCustomDesers); + } } public MessagePackFactory setReuseResourceInGenerator(boolean reuseResourceInGenerator) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java index f9ce8629c..478b73a76 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.AnnotationIntrospector; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector; import org.junit.Test; import org.msgpack.core.MessagePack; @@ -58,7 +59,36 @@ public void testCreateParser() } @Test - public void copy() + public void copyWithDefaultConfig() + throws IOException + { + MessagePackFactory messagePackFactory = new MessagePackFactory(); + ObjectMapper copiedObjectMapper = new ObjectMapper(messagePackFactory).copy(); + JsonFactory copiedFactory = copiedObjectMapper.getFactory(); + assertThat(copiedFactory, is(instanceOf(MessagePackFactory.class))); + MessagePackFactory copiedMessagePackFactory = (MessagePackFactory) copiedFactory; + + assertThat(copiedMessagePackFactory.getPackerConfig().isStr8FormatSupport(), is(true)); + + assertThat(copiedMessagePackFactory.getExtTypeCustomDesers(), is(nullValue())); + + assertThat(copiedMessagePackFactory.isEnabled(JsonGenerator.Feature.AUTO_CLOSE_TARGET), is(true)); + assertThat(copiedMessagePackFactory.isEnabled(JsonParser.Feature.AUTO_CLOSE_SOURCE), is(true)); + + Collection annotationIntrospectors = copiedObjectMapper.getSerializationConfig().getAnnotationIntrospector().allIntrospectors(); + assertThat(annotationIntrospectors.size(), is(1)); + assertThat(annotationIntrospectors.stream().findFirst().get(), is(instanceOf(JacksonAnnotationIntrospector.class))); + + HashMap map = new HashMap<>(); + map.put("one", 1); + Map deserialized = copiedObjectMapper + .readValue(objectMapper.writeValueAsBytes(map), new TypeReference>() {}); + assertThat(deserialized.size(), is(1)); + assertThat(deserialized.get("one"), is(1)); + } + + @Test + public void copyWithAdvancedConfig() throws IOException { ExtensionTypeCustomDeserializers extTypeCustomDesers = new ExtensionTypeCustomDeserializers(); From 5505124e618e4079daff17a32190ab595918d7f1 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Sat, 14 Apr 2018 22:26:47 +0900 Subject: [PATCH 2/3] Refactor the test --- .../dataformat/MessagePackFactoryTest.java | 112 ++++++++++-------- 1 file changed, 65 insertions(+), 47 deletions(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java index 478b73a76..c36360c52 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java @@ -19,6 +19,7 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.AnnotationIntrospector; import com.fasterxml.jackson.databind.ObjectMapper; @@ -58,76 +59,93 @@ public void testCreateParser() assertEquals(MessagePackParser.class, parser.getClass()); } - @Test - public void copyWithDefaultConfig() + private void assertCopy(boolean advancedConfig) throws IOException { - MessagePackFactory messagePackFactory = new MessagePackFactory(); - ObjectMapper copiedObjectMapper = new ObjectMapper(messagePackFactory).copy(); - JsonFactory copiedFactory = copiedObjectMapper.getFactory(); - assertThat(copiedFactory, is(instanceOf(MessagePackFactory.class))); - MessagePackFactory copiedMessagePackFactory = (MessagePackFactory) copiedFactory; + // Build base ObjectMapper + ObjectMapper objectMapper; + if (advancedConfig) { + ExtensionTypeCustomDeserializers extTypeCustomDesers = new ExtensionTypeCustomDeserializers(); + extTypeCustomDesers.addTargetClass((byte) 42, TinyPojo.class); - assertThat(copiedMessagePackFactory.getPackerConfig().isStr8FormatSupport(), is(true)); + MessagePack.PackerConfig msgpackPackerConfig = new MessagePack.PackerConfig().withStr8FormatSupport(false); - assertThat(copiedMessagePackFactory.getExtTypeCustomDesers(), is(nullValue())); + MessagePackFactory messagePackFactory = new MessagePackFactory(msgpackPackerConfig); + messagePackFactory.setExtTypeCustomDesers(extTypeCustomDesers); - assertThat(copiedMessagePackFactory.isEnabled(JsonGenerator.Feature.AUTO_CLOSE_TARGET), is(true)); - assertThat(copiedMessagePackFactory.isEnabled(JsonParser.Feature.AUTO_CLOSE_SOURCE), is(true)); + objectMapper = new ObjectMapper(messagePackFactory); - Collection annotationIntrospectors = copiedObjectMapper.getSerializationConfig().getAnnotationIntrospector().allIntrospectors(); - assertThat(annotationIntrospectors.size(), is(1)); - assertThat(annotationIntrospectors.stream().findFirst().get(), is(instanceOf(JacksonAnnotationIntrospector.class))); + objectMapper.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false); + objectMapper.configure(JsonParser.Feature.AUTO_CLOSE_SOURCE, false); - HashMap map = new HashMap<>(); - map.put("one", 1); - Map deserialized = copiedObjectMapper - .readValue(objectMapper.writeValueAsBytes(map), new TypeReference>() {}); - assertThat(deserialized.size(), is(1)); - assertThat(deserialized.get("one"), is(1)); - } + objectMapper.setAnnotationIntrospector(new JsonArrayFormat()); + } + else { + MessagePackFactory messagePackFactory = new MessagePackFactory(); + objectMapper = new ObjectMapper(messagePackFactory); + } - @Test - public void copyWithAdvancedConfig() - throws IOException - { - ExtensionTypeCustomDeserializers extTypeCustomDesers = new ExtensionTypeCustomDeserializers(); - extTypeCustomDesers.addTargetClass((byte) 42, TinyPojo.class); + // Use the original ObjectMapper in advance + { + byte[] bytes = objectMapper.writeValueAsBytes(1234); + assertThat(objectMapper.readValue(bytes, Integer.class), is(1234)); + } - MessagePack.PackerConfig msgpackPackerConfig = new MessagePack.PackerConfig().withStr8FormatSupport(false); + // Copy the ObjectMapper + ObjectMapper copiedObjectMapper = objectMapper.copy(); - MessagePackFactory messagePackFactory = new MessagePackFactory(msgpackPackerConfig); - messagePackFactory.setExtTypeCustomDesers(extTypeCustomDesers); + // Assert the copied ObjectMapper + JsonFactory copiedFactory = copiedObjectMapper.getFactory(); + assertThat(copiedFactory, is(instanceOf(MessagePackFactory.class))); + MessagePackFactory copiedMessagePackFactory = (MessagePackFactory) copiedFactory; - ObjectMapper objectMapper = new ObjectMapper(messagePackFactory); + Collection annotationIntrospectors = + copiedObjectMapper.getSerializationConfig().getAnnotationIntrospector().allIntrospectors(); + assertThat(annotationIntrospectors.size(), is(1)); - objectMapper.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false); - objectMapper.configure(JsonParser.Feature.AUTO_CLOSE_SOURCE, false); + if (advancedConfig) { + assertThat(copiedMessagePackFactory.getPackerConfig().isStr8FormatSupport(), is(false)); - objectMapper.setAnnotationIntrospector(new JsonArrayFormat()); + assertThat(copiedMessagePackFactory.getExtTypeCustomDesers().getDeser((byte) 42), is(notNullValue())); + assertThat(copiedMessagePackFactory.getExtTypeCustomDesers().getDeser((byte) 43), is(nullValue())); - ObjectMapper copiedObjectMapper = objectMapper.copy(); - JsonFactory copiedFactory = copiedObjectMapper.getFactory(); - assertThat(copiedFactory, is(instanceOf(MessagePackFactory.class))); - MessagePackFactory copiedMessagePackFactory = (MessagePackFactory) copiedFactory; + assertThat(copiedMessagePackFactory.isEnabled(JsonGenerator.Feature.AUTO_CLOSE_TARGET), is(false)); + assertThat(copiedMessagePackFactory.isEnabled(JsonParser.Feature.AUTO_CLOSE_SOURCE), is(false)); - assertThat(copiedMessagePackFactory.getPackerConfig().isStr8FormatSupport(), is(false)); + assertThat(annotationIntrospectors.stream().findFirst().get(), is(instanceOf(JsonArrayFormat.class))); + } + else { + assertThat(copiedMessagePackFactory.getPackerConfig().isStr8FormatSupport(), is(true)); - assertThat(copiedMessagePackFactory.getExtTypeCustomDesers().getDeser((byte) 42), is(notNullValue())); - assertThat(copiedMessagePackFactory.getExtTypeCustomDesers().getDeser((byte) 43), is(nullValue())); + assertThat(copiedMessagePackFactory.getExtTypeCustomDesers(), is(nullValue())); - assertThat(copiedMessagePackFactory.isEnabled(JsonGenerator.Feature.AUTO_CLOSE_TARGET), is(false)); - assertThat(copiedMessagePackFactory.isEnabled(JsonParser.Feature.AUTO_CLOSE_SOURCE), is(false)); + assertThat(copiedMessagePackFactory.isEnabled(JsonGenerator.Feature.AUTO_CLOSE_TARGET), is(true)); + assertThat(copiedMessagePackFactory.isEnabled(JsonParser.Feature.AUTO_CLOSE_SOURCE), is(true)); - Collection annotationIntrospectors = copiedObjectMapper.getSerializationConfig().getAnnotationIntrospector().allIntrospectors(); - assertThat(annotationIntrospectors.size(), is(1)); - assertThat(annotationIntrospectors.stream().findFirst().get(), is(instanceOf(JsonArrayFormat.class))); + assertThat(annotationIntrospectors.stream().findFirst().get(), + is(instanceOf(JacksonAnnotationIntrospector.class))); + } - HashMap map = new HashMap<>(); + // Check the copied ObjectMapper works fine + Map map = new HashMap<>(); map.put("one", 1); Map deserialized = copiedObjectMapper .readValue(objectMapper.writeValueAsBytes(map), new TypeReference>() {}); assertThat(deserialized.size(), is(1)); assertThat(deserialized.get("one"), is(1)); } + + @Test + public void copyWithDefaultConfig() + throws IOException + { + assertCopy(false); + } + + @Test + public void copyWithAdvancedConfig() + throws IOException + { + assertCopy(true); + } } From 2b6487f912dffe6cd04b253c6aee9c962e007bec Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Sat, 14 Apr 2018 22:46:37 +0900 Subject: [PATCH 3/3] Remove unused import --- .../org/msgpack/jackson/dataformat/MessagePackFactoryTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java index c36360c52..c3379c3df 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackFactoryTest.java @@ -19,7 +19,6 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.AnnotationIntrospector; import com.fasterxml.jackson.databind.ObjectMapper;