Skip to content

fix(NET-10): Pass request URLs per call instead of mutating shared BaseUrl - #33

Open
cozminu wants to merge 10 commits into
cozmin/refactorfrom
cozmin/net-10
Open

fix(NET-10): Pass request URLs per call instead of mutating shared BaseUrl#33
cozminu wants to merge 10 commits into
cozmin/refactorfrom
cozmin/net-10

Conversation

@cozminu

@cozminu cozminu commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

Removes the shared mutable BaseUrl from ResourceClientBase and AuthClientBase so a singleton client can serve concurrent requests to different wallet addresses without cross-talk. Ten hand-written methods on ResourceServerClient/AuthServerClient now take their target Uri as an explicit first parameter and build their URL from that parameter, instead of reading a _baseUrl field their wrapper mutated immediately before the call. This is the same pattern already used by ContinueGrantAsync(Uri continueUrl, ...), RotateTokenAsync(Uri tokenUrl, ...), and WalletAddressClient.GetWalletAddressAsync(string walletAddress, ...).

  • Closes Fix thread-safety: singleton clients mutate a shared BaseUrl #16.
  • Three URLs change on the wire, all corrections: the three GET endpoints lose a trailing slash, complete-incoming-payment loses a double slash, and grant requests to a pathful auth server lose a trailing slash. Because @target-uri is a signed component of the HTTP message signature, the previous code also signed over the malformed URI.
  • ResourceServerClient and AuthServerClient are public types, so their hand-written method signatures changing is source-breaking for anyone calling them directly rather than through IAuthenticatedClient. The supported surface (IResourceClientBase, IAuthClientBase, IAuthenticatedClient) is unchanged.
  • Adds a CI guard (.github/workflows/build.yaml) that fails the build if BaseUrl = is reintroduced anywhere in hand-written code under OpenPayments.Sdk/ (excluding generated *.g.cs constructors).

Test plan

  • dotnet test — 78/78 passing (49 SDK tests, 29 HttpSignatureUtils tests)
  • dotnet build --configuration Release — clean, no new warnings
  • dotnet build OpenPayments.Snippets/OpenPayments.Snippets.csproj — compiles unchanged, confirming the public surface didn't move
  • New RecordingHandler test double + cross-host/200-iteration parallel tests exercise the concurrency fix directly
  • CI guard verified to catch a reintroduced BaseUrl = write and to pass cleanly on the current tree

cozminu added 10 commits July 29, 2026 12:20
Adds the missing <param> tags for the Uri parameters introduced by the
thread-safe base URL refactor (resourceServerUrl/incomingPaymentUrl/
outgoingPaymentUrl) in the Incoming/Outgoing Payment partials, clearing
the CS1573 warnings from the documentation build.

Also hardens the CI BaseUrl guard: scans all of OpenPayments.Sdk/ (not
just Clients/) while excluding generated *.g.cs files, tolerates
whitespace around the '=' in "BaseUrl =", and fails loudly instead of
silently passing if its scan target ever disappears or is renamed.
@cozminu cozminu self-assigned this Jul 29, 2026
@cozminu cozminu added the bug Something isn't working label Jul 29, 2026
@cozminu cozminu changed the title Pass request URLs per call instead of mutating shared BaseUrl feat(NET-10): Pass request URLs per call instead of mutating shared BaseUrl Jul 29, 2026
@cozminu cozminu changed the title feat(NET-10): Pass request URLs per call instead of mutating shared BaseUrl bug(NET-10): Pass request URLs per call instead of mutating shared BaseUrl Jul 29, 2026
@cozminu cozminu changed the title bug(NET-10): Pass request URLs per call instead of mutating shared BaseUrl fix(NET-10): Pass request URLs per call instead of mutating shared BaseUrl Jul 29, 2026
@cozminu
cozminu changed the base branch from main to cozmin/refactor July 31, 2026 09:30
@cozminu
cozminu requested review from golobitch and koekiebox July 31, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix thread-safety: singleton clients mutate a shared BaseUrl

1 participant