Skip to content

Move socket connection interfaces to NATS.Client.Abstractions#1192

Merged
mtmk merged 1 commit into
release/3.0from
move-socket-interfaces-to-abstractions
Jun 25, 2026
Merged

Move socket connection interfaces to NATS.Client.Abstractions#1192
mtmk merged 1 commit into
release/3.0from
move-socket-interfaces-to-abstractions

Conversation

@mtmk

@mtmk mtmk commented Jun 17, 2026

Copy link
Copy Markdown
Member

Move the socket transport contracts INatsSocketConnection and INatsTlsUpgradeableSocketConnection to NATS.Client.Abstractions so custom transport implementers can depend on the contract without referencing Core, matching the serialization interfaces already there. The types keep the NATS.Client.Core namespace and are forwarded via TypeForwardedTo to preserve source and binary compatibility; the obsolete ISocketConnection and INatsSocketConnectionFactory stay in Core. The ABI compatibility harness is extended to cover both moved interfaces through a transient dependency compiled against an older release.

Fixes #1185

Move INatsSocketConnection and INatsTlsUpgradeableSocketConnection to
NATS.Client.Abstractions so custom transport implementers can depend on the
contract without referencing Core, same rationale as the serialization
interfaces already there. Both types are BCL-only.

Keep the NATS.Client.Core namespace on the moved types and add
TypeForwardedTo in Core to preserve source and binary compatibility. The
obsolete ISocketConnection and INatsSocketConnectionFactory stay in Core; the
factory takes NatsOpts so it cannot move without dragging Core options along.

Add Microsoft.Bcl.AsyncInterfaces to the Abstractions netstandard2.0 group for
IAsyncDisposable and ValueTask, which are not in that target's BCL.
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 2m 22s —— View job


PR Review

  • Gather context
  • Read PR diff and changed files
  • Analyze correctness, API compatibility, binary compat
  • Post review findings

Overall: The change is correct and well-structured. Follows the established pattern used for serialization interfaces. No bugs or security issues.

What looks good

  • Namespace preservation (namespace NATS.Client.Core; in the Abstractions project file) maintains source compatibility so existing consumers don't need new using directives.
  • TypeForwardedTo usage is correct — both INatsSocketConnection and INatsTlsUpgradeableSocketConnection are forwarded in TypeForwarders.cs, which handles binary compatibility for code compiled against older assemblies.
  • Microsoft.Bcl.AsyncInterfaces version (8.0.0) is consistent with what NATS.Client.Core already specifies for netstandard2.0. The dependency is necessary since IAsyncDisposable isn't in netstandard2.0 natively.
  • ABI harness is extended correctly: both interfaces are checked for assembly residency, and the transient-lib scenario (simulate old binary loading against new runtime) is exercised for INatsSocketConnection usage.

Notes / things to be aware of

System.Net.Sockets.Socket in Abstractions: INatsTlsUpgradeableSocketConnection.Socket introduces a concrete BCL networking type into what is otherwise a serialization/contract package. This is available on netstandard2.0 so it won't break builds, and preserving the interface shape is required for compat — just noting that Abstractions now has a platform-networking surface.

Factory still requires Core: The goal stated in the PR description is "custom transport implementers can depend on the contract without referencing Core." That's true for the connection side — an implementation library only needs Abstractions. However, INatsSocketConnectionFactory (the entry point for registering the custom transport via NatsOpts.SocketConnectionFactory) stays in Core because it takes NatsOpts. So the application wiring still needs Core, but this is expected and the PR is a meaningful step toward decoupling.

ABI test sends 7 bytes: MySocketConnection.SendAsync returns buffer.Length and the test asserts the return equals 7 — this is intentional and correct for a stub implementation.

No blocking issues. LGTM.

@github-actions

Copy link
Copy Markdown

@mtmk
mtmk requested a review from caleblloyd June 17, 2026 13:53
@mtmk mtmk added this to the Release 3.0 milestone Jun 17, 2026
@mtmk mtmk linked an issue Jun 17, 2026 that may be closed by this pull request
@mtmk
mtmk requested a review from scottf June 22, 2026 10:57
@mtmk mtmk added the ready label Jun 24, 2026

@scottf scottf 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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move socket connection interfaces to NATS.Client.Abstractions

2 participants