[BREAKING]: Normalizes ICacheClient IncrementAsync TTL behavior, Removes ListRemoveAsync ExpiresIn argument to align with all other Removes, Hybrid Cache Improvements

[niemyjski]

Normalizes the behavior of IncrementAsync regarding TTL handling, aligning it with SetAsync. Previously, IncrementAsync preserved existing TTLs when expiresIn was null. Now, passing null removes the TTL, ensuring consistency across cache operations.

This change simplifies the hybrid cache implementation and avoids potential inconsistencies between L1 and L2 caches.

Removes the expiresIn parameter from the ListRemoveAsync method to simplify its usage, as expiration was not being correctly handled.

Updates documentation and tests to reflect the changes in TTL behavior and the removal of the expiresIn parameter from ListRemoveAsync.

[Copilot]

Pull request overview

This PR introduces breaking changes to normalize TTL behavior across cache operations. The primary changes are:

• IncrementAsync TTL normalization: Changed behavior from preserving existing TTL when expiresIn is null to removing it, aligning with SetAsync behavior for consistency
• ListRemoveAsync simplification: Removed the expiresIn parameter which was not properly implemented and caused confusion

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/Foundatio/Caching/ICacheClient.cs	Updated interface: removed expiresIn from ListRemoveAsync, updated IncrementAsync documentation
src/Foundatio/Caching/InMemoryCacheClient.cs	Updated IncrementAsync to always set TTL instead of conditionally preserving it; updated ListRemoveAsync signature; reordered empty check in SetAllAsync
src/Foundatio/Caching/HybridCacheClient.cs	Simplified IncrementAsync local cache handling to leverage new TTL behavior; added expiration checks to SetIfHigher/SetIfLower methods; updated ListRemoveAsync signature
src/Foundatio/Caching/HybridAwareCacheClient.cs	Updated ListRemoveAsync to remove expiresIn parameter
src/Foundatio/Caching/ScopedCacheClient.cs	Updated ListRemoveAsync to remove expiresIn parameter
src/Foundatio/Caching/NullCacheClient.cs	Updated ListRemoveAsync signature
src/Foundatio/Extensions/CacheClientExtensions.cs	Updated extension methods to remove expiresIn from ListRemoveAsync calls
docs/guide/caching.md	Updated TTL behavior documentation to reflect IncrementAsync changes and ListRemoveAsync parameter removal
docs/guide/implementations/hybrid-cache.md	Updated synchronization strategy documentation for the new IncrementAsync behavior
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.cs	Updated tests to validate new TTL behavior and ListRemoveAsync changes
src/Foundatio.TestHarness/Caching/HybridCacheClientTestBase.cs	Added tests for IncrementAsync with zero expiration; updated local cache count expectations
tests/Foundatio.Tests/Caching/*.cs	Renamed test methods from ListRemoveAsync_WithExpiration_SetsExpirationCorrectly to ListRemoveAsync_WithMultipleItems_RemovesCorrectly

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[niemyjski]

Regarding code comments:

Edge Case: SetIfHigherAsync/SetIfLowerAsync with Value 0

The Issue

When SetIfHigherAsync or SetIfLowerAsync is called with value 0 on a new key, it creates a temporary L1/L2 inconsistency:

await hybridCache.SetIfHigherAsync("newkey", 0.0);

What happens:

1. Key doesn't exist, so distributed cache (L2) creates it with value 0.0 ✅
2. According to the ICacheClient contract, for new keys the method returns the value itself: 0.0
3. HybridCacheClient checks if (difference != 0) → 0.0 != 0 is false
4. Local cache (L1) doesn't set the key
5. Result: Key exists in L2 with value 0.0, but not in L1 ❌

Why This Is Acceptable (Self-Healing Design)

This edge case is documented and by design for the following reasons:

1. Extremely rare in practice: Setting a new counter/metric to exactly 0 is uncommon (usually start at 1 or higher)

2. Self-healing on next read:

   // First read after SetIfHigherAsync with 0
   var value = await cache.GetAsync<double>("newkey");
   // ✅ Fetches 0.0 from L2 and populates L1

3. Conservative approach: The ambiguity of 0 as a return value makes it safer to remove from L1 rather than risk caching incorrect state

4. No data loss: The value is correctly stored in L2 (source of truth) and will be retrieved on the next operation

Alternative Considered: Complex State Tracking

We could theoretically distinguish between "no update" (existing value was already higher/lower) vs "new key set to 0" by:

• Adding additional return metadata
• Making a second call to check if the key exists
• Tracking state across operations

Decision: Not worth the complexity. The self-healing property handles this edge case gracefully, and the added code complexity would impact all operations for an extremely rare scenario.

Documentation

This behavior is documented in /docs/guide/implementations/hybrid-cache.md under "Edge Cases with Zero Values" alongside the similar IncrementAsync edge case.

---

Summary: The difference != 0 fix resolves the negative value bug while maintaining the documented self-healing behavior for the edge case of setting new keys to exactly 0.

[niemyjski]

Bug Fix: SetIfHigherAsync/SetIfLowerAsync Local Cache Synchronization

The Problem

HybridCacheClient was using if (difference > 0) to determine whether to update the local cache (L1) after calling SetIfHigherAsync/SetIfLowerAsync on the distributed cache (L2). This caused a bug when setting new keys with negative values.

Example scenario:

await hybridCache.SetIfHigherAsync("key", -5.0);

1. Key doesn't exist, so distributed cache creates it with value -5.0 and returns -5.0 (the value itself, per the ICacheClient contract)
2. HybridCacheClient checks if (difference > 0) → -5.0 > 0 is false
3. Local cache incorrectly removes the key instead of setting it
4. Result: Key exists in L2 with value -5.0, but not in L1 (inconsistency)

The Root Cause

According to the ICacheClient contract and all implementations (Redis Lua scripts, InMemoryCacheClient):

Scenario	Return Value
New key created	The value itself (can be negative!)
Existing key updated	Positive difference
No update	0

The difference > 0 check incorrectly assumed the return value would always be positive for successful operations.

The Fix

Changed the condition from if (difference > 0) to if (difference != 0) in all four methods:

• SetIfHigherAsync (double and long overloads)
• SetIfLowerAsync (double and long overloads)

This correctly interprets:

• != 0 → Operation succeeded (either new key created or existing key updated), sync to L1
• == 0 → No update occurred, remove from L1 since we don't know the actual value

Test Coverage

Extended the existing SetIfHigherAsync_WithFloatingPointDecimals_ComparesCorrectly and SetIfLowerAsync_WithFloatingPointDecimals_ComparesCorrectly tests to include negative value scenarios:

• Initialize new keys with negative values
• Update existing keys with more/less negative values
• Verify both double and long overloads handle negative values correctly

All 1696 tests pass with the fix in place.

---

This maintains the self-healing property of the hybrid cache while correctly handling the full numeric range including negative values.