Add test code and write tips for issue 508

komamitsu · komamitsu · commit 961fd266f9d8 · 2019-08-28T00:10:06.000+09:00
diff --git 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));
+```
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
@@ -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;
+        }
+    }
 }
