refactor(cache)!: move the interface forwarders to extension methods by cosmin-staicu · Pull Request #146 · UiPath/dotnet-caching

cosmin-staicu · 2026-09-02T18:24:58Z

Summary

Stacked on #144 — base branch is feat/conditional-add-tryaddasync, so review that one first and this diff shows only the refactor on top.

Six partials across the cache interfaces held 101 default interface methods that were pure forwarders — no behavior of their own, each delegating to a real member. They are now extension methods, which is where such forwarders belong: the interfaces shrink to the operations an implementation actually has to implement, and an implementor writes one member per operation instead of one plus an inherited forwarder they could accidentally override.

42 pre-CachePolicy convenience overloads (ICache.Compat.cs, IHashCache.Compat.cs, ISetCache.Compat.cs), each forwarding to the policy-bearing member with policy: null.
59 blocking sync forwarders (ICacheOfT.Sync.cs, IHashCacheOfT.Sync.cs, ISetCacheOfT.Sync.cs), each blocking on the async member via .AsTask().GetAwaiter().GetResult().

No call site in the repo needed an edit.

Changes

CacheExtensions, HashCacheExtensions (UiPath.Caching.Abstractions) and SetCacheExtensions (UiPath.Caching.Queue) — the 42 forwarders, same UiPath.Caching namespace as the interfaces so no new using is required. [ExcludeFromCodeCoverage] on the class, matching what the Compat files carried per-member.
ICache.Compat.cs, IHashCache.Compat.cs, ISetCache.Compat.cs — deleted. partial comes off ICache / IHashCache / ISetCache, which nothing else extends now. The ICache<T> / IHashCache<T> / ISetCache<T> partials stay — their .Sync.cs halves are unaffected.
CachePolicy? policy is now required on every member that takes one, along with the expiration / setOption parameters that precede it (C# forbids an optional parameter before a required one).
Implementations — MultilayerCache, RedisCache, MultilayerHashCache, RedisHashCache, MultilayerSetCache, RedisSetCache, NullCache, NullHashCache, NullSetCache, plus the DictionaryCache test fake, all drop the defaults so behavior is identical through the interface or the concrete type. The policy ??= DefaultPolicy bodies are untouched: passing null still resolves the default exactly as before.
CacheSyncExtensions, HashCacheSyncExtensions, SetCacheSyncExtensions — the 59 blocking forwarders off the three *.Sync.cs partials. Blocking behavior is unchanged; T becomes a method type parameter inferred from the receiver, so call sites are unchanged and the forwarders stay reachable through the concrete Cache<T> / HashCache<T> / SetCache<T> as well as the interfaces. partial comes off ICache<T> / IHashCache<T> / ISetCache<T>, leaving all three as pure async contracts. This half needed no signature change — the forwarders are distinct names (Get, not GetAsync) rather than overloads of what they forward to, so no instance member shadows them.
Docs — interfaces.md code blocks track the new signatures, all four surfaces gain a note on their extension surface; the stale ICache.Compat.cs reference in the multi-key GetOrAddAsync note and the "every parameter after the generator is optional" line in batch-get-or-add.md are corrected.

Why `policy` had to become required

This is the part worth a second opinion. Making policy required is not cosmetic — it is what makes the extensions reachable at all.

Instance members always beat extension members in overload resolution. While the interfaces still declared policy = null, an applicable interface overload existed for every short call, so cache.GetAsync<T>(key, token) kept binding to the interface and the extension methods were unreachable dead code. I built that intermediate state and it compiled clean, which is exactly the problem — nothing tells you the extensions are never called.

With policy required, no interface overload is applicable to the short forms and there is exactly one way to spell each call. Two follow-on benefits: an implementation no longer gets to declare its own default for "no policy", and the interface stops carrying two spellings of the same operation.

What no longer compiles is an interface call that leaned on the defaults to skip the policy slot positionally. Named arguments (policy:, token:) and the short forms are unaffected.

Compatibility

Source-compatible for callers; binary-breaking for external implementors of any of these interfaces, which is what the 183 entries leaving PublicAPI.Shipped.txt record (142 in Abstractions, 41 in Queue). Three **BREAKING:** CHANGELOG entries under Unreleased.

One call shape does not compile, before or after: Set(pairs) with a single argument, where the KeyValuePair[] overloads carrying TimeSpan? expiration = null and DateTimeOffset? expiration = null tie with the token-only overload. I verified that ambiguity is pre-existing by reproducing the old shape as default interface methods — it fails identically there. No call site uses it, and the async twin SetAsync(pairs) on ICache<T> has the same wart, so I left both alone rather than widen the break.

Test plan

dotnet test — 1503 passed / 0 failed, net8.0 and net10.0, Debug and Release (rebased onto 63794f7).
Debug and Release builds clean; the only warnings are the 8 pre-existing CS0618 StackExchange.Redis obsoletions. Every build error at any point in this refactor was RS0016/RS0017 API-baseline bookkeeping — never a CS error, which is the evidence that call sites are genuinely unchanged.
Compile-probed all 82 affected call shapes against the new surface — 23 short policy-free shapes (11 cache, 12 set) and 59 sync shapes, the latter including calls through the concrete Cache<T> and SetCache<T> — to confirm they bind to the extensions rather than silently resolving elsewhere. Probes removed afterwards.
Forced GenerateDocumentationFile=true to check the <inheritdoc cref> targets in all six new files — zero CS1574/CS1580. Doc generation is off in this repo, so a bad cref would otherwise fail silently.
PublicAPI.Shipped.txt / PublicAPI.Unshipped.txt updated in both packages.
CHANGELOG.md updated.

Also in this PR: two test de-flakes (first commit)

Running the Release suite repeatedly surfaced two pre-existing non-determinisms on net10.0 — 2 failures in 5 runs, a different test each time. Both are unrelated to the refactor and are fixed in 7b72bd9, separated so they can be reviewed or cherry-picked on their own:

ResiliencePipelineFactoryTest.Pipeline_works_as_expected checked that the breaker closes within a fixed 250ms + 4x100ms budget against a DurationOfBreak of 500ms — 150ms of slack, measured from wherever the preceding exception loop happened to finish. Under parallel load the breaker is still open on the fourth probe. Now polls at 20ms under a 30s ceiling, the shape ConnectionStateMonitorTests.WaitUntilAsync already uses.
The four tests asserting ThrowAsync<TimeoutException> passed the ambient xunit token as the caller token, but FactoryTimeout.RunAsync only converts cancellation to TimeoutException while that token is uncancelled (when (linkedCts.IsCancellationRequested && !token.IsCancellationRequested)). If the runner cancels it, a raw TaskCanceledException escapes. Each now uses a CancellationTokenSource it owns, matching GetOrAdd_FactoryTimeout_does_not_swallow_caller_cancellation next door. All four are fixed, not just the one observed failing.

Verified with three consecutive full Release runs, green on both TFMs, two of them at 3m35s–3m59s against a 1m25s baseline — i.e. heavier load than the runs that originally flaked.

One caveat stated plainly: I never captured the original assertion message for the Redis FactoryTimeout failure, so that diagnosis comes from reading the catch filter rather than from the failure text. The filter is a genuine non-determinism either way and the change is a strict improvement, but I would rather flag that than overstate it.

An unresolved third flake (`f198c1f`)

A third intermittent net10.0 failure showed up while working on this — RedisStreamSubjectWriterTests.Unknown_command_quarantine_is_lifted_when_the_connection_reconnects, ~21s against 352ms in isolation, recovered false after its 10s budget. It has no contact with anything this PR changes.

f198c1f closes a real data race found while investigating it: the test flips a captured fail bool from the test thread while the writer's fetch loop reads it from its own, with no barrier — and the neighbouring test in the same file already reads its attempts counter through Interlocked/Volatile, so this flag was the outlier.

That did not fix the flake, and the commit message says so. The test was still seen failing after the change. Two hypotheses are ruled out: the wake is not lost (the retry gate is a SemaphoreSlim, so a Release preceding WaitAsync is preserved — and ReleaseRetryGate deliberately swallows SemaphoreFullException for exactly that case), and it is not this data race. The failure then declined to reproduce across five subsequent loaded runs, so I could not capture the assertion message and the root cause is still open. The race fix stands on its own merits; the flake needs a separate look.

Linked issues

Fixes #

Contributor declaration

I signed off my commits per the DCO (git commit -s).
I am contributing on behalf of my employer, or in the course of employment / using employer resources. _{(If checked, your employer may hold IP rights in this work, which can require a signed CLA — a maintainer will follow up. See CONTRIBUTING.md.)}

Note

Both commits carry a Signed-off-by trailer. The employer box is left for the author to confirm.

🤖 Generated with Claude Code

https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1

Adds a create-if-absent member to ICache and ICache<T> (plus a blocking TryAdd on the typed surface). On Redis-backed caches it maps to StackExchange.Redis When.NotExists — SET key value EX .. NX — a single atomic round-trip with the TTL applied by the same command, so exactly one caller across all nodes wins a given key and a won key is never briefly immortal between the write and a follow-up EXPIRE. Intended for at-most-once semantics keyed by something: idempotency keys, dedup markers whose TTL is the dedup window, electing which replica runs a job. Previously the only NX primitive in the library was IDistributedLock, which is a lease rather than a value store, and GetOrAddAsync's check-then-write is not a substitute — the gap between probe and write is exactly what NX removes. Contract decisions: - false is fail-closed and deliberately ambiguous: the key already existed, or the write could not be completed (disconnected, threw, or a null/default value the cache cannot represent). A caller treating true as "I own this key" is never wrongly told it won. Same conflation IDistributedLock.TryAcquireAsync already documents. - Never deletes. Where SetAsync removes the key when handed a null with CacheNullValues off, TryAddAsync reports false and leaves it untouched. - A win is never downgraded. On MultilayerCache the L2 arbitrates and the L1 write plus invalidation broadcast are best-effort *after* the win — reporting a loss there would strand the entry with no owner until its TTL. - L1 never arbitrates while an L2 exists, since a key absent locally may be present in the shared store. With the L2 disconnected the call returns false rather than granting a local-only claim every node would also get (SetAsync degrades to a local write there). The memory-only provider has no L2, so the local tier arbitrates and exclusion narrows to in-process, serialized by Lock.LocalLockEnabled. Ships as default interface methods so existing implementations keep compiling, per the convention established for the 1.3.0 ICache additions. The default body throws NotSupportedException rather than emulating the operation with a probe followed by a write, which would not be atomic and would silently void the only guarantee the method makes. No multi-key overload: Redis has no atomic multi-key NX, and all-or-nothing versus per-key semantics would be a guess. No hash-surface member: NX there is per-field (HSETNX) and a different shape. Also corrects interfaces.md, which described the IHashCache<T>.SetAsync(.., HashCacheEntryOptions, ..) overload as offering "conditional set, individual field TTL". It offers neither — HashCacheSetOption selects write scope, and there is no per-field TTL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-02T18:25:15Z

🔎 Maintainer heads-up: automated triage flagged this PR as potentially material, so it may need a signed CLA in addition to the DCO sign-off.

Strong signals

adds public API surface (PublicAPI.Unshipped.txt in src/UiPath.Caching.Abstractions, src/UiPath.Caching.Queue, src/UiPath.Caching)
changes shipped public API — possible removal/breaking change (PublicAPI.Shipped.txt in src/UiPath.Caching.Abstractions, src/UiPath.Caching.Queue)

Other signals

large production change (+618 lines under src/)

This is advisory only — the bot does not decide. Please judge against the CLA criteria (material, product-critical, patent-sensitive, corporate contributor, broad commercial use). Note that thresholds can be gamed by splitting PRs, so use your judgement.

If a CLA is needed → add the cla-required label (a contributor comment with signing steps is posted automatically).
If it is not needed → replace needs-cla-review with cla-not-required so later pushes don't re-flag it.

Review follow-up on the conditional add. The contract the member exists to make is "a caller told true owns this key"; several paths could break it. MultilayerCache, memory-only tier: AcquireLocalLockAsync answered null both when Lock.LocalLockEnabled was off and when the acquire timed out, and the probe-then-write ran unserialized anyway — 11 of 32 concurrent callers were told they added the key with the lock disabled, 20 of 32 with a contended acquire. The local lock is the whole guarantee here rather than a single-flight optimization, so it is now taken regardless of Lock.LocalLockEnabled, and a caller that cannot acquire it within Lock.LocalLockTimeout is told it lost. NullCache.TryAddAsync returns false. It is the one member where the type does not degrade to "caching is off, carry on": it cannot complete the write, which is what a fail-closed false means, and true there hands every caller a claim of exclusive ownership. It is also reached by accident, being what ICacheFactory.CreateCache resolves to when the requested provider is absent or has Enabled=false, so the old true turned at-most-once into at-least-once with no error. NullSetCache.AddAsync — SADD, the same question — already answered false; this aligns the two. Local arbitration is restricted to the InMemory provider. NullCache is the L2 both for the memory-only provider by design and for any provider whose real L2 was absent or disabled, since CacheFactory falls back to it — so an InMemoryRedis configured with DefaultCache=Redis and no Redis provider was routed through LocalTryAddAsync and handed every process its own winner, under a provider name that promises cross-node exclusion. It now reaches the L2 and takes NullCache's fail-closed false, and the construction-time warning covers this composition as well as a nested multilayer L2. A nested in-process arbiter fails closed instead of being delegated to. An InMemoryRedis whose DefaultCache is InMemory resolves that provider's multilayer cache as its L2, and delegating let its local arbiter grant a win per process under a provider name that promises cross-node exclusion; a construction-time warning did not change that, so the add path now reports false. MemorySet reporting success is no longer taken as retention on the local path. A size-limited IMemoryCache declines an entry it cannot fit without throwing, and MemoryCacheSetter still returns true, so the key was probed after the write. The L2 gate no longer runs through GetInnerCacheDisconnected, whose state aggregates the broadcast transport as well as the inner cache: with UseLocalOnlyWhenDisconnected on, a dead topic stopped an otherwise healthy Redis from arbitrating, in a method that already treats broadcast as best-effort after a win. The L2 is asked instead and fails closed on its own — RedisCache checks its connection before issuing NX — so a disconnected L2 still yields false rather than a local-only claim. The test that covered this disconnected only the topic provider, which is exactly the case that should now succeed; it is split into that expectation and one on the L2's own answer. A publish that reports false rather than throwing is logged too. CacheSetAsync signals an ordinary failure — a disconnected topic among them — with a false return, which both broadcast sites discarded, so a win could leave peers on stale L1 data without the propagation warning the code promises. The local lock serializes conditional adds against each other only: SetAsync and RemoveAsync take no lock, so a set landing between the probe and the write is overwritten by the claim, which still reports true. IMemoryCache has no create-if-absent primitive to close that with, and locking every local mutation is a change to a hot path well outside this member, so the limit is documented on the method, in the recipe and in the interfaces.md exclusion column instead. Redis has no such gap, NX being atomic against a concurrent SET. On the L2-win path the invalidation broadcast and the L1 write are now separate best-effort steps: sharing one try/catch meant a dead topic also cost the winning node its local copy, though they are described as independent. The local path publishes no broadcast. It runs only on the InMemory provider, and ChangeTokenFactory accepts only CacheRemoved and CacheRefreshed there, so peers ignore CacheSet — deliberately, since each node's memory is the store rather than a copy of a shared one, and a peer's write says nothing about this node's entry. A non-positive effective local retention reports false on the InMemory provider, where it is the only retention: MemoryCacheSetter writes an entry IMemoryCache evicts on arrival and still returns true, so every later caller would win too. CachePolicyFactoryValidator catches the options-level value, but a per-call CachePolicy is not validated at all, which is the path that reaches this. An expiration that is not in the future now reports false on both tiers. IMemoryCache evicts such an entry on the way in, so MemorySet reported success while retaining nothing and the next caller won too; Redis rejected the negative PX and answered false. A win on the local tier now publishes the invalidation broadcast, as SetAsync does — without it a broadcast-enabled memory provider leaves peers serving a stale copy of a key this node believes it just claimed. An inner cache's NotSupportedException is no longer swallowed into false. That exception is the ICache default body saying the store has no atomic create-if-absent primitive; reported as false it is indistinguishable from permanent contention, so no caller ever wins and the guarded work silently never runs. A cancellation raised while the write is in flight now propagates rather than being reported as false, which would assert the key belongs to someone else — a fact the cancelled call never established. Both tiers do this, so InMemoryRedis and Redis agree. The write itself stays on the shared Write resilience pipeline: retries fire on exceptions only, and re-issuing SET .. NX is harmless — an attempt whose reply was lost is refused by the key it just wrote and reports the same false the exception would have, while an attempt that never reached Redis is recovered as the true it should have been. Contrast SPOP behind ISetCache.PopAsync, where a retry pops a second item and loses the first, which is why RedisSetCacheOptions.ResilienceKeyName exists. MultilayerCache warns at construction when it resolved another multilayer cache as its distributed tier, which arbitrates in-process only under a provider name that suggests otherwise. Reaching that state takes a deliberate misconfiguration — with the default DefaultCache the same composition fails loudly on Lazy re-entrancy instead — so it stays a warning next to the existing innerCache is NullCache test rather than earning a capability member on ICache. The docs no longer offer IConnectionState as a way to tell an outage from a lost race, because it is not one: a serialization or command failure returns false with the connection snapshot still healthy, and IDistributedLock.TryAcquireAsync conflates backend-unavailable with already-held in the same way. The ambiguity is documented as unrecoverable — design the false branch so that not proceeding is safe — and interfaces.md describes IConnectionState as the cache-health signal it actually is. For the same reason the recipe's worked example is now a daily digest rather than a payment capture: a claim marker records that someone started, never that anyone finished, so an at-least-once operation needs a recorded outcome and no branching on false substitutes for one. The XML docs on the conditional-add members are cut to a couple of lines each, pointing at docs/recipes/conditional-add.md for the contract. Every other member of ICache carries no doc comment at all, and the reference docs had drifted from these ones twice already. ICache.Compat.cs gains the three token-positional TryAddAsync forwarders, so cache.TryAddAsync(key, value, ttl, ct) compiles like the SetAsync it is written next to — the only public API this commit adds. RunUnderLocksAsync and AcquireLocalLockAsync now resolve the local-lock policy through one ResolveLocalLock helper instead of two copies of the same expressions. Tests: 21 added, pinning each of the above — including the two paths where the contract actually broke (the lock-disabled and lock-timeout local paths), the NotSupportedException surfacing, and cancellation crossing both tiers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkpKLown2fG6juC2DDiQgS Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Four findings Sonar raised on this PR's changed lines. All three rules sit at their default Info severity, so they surface in the IDE and in Sonar's import but never as build warnings — which is why the build has been clean throughout. CA1859 on RedisCacheTryAddTests._serializer needs care: narrowing the field to SystemJsonSerializerProxy as the rule suggests silently breaks the fixture, because AutoFixture's Inject binds on the argument's *static* type. With a bare Inject the concrete type gets registered, RedisCache resolves a substitute for ISerializerProxy<RedisValue> instead of the real serializer, and TryAdd_writes_a_payload_a_reader_can_deserialize fails with "JsonException: 'v' is an invalid start of a value" — the payload was written raw rather than as JSON. Verified by applying the naive form first. The registration is now pinned with an explicit type argument, with a comment saying why. CA1816 on both DisposeAsync hooks: xunit v3's IAsyncLifetime derives from IAsyncDisposable, so the rule fires on what is really a runner-invoked lifecycle hook. Neither class has a finalizer, so the call is a no-op, but it is a one-liner and keeps the file clean. CA2012 on the NSubstitute arrange: suppressed with a pragma and a justification. NSubstitute intercepts the call and Returns only uses the ValueTask as its receiver — it is never awaited, so there is no single-consumption hazard. Scope note: these rules fire 57 times across the repo (42 CA1816, all in tests; 13 CA1859, 2 of them in src; 2 CA2012), and this commit clears the 4 that Sonar attributed to this PR, leaving 53. The remaining CA1816 and CA2012 hits are the same two false-positive patterns — xunit lifecycle hooks and NSubstitute arranges — so silencing them for the test project in .editorconfig would be a better fix than 40 more SuppressFinalize calls; the 2 src CA1859 hits (MemorySetCache/RedisSetCache Deserialize returning IReadOnlyCollection<T?> where List<T?> would do) are legitimate and worth their own change. Verified: Release build clean (16 warnings, all pre-existing CS0618), 1503/1503 on net8.0 and net10.0, and the 4 findings confirmed gone by temporarily raising the three rules to warning (57 -> 53). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1 Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Two unrelated non-determinisms, both surfaced by running the Release suite under parallel load on net10.0. ResiliencePipelineFactoryTest.Pipeline_works_as_expected checked that the circuit closes using a fixed 250ms + 4x100ms budget against a DurationOfBreak of 500ms — 150ms of slack, measured from wherever the preceding exception loop happened to finish. Under load the breaker is still open on the fourth probe. Replaced with the polling shape already used by ConnectionStateMonitorTests.WaitUntilAsync: 20ms polls under a 30s ceiling, so a slow agent costs latency rather than a failure. The guard CTS goes from 5s to 30s for the same reason — it exists only to stop a hang. The four tests asserting ThrowAsync<TimeoutException> passed the ambient xunit token as the *caller* token. FactoryTimeout.RunAsync only converts cancellation to TimeoutException while that token is uncancelled: catch (OperationCanceledException) when (linkedCts.IsCancellationRequested && !token.IsCancellationRequested) so the assertion depended on the runner not cancelling it, and a raw TaskCanceledException escapes when it does. Each now uses a CancellationTokenSource it owns, matching GetOrAdd_FactoryTimeout_does_not_swallow_caller_cancellation next door. The 50ms FactoryTimeout is what bounds these calls, so dropping the ambient token cannot hang them; the batch test keeps it on its Task.WhenAny guard. All four are fixed, not just the one observed failing — the Multilayer, Multilayer hash and both RedisCacheTests cases share the same defect. Verified with three consecutive full Release runs, 1498/1498 on net8.0 and net10.0, two of them at 3m35s-3m59s against a 1m25s baseline (i.e. heavier load than the runs that originally flaked). Before: 2 failures in 5 runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1 Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

…hods ICache.Compat.cs, IHashCache.Compat.cs and ISetCache.Compat.cs carried 42 convenience overloads as default interface methods — GetAsync<T>(key, token), SetAsync<T>(key, value, expiration, token), TryAddAsync<T>(key, value, token), AddAsync<T>(key, item, token), PopAsync<T>(key, token) and so on — each forwarding to the policy-bearing member with policy: null. They are now extension methods on CacheExtensions, HashCacheExtensions and SetCacheExtensions, in the same UiPath.Caching namespace, so no call site needed an edit. The interfaces shrink to just the policy-bearing members. An implementation now writes one member per operation instead of one plus an inherited forwarder it could accidentally override, and no implementation in the repo had declared the forwarders, so nothing had to be rewritten. CachePolicy? policy also becomes *required* on every member that takes one, along with the expiration / setOption parameters that precede it (C# forbids an optional parameter before a required one). This is what makes the extensions load-bearing rather than decorative: instance members always beat extension members in overload resolution, so while the interfaces still declared policy = null an applicable interface overload existed for every short call and the extensions were unreachable. With policy required there is exactly one way to spell each call, and an implementation no longer gets to declare its own default for "no policy". Implementations drop the defaults too — MultilayerCache, RedisCache, their hash counterparts, MultilayerSetCache, RedisSetCache, NullCache, NullHashCache, NullSetCache — so behavior is identical whether the call goes through the interface or the concrete type. The policy ??= DefaultPolicy bodies are untouched, so passing null still resolves the default exactly as before. The typed ICache<T> / IHashCache<T> / ISetCache<T> facades are unchanged; they never had a policy parameter. Binary-breaking for external implementors: 127 entries leave PublicAPI.Shipped.txt across the two packages. Verified: Debug and Release builds clean (only the 8 pre-existing CS0618 warnings), 1498/1498 tests on net8.0 and net10.0. Every existing call site compiled unchanged — the only build errors at any point were RS0016/RS0017 baseline bookkeeping. Compile-probed all 23 short call shapes to confirm they bind to the extensions, and forced GenerateDocumentationFile on to confirm the inheritdoc crefs in the three new files resolve (doc generation is off in this repo, so bad crefs fail silently). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1 Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Same treatment as the Compat partials, applied to ICacheOfT.Sync.cs, IHashCacheOfT.Sync.cs and ISetCacheOfT.Sync.cs. The 59 blocking forwarders they carried as default interface methods — Get, GetOrAdd, Set, TryAdd, Refresh, Remove, Contains, TimeToLive, ExpireTime, the hash surface's GetItem, GetCacheEntry, GetMetadata and SetMetadata, and the set surface's Add, Pop, Members, ContainsItem, Count, RemoveItem and RemoveItems — now live on CacheSyncExtensions, HashCacheSyncExtensions and SetCacheSyncExtensions. Each still blocks on the async member via .AsTask().GetAwaiter().GetResult(); nothing about the blocking behavior changed. T becomes a method type parameter inferred from the receiver, so call sites are unchanged, and the forwarders stay reachable through the concrete Cache<T> / HashCache<T> / SetCache<T> classes as well as the interfaces. No implementation declared them, so nothing had to be rewritten. partial comes off ICache<T>, IHashCache<T> and ISetCache<T>, which nothing else extends now. That leaves all three as pure async contracts: an implementation writes only the members it implements rather than inheriting blocking forwarders it could accidentally override. Unlike the Compat move this needs no signature change, because the forwarders are distinct names (Get, not GetAsync) rather than overloads of the members they forward to — so no instance member shadows them. Verified: Release build clean (16 warnings across both TFMs, all pre-existing CS0618), 1503/1503 tests on net8.0 and net10.0. Compile-probed all 59 sync call shapes, including through the concrete Cache<T> and SetCache<T>, then removed the probes. 56 entries leave PublicAPI.Shipped.txt (43 Abstractions, 13 Queue) and 3 leave Unshipped.txt, replaced by 62 extension entries. One call shape does not compile, before or after: Set(pairs) with a single argument, where the KeyValuePair[] overloads with `TimeSpan? expiration = null` and `DateTimeOffset? expiration = null` tie with the token-only overload. That ambiguity is pre-existing and was verified against a default-interface-method reproduction of the old shape — no call site uses it, and the async twin SetAsync(pairs) has the same wart on ICache<T>. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1 Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

Found while investigating a third intermittent net10.0 failure: RedisStreamSubjectWriterTests.Unknown_command_quarantine_is_lifted_when_the_connection_reconnects flips a captured `fail` bool from the test thread while the writer's fetch loop reads it from its own thread, with no barrier. The neighbouring test in the same file already reads its `attempts` counter through Interlocked/Volatile; this flag was the outlier. Now written with Volatile.Write before the retry-gate release and read with Volatile.Read, so a thread observing the release also observes the flip. This does NOT fix the flake. The test was still seen failing under parallel load after this change, with `recovered` false after its 10s budget and the same ~21s duration. Two hypotheses are ruled out: the wake is not lost (the retry gate is a SemaphoreSlim, so a Release preceding WaitAsync is preserved — and ReleaseRetryGate deliberately swallows SemaphoreFullException for exactly that case), and it is not this data race. I could not capture the assertion message: the failure did not reproduce in five subsequent loaded runs, so the root cause is still open. Committing the race fix on its own merits rather than leaving an unsynchronized cross-thread flag in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015RLSnsYiZsqeaHHYYybXW1 Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>

cosmin-staicu requested review from alinahornet, cosminvlad and litheon as code owners September 2, 2026 18:25

cosmin-staicu force-pushed the feat/conditional-add-tryaddasync branch from ccfc7eb to 2ae1a9e Compare September 2, 2026 18:25

cosmin-staicu requested review from alinahornet, cosminvlad, litheon, lucianaparaschivei and razvalex as code owners September 2, 2026 18:25

github-actions Bot added the needs-cla-review A maintainer should assess whether a signed CLA is required (see CONTRIBUTING.md) label Sep 2, 2026

cosmin-staicu force-pushed the refactor/compat-to-extensions branch from 450f68a to ad7c491 Compare September 2, 2026 18:30

cosmin-staicu force-pushed the feat/conditional-add-tryaddasync branch 2 times, most recently from 5fbd334 to ab54312 Compare September 2, 2026 18:46

cosmin-staicu force-pushed the refactor/compat-to-extensions branch from ad7c491 to bf7b1f6 Compare September 2, 2026 18:56

cosmin-staicu changed the title ~~refactor(cache)!: move the pre-CachePolicy overloads to extension methods~~ refactor(cache)!: move the interface forwarders to extension methods Sep 2, 2026

cosmin-staicu force-pushed the refactor/compat-to-extensions branch from bf7b1f6 to 3bbf697 Compare September 2, 2026 19:03

cosmin-staicu force-pushed the feat/conditional-add-tryaddasync branch from ab54312 to 63794f7 Compare September 2, 2026 19:07

cosmin-staicu force-pushed the refactor/compat-to-extensions branch from 3bbf697 to f198c1f Compare September 2, 2026 19:28

cosmin-staicu and others added 5 commits September 3, 2026 00:28

cosmin-staicu force-pushed the refactor/compat-to-extensions branch from f198c1f to 628b6fd Compare September 2, 2026 21:32

cosmin-staicu force-pushed the feat/conditional-add-tryaddasync branch from 1526da7 to 7208d58 Compare September 3, 2026 04:40

cosmin-staicu force-pushed the feat/conditional-add-tryaddasync branch 3 times, most recently from 2722e0a to b0b4171 Compare September 3, 2026 08:17

This was referenced Sep 3, 2026

feat(cache)!: make the per-call expiration non-nullable #150

Open

feat(cache)!: floor the resolved expiration at one hour #151

Open

CalinMPopa force-pushed the feat/conditional-add-tryaddasync branch 2 times, most recently from 5e8f1a9 to 4b2e0b5 Compare September 3, 2026 14:06

Base automatically changed from feat/conditional-add-tryaddasync to main September 3, 2026 14:25

Navigation Menu

Uh oh!

refactor(cache)!: move the interface forwarders to extension methods - #146

refactor(cache)!: move the interface forwarders to extension methods#146
cosmin-staicu wants to merge 7 commits into
mainfrom
refactor/compat-to-extensions

cosmin-staicu commented Sep 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cosmin-staicu commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Why policy had to become required

Compatibility

Test plan

Also in this PR: two test de-flakes (first commit)

An unresolved third flake (f198c1f)

Linked issues

Contributor declaration

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cosmin-staicu commented Sep 2, 2026 •

edited

Loading

Why `policy` had to become required

An unresolved third flake (`f198c1f`)