Skip to content

Commit efea397

Browse files
#49 Support MaxValue "never expire" sentinels; polish range errors and logs
Address review feedback from mtmk: - Treat DateTimeOffset.MaxValue / TimeSpan.MaxValue expirations as "cache forever" (no TTL) instead of rejecting them as out-of-range — a common "never expire" idiom, normalized to no expiration in GetTtl/CreateCacheEntry. - Include the maximum window in the range-check exception messages. - Use PascalCase structured log property names ({Key}). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matthew DeVenny <matt@codecargo.com>
1 parent ee12ffa commit efea397

5 files changed

Lines changed: 94 additions & 32 deletions

File tree

‎src/NatsDistributedCache/CacheEntryBinarySerializer.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ internal sealed class CacheEntryBinarySerializer : INatsSerialize<CacheEntry>, I
3737
/// </summary>
3838
internal const long MaxTtlTicks = int.MaxValue * TimeSpan.TicksPerSecond;
3939

40+
/// <summary>
41+
/// <see cref="MaxTtlTicks"/> as a <see cref="TimeSpan"/>, for range-check exception messages.
42+
/// </summary>
43+
internal static readonly TimeSpan MaxTtl = TimeSpan.FromTicks(MaxTtlTicks);
44+
4045
private const byte HasAbsoluteExpiration = 0b0000_0001;
4146
private const byte HasSlidingExpiration = 0b0000_0010;
4247
private const byte KnownFlags = HasAbsoluteExpiration | HasSlidingExpiration;

‎src/NatsDistributedCache/NatsCache.Log.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ private void LogSwallowedException(Exception exception) =>
1616
private void LogUndeserializableEntry(string key) =>
1717
_logger.LogDebug(
1818
EventIds.UndeserializableEntry,
19-
"Cache entry for key {key} could not be deserialized (legacy or corrupt format); returning a cache miss",
19+
"Cache entry for key {Key} could not be deserialized (legacy or corrupt format); returning a cache miss",
2020
key);
2121

2222
private static class EventIds

‎src/NatsDistributedCache/NatsCache.cs‎

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -206,50 +206,56 @@ public async ValueTask<bool> TryGetAsync(
206206

207207
internal TimeSpan? GetTtl(DistributedCacheEntryOptions options)
208208
{
209-
if (options.AbsoluteExpiration.HasValue && options.AbsoluteExpiration.Value <= TimeProvider.GetUtcNow())
209+
// Maximum-value sentinels mean "never expire": normalize them to no expiration so they are not
210+
// rejected as out-of-range below. Callers commonly use DateTimeOffset.MaxValue / TimeSpan.MaxValue
211+
// as a "cache forever" idiom, which is equivalent to omitting expiration entirely.
212+
var absoluteExpiration = EffectiveAbsoluteExpiration(options);
213+
var relativeToNow = EffectiveRelativeToNow(options);
214+
var slidingExpiration = EffectiveSlidingExpiration(options);
215+
216+
if (absoluteExpiration.HasValue && absoluteExpiration.Value <= TimeProvider.GetUtcNow())
210217
{
211218
throw new ArgumentOutOfRangeException(
212219
nameof(DistributedCacheEntryOptions.AbsoluteExpiration),
213-
options.AbsoluteExpiration.Value,
220+
absoluteExpiration.Value,
214221
"The absolute expiration value must be in the future.");
215222
}
216223

217-
if (options.AbsoluteExpirationRelativeToNow.HasValue &&
218-
options.AbsoluteExpirationRelativeToNow.Value <= TimeSpan.Zero)
224+
if (relativeToNow.HasValue && relativeToNow.Value <= TimeSpan.Zero)
219225
{
220226
throw new ArgumentOutOfRangeException(
221227
nameof(DistributedCacheEntryOptions.AbsoluteExpirationRelativeToNow),
222-
options.AbsoluteExpirationRelativeToNow.Value,
228+
relativeToNow.Value,
223229
"The relative expiration value must be positive.");
224230
}
225231

226-
if (options.SlidingExpiration.HasValue && options.SlidingExpiration.Value <= TimeSpan.Zero)
232+
if (slidingExpiration.HasValue && slidingExpiration.Value <= TimeSpan.Zero)
227233
{
228234
throw new ArgumentOutOfRangeException(
229235
nameof(DistributedCacheEntryOptions.SlidingExpiration),
230-
options.SlidingExpiration.Value,
236+
slidingExpiration.Value,
231237
"The sliding expiration value must be positive.");
232238
}
233239

234240
// Reject sliding windows the serializer/TTL encoding could not round-trip. The read path fails
235241
// closed above MaxTtlTicks, so accepting a larger value here would silently store an entry that
236242
// later reads back as an undeserializable cache miss.
237-
if (options.SlidingExpiration.HasValue &&
238-
options.SlidingExpiration.Value.Ticks > CacheEntryBinarySerializer.MaxTtlTicks)
243+
if (slidingExpiration.HasValue &&
244+
slidingExpiration.Value.Ticks > CacheEntryBinarySerializer.MaxTtlTicks)
239245
{
240246
throw new ArgumentOutOfRangeException(
241247
nameof(DistributedCacheEntryOptions.SlidingExpiration),
242-
options.SlidingExpiration.Value,
243-
"The sliding expiration value is too large.");
248+
slidingExpiration.Value,
249+
$"The sliding expiration value is too large. The maximum is {CacheEntryBinarySerializer.MaxTtl}.");
244250
}
245251

246-
var absoluteExpiration = ResolveAbsoluteExpiration(options);
247-
if (!absoluteExpiration.HasValue)
252+
var resolvedAbsolute = ResolveAbsoluteExpiration(options);
253+
if (!resolvedAbsolute.HasValue)
248254
{
249-
return options.SlidingExpiration;
255+
return slidingExpiration;
250256
}
251257

252-
var ttl = absoluteExpiration.Value - TimeProvider.GetUtcNow();
258+
var ttl = resolvedAbsolute.Value - TimeProvider.GetUtcNow();
253259
if (ttl.TotalMilliseconds <= 0)
254260
{
255261
// Value is in the past, remove it
@@ -258,28 +264,28 @@ public async ValueTask<bool> TryGetAsync(
258264

259265
// If there's also a sliding expiration, use the minimum of the two. Sliding is bounded to
260266
// MaxTtlTicks above, so the minimum is always within the encodable range.
261-
if (options.SlidingExpiration.HasValue)
267+
if (slidingExpiration.HasValue)
262268
{
263-
return TimeSpan.FromTicks(Math.Min(ttl.Ticks, options.SlidingExpiration.Value.Ticks));
269+
return TimeSpan.FromTicks(Math.Min(ttl.Ticks, slidingExpiration.Value.Ticks));
264270
}
265271

266272
// Absolute-only: the TTL spans the full window to the absolute instant. Reject windows the NATS
267273
// TTL encoding cannot represent (see CacheEntryBinarySerializer.MaxTtlTicks) rather than emitting
268274
// an overflowed header.
269275
if (ttl.Ticks > CacheEntryBinarySerializer.MaxTtlTicks)
270276
{
271-
if (options.AbsoluteExpirationRelativeToNow.HasValue)
277+
if (relativeToNow.HasValue)
272278
{
273279
throw new ArgumentOutOfRangeException(
274280
nameof(DistributedCacheEntryOptions.AbsoluteExpirationRelativeToNow),
275-
options.AbsoluteExpirationRelativeToNow.Value,
276-
"The relative expiration value is too large.");
281+
relativeToNow.Value,
282+
$"The relative expiration value is too large. The maximum is {CacheEntryBinarySerializer.MaxTtl}.");
277283
}
278284

279285
throw new ArgumentOutOfRangeException(
280286
nameof(DistributedCacheEntryOptions.AbsoluteExpiration),
281-
options.AbsoluteExpiration!.Value,
282-
"The absolute expiration is too far in the future.");
287+
absoluteExpiration!.Value,
288+
$"The absolute expiration is too far in the future. The maximum window is {CacheEntryBinarySerializer.MaxTtl}.");
283289
}
284290

285291
return ttl;
@@ -290,7 +296,7 @@ internal CacheEntry CreateCacheEntry(byte[] value, DistributedCacheEntryOptions
290296
{
291297
Data = value,
292298
AbsoluteExpiration = ResolveAbsoluteExpiration(options),
293-
SlidingExpirationTicks = options.SlidingExpiration?.Ticks
299+
SlidingExpirationTicks = EffectiveSlidingExpiration(options)?.Ticks
294300
};
295301

296302
// An entry is absolutely expired once the clock reaches its absolute expiration instant. The
@@ -326,12 +332,27 @@ internal NatsKVConfig BuildBucketConfig()
326332
return config;
327333
}
328334

335+
// "Never expire" sentinels: a DateTimeOffset.MaxValue absolute instant or a TimeSpan.MaxValue window
336+
// is normalized to no expiration, so it is not rejected as out-of-range and the entry lives forever.
337+
private static DateTimeOffset? EffectiveAbsoluteExpiration(DistributedCacheEntryOptions options) =>
338+
options.AbsoluteExpiration == DateTimeOffset.MaxValue ? null : options.AbsoluteExpiration;
339+
340+
private static TimeSpan? EffectiveRelativeToNow(DistributedCacheEntryOptions options) =>
341+
options.AbsoluteExpirationRelativeToNow == TimeSpan.MaxValue ? null : options.AbsoluteExpirationRelativeToNow;
342+
343+
private static TimeSpan? EffectiveSlidingExpiration(DistributedCacheEntryOptions options) =>
344+
options.SlidingExpiration == TimeSpan.MaxValue ? null : options.SlidingExpiration;
345+
329346
// Resolves the effective absolute expiration instant: a relative expiration (offset from the
330347
// current clock) takes precedence over an explicit absolute expiration when both are set.
331-
private DateTimeOffset? ResolveAbsoluteExpiration(DistributedCacheEntryOptions options) =>
332-
options.AbsoluteExpirationRelativeToNow.HasValue
333-
? TimeProvider.GetUtcNow().Add(options.AbsoluteExpirationRelativeToNow.Value)
334-
: options.AbsoluteExpiration;
348+
// Maximum-value sentinels are treated as "no expiration" (see the Effective* helpers).
349+
private DateTimeOffset? ResolveAbsoluteExpiration(DistributedCacheEntryOptions options)
350+
{
351+
var relativeToNow = EffectiveRelativeToNow(options);
352+
return relativeToNow.HasValue
353+
? TimeProvider.GetUtcNow().Add(relativeToNow.Value)
354+
: EffectiveAbsoluteExpiration(options);
355+
}
335356

336357
private string GetEncodedKey(string key) =>
337358
string.IsNullOrEmpty(_keyPrefix)

‎test/UnitTests/Cache/TimeExpirationUnitTests.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,18 @@ public void TooLargeSlidingExpirationThrows()
9898
var key = MethodKey();
9999
var value = new byte[1];
100100

101-
// TimeSpan.MaxValue is far beyond the ceiling the NATS TTL encoding supports (int.MaxValue
102-
// seconds), so the write path must reject it rather than store an entry that later reads back
101+
// A window beyond the NATS TTL encoding ceiling (~68 years) but short of the TimeSpan.MaxValue
102+
// "never expire" sentinel must be rejected rather than stored as an entry that later reads back
103103
// as an undeserializable miss or emits an overflowed TTL header.
104+
var sliding = TimeSpan.FromDays(365 * 100);
104105
ExceptionAssert.ThrowsArgumentOutOfRange(
105106
() =>
106107
{
107-
Cache.Set(key, value, new DistributedCacheEntryOptions().SetSlidingExpiration(TimeSpan.MaxValue));
108+
Cache.Set(key, value, new DistributedCacheEntryOptions().SetSlidingExpiration(sliding));
108109
},
109110
nameof(DistributedCacheEntryOptions.SlidingExpiration),
110111
"The sliding expiration value is too large.",
111-
TimeSpan.MaxValue);
112+
sliding);
112113
}
113114

114115
[Fact]

‎test/UnitTests/Cache/TimeProviderExpirationUnitTests.cs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,39 @@ public void GetTtlWithFarAbsoluteAndSlidingReturnsSliding()
119119

120120
Assert.Equal(TimeSpan.FromMinutes(10), ttl);
121121
}
122+
123+
// DateTimeOffset.MaxValue / TimeSpan.MaxValue mean "cache forever": no TTL, not an out-of-range throw.
124+
[Fact]
125+
public void GetTtlTreatsMaxAbsoluteInstantAsNoExpiration() =>
126+
Assert.Null(Cache.GetTtl(new DistributedCacheEntryOptions().SetAbsoluteExpiration(DateTimeOffset.MaxValue)));
127+
128+
[Fact]
129+
public void GetTtlTreatsMaxRelativeExpirationAsNoExpiration() =>
130+
Assert.Null(Cache.GetTtl(new DistributedCacheEntryOptions().SetAbsoluteExpiration(TimeSpan.MaxValue)));
131+
132+
[Fact]
133+
public void GetTtlTreatsMaxSlidingExpirationAsNoExpiration() =>
134+
Assert.Null(Cache.GetTtl(new DistributedCacheEntryOptions().SetSlidingExpiration(TimeSpan.MaxValue)));
135+
136+
[Fact]
137+
public void CreateCacheEntryTreatsMaxValueSlidingAsNoExpiration()
138+
{
139+
var entry = Cache.CreateCacheEntry(
140+
new byte[1],
141+
new DistributedCacheEntryOptions().SetSlidingExpiration(TimeSpan.MaxValue));
142+
143+
Assert.Null(entry.SlidingExpirationTicks);
144+
Assert.Null(entry.AbsoluteExpiration);
145+
}
146+
147+
[Fact]
148+
public void CreateCacheEntryTreatsMaxValueAbsoluteAsNoExpiration()
149+
{
150+
var entry = Cache.CreateCacheEntry(
151+
new byte[1],
152+
new DistributedCacheEntryOptions().SetAbsoluteExpiration(DateTimeOffset.MaxValue));
153+
154+
Assert.Null(entry.AbsoluteExpiration);
155+
Assert.Null(entry.SlidingExpirationTicks);
156+
}
122157
}

0 commit comments

Comments
 (0)