Repository navigation
Conversation
The three Cache_On_Local_Without_Redis_* tests raced a fixed 100ms delay against a 100ms local expiry; MemoryCache evaluates absolute expiry lazily against its own clock, which can lag Task.Delay by a few milliseconds, so the entry was occasionally still alive on read. Poll until the entry expires instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CgQoE35KUm7e9AgVoYdxLZ
The worktree itself is long gone and the path is already covered by .gitignore; only the leftover gitlink entry remained, showing up as a permanent deletion in git status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S5FvtrvDZo72RrQMx8KPyZ
…cache lookups Replaces the prometheus-net dependency with System.Diagnostics.Metrics, so hosts collect through OpenTelemetry (or any MeterListener) by registering the meter: AddMeter(KeyMeter.MeterName). Adds a `hybrid_cache_lookups` counter recording every read, tagged with `cache`, `layer` (local, redis) and `result` (hit, miss). A local miss falls through to redis, so one read can produce two measurements. Both error paths in the read methods now record a redis miss; previously only the deserialization failure did, so reads lost to a connection error vanished from the counter. The data-size histogram is shared by instrument name rather than created per KeyMeter instance: its meter is static, so a per-instance instrument published a duplicate per cache and was never released. Both instruments carry the `cache` tag (InstancesSharedName) so several caches in one process stay distinguishable — cardinality is the number of caches, not keys. Documents the meter in README.md, since existing EnableMeterData users now have to register it or collect nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
Both are non-packable apps, so this only requires the .NET 10 SDK locally. The library itself still multi-targets net8.0;net9.0;net10.0, so consumer support is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
Library: - StackExchange.Redis 3.0.17 -> 3.3.1 (raises the minimum consumers resolve) - MessagePack 3.1.8 -> 3.1.10 - Microsoft.Extensions.Caching.Memory 10.0.10 -> 10.0.12 Tests: - Microsoft.Garnet 1.1.10 -> 2.1.8 (major; the in-process fixture needed no change) - xunit.runner.visualstudio 3.1.5 -> 4.0.0 (major; still discovers the xunit v2 suite) - Microsoft.NET.Test.Sdk 18.8.1 -> 18.10.1 - Testcontainers.Redis / Testcontainers.Xunit 4.13.0 -> 4.15.0, which drops the transitive SSH.NET 2025.1.0 carrying two high-severity advisories (NU1903). Redis 3.3.1 deprecated RedisConnectionException(ConnectionFailureType, string), so the connect-time throw moves to the overload taking CommandFlags; no command is in flight there, hence CommandFlags.None. The build is warning-free again. MemoryPack, Newtonsoft.Json, BenchmarkDotNet, EasyCaching.* and xunit itself were already current (xunit 2.9.3 is the latest v2; v3 is a separate package). Verified: all three TFMs build with 0 warnings, 409/409 tests pass, no vulnerable packages remain, and the nupkg records the new dependency floors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
xunit 2.9.3 -> xunit.v3 4.0.1, Testcontainers.Xunit -> Testcontainers.XunitV3. xunit v3 runs on Microsoft.Testing.Platform and the .NET 10 SDK removed the VSTest bridge for it, so `dotnet test` has to use the MTP runner: global.json opts in, and Microsoft.NET.Test.Sdk, xunit.runner.visualstudio and coverlet.collector (all VSTest components) give way to Microsoft.Testing.Extensions.CodeCoverage. CI follows: the solution goes through --solution, and coverage is --coverage --coverage-output-format cobertura instead of --collect:"XPlat Code Coverage". API changes: - Xunit.Abstractions is gone; ITestOutputHelper now lives in Xunit. - IAsyncLifetime returns ValueTask, so InProcessRedisFixture changes shape. Two regressions this would have introduced silently, both fixed here: - ContainerTest implements IAsyncLifetime/IAsyncDisposable explicitly in v3, so BaseCacheTest.DisposeAsync() still compiled but was never called and the HybridCache was never disposed. Moved to the DisposeAsyncCore() hook. - v3 enforces [Fact(Timeout = ...)] by cancelling TestContext.Current.CancellationToken. The five tests declaring a Timeout ignored it, so two that block on semaphore.WaitAsync() waiting for a bus message would hang the whole run instead of failing. They now pass the token. xUnit1051 (pass TestContext.Current.CancellationToken everywhere) fires ~1000 times and is advisory, so it is suppressed project-wide; the tests that depend on the token for real pass it explicitly. Docs: VSTest's --filter "FullyQualifiedName~..." no longer exists. The no-Docker subset now runs through xunit's own runner (dotnet run) with its query filter language; MTP's --filter does not accept those queries. Verified: 0 warnings, 409/409 tests pass, and the CI command produces TestResults/coverage.cobertura.xml for Codecov. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
…051 suppression) Removes <NoWarn>xUnit1051</NoWarn> and fixes all 509 sites instead. xunit cancels this token on run cancellation and when a test's Timeout elapses, so a hung call now ends the test rather than the run. BaseCacheTest and InProcessCacheTest expose `TestToken` (TestContext.Current.CancellationToken); CacheLookupMeteringTests, which has no base class, declares its own. The analyzer accepts the helper property, so call sites stay readable. Two wrinkles worth knowing: - The argument is `token:` across most of the API but `cancellationToken:` on the lock methods (TryLockKeyAsync, LockKeyAsync, TryExtendLockAsync, TryReleaseLockAsync), where `token` already names the lock's own token. That mismatch is why 39 sites first failed to compile with CS1744/CS1739. - AsyncMethods_WithDefaultToken_CompleteNormally keeps the default token behind a narrow pragma: it exists to cover the fast path that skips WaitAsync when the token cannot be cancelled, and a real token would have quietly made it assert the opposite. Verified: solution builds with 0 warnings and no suppression, 409/409 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
…eaked MemoryCache Three defects found by review, each with a regression test that fails without its fix. **The data retriever ran for every caller.** `_dataRetrieverTasks` stored the retriever delegate rather than its in-flight task, so `GetOrAdd` de-duplicated nothing: 20 concurrent misses on one key produced 20 executions. Worse, a caller that lost the race executed the *winner's* delegate, so it could receive a value produced by someone else's closure, and when two different `T`s shared a key the cast failed and the method returned `default` without calling any retriever and without an error. The dictionary now holds a `Lazy<Task>`: the first caller runs the retriever and the rest await that task. Callers sharing a key share one result by design, which is now documented on the method. Removal uses the compare-and-remove `TryRemove` overload so a late caller cannot evict a task others still await. **Clearing the local cache raced with every read of it.** `ClearLocalMemory` took `lock (_memoryCache)`, then disposed that instance and reassigned the field — so the lock guarded an object it was about to replace (two clears could hold different objects) while readers, which never took the lock, could touch a disposed `MemoryCache` and throw `ObjectDisposedException`. It runs on the bus thread and on reconnect, concurrently with application reads. Both caches are now `readonly` instances emptied in place with `MemoryCache.Clear()`, so no reference ever changes and no lock is needed. **`_recentlySetKeys` was never disposed.** `Dispose`/`DisposeAsync` released only `_memoryCache`, leaking the second cache's timer per instance. Also drops the redundant `RedisDb.Multiplexer` dispose, which is the same object as `_connection`. Also aligns `IHybridCache.Exists`'s default flag with the implementation. This one is cosmetic, not a behaviour change: `Flags.None` and `Flags.PreferMaster` are both `0`. The new `InterfaceOptionalArguments_MatchTheImplementation` theory walks the interface map of both contracts (77 members) and fails on real drift, which is what the ExistsAsync class of bug needed to be caught automatically. Verified: solution builds with 0 warnings, 415/415 tests pass, and the three regression tests fail against the previous implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
Two ways the reconnect loop's health check answered "down" for a server that was fine, each costing a full tear-down and rebuild of the connection: - `ConnectRetry = 0` made the retry loop's body never run, so the method returned false without sending a single PING. It now always attempts at least once. - A reply slower than two seconds counted as a failure. A reply means the server is reachable, however loaded; latency is no longer a verdict. Also drops the 500ms sleep that followed the final failed attempt, which delayed the answer without buying another try. `SelfWriteNotificationWindow` replaces the hard-coded five seconds an instance remembers its own write for. The default is unchanged, so behaviour is the same; the point is that the trade-off is now documented and reachable: too short costs a local miss when a notification is delayed, too long risks mistaking another instance's write for our own and serving a stale local value. Worth recording: the suspected self-invalidation on repeated writes to one key does not reproduce. Two probes on real Redis (asserting on the local hit/miss counters, 2 writes and 25 writes in a loop) both showed the local copy surviving, so the markers and notifications stay paired in practice. No fix was warranted and none was made. Verified: 0 warnings, 420/420 tests pass, and the new PingRetryTests fail against the previous implementation for retryCount 0 and -1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017zuwcYdNke8tXabBtpJerC
…on claim Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 7 medium |
| CodeStyle | 18 minor |
🟢 Metrics 4 complexity
Metric Results Complexity 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…nd server admin commands RENAME now invalidates the local copies of both keys on every instance (rename_from / rename_to key-space events). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
…&D change Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
Sync GetAll blocked on pipelined tasks (SER308), which can starve the thread pool; it now uses the sync Redis API. Test pattern formatting is now culture-safe (CA1305). CLAUDE.md gets a local analyzer gate that mirrors Codacy, since app.codacy.com is not reachable from the sandbox. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
- servers[0] instead of First() (S6608), lambda parameter in GetOrAdd (S6612) - regression test now asserts (S2699); in-process test awaits the async flush (S6966) - Markdown: fence languages, blank lines, no $ prompts; .markdownlint.json with 120-char lines - CLAUDE.md: how to run SonarC# and markdownlint locally, plus the new rules Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
This branch was successfully deployed
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
GetAsyncwith a data retriever runs once per key for concurrent misses).Docs only, no code change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FW752zWEpcuVRaYy5a6SEQ
Generated by Claude Code