ICacheProvider.Get and GetOrAdd treat a legitimately cached null as a cache miss

[matt-edmondson]

What's wrong

ICacheProvider<TKey,TValue>'s default interface methods Get and GetOrAdd (Essentials/ICacheProvider.cs:52-80) check value is null in addition to the TryGet boolean result:

if (!TryGet(key, out TValue? value) || value is null)
    throw new KeyNotFoundException(...);

if (TryGet(key, out TValue? value) && value is not null)
    return value;

For a nullable TValue (e.g. string? or a reference type where null is a legitimate cached result), InMemoryCacheProvider<TKey,TValue> and other implementations correctly store and return value = null with TryGet returning true. But the default Get/GetOrAdd (and the async wrappers built on them, GetAsync/GetOrAddAsync) then treat that as "not found": Get throws KeyNotFoundException even though the key was found, and GetOrAdd silently discards the cached null and re-invokes (and re-Sets) the factory on every single call — defeating caching entirely for any factory that can legitimately produce null.

Failure scenario

A consumer caches the (correct) result of a lookup that returns null — e.g. cache.GetOrAdd("missing-user", () => LookupUser(id)) where LookupUser returns User? and legitimately returns null for a nonexistent id. Every call re-invokes LookupUser instead of reusing the cached null, and any caller using Get for that same key gets an unexpected KeyNotFoundException instead of null.

Suggested fix

Use only the bool result from TryGet, not a null check on value:

if (!TryGet(key, out TValue? value))
    throw new KeyNotFoundException(...);
...
if (TryGet(key, out TValue? value))
    return value!;

Acceptance criteria

A test that seeds a cache entry with a null value via Set(key, null) asserts Get returns null without throwing, and GetOrAdd returns the cached null without invoking the factory again.

[matt-edmondson]

Triage

Category: Bug
Priority: High
Area: Essentials — ICacheProvider<TKey,TValue> default interface methods

High because this is a published library's default interface implementation, so the defect ships to every consumer with a nullable TValue and cannot be worked around without bypassing Get/GetOrAdd entirely. Two distinct failures from one root cause:

• Get throws KeyNotFoundException for a key that was found — a contract violation the caller cannot distinguish from a genuine miss.
• GetOrAdd silently discards the cached null and re-invokes and re-Sets the factory on every call, defeating caching completely for any factory that can legitimately return null. Silent, unbounded, and exactly the case a cache is most often reached for (negative lookups).

The async wrappers GetAsync/GetOrAddAsync inherit both, so the surface is wider than the two methods named.

Suggested assignment: whoever owns the Essentials caching abstractions.

Duplicates: none. Shares a theme with #31 in this repo — both are is not null used as a proxy for "was a value supplied" — but they are separate call sites in unrelated components and should be fixed independently. Worth a quick sweep for the same pattern elsewhere in Essentials while in there.
Open PR covering this: none.

Suggested next step: the fix in the body is right and minimal — trust the bool from TryGet and drop the null check. The value! forgiveness operator in the GetOrAdd path is correct here rather than a papered-over warning, since TryGet returning true is the actual guarantee. Ship it with the Set(key, null) regression test from the acceptance criteria, which is what would have caught this.

---

Generated by Claude Code