Skip to content

Add SharedCacheConfiguration for unified API client registrations - #248

Merged
frasermolyneux merged 2 commits into
mainfrom
agents/unified-api-client-registration-fix
Aug 3, 2026
Merged

Add SharedCacheConfiguration for unified API client registrations#248
frasermolyneux merged 2 commits into
mainfrom
agents/unified-api-client-registration-fix

Conversation

@frasermolyneux

Copy link
Copy Markdown
Owner

Summary

Add a first-class, reflection-free SharedCacheConfiguration capability so a single cache-configuration delegate can be applied across many typed sub-API registrations without throwing for sibling sub-APIs. Locks the fix with a DI-composition regression test that would have caught the production incident.

Closes the production outage where a downstream unified client (XtremeIdiots Portal Repository client) crashed Functions hosts at startup with ArgumentException: The expression must invoke a method declared by ...IAdminActionsApi ... when the shared configureOptions delegate contained cache expressions targeting multiple sub-APIs.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)

Root cause (recap)

ApiClientOptionsBuilder<>.WithCaching(Action<CacheBuilder>) constructs a CacheBuilder scoped to the current typed client type. When the same delegate is replayed across N typed clients (one call per sub-API), an expression declared on IGameServersApi throws the moment the delegate runs while the builder is scoped to a different sub-API (e.g. IAdminActionsApi). The throw happens inside DI registration, before the host is built.

Fix approach (Option A: capture-once, apply-filtered): keep the existing single-client WithCaching(...) scope safety unchanged, and add a new sibling WithSharedCaching(SharedCacheConfiguration) that captures the shared cache intent once and applies only the operations whose declaring interface is assignable from the current typed client. A final ValidateAllOperationsMatched() call surfaces genuine typos where an operation targets an interface that was never registered.

Public API added

// MX.Api.Client.Configuration
public sealed class SharedCacheConfiguration
{
    public SharedCacheConfiguration(Action<CacheBuilder> configure);
    public void ValidateAllOperationsMatched();
}

// MX.Api.Client.Configuration.ApiClientOptionsBuilder<TOptions, TBuilder>
public TBuilder WithSharedCaching(SharedCacheConfiguration sharedConfiguration);

Nothing in the existing public surface changes. WithCaching(...) continues to throw on real single-client scope mismatch, preserving typo safety.

Consumer impact

Published contract change (MX.Api.Client NuGet): additive only — two new public members. No breaking changes.

Downstream migration (XtremeIdiots Portal Repository unified client — currently using private-reflection stopgap):

Before (unsupported, uses private reflection):

services.AddTypedApiClient<IGameServersApi, GameServersApi, TOptions, TBuilder>(configureOptions);
// configureOptions contains .WithCaching(c => c.UseLibraryDefaults()
//     .InMemory<IGameServersApi, Task<...>>(x => x.GetGameServer(default, default), TimeSpan.FromSeconds(60)))
// -> throws when applied to sibling sub-APIs

After (supported, no reflection):

using MX.Api.Client.Configuration;

var sharedCache = new SharedCacheConfiguration(c => c
    .UseLibraryDefaults()
    .InMemory<IGameServersApi, Task<ApiResult<GameServerDto>>>(
        x => x.GetGameServer(default, default), TimeSpan.FromSeconds(60))
    .NotCached<IAdminActionsApi, Task<ApiResult<AdminActionDto>>>(
        x => x.CreateAdminAction(default, default)));

void ConfigureOptions(UnifiedApiOptionsBuilder b) => b
    .WithBaseUrl(baseUrl)
    .WithApiKeyAuthentication(apiKey)
    .WithCachePartition(partition)
    .WithSharedCaching(sharedCache);

services.AddTypedApiClient<IGameServersApi, GameServersApi, UnifiedApiOptions, UnifiedApiOptionsBuilder>(ConfigureOptions);
services.AddTypedApiClient<IAdminActionsApi, AdminActionsApi, UnifiedApiOptions, UnifiedApiOptionsBuilder>(ConfigureOptions);
// ... repeat for each sub-API

sharedCache.ValidateAllOperationsMatched(); // surfaces any operation whose interface was never registered

The unified client's private reflection into _configuredClientType and ApiClientOptionsBase.SetCachePolicyOperation can then be deleted.

Validation evidence

Build

> dotnet build src/MX.Api.Abstractions.sln -nologo
Build succeeded.
    0 Error(s)

Tests (default CI filter — the DI-composition regression test is included here)

> dotnet test src/MX.Api.Abstractions.sln --filter "FullyQualifiedName!~IntegrationTests" --nologo
Passed!  - Failed:     0, Passed:    68, Skipped:     0, Total:    68 - MX.Api.Abstractions.Tests.dll (net9.0)
Passed!  - Failed:     0, Passed:    68, Skipped:     0, Total:    68 - MX.Api.Abstractions.Tests.dll (net10.0)
Passed!  - Failed:     0, Passed:    30, Skipped:     0, Total:    30 - MX.Api.Web.Extensions.Tests.dll (net9.0)
Passed!  - Failed:     0, Passed:    30, Skipped:     0, Total:    30 - MX.Api.Web.Extensions.Tests.dll (net10.0)
Passed!  - Failed:     0, Passed:   142, Skipped:     0, Total:   142 - MX.Api.Client.Tests.dll (net9.0)
Passed!  - Failed:     0, Passed:   142, Skipped:     0, Total:   142 - MX.Api.Client.Tests.dll (net10.0)

New tests (all under default filter):

  • MX.Api.Client.Tests.Extensions.UnifiedApiClientRegistrationTests
    • SharedWithCachingDelegate_AcrossMultipleTypedClients_StillThrows_ProvingRepro — proves the incident under the old API.
    • SharedWithCachingDelegate_ViaSharedCacheConfiguration_ComposesAllTypedClientsCleanly — DI composition succeeds via the new API.
    • SharedCacheConfiguration_OperationsLandOnMatchingClientOnly_NoCrossClientBleed — per-client landing correctness.
    • ValidateAllOperationsMatched_UnregisteredSubApi_SurfacesClearError — real typo still surfaces.
  • MX.Api.Client.Tests.SharedCacheConfigurationTests — 11 focused unit tests for capture, filter/apply, opt-in defaults, and validation semantics (including the regression test that WithCaching single-client scope mismatch still throws).

Format check

> dotnet format src/MX.Api.Abstractions.sln --verify-no-changes
(no output — clean)

Code review

Ran the code-review sub-agent on the change set. Findings:

  • Filtering (IsAssignableFrom) — correct; handles inheritance and skips siblings.
  • ValidateAllOperationsMatched — no false-positive on UseLibraryDefaults()-only usage.
  • Thread-safety of ApplyTo — safe: AddTypedApiClient invokes configureOptions synchronously during DI composition on a single thread (see ApiClientExtensions.cs line 146). Flagged as a note if the delegate ever migrates to a lazy IOptionsFactory path in future.
  • Existing WithCaching scope safety — preserved.
  • Test coverage — all four required scenarios present.

No High or Medium findings requiring resolution.

Risk and rollout

  • Blast radius: additive to MX.Api.Client — two new public members. No existing behavior changes. Downstream consumers not using SharedCacheConfiguration see zero effect.
  • Auto-deploy? No (NuGet library; nbgv/height-derived versioning). A maintainer will tag the release after merge (expected 2.3.77 or 2.4.0).
  • Manual steps post-merge: (1) publish NuGet, (2) downstream (XtremeIdiots Portal Repository client) migrates from private reflection to SharedCacheConfiguration per the "Consumer impact" snippet above.
  • Rollback plan: revert this PR and re-publish the prior NuGet version. Downstream can temporarily reinstate its private-reflection stopgap until the next release.

Agent attestation

  • Read the required-reading list (.github/copilot-instructions.md, personal working preferences, org catalog, stack-specific instructions).
  • Ran dotnet build src/MX.Api.Abstractions.sln — succeeded, 0 warnings, 0 errors.
  • Ran dotnet test src/MX.Api.Abstractions.sln --filter "FullyQualifiedName!~IntegrationTests" — all 480 tests passed across net9.0 + net10.0 TFMs.
  • Ran dotnet format src/MX.Api.Abstractions.sln --verify-no-changes — clean.
  • Ran the code-review sub-agent — no High/Medium findings requiring resolution.
  • No secrets, tokens, connection strings, or GUIDs introduced.
  • No edits to .github/workflows/, version.json, Directory.*.props, or cross-repo contracts.
  • New public API surface documented above (Consumer impact section) so downstream migration is unambiguous.

Introduces first-class, reflection-free support for the `unified API client`
scenario where a single configureOptions delegate is applied across many typed
sub-API clients that share one TOptions/TBuilder type. Previously such a delegate
threw at DI-composition time whenever a cache expression referenced a sibling
sub-API interface, aborting host startup.

- Adds `SharedCacheConfiguration` (public sealed): captures cache intent once
  against an unscoped `CacheBuilder` and applies operations per typed client
  filtered by declaring-type assignability. Exposes
  `ValidateAllOperationsMatched` to surface real typos (operations whose
  declaring interface never matched a registered sub-API).
- Adds `ApiClientOptionsBuilder<TOptions, TBuilder>.WithSharedCaching(...)`:
  the per-registration entry point that applies a shared configuration to the
  current typed client, skipping sibling sub-APIs instead of throwing.
- Preserves the existing `WithCaching(...)` behavior for single-client
  registrations (still throws on real scope mismatch).
- Adds a DI-composition regression test (runs under the default CI filter) that
  reproduces the production incident with the old API and proves the new API
  composes many sub-APIs cleanly with no cross-client bleed, plus a negative
  test for unregistered-interface typos.
- Adds focused unit tests for capture, filter-and-apply, opt-in library defaults,
  and validation semantics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 3, 2026 06:08
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a first-class, reflection-free way to share a single caching configuration across multiple typed sub-API client registrations, preventing DI-time failures when the shared delegate includes cache expressions for sibling sub-APIs. It introduces SharedCacheConfiguration plus a new WithSharedCaching(...) builder method, and locks the scenario with DI-composition regression tests.

Changes:

  • Added SharedCacheConfiguration to capture cache operations once and apply only operations that match the current typed client during registration.
  • Added ApiClientOptionsBuilder<,>.WithSharedCaching(SharedCacheConfiguration) to apply shared caching safely per sub-API registration.
  • Added focused unit tests and DI-composition regression tests mirroring the unified-client registration pattern.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/MX.Api.Client/Configuration/SharedCacheConfiguration.cs New stateful shared-cache capture/apply + validation surface for unified typed-client registrations.
src/MX.Api.Client/Configuration/ApiClientOptionsBuilder.cs Adds WithSharedCaching(...) to apply shared cache operations filtered by configured client type.
src/MX.Api.Client.Tests/TestClients/UnifiedSubApis.cs Adds fake sub-API interfaces/implementations for unified registration/caching tests.
src/MX.Api.Client.Tests/SharedCacheConfigurationTests.cs Unit tests for capture, filtering, defaults propagation, and validation semantics.
src/MX.Api.Client.Tests/Extensions/UnifiedApiClientRegistrationTests.cs DI-composition regression tests reproducing the incident and validating the new shared-caching path.
Suppressed comments (1)

src/MX.Api.Client/Configuration/SharedCacheConfiguration.cs:116

  • The error text says to verify the declaring interface is registered as a typed client, but the matching logic allows operations to match on inherited interfaces (DeclaringType.IsAssignableFrom(configuredClientType)). The message should reflect that to avoid sending consumers down the wrong path.
        throw new InvalidOperationException(
            "The following shared cache operations did not match any typed API client registered via WithSharedCaching. " +
            "Verify that each declaring interface is registered as a typed API client and that the shared configuration " +
            "is passed to every registration:" + Environment.NewLine + summary);

Comment thread src/MX.Api.Client/Configuration/SharedCacheConfiguration.cs Outdated
Comment thread src/MX.Api.Client/Configuration/SharedCacheConfiguration.cs
Comment thread src/MX.Api.Client/Configuration/SharedCacheConfiguration.cs Outdated
- Document stateful/single-use nature and thread-safety expectations in remarks.

- Include full method signature (return type + parameter types) in the unmatched-operations summary so overloaded methods are unambiguous.

- Clarify ValidateAllOperationsMatched exception docs and message to reflect the actual matching rule (declaring interface assignable from a registered typed client), not strict registration of the declaring interface.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 3, 2026 06:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

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