Skip to content

Commit d9b0dc4

Browse files
committed
Update preexisting-issues.md to reflect generator contract fixes
1 parent dcfccb0 commit d9b0dc4

1 file changed

Lines changed: 27 additions & 10 deletions

File tree

plans/preexisting-issues.md

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66

77
**File:** `msgpack-jackson3/.../MessagePackGenerator.java` (close method)
88

9-
Fixed by setting `_closed = true` directly in the `finally` block of `close()`, without
10-
calling `super.close()` (which would unconditionally close the underlying stream via
11-
`_closeInput()`, ignoring the `AUTO_CLOSE_TARGET` flag).
9+
Fixed by delegating to `super.close()` (which sets `_closed = true` and calls
10+
`_releaseBuffers()`). `AUTO_CLOSE_TARGET` handling was moved to `_closeInput()` so the
11+
base-class lifecycle is used correctly. Our `close()` override calls `flush()` first to
12+
drain pending nodes, then delegates the rest to `super.close()`.
1213

1314
### 2. `MessagePackFactory.snapshot()` returns `this` — FIXED
1415

@@ -22,20 +23,36 @@ Fixed: `snapshot()` now delegates to `copy()`, and `rebuild()` is implemented vi
2223
running on Java 17+. Developers on older JDKs and CI on older JDK matrix entries
2324
skip the module cleanly.
2425

25-
### 4. `MessagePackGenerator.streamWriteContext()` returns null — FIXED
26+
### 4. `MessagePackGenerator` Jackson 3 generator contract — FIXED
2627

2728
**File:** `msgpack-jackson3/.../MessagePackGenerator.java`
2829

29-
Fixed by adding a `SimpleStreamWriteContext writeContext` field initialized to
30+
Several issues fixed across two passes:
31+
32+
**Pass 1**`streamWriteContext()` returns null:
33+
Added a `SimpleStreamWriteContext writeContext` field initialized to
3034
`SimpleStreamWriteContext.createRootContext(null)`. `streamWriteContext()` returns
3135
it; `writeStartArray/Object` push a child context; `endCurrentContainer` pops via
32-
`clearAndGetParent()`; `writeName` calls `writeContext.writeName(name)`;
33-
`_verifyValueWrite` calls `writeContext.writeValue()`. `currentValue()` and
34-
`assignCurrentValue()` now delegate to the write context.
36+
`clearAndGetParent()`; `currentValue()` and `assignCurrentValue()` delegate to it.
3537

3638
Also fixed in the same pass:
37-
- `version()` now returns `PackageVersion.VERSION` (0.9.12) in generator, parser,
38-
and factory, replacing `Version.unknownVersion()`.
39+
- `version()` now returns `PackageVersion.VERSION` in generator, parser, and factory.
40+
41+
**Pass 2** — Align with CBORGenerator pattern (reference: jackson-dataformats-binary 3.x):
42+
- `_verifyValueWrite`: checks return value of `writeContext.writeValue()` and calls
43+
`_reportError` on failure, so writing a value inside an object without a preceding
44+
property name is detected at the Jackson API level.
45+
- `writeName(String/SerializableString)`: checks return value of
46+
`writeContext.writeName()` and calls `_reportError` when not in Object context or
47+
when a name is written twice without an intervening value.
48+
- `writeStartArray/writeStartObject`: call `_verifyValueWrite` before creating the
49+
child context, matching the CBORGenerator pattern.
50+
- `addValueNode`: calls `writeContext.writeValue()` to keep Jackson context state in
51+
sync with our internal node-based state. This was the root cause of all `writeName`
52+
failures: `GeneratorBase` does NOT auto-call `_verifyValueWrite` for abstract write
53+
methods (writeString, writeNumber, etc.), so without this call `_gotPropertyId` was
54+
never reset after the first `writeName`, making every subsequent `writeName` return
55+
false.
3956

4057
Note: `messageBufferOutputHolder` ThreadLocal was NOT fixed here — calling
4158
`messageBufferOutputHolder.remove()` in `_releaseBuffers()` caused a 23% serialization

0 commit comments

Comments
 (0)