Simple MemoryCache implementation for thread safe caching

Question

After much prompting from this post I wanted to build a simple, in-memory, thread-safe cache.

The only caveat (as far as I was originally concerned) was the need for two different absolute expiration times for cached objects - those being based on a property of the item being cached (IsFailureItem). This is for a .NET Framework 4.6.1 solution.

I am primarily interested in anything which makes this either thread-unsafe, leak memory or simply bad practice. I know the class itself could be generically typed, but that is a decision for the future.

public class CacheItem
{
    public IEnumerable<DataItem> Response { get; set; }
    public bool IsFailureItem { get; set; }
}

public class CacheHelper
{
    public static CacheHelper Cache { get; set; }
    private static IMemoryCache InMemoryCache { get; set; }

    static CacheHelper()
    {
        Cache = new CacheHelper();
        InMemoryCache = new MemoryCache(new MemoryCacheOptions { });
    }

    private CacheHelper() { }

    public CacheItem this[string key]
    {
        get => InMemoryCache.Get<CacheItem>(key);
        set => InMemoryCache.Set<CacheItem>(key, value, value.IsFailureItem ? FailureCacheEntryOption : SuccessCacheEntryOption );
    }

    private static MemoryCacheEntryOptions FailureCacheEntryOption => new MemoryCacheEntryOptions()
    { AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(EnvironmentHelper.CacheFailureSecondsToLive) };

    private static MemoryCacheEntryOptions SuccessCacheEntryOption => new MemoryCacheEntryOptions()
    { AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(EnvironmentHelper.CacheSuccessSecondsToLive) };
}

Answer 1

No reason to have a public set on public static CacheHelper Cache { get; set; } not anyway to set it so why expose it?

I think CacheItem should be immutable. If calling Set with a CacheItem that IsFailureItem is true but later can be set to false, wouldn't effect time. Same with Response. Is it to be expected to be able to retrieve the CacheItem and update/replace the Response property?

Answer 2

Here is the code, after applying changes as advised by the wise commenters here:

public class CacheItem
{
    public IEnumerable<DataItem> Response { get; }
    public bool IsFailureItem { get; }

    public CacheItem(bool isFailureItem, IEnumerable<DataItem> response = null)
    {
        IsFailureItem = isFailureItem;
        Response = response;
    }
}

public class CacheHelper
{
    public static CacheHelper Cache { get; }
    private static readonly IMemoryCache InMemoryCache;

    static CacheHelper()
    {
        Cache = new CacheHelper();
        InMemoryCache = new MemoryCache(new MemoryCacheOptions { });
    }

    private CacheHelper() { }

    public CacheItem this[string key]
    {
        get => InMemoryCache.Get<CacheItem>(key);
        set => InMemoryCache.Set<CacheItem>(key, value, value.IsFailureItem ? FailureCacheEntryOption : SuccessCacheEntryOption );
    }

    private static MemoryCacheEntryOptions FailureCacheEntryOption => new MemoryCacheEntryOptions()
    { AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(EnvironmentHelper.CacheFailureSecondsToLive) };

    private static MemoryCacheEntryOptions SuccessCacheEntryOption => new MemoryCacheEntryOptions()
    { AbsoluteExpirationRelativeToNow = TimeSpan.FromSeconds(EnvironmentHelper.CacheSuccessSecondsToLive) };
}