fix: honor registered ext coders for named non-struct types across encode/decode and stream APIs (#55) - #117
Open
shamaton wants to merge 2 commits into
Open
fix: honor registered ext coders for named non-struct types across encode/decode and stream APIs (#55)#117shamaton wants to merge 2 commits into
shamaton wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #117 +/- ##
==========================================
- Coverage 94.90% 92.91% -2.00%
==========================================
Files 74 74
Lines 6461 7017 +556
==========================================
+ Hits 6132 6520 +388
- Misses 191 311 +120
- Partials 138 186 +48 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
type Role uint8) being silently ignored outside the struct dispatch path, on bothMarshal/Unmarshaland the streamingEncoder/DecoderAPIs.Background / Motivation
The ext registry (
AddExtCoder/AddExtEncoder) accepts anyreflect.Type, but was only consulted from the struct dispatch (calcStruct/writeStructon encode,setStructon decode). The general dispatch functions (calcSize/createon encode,decodeon decode) switch purely onreflect.Kind, so an ext coder registered for a named non-struct type was silently dropped at the top level and inside slices —Marshalfell back to the plain kind encoding andUnmarshalcouldn't recover the value, making the round-trip lossy. The same gap existed in the streaming API, which shares none of this dispatch code with the non-streaming path.Credit: originally reported and root-caused by @youdie006 in #110 (repro, root-cause analysis, and an initial encode-side fix for the non-stream API). This PR takes a different implementation approach to also cover decode, the streaming API, and avoid a performance regression on the non-ext hot path (see below) — closing #110 with thanks for the report.
Changes
internal/encoding: kind-partitioned ext encoder registry (atomic.Pointer+ copy-on-write update) socalcSize/createonly consult ext coders for kinds that actually have one registered, instead of scanning all registered coders for every value.internal/decoding: newtryExtDecodehelper, used from bothdecode()andsetStruct, that recognizes ext wire frames viaextEndOffset(full boundary validation, so truncated ext frames are still rejected before any custom decoder runs) and dispatches to the first registered decoder whose Go type matches the destination.internal/stream/encoding,internal/stream/decoding: mirror the same designs for the streaming API.Testing
go test ./...,go vet ./...,go build ./...all clean; race detector clean on the new stream decode tests.issue55_ext_test.go(public API, non-stream + stream), plus package-level tests ininternal/encoding,internal/decoding,internal/stream/encoding,internal/stream/decoding.internal/encodingandinternal/decodingcomparing plain-value and ext-slice encode/decode paths.Closes #55.