MichaCo / CacheManager

CacheManager is an open source caching abstraction layer for .NET written in C#. It supports various cache providers and implements many advanced features.
http://cachemanager.michaco.net
Apache License 2.0
2.34k stars 456 forks source link

ConfigurationBuilderCacheHandlePart.WithExpiration throws error when ExpirationMode.Default given #192

Closed brandondahler closed 6 years ago

brandondahler commented 6 years ago

ConfigurationBuilderCacheHandlePart.WithExpiration throws the error "If expiration mode is not set to 'None', timeout cannot be zero." when ExpirationMode.Default is provided with a timeout of TimeSpan.Zero.

Severity: Low

Use Case

Writing an abstraction layer for application-specific caching which uses CacheManager underneath. Code using abstraction layer can optionally specify the ExpirationMode, which is then passed to the CacheFactory settings. By default ExpriationMode is configured to ExpirationMode.Default and Timeout is configured to TimeSpan.Zero.

Reproduction Code

var expirationMode = ExpirationMode.Default;
var timeout = TimeSpan.Zero;

var cacheManager = CacheFactory.Build<string>(
    cacheName,
    s => s
        .WithJsonSerializer()
        .WithMicrosoftMemoryCacheHandle(cacheName)
        .WithExpiration(expirationMode, timeout));

Expected Results

cacheManager is set to a non-null value, configured for Json serialization, MicrosoftMemoryCacheHandler, and an ExpirationMode of None (as Default is equivalent to None per src/CacheManager.Core/ExpirationMode.cs#L11).

Actual Results

InvalidOperationException thrown with message of "If expiration mode is not set to 'None', timeout cannot be zero.".

Recommended Fix

Update src/CacheManager.Core/Configuration/ConfigurationBuilder.cs#L496 to

if (expirationMode != ExpirationMode.None && expirationMode != ExpirationMode.Default && timeout == TimeSpan.Zero)

Alternative Fix

Remove ExpirationMode.Default as a valid value, which would be a breaking change.

Work Around

Use ExpirationMode.None as default value if default value for ExpirationMode is needed.

MichaCo commented 6 years ago

Totally forgot to post an answer to this -.- Thanks for the very detailed report! I will look into this

MichaCo commented 6 years ago

fixed in release 1.1.2