Skip to content

Commit 1816b1e

Browse files
authored
Merge pull request #512 from msgpack/follow-up-issue-508
Add test and update README to follow up issue 508
2 parents 6f6cc4f + d7f32ca commit 1816b1e

File tree

2 files changed

+90
-0
lines changed

2 files changed

+90
-0
lines changed

msgpack-jackson/README.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,3 +315,42 @@ When you want to use non-String value as a key of Map, use `MessagePackKeySerial
315315
System.out.println(objectMapper.readValue(bytes, Object.class));
316316
// => Java
317317
```
318+
319+
### Serialize a nested object that also serializes
320+
321+
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.
322+
323+
```java
324+
@Test
325+
public void testNestedSerialization() throws Exception
326+
{
327+
ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory());
328+
objectMapper.writeValueAsBytes(new OuterClass());
329+
}
330+
331+
public class OuterClass
332+
{
333+
public String getInner() throws JsonProcessingException
334+
{
335+
ObjectMapper m = new ObjectMapper(new MessagePackFactory());
336+
m.writeValueAsBytes(new InnerClass());
337+
return "EFG";
338+
}
339+
}
340+
341+
public class InnerClass
342+
{
343+
public String getName()
344+
{
345+
return "ABC";
346+
}
347+
}
348+
```
349+
350+
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.
351+
352+
```java
353+
ObjectMapper objectMapper = new ObjectMapper(
354+
new MessagePackFactory().setReuseResourceInGenerator(false));
355+
```
356+

msgpack-jackson/src/test/java/org/msgpack/jackson/dataformat/MessagePackGeneratorTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//
1616
package org.msgpack.jackson.dataformat;
1717

18+
import com.fasterxml.jackson.annotation.JsonProperty;
1819
import com.fasterxml.jackson.core.JsonGenerator;
1920
import com.fasterxml.jackson.core.JsonEncoding;
2021
import com.fasterxml.jackson.core.JsonProcessingException;
@@ -884,4 +885,54 @@ public void serializeStringAsBigInteger()
884885
MessagePack.newDefaultUnpacker(objectMapper.writeValueAsBytes(bi)).unpackDouble(),
885886
is(bi.doubleValue()));
886887
}
888+
889+
@Test
890+
public void testNestedSerialization() throws Exception
891+
{
892+
// The purpose of this test is to confirm if MessagePackFactory.setReuseResourceInGenerator(false)
893+
// works as a workaround for https://github.com/msgpack/msgpack-java/issues/508
894+
ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory().setReuseResourceInGenerator(false));
895+
OuterClass outerClass = objectMapper.readValue(
896+
objectMapper.writeValueAsBytes(new OuterClass("Foo")),
897+
OuterClass.class);
898+
assertEquals("Foo", outerClass.getName());
899+
}
900+
901+
static class OuterClass
902+
{
903+
private final String name;
904+
905+
public OuterClass(@JsonProperty("name") String name)
906+
{
907+
this.name = name;
908+
}
909+
910+
public String getName()
911+
throws IOException
912+
{
913+
// Serialize nested class object
914+
ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory());
915+
InnerClass innerClass = objectMapper.readValue(
916+
objectMapper.writeValueAsBytes(new InnerClass("Bar")),
917+
InnerClass.class);
918+
assertEquals("Bar", innerClass.getName());
919+
920+
return name;
921+
}
922+
}
923+
924+
static class InnerClass
925+
{
926+
private final String name;
927+
928+
public InnerClass(@JsonProperty("name") String name)
929+
{
930+
this.name = name;
931+
}
932+
933+
public String getName()
934+
{
935+
return name;
936+
}
937+
}
887938
}

0 commit comments

Comments
 (0)