IMemoryCache.Set() silently fails when Size >= SizeLimit #1889

When adding an item via IMemoryCache.Set() (the extension method), and the number of items in the cache is already at MemoryCacheOptions.SizeLimit, the item is not added, but the add also fails silently, and there appears to be no way to readily know this.

The only indication of failure appears to be that CacheEntry.EvictionReason is set to Capacity, but a) this is only set on disposal of the ICacheEntry, b) using .Set() does not expose the ICacheEntry anyway, and c) even if trying to use e.g. IMemoryCache.CreateEntry() instead, CacheEntry and EvictionReason are internal.

To Reproduce

[Test]
public void OverCapacity_IsHandled_Eg_ByCompaction()
{
    var cacheOptions = new MemoryCacheOptions { SizeLimit = 2, CompactionPercentage = 0.5D };
    var entryOptions = new MemoryCacheEntryOptions { SlidingExpiration = TimeSpan.FromMinutes(1.0), Size = 1 };

    var value1 = new object();
    var value2 = new object();
    var value3 = new object();

    using (var memoryCache = new MemoryCache(cacheOptions))
    {
        object added1 = memoryCache.Set("key1", value1, entryOptions);
        Assert.That(added1, Is.Not.Null.And.EqualTo(value1));
        Assert.That(memoryCache.Get("key1"), Is.Not.Null);

        object added2 = memoryCache.Set("key2", value2, entryOptions);
        Assert.That(added2, Is.Not.Null.And.EqualTo(value2));
        Assert.That(memoryCache.Get("key2"), Is.Not.Null);

        object added3 = memoryCache.Set("key3", value3, entryOptions); //// Might expect this to make some noise as it failed...
        Assert.That(added3, Is.Not.Null.And.EqualTo(value3)); //// <-- ...and this doesn't indicate an issue either
        Assert.That(memoryCache.Get("key3"), Is.Not.Null); //// <-- Assertion fails; the item is not in the cache.
    }
}

Expected behavior

In most- to least-preferred order:

• call OvercapacityCompaction(), add the new item, succeed. This will take more execution time, but it's probably what the caller wants to achieve anyway, and the frequency of this can be regulated with MemoryCacheOptions.CompactionPercentage.
• throw an exception. The caller will probably then still want to remove LRUs, then add the new value, so it doesn't seem like a good option when MemoryCache could assume responsibility for this instead.

Additional context

Issue 1087 alludes to the same issue, but it seems to not be getting much attention and I thought it needed a stronger title.

I am using Microsoft.Extensions.Caching 2.2.0

Some workarounds I considered:

• Calling .Compact() before .Set(), but this scans every item in the cache regardless of whether any compaction is actually needed, so is very inefficient. Also .Compact() is available on MemoryCache, not IMemoryCache, so is not available to the latter when injected, without assuming knowledge of the former.
• Comparing .Size to MemoryCacheOptions.SizeLimit just before using .Set(), and then only calling .Compact() if needed, but again neither is available on IMemoryCache and .Size is internal anyway. In my case I know each item is of size 1, so I could use .Count as equivalent, but this is not always the case.
• Inheriting MemoryCache to create e.g. AutoCompactingMemoryCache to get around the limitations above, but while MemoryCache is not sealed, nothing of note is virtual to allow this, and the use of extension methods makes this even harder.

I eventually worked around this by creating an AutoCompactingMemoryCache inheriting MemoryCache and re-implementing IMemoryCache.CreateEntry() explicitly, with the known assumption of .Size == .Count, but the assumption, while it works for me, is not the general case.