Skip to content

Commit 5e3ef27

Browse files
committed
Add regression tests and document writeRaw* design decision
- Add testWriteContextCurrentNameIsUpdatedForEveryProperty: verifies that addValueNode calls writeContext.writeValue() so _gotPropertyId is reset after each value, keeping currentName() correct for multi-property objects - Add testWritePropertyIdUpdatesWriteContext: verifies that writePropertyId calls writeContext.writeName() when supportIntegerKeys=true, so streamWriteContext().currentName() reflects the integer key - Document writeRaw* / writeRawValue* design decision in preexisting-issues.md: current behavior (delegate to addValueNode like writeString) satisfies the contract and is preferable to throwing UnsupportedOperationException
1 parent 1038199 commit 5e3ef27

2 files changed

Lines changed: 62 additions & 0 deletions

File tree

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,4 +1207,46 @@ public void testSerializedStringMethods()
12071207
assertEquals(5, written);
12081208
assertArrayEquals("hello".getBytes(java.nio.charset.StandardCharsets.UTF_8), baos.toByteArray());
12091209
}
1210+
1211+
// Regression: addValueNode must call writeContext.writeValue() so that the Jackson write
1212+
// context resets _gotPropertyId after each value. Without it, the second writeName() in the
1213+
// same object finds _gotPropertyId still set from the first writeName() and returns false
1214+
// without updating currentName(), leaving streamWriteContext().currentName() stale.
1215+
@Test
1216+
public void testWriteContextCurrentNameIsUpdatedForEveryProperty()
1217+
throws IOException
1218+
{
1219+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
1220+
JsonGenerator generator = factory.createGenerator(ObjectWriteContext.empty(), baos);
1221+
generator.writeStartObject();
1222+
1223+
generator.writeName("alpha");
1224+
generator.writeNumber(1);
1225+
1226+
generator.writeName("beta");
1227+
assertEquals("beta", generator.streamWriteContext().currentName());
1228+
generator.writeNumber(2);
1229+
1230+
generator.writeEndObject();
1231+
generator.close();
1232+
}
1233+
1234+
// Regression: writePropertyId() must call writeContext.writeName() when supportIntegerKeys
1235+
// is true. Without it, streamWriteContext() never learns a name was written, so currentName()
1236+
// returns null and any downstream code relying on context state (e.g. duplicate-name
1237+
// detection, error messages) sees wrong state.
1238+
@Test
1239+
public void testWritePropertyIdUpdatesWriteContext()
1240+
throws IOException
1241+
{
1242+
MessagePackFactory intKeyFactory = new MessagePackFactory().setSupportIntegerKeys(true);
1243+
ByteArrayOutputStream baos = new ByteArrayOutputStream();
1244+
JsonGenerator generator = intKeyFactory.createGenerator(ObjectWriteContext.empty(), baos);
1245+
generator.writeStartObject();
1246+
generator.writePropertyId(42L);
1247+
assertEquals("42", generator.streamWriteContext().currentName());
1248+
generator.writeString("value");
1249+
generator.writeEndObject();
1250+
generator.close();
1251+
}
12101252
}

plans/preexisting-issues.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,26 @@ buffer) into a `StringBuilder`, matching the pattern already used in the len<0 b
7272
The len<0 path (StringBuilder + 1024-char chunk) still allocates an extra copy vs.
7373
`CharArrayWriter`, but the overhead is minor and not worth optimizing at this layer.
7474

75+
### 6. `writeRaw*` / `writeRawValue*` — FYI, no action needed
76+
77+
**File:** `msgpack-jackson3/.../MessagePackGenerator.java`
78+
79+
CBORGenerator and SmileGenerator both throw `UnsupportedOperationException` for all
80+
`writeRaw*` / `writeRawValue*` overloads. This was flagged in code review as something
81+
MessagePackGenerator should match.
82+
83+
After investigation, the current implementation (which delegates to `addValueNode`, same
84+
as `writeString`) is correct and need not change:
85+
86+
- The `writeRaw` contract requires: no escaping, no separators, content preserved
87+
unchanged. All three are satisfied — MessagePack has no JSON-style escaping or
88+
separators, and the text is packed verbatim via `packString`.
89+
- CBOR/Smile throwing is a design choice, not a technical requirement. They could
90+
equally implement it as "write as a text value" — they chose to be strict instead.
91+
- Our lenient approach (degrade gracefully to `writeString` behaviour) is just as
92+
defensible, and avoids breaking callers that use `writeRaw` as a `writeString`
93+
synonym.
94+
7595
---
7696

7797
## msgpack-jackson pre-existing issues

0 commit comments

Comments
 (0)