client: consume MX.Api.Client 2.3.77 SharedCacheConfiguration, remove reflection - #849
Merged
Merged
Conversation
…ent expression leaks The V1 Repository client registers ~30 typed sub-API clients via a single consumer configureOptions delegate. When consumers invoked .WithCaching(c => c.UseLibraryDefaults()) plus per-sub-API cache expressions (e.g. c => c.GetGameServer(...)), MX.Api.Client 2.3.76 scoped the CacheBuilder to each typed client's _configuredClientType, causing ArgumentException at startup for every non-matching typed client (e.g. IAdminActionsApi rejecting an IGameServersApi expression). Fix: override WithCaching(Action<CacheBuilder>) on RepositoryApiOptionsBuilder. The consumer delegate is captured once, executed against an unscoped shadow builder per typed client, and only the CachePolicyOperations whose declaring types are assignable from the current typed client are replayed onto the real builder. UseLibraryDefaults() is re-applied through the public API. Access to the internal SetCachePolicyOperation and _configuredClientType members is via pinned reflection with clear fail-fast messages if MX.Api.Client contract changes. No change to the shipped public API, safe policy matrix, or AddDefaultCachePolicies registrations. Not an MX.Api.Client bug; per-client scoping is intentional. Tests: RepositoryClientRegistrationTests covers resolution of the unified client, all 34 registered sub-APIs (with and without caching), per-typed-client override placement, and confirms no cross-client bleed for both single and multi-sub-API consumer overrides. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Dependency ReviewThe following issues were found:
License Issuessrc/XtremeIdiots.Portal.Repository.Abstractions.V1/XtremeIdiots.Portal.Repository.Abstractions.V1.csproj
src/XtremeIdiots.Portal.Repository.Abstractions.V2/XtremeIdiots.Portal.Repository.Abstractions.V2.csproj
src/XtremeIdiots.Portal.Repository.Api.Client.Testing/XtremeIdiots.Portal.Repository.Api.Client.Testing.csproj
src/XtremeIdiots.Portal.Repository.Api.Client.V1/XtremeIdiots.Portal.Repository.Api.Client.V1.csproj
src/XtremeIdiots.Portal.Repository.Api.Client.V2/XtremeIdiots.Portal.Repository.Api.Client.V2.csproj
src/XtremeIdiots.Portal.Repository.Api.V1/XtremeIdiots.Portal.Repository.Api.V1.csproj
src/XtremeIdiots.Portal.Repository.Api.V2/XtremeIdiots.Portal.Repository.Api.V2.csproj
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Pull request overview
Fixes a DI startup ArgumentException in the V1 Repository API client when consumers combine .WithCaching(c => c.UseLibraryDefaults()) with per-sub-API cache expressions, by scoping cache policy operations to the currently-configured typed sub-API during registration.
Changes:
- Added a scope-aware
WithCaching(Action<CacheBuilder>)implementation onRepositoryApiOptionsBuilderthat captures cache intent once and replays only compatible operations per typed sub-API. - Added new DI resolution + scoping regression tests covering the crash repro and cross-client isolation guarantees.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/XtremeIdiots.Portal.Repository.Api.Client.V1/RepositoryApiOptionsBuilder.cs | Introduces scope-isolated caching configuration to prevent cross-sub-API cache expression failures during DI registration. |
| src/XtremeIdiots.Portal.Repository.Api.Client.Tests.V1/RepositoryClientRegistrationTests.cs | Adds regression tests to reproduce the prior startup crash and verify per-typed-client cache policy isolation. |
frasermolyneux
temporarily deployed
to
Development
August 2, 2026 21:34 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
⛔ Superseded — A newer run has replaced this result.⛔ Superseded — A newer run has replaced this result.🏗️ Terraform Plan
✅ Validate — Passed ✅ Plan
|
Address PR review feedback: consolidate the accidental duplicate using block in RepositoryClientRegistrationTests and drop the unused System.Collections.Generic / System.Linq / System.Reflection usings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/XtremeIdiots.Portal.Repository.Api.Client.V1/RepositoryApiOptionsBuilder.cs:71
WithCaching(Action<CacheBuilder>)never setsOptions.EnableCachingtotrue. If a consumer has previously calledWithCaching(false)(or the option is otherwise false), the cache policy operations you replay here will be configured but never used because caching remains disabled. Consider explicitly enabling caching when this overload is used to match the expected semantics of “WithCaching(…)”.
public new RepositoryApiOptionsBuilder WithCaching(Action<CacheBuilder> configure)
{
ArgumentNullException.ThrowIfNull(configure);
frasermolyneux
temporarily deployed
to
Development
August 2, 2026 21:50 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
⛔ Superseded — A newer run has replaced this result.🏗️ Terraform Plan
✅ Validate — Passed ✅ Plan
|
…rop reflection Replaces the private-reflection cache-scoping hack introduced in the prior commit with MX.Api.Client 2.3.77's new first-class API surface: - RepositoryApiOptionsBuilder.WithCaching(Action<CacheBuilder>) now only CAPTURES the consumer delegate onto an internal CapturedCacheConfigure property; no application, no reflection, no throws. - ServiceCollectionExtensions.AddRepositoryApiClient runs configureOptions once on a throwaway probe builder to extract the captured cache delegate, constructs one SharedCacheConfiguration per composition, and wraps every AddTypedApiClient call with a perClient delegate that applies WithSharedCaching(sharedCache) after configureOptions. After all typed clients are registered, ValidateAllOperationsMatched() is invoked exactly once to surface consumer typos as InvalidOperationException. - Consumer-facing API unchanged: .WithCaching(c => c.UseLibraryDefaults()...) on RepositoryApiOptionsBuilder continues to work exactly as before. - Bumped MX.Api.Client + MX.Api.Abstractions from 2.3.76 to 2.3.77 across V1/V2 Client, Abstractions V1/V2, Api V1/V2, and Client.Testing csprojs for solution-wide version consistency. - Added guard test AddRepositoryApiClient_ConsumerOverrideTargetingUnregisteredInterface_ThrowsValidationError proving orphaned cache expressions surface ValidateAllOperationsMatched(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
frasermolyneux
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/XtremeIdiots.Portal.Repository.Api.Client.V1/RepositoryApiOptionsBuilder.cs:56
- The XML doc cref targets
Abstractions.Interfaces.V1.*, but the interfaces live inXtremeIdiots.Portal.Repository.Abstractions.Interfaces.V1. As written, these<see cref>links won’t resolve in generated docs / IDE tooltips.
/// <see cref="ServiceCollectionExtensions.AddRepositoryApiClient"/> re-invokes the consumer's configuration
/// delegate once per typed sub-API (<see cref="Abstractions.Interfaces.V1.IAdminActionsApi"/>,
/// <see cref="Abstractions.Interfaces.V1.IGameServersApi"/>, etc.). The base
src/XtremeIdiots.Portal.Repository.Api.Client.Tests.V1/RepositoryClientRegistrationTests.cs:116
GetRequiredService<RepositoryApiClientOptions>()returns the last registeredRepositoryApiClientOptionswhen multiple registrations exist, so the variable namegameServersOptionsis misleading here and may confuse future readers of the test.
var gameServersOptions = provider.GetRequiredService<RepositoryApiClientOptions>();
Assert.True(gameServersOptions.UseLibraryCacheDefaults);
src/XtremeIdiots.Portal.Repository.Api.Client.V2/XtremeIdiots.Portal.Repository.Api.Client.V2.csproj:25
- PR description/risk section says the blast radius is the V1 client only and that the V2 client is untouched, but this PR also bumps MX.Api.* dependencies in the V2 client package. Either update the PR narrative/risk assessment to include V2, or revert/split these version bumps if they’re not required for the V1 fix.
<PackageReference Include="MX.Api.Client" Version="2.3.77" />
<PackageReference Include="MX.Api.Abstractions" Version="2.3.77" />
|
frasermolyneux
temporarily deployed
to
Development
August 3, 2026 07:03 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
🏗️ Terraform Plan
✅ Validate — Passed ✅ Plan
📋 Resource Details
|
This was referenced Aug 3, 2026
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
Fix the startup
ArgumentExceptionseen by consumers that enabled.WithCaching(c => c.UseLibraryDefaults())onAddRepositoryApiClient(regression exposed in 4.2.21 on MX.Api.Client 2.3.76). This revision replaces the earlier private-reflection cache-scoping fix with MX.Api.Client 2.3.77's new first-classSharedCacheConfiguration/WithSharedCachingAPI. All reflection into MX internals is gone.Closes #4221
Type of change
Root cause
ApiClientOptionsBuilder<TOptions,TBuilder>.WithCaching(Action<CacheBuilder>)(2.3.76) evaluates every cache expression the consumer supplies against the currently-scoped typed client's interface and throwsArgumentException: The expression must invoke a method declared by ... IAdminActionsApi or inherited interfaceswhen the expression's declaring interface isn't assignable to the scoped client.AddRepositoryApiClientinvokes the same consumerconfigureOptionsdelegate for every one of ~34AddTypedApiClientregistrations, so a single consumer.WithCaching(...)call containing operations for multiple sub-APIs unavoidably crashes on the first mismatched sibling.Fix
MX.Api.Client 2.3.77 shipped a purpose-built API for the unified-client scenario. Repository consumes it:
RepositoryApiOptionsBuilder.WithCaching(Action<CacheBuilder>)now only captures the consumer delegate onto an internalCapturedCacheConfigureproperty. It never throws and never applies operations itself. All private reflection into MX (_configuredClientType,SetCachePolicyOperation) is removed.ServiceCollectionExtensions.AddRepositoryApiClientrunsconfigureOptionsonce against a throwaway probe builder to extractCapturedCacheConfigure, then constructs oneSharedCacheConfigurationfor the composition. AperClientwrapper invokesconfigureOptions(builder)and thenbuilder.WithSharedCaching(sharedCache).WithSharedCachingapplies only the operations whose declaring interface is assignable from the currently-scoped typed client and skips siblings instead of throwing. After all typed clients are registered,sharedCache?.ValidateAllOperationsMatched()is invoked exactly once — this surfaces genuine consumer typos (a cache expression targeting an interface that is not a registered Repository sub-API) as a clearInvalidOperationException.AddDefaultCachePolicies<TClient>calls and the safe policy matrix are unchanged.The consumer-facing surface is unchanged:
.WithCaching(c => c.UseLibraryDefaults()....)continues to work exactly as documented.SharedCacheConfigurationis a Repository-side implementation detail.Consumer impact
.WithCaching(c => c.UseLibraryDefaults()....)— no code change required in downstream consumers.InvalidOperationExceptionat startup if their cache expression targets an interface that is not a registered Repository sub-API (previously such typos would be silently mapped to the wrong client). This is intentional and matches the shipping MX.Api.Client 2.3.77 contract.Api.Client.Testingpackage: no in-memory fake or DTO factory changed; version bump is transitive-consistency only.Validation evidence
Build (
dotnet build src/XtremeIdiots.Portal.Repository.sln):Tests (
dotnet test src/XtremeIdiots.Portal.Repository.sln --filter "FullyQualifiedName!~IntegrationTests") — all 12 non-integration assemblies pass on net9.0 and net10.0:The Repository client test suite covers:
AddRepositoryApiClient+BuildServiceProvider()(theory overAllSubApiInterfaces).WithCachingoverride targeting one sub-API lands only on that typed client's registered options (no cross-client bleed).INotARegisteredRepositoryApi) triggersValidateAllOperationsMatched()→InvalidOperationExceptionat startup.Format (
dotnet format src/XtremeIdiots.Portal.Repository.sln --verify-no-changes): clean (no changes required).Code-review sub-agent: no significant issues. Confirmed (a) probe pattern is side-effect free because the overridden
WithCachingonly stores the delegate, (b)ValidateAllOperationsMatched()runs after every typed client has been visited becauseAddTypedApiClientinvokes the configure delegate eagerly at registration time, (c)WithSharedCachingcomposes correctly with the pre-existingAddDefaultCachePolicies<T>singleton lookup.Risk and rollout
version.jsonuntouched).XtremeIdiots.Portal.Repository.Api.Client.V1should upgrade to the new package to pick up the fix.Agent attestation
AGENTS.mdbefore starting work.version.json,Directory.Build.props, or.github/workflows/.DataLib(no schema change; regeneration not required)..WithCaching(Action<CacheBuilder>),RepositoryApiOptionsBuilder) remain source-compatible.code-reviewsub-agent run; no High/Medium findings.src/XtremeIdiots.Portal.Repository.Api.Client.V1/(verified by grep).