[Split 2/3] Add Ably.PubSub.Device and Ably.PubSub.Server door packages with side-declaring agents - #1334
Draft
umair-ably wants to merge 5 commits into
Draft
Conversation
The core stamped `ably-dotnet/<version>` as its RSC7d1 family identifier. At the Pub/Sub package split (PDR-091b) it becomes `ably-pubsub-dotnet/<version>`, registered in ably-common#361 as a versioned `sdk` entry. The flip is the point: with the 1.x maintenance branch keeping the old identifier, the string alone partitions legacy traffic from new-package traffic, so no version correlation is needed to tell them apart. Updates the four spec files that assert on the token: the shared realtime connection-parameter specs, the shared REST HTTP client specs, and the Unity EditMode/PlayMode specs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports the ably-js `packages/shared/side.ts` contract (ably-js#2293,
ably-java#1233).
`src/Ably.PubSub.Side/Side.cs` is a single shared source file compiled
into both door assemblies by a `<Compile Include>` item rather than
published as a third package - the .NET analogue of ably-js's
`shared/side.ts` and ably-java's `shared/.../Side.java`. It holds both
side identifiers, with the load-bearing-suffix warning, and
`WithSideAgent`, which:
- builds a new `Agents` dictionary from the caller's entries, so the
caller's own dictionary instance is never mutated;
- applies the side entry last with a null value, so the flag is
versionless (`Agent.AddAgentIdentifier` already emits a bare token
for a null version) and wins a collision on its own key.
`PubSubServer` (`CreateRealtimeClient` / `CreateHttpClient`) and
`PubSubDevice` (`CreateClient`) each take the three shapes the core
constructors accept - key or token string, `ClientOptions`,
`Action<ClientOptions>` - and return the concrete `AblyRealtime` /
`AblyRest`. One device door by design: device-side connectionless
operations stay available on that client, so there is no device HTTP
door.
Both doors are strong-named in every configuration, so a consumer of
the signed core never mixes signed and unsigned Ably assemblies, and
both target the core's `netstandard2.0;net6.0;net7.0`. Server also gets
an old-style `net462` head mirroring the core's, so the package has a
`lib/net46` asset; there is deliberately no `net462` Device.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ably.pubsub.device.nuspec` and `ably.pubsub.server.nuspec` carry the same authors/licence/icon/repository metadata as the core's, and each declares an exact pin on the core (`[$version$]`) per target-framework group - the NuGet analogue of ably-js's exact `peerDependencies`, so a consumer can never resolve two core versions. Server ships net46 (from its .NET Framework head), netstandard2.0, net6.0 and net7.0; Device ships the same minus net46. `_Package_Create_NuGet` packs core, then device, then server, matching the publish order stack PR 3 will need. The `[$version$]` substitution inside a dependency version attribute is marked TODO to verify on the release dry run - no nuget.exe or Mono locally to check it. Stack PR 3 adds the pre-flight that asserts the packed pin equals the version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thirteen specs in Ably.PubSub.Tests.DotNET/PubSub/PubSubPackageSpecs.cs.
They live in the .NET test head rather than Ably.PubSub.Tests.Shared
because Ably.PubSub.Device has no net462 asset, so the shared project
cannot reference it from the .NET Framework head.
What they pin, using the existing MockHttp/FakeHttpMessageHandler
capture for the header and the fake transport factory for the realtime
connection params, so nothing here opens a network connection:
- the `Ably-Agent` header a door-created REST client actually puts on
the wire contains the bare side flag, no versioned form of it, the
versioned `ably-pubsub-dotnet/<version>` family identifier, and not
the other side's flag;
- the same for the realtime connection params from
`TransportParams.GetParams()`;
- caller `Agents` entries survive alongside the flag; a caller entry
under the side's own key is overridden; the caller's own dictionary
instance is untouched;
- null options and a null configure action throw
ArgumentNullException from every door;
- all three overload shapes return the concrete core types;
- the string overload's key-versus-token colon rule.
The identifiers are written out as literals, not read from `Side`, so a
rename of the constant fails here instead of passing silently.
Verified by mutation: giving the flag a version fails 8 of the 13.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the "added in the next PR in this stack" placeholders in the 2.0-in-development note with install commands and the door snippets for both sides, and states plainly that the doors are the supported entry points and Ably.PubSub.Core is internal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Second PR in the PDR-091b split stack. Stacked on #1333 (
integration/split-1-core), which is its base, so the diff shows only this PR's changes. Reference: ably-js#2293 (packages/shared/side.ts, the contract this ports), ably-js#2297 (versionless flags) and ably-java#1233.Supersedes the door-package half of the declined-shape #1330, whose door classes, argument guards and XML docs are reused, re-based onto the new core.
What this PR does
Family identifier flip.
Agent.csstampsably-pubsub-dotnet/<version>instead ofably-dotnet/<version>. The flip is the point: the 1.x maintenance branch keeps the old identifier, so the string alone partitions legacy traffic from new-package traffic without correlating versions. Registered in ably-common#361 as a versionedsdkentry. Four spec files updated (shared realtime connection-parameter specs, shared REST HTTP client specs, Unity EditMode + PlayMode specs).Shared side contract —
src/Ably.PubSub.Side/Side.cs,internal static class SideinIO.Ably.PubSub.Internal. A single.csfile pulled into both door assemblies with a<Compile Include>item rather than published as a third package: the .NET analogue of ably-js'spackages/shared/side.tsand ably-java'sshared/.../Side.java. It holds:DeviceAgentIdentifier/ServerAgentIdentifier, both here so the naming scheme changes in one place, carrying the load-bearing-suffix warning verbatim — on API-key auth the realtime system grants the MAU exemption by matching an agent entry ending in-server, and an identifier not yet in the registry is classified by that suffix alone, so renaming either without preserving its suffix silently reclassifies every client the package constructs;WithSideAgent(ClientOptions, string), which builds a newAgentsdictionary from the caller's entries and sets[identifier] = nulllast. Caller entries survive (a layered SDK keeps its attribution); the side wins a collision on its own key (which side the package declares is the package's to state, not the caller's to redefine); the caller's own dictionary instance is never mutated.nulloptions throwsArgumentNullExceptionnaming the door's parameter rather than failing vaguely deeper in the core.Configure(Action<ClientOptions>), since the core clients have no such constructor overload.Ably.PubSub.Server(netstandard2.0;net6.0;net7.0, assemblyAbly.PubSub.Server, namespaceIO.Ably.PubSub.Server,ProjectReferenceto the core):PubSubServer.CreateRealtimeClient(...)→AblyRealtimeandPubSubServer.CreateHttpClient(...)→AblyRest, three overloads each (key-or-tokenstring,ClientOptions,Action<ClientOptions>). The string overload usesnew ClientOptions(keyOrToken), so the core's own colon rule does the key-versus-token disambiguation.Ably.PubSub.Device(same TFMs minusnet462, decision 5b):PubSubDevice.CreateClient(...)→AblyRealtime. One door by design — device-side connectionless operations (history, presence reads, token requests,Request) are available on the returned client, so there is deliberately no device HTTP door. The API-key overloads are kept on purpose (PDR-091: device-side API keys stay allowed at launch; enforcement is server-side).Both doors are strong-named with
IO.Ably.snkin every configuration, not just the package one, so a consumer of the signed core never mixes signed and unsigned Ably assemblies; both haveGenerateDocumentationFile, StyleCop +stylecop.json+IO.Ably.ruleset, linkedCommonAssemblyInfo.csandGenerateAssemblyInfo=false, matching the core.Ably.PubSub.Server.NETFrameworkis an old-style head mirroring the core's, so the Server package has alib/net46asset.Packaging.
nuget/ably.pubsub.device.nuspecandnuget/ably.pubsub.server.nuspec, same authors/licence/icon/repository metadata as the core's, each declaring an exact pin<dependency id="Ably.PubSub.Core" version="[$version$]" />per target-framework group — the NuGet analogue of ably-js's exactpeerDependencies, so a consumer can never resolve two core versions._Package_Create_NuGetpacks core → device → server, matching the publish order PR 3 needs.Both door projects added to
Ably.PubSub.sln,Ably.PubSub.NetStandard.slnandAbly.PubSub.Package.sln; the Server .NET Framework head toAbly.PubSub.sln,Ably.PubSub.Package.slnandAbly.PubSub.NetFramework.sln.README: the 2.0-in-development note now lists all three packages with install commands and door snippets, and states that the doors are the supported entry points and
Ably.PubSub.Coreis internal.Agent contract (wire shape)
Captured live against sandbox from a throwaway console app (not committed) that creates a client through each door with a
DelegatingHandleronClientOptions.HttpClientand callsTimeAsync():The side flag is a bare token. A version on a cross-SDK flag says version-of-what: the door ships in lockstep with the core, whose version the family identifier already carries, and ably-common#361 models both flags with
versioned: false, likebrowser.Agent.AddAgentIdentifierin the core already emits a bare token for a null or empty version, so no core change was needed.Tests (what billing reads)
13 new specs in
src/Ably.PubSub.Tests.DotNET/PubSub/PubSubPackageSpecs.cs. They are in the .NET test head rather thanAbly.PubSub.Tests.SharedbecauseAbly.PubSub.Devicehas nonet462asset, so the shared project cannot reference it from the .NET Framework head.They use the existing infrastructure —
FakeHttpMessageHandleronClientOptions.HttpClientfor the header, the fake transport factory plusTransportParams.GetParams()for the realtime params — so no unit test here opens a network connection.PubSubServer.CreateHttpClient(...): theAbly-Agentheader actually put on the wire, split on spaces, contains the bareably-pubsub-server, no token startingably-pubsub-server/,ably-pubsub-dotnet/<Defaults.LibraryVersion>, and noably-pubsub-device.PubSubServer.CreateRealtimeClient(...): the same, from the realtime connection params.PubSubDevice.CreateClient(...): the same withably-pubsub-deviceand noably-pubsub-server— header via itsRestClient, and realtime params.Agentsentries survive ({"chat-dotnet": "1.0.0"}appears aschat-dotnet/1.0.0alongside the flag); a caller entry{"ably-pubsub-server": "9.9.9"}is overridden to the bare flag; the caller's own dictionary instance is unchanged afterwards.nulloptions and anullconfigure action throwArgumentNullExceptionfrom every door.AutoConnect = falseand return the concreteAblyRealtime/AblyRest.Options.Key, a token-shaped one inOptions.Token, and both are still stamped.The identifiers are written out as literals, not read from
Side, so renaming the constant fails here instead of passing silently. Verified by mutation: giving the flag a version (agents[identifier] = "9.9.9") fails 8 of the 13.Local runs (
export DOTNET_ROOT=$HOME/.dotnet):dotnet build src/Ably.PubSub.NetStandard.sln -c Release./build.sh --target=Test.NetStandard.Unit.WithRetry --framework=net6.0./build.sh --target=Test.NetStandard.Unit.WithRetry --framework=net7.0#1333's baseline was 1093 passed / 19 skipped; 1106 − 1093 = exactly the 13 new specs, so nothing regressed.
Open items flagged for review
ably-pubsub-dotnetversionedsdk;ably-pubsub-device/ably-pubsub-serverunversionedruntime). If Presence re-entry requirement change for 1.1 #361 is reshaped in review, mirror it here.TransportParams.GetParams()sends the agent under the keyAbly-Agent, while spec RTN2g names the parameteragent. This PR does not change that key and the specs assert on whatever key the core currently uses, so they do not pre-empt the answer. Plan step 14 asks for this to be confirmed against RTN2g separately; if it is a latent bug it should be fixed onmainfirst so the 1.x maintenance line gets it too.[$version$]inside the dependencyversionattribute is unverified locally — nonuget.exeand no Mono on this machine. nuget.exe documents replacement tokens as applying to the whole nuspec, and Cake'sNuGetPackis givenVersion = version, so it should substitute; there is aTODO(verify on the release dry run)comment in both nuspecs. PR 3 adds the pre-flight that asserts the packed pin equals the packed version, which is where this gets proven.Ably.PubSub.Server.NETFrameworkis unverified locally (no Mono/MSBuild here; CI coversnet462). It is a csproj plus the sameCompileincludes as the SDK-style head, referencingAbly.PubSub.Core.NETFramework, and is registered in the main, Package and NetFramework solutions.CreateRealtimeClient(string)andCreateClient(string)cannot disableAutoConnect, so constructing one in a unit test would open a real websocket — the flakiness CodeRabbit flagged on PubSub packages split and related documentation #1330. They share the exact code path that is tested (new ClientOptions(keyOrToken)thenWithSideAgent), the colon rule is covered through the HTTP door, and realtime stamping is covered through the action overload with a fake transport. Noted in the spec file.WithSideAgentassigns onto the caller'sClientOptionsinstance rather than a copy, replacing onlyAgents. There is noClientOptions.Clone()in the core, and adding one would put a core change on the critical path (ably-java#1233 had to fix itscopy()for exactly this). The caller's ownAgentsdictionary instance is still never mutated, which is the part that matters for reuse. Documented on the method. Say the word if a copy is wanted instead.IO.Ably.PubSub.Internal.Side— the same pattern as ably-java's duplicatedSide; harmless if both packages ever meet in one project, since the type isinternal.Agent.PlatformRuntimestill recognises only .NET 6 and 7, so .NET 8/9/10 apps reportdotnet-standard. Pre-existing, unrelated to the split, and needsdotnet8/dotnet10registry entries — plan step 5 suggests bundling it into the ably-common follow-up.Where the stack deliberately stops
_Release_Preflighttask, the packaging assertions,publish.yml, trusted publishing and the ordered core-first push are all PR 3.repositorymetadata and every link still sayably-dotnet.IRestClient/IRealtimeClientnarrowing and hiding push admin from Device are step-19 items.ably.io.push.android/ably.io.push.ios1.x stay on the maintenance branch, and a modernAbly.PubSub.Push.*onnet8.0-android/net8.0-iosis a post-GA 2.x addition.IO.Ablyfor the core (decision 2); only the doors take a new namespace.net462Ably.PubSub.Devicehead, and none is planned (decision 5b).🤖 Generated with Claude Code