From 961fd266f9d853e5ee07d9bc1ae031d2b1d54e78 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Wed, 28 Aug 2019 00:10:06 +0900 Subject: [PATCH 1/3] Add test code and write tips for issue 508 --- msgpack-jackson/README.md | 38 ++++++++++++++ .../dataformat/MessagePackGeneratorTest.java | 51 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/msgpack-jackson/README.md b/msgpack-jackson/README.md index 52149f047..fe76f0e2b 100644 --- a/msgpack-jackson/README.md +++ b/msgpack-jackson/README.md @@ -315,3 +315,41 @@ When you want to use non-String value as a key of Map, use `MessagePackKeySerial System.out.println(objectMapper.readValue(bytes, Object.class)); // => Java ``` + +### Serialize a nested object that also serializes + +When you serialize an object that has a nested object also serializing with ObjectMapper and MessagePackFactory like the following code + +```java + @Test + public void testNestedSerialization() throws Exception + { + ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory()); + objectMapper.writeValueAsBytes(new OuterClass()); + } + + public class OuterClass + { + public String getInner() throws JsonProcessingException + { + ObjectMapper m = new ObjectMapper(new MessagePackFactory()); + m.writeValueAsBytes(new InnerClass()); + return "EFG"; + } + } + + public class InnerClass + { + public String getName() + { + return "ABC"; + } + } +``` + +This code throws NullPointerException since the nested MessagePackFactory modifies a shared state stored in ThreadLocal. There are a few options to fix this issue, but they introduce performance degredations while this usage is a corner case. A workaround that doesn't affect performance is to call `MessagePackFactory#setReuseResourceInGenerator(false)`. I think it might be inconvenient to call the API for users, but it's a reasonable tradeoff with performance for now. + +```java + ObjectMapper objectMapper = new ObjectMapper( + new MessagePackFactory().setReuseResourceInGenerator(false)); +``` \ No newline at end of file diff --git a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java index 7a8db2805..549f2f90a 100644 --- a/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java +++ b/msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java @@ -15,6 +15,7 @@ // package org.msgpack.jackson.dataformat; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonProcessingException; @@ -884,4 +885,54 @@ public void serializeStringAsBigInteger() MessagePack.newDefaultUnpacker(objectMapper.writeValueAsBytes(bi)).unpackDouble(), is(bi.doubleValue())); } + + @Test + public void testNestedSerialization() throws Exception + { + // The purpose of this test is to confirm if MessagePackFactory.setReuseResourceInGenerator(false) + // works as a workaround for https://github.com/msgpack/msgpack-java/issues/508 + ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory().setReuseResourceInGenerator(false)); + OuterClass outerClass = objectMapper.readValue( + objectMapper.writeValueAsBytes(new OuterClass("Foo")), + OuterClass.class); + assertEquals("Foo", outerClass.getName()); + } + + static class OuterClass + { + private final String name; + + public OuterClass(@JsonProperty("name") String name) + { + this.name = name; + } + + public String getName() + throws IOException + { + // Serialize nested class object + ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory()); + InnerClass innerClass = objectMapper.readValue( + objectMapper.writeValueAsBytes(new InnerClass("Bar")), + InnerClass.class); + assertEquals("Bar", innerClass.getName()); + + return name; + } + } + + static class InnerClass + { + private final String name; + + public InnerClass(@JsonProperty("name") String name) + { + this.name = name; + } + + public String getName() + { + return name; + } + } } From b67c13b91232fd15870d04f4ade689b8ec957427 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Fri, 30 Aug 2019 22:07:18 +0900 Subject: [PATCH 2/3] Minor change --- msgpack-jackson/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/msgpack-jackson/README.md b/msgpack-jackson/README.md index fe76f0e2b..d53ecfefb 100644 --- a/msgpack-jackson/README.md +++ b/msgpack-jackson/README.md @@ -347,9 +347,10 @@ When you serialize an object that has a nested object also serializing with Obje } ``` -This code throws NullPointerException since the nested MessagePackFactory modifies a shared state stored in ThreadLocal. There are a few options to fix this issue, but they introduce performance degredations while this usage is a corner case. A workaround that doesn't affect performance is to call `MessagePackFactory#setReuseResourceInGenerator(false)`. I think it might be inconvenient to call the API for users, but it's a reasonable tradeoff with performance for now. +This code throws NullPointerException since the nested MessagePackFactory modifies a shared state stored in ThreadLocal. There are a few options to fix this issue, but they introduce performance degredations while this usage is a corner case. A workaround that doesn't affect performance is to call `MessagePackFactory#setReuseResourceInGenerator(false)`. It might be inconvenient to call the API for users, but it's a reasonable tradeoff with performance for now. ```java ObjectMapper objectMapper = new ObjectMapper( new MessagePackFactory().setReuseResourceInGenerator(false)); -``` \ No newline at end of file +``` + From d7f32ca9381980ddbc225e4065059db0c544e003 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Fri, 30 Aug 2019 22:22:45 +0900 Subject: [PATCH 3/3] Minor update --- msgpack-jackson/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msgpack-jackson/README.md b/msgpack-jackson/README.md index d53ecfefb..00be68c67 100644 --- a/msgpack-jackson/README.md +++ b/msgpack-jackson/README.md @@ -318,7 +318,7 @@ When you want to use non-String value as a key of Map, use `MessagePackKeySerial ### Serialize a nested object that also serializes -When you serialize an object that has a nested object also serializing with ObjectMapper and MessagePackFactory like the following code +When you serialize an object that has a nested object also serializing with ObjectMapper and MessagePackFactory like the following code, it throws NullPointerException since the nested MessagePackFactory modifies a shared state stored in ThreadLocal. ```java @Test @@ -347,7 +347,7 @@ When you serialize an object that has a nested object also serializing with Obje } ``` -This code throws NullPointerException since the nested MessagePackFactory modifies a shared state stored in ThreadLocal. There are a few options to fix this issue, but they introduce performance degredations while this usage is a corner case. A workaround that doesn't affect performance is to call `MessagePackFactory#setReuseResourceInGenerator(false)`. It might be inconvenient to call the API for users, but it's a reasonable tradeoff with performance for now. +There are a few options to fix this issue, but they introduce performance degredations while this usage is a corner case. A workaround that doesn't affect performance is to call `MessagePackFactory#setReuseResourceInGenerator(false)`. It might be inconvenient to call the API for users, but it's a reasonable tradeoff with performance for now. ```java ObjectMapper objectMapper = new ObjectMapper(