Skip to content

Add per-call SerializationContext overloads - #1085

Merged
AArnott merged 3 commits into
mainfrom
aarnott-cautious-fishstick
Sep 29, 2026
Merged

AArnott merged 3 commits into
mainfrom
aarnott-cautious-fishstick

Conversation

@AArnott

@AArnott AArnott commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Why

Advanced callers need per-operation converter state and settings without creating a separate serializer for every call. Adding a context parameter to every overload would unnecessarily multiply the public API surface.

What changed

  • Add context-taking overloads for the core sync and async typed and untyped serialize/deserialize entry points (MessagePackWriter/MessagePackReader and PipeWriter/PipeReader). Existing overloads preserve their signatures and delegate using StartingContext; the context-taking overloads use the supplied context and its cancellation token.
  • Share implementation through private cores. Sync cores initialize the context by reference to avoid an additional large-struct copy. Give the new overloads lower overload-resolution priority so explicit default continues to bind to the existing cancellation-token overloads.
  • Reject contexts already active in another serialization operation, directing converters to SerializationContext.GetConverter for nested values. Add a protected CreateSerializationContext overload accepting a per-call context and move internal callers away from the overload that implicitly uses StartingContext.
  • Document per-call state and add a sample and tests.

Validation

  • Build/pack and formatting verification pass; DocFX builds; AOT console sample ends with Success.
  • The 13 new tests pass on net8.0, net9.0, net10.0, and net472.
  • The full test run has 19 net472 failures involving Newtonsoft.Json.Linq.JToken.WriteTo MissingMethodException; the same 19 failures reproduce with the changes stashed.

…hods

Adds overloads to the 8 core Serialize/Deserialize(Object)(Async) methods over
MessagePackWriter/Reader and PipeWriter/Reader that take a SerializationContext
in place of the CancellationToken. The cancellation token comes from the context.

Existing overloads delegate to shared private cores passing StartingContext.
Sync cores initialize the context in place by ref to avoid an extra copy of
the large struct. New overloads carry [OverloadResolutionPriority(-1)] so an
explicit 'default' argument still binds to the CancellationToken overloads.

A context already initialized for an in-flight operation (e.g. one handed to
a converter) is rejected with ArgumentException.

Adds a protected CreateSerializationContext(ITypeShapeProvider,
SerializationContext) overload; library code no longer calls the overload
that implicitly uses StartingContext.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation and tests are coherent; remaining feedback concerns minor documentation and message accuracy.

Review effort: Balanced
Findings: 3 Low severity

Open (3)
What changed in this PR

Adds per-call SerializationContext support across core serialization APIs.

Changes:

  • Adds sync/async typed and untyped context overloads.
  • Rejects previously initialized contexts and consolidates shared implementation.
  • Adds documentation, samples, and comprehensive tests.
File Description
MessagePackSerializer.cs Adds context overloads and shared cores.
SerializationContext.cs Adds initialized-context validation.
PerCallSerializationContextTests.cs Tests context state, cancellation, depth, and overload resolution.
CustomConverters.cs Demonstrates per-call state.
custom-converters.md Documents the new API.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docfx/docs/custom-converters.md Outdated
Comment thread src/Nerdbank.MessagePack/MessagePackSerializer.cs Outdated
Comment thread src/Nerdbank.MessagePack/SerializationContext.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 29, 2026 00:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation is consistent across entry points and adequately tested and documented.

Review effort: Balanced
Findings: None

Resolved since last review (3)

Avoid reflection-based type shape resolution in the new tests so they work under NativeAOT, including the Linux CI test executable.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 29, 2026 00:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation, validation, documentation, and tests consistently cover the new per-call context behavior.

Review effort: Balanced
Findings: None

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.89796% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.27%. Comparing base (15f3ca3) to head (1fffefc).

Files with missing lines Patch % Lines
src/Nerdbank.MessagePack/MessagePackSerializer.cs 92.53% 8 Missing and 2 partials ⚠️
...sagePack.TUnit/PerCallSerializationContextTests.cs 96.75% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1085      +/-   ##
==========================================
+ Coverage   82.01%   82.27%   +0.25%     
==========================================
  Files         256      257       +1     
  Lines       20866    21057     +191     
  Branches     2794     2795       +1     
==========================================
+ Hits        17113    17324     +211     
+ Misses       2864     2845      -19     
+ Partials      889      888       -1     
Flag Coverage Δ
Linux 82.01% <94.89%> (+0.20%) ⬆️
Windows 82.10% <94.89%> (+0.10%) ⬆️
macOS 82.02% <94.89%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AArnott
AArnott merged commit 2626dfe into main Sep 29, 2026
10 checks passed
@AArnott
AArnott deleted the aarnott-cautious-fishstick branch September 29, 2026 01:16
@AArnott AArnott added this to the 1.4 milestone Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants