Align ICache.TryRemove overloads with ConcurrentDictionary

BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs

@@ -13,11 +13,18 @@ namespace BitFaster.Caching.UnitTests.Atomic
     public class AtomicFactoryCacheTests
     {
         private const int capacity = 6;
-        private readonly AtomicFactoryCache<int, int> cache = new(new ConcurrentLru<int, AtomicFactory<int, int>>(capacity));
+        private readonly ConcurrentLru<int, AtomicFactory<int, int>> innerCache;
+        private readonly AtomicFactoryCache<int, int> cache;
 
         private List<ItemRemovedEventArgs<int, int>> removedItems = new();
         private List<ItemUpdatedEventArgs<int, int>> updatedItems = new();
 
+        public AtomicFactoryCacheTests()
+        {
+            innerCache = new ConcurrentLru<int, AtomicFactory<int, int>>(capacity);
+            cache = new(innerCache);
+        }
+
         [Fact]
         public void WhenInnerCacheIsNullCtorThrows()
         {
@@ -70,10 +77,52 @@ public void WhenRemovedEventHandlerIsRegisteredItIsFired()
             this.cache.TryRemove(1);
 
             this.removedItems.First().Key.Should().Be(1);
+        }
+
+        // backcompat: remove conditional compile
+#if NETCOREAPP3_0_OR_GREATER
+        [Fact]
+        public void WhenRemovedValueIsReturned()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(1, out var value);
+
+            value.Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenNotRemovedValueIsDefault()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(2, out var value);
+
+            value.Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueDoesntMatchDontRemove()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueDoesMatchThenRemove()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueIsNotCreatedDoesNotRemove()
+        {
+            // seed the inner cache with an not yet created value
+            this.innerCache.AddOrUpdate(1, new AtomicFactory<int, int>());
+
+            // try to remove with the default value (0)
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 0)).Should().BeFalse();
         }
 
-// backcompat: remove conditional compile
-#if NETCOREAPP3_0_OR_GREATER
         [Fact]
         public void WhenUpdatedEventHandlerIsRegisteredItIsFired()
         {

BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs

@@ -63,6 +63,58 @@ public void WhenValueCreatedArgGetValueReturnsOriginalValue()
             a.GetValue(1, (k, a) => k + a, 9).Should().Be(8);
         }
 
+        [Fact]
+        public void WhenValueNotCreatedHashCodeIsZero()
+        {
+            new AtomicFactory<int, int>()
+                .GetHashCode()
+                .Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenValueCreatedHashCodeIsValueHashCode()
+        {
+            new AtomicFactory<int, int>(1)
+                .GetHashCode()
+                .Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenValueNotCreatedEqualsFalse()
+        {
+            var a = new AtomicFactory<int, int>();
+            var b = new AtomicFactory<int, int>();
+
+            a.Equals(b).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenOtherValueNotCreatedEqualsFalse()
+        {
+            var a = new AtomicFactory<int, int>(1);
+            var b = new AtomicFactory<int, int>();
+
+            a.Equals(b).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenArgNullEqualsFalse()
+        {
+            new AtomicFactory<int, int>(1)
+                .Equals(null)
+                .Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenArgObjectValuesAreSameEqualsTrue()
+        {
+            object other = new AtomicFactory<int, int>(1);
+
+            new AtomicFactory<int, int>(1)
+                .Equals(other)
+                .Should().BeTrue();
+        }
+
         [Fact]
         public async Task WhenCallersRunConcurrentlyResultIsFromWinner()
         {

BitFaster.Caching.UnitTests/CacheTests.cs

@@ -1,5 +1,6 @@
 
 using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using FluentAssertions;
 using Moq;
@@ -25,6 +26,28 @@ public void WhenCacheInterfaceDefaultGetOrAddFallback()
                 1, 
                 (k, a) => k + a, 
                 2).Should().Be(3);
+        }
+
+        [Fact]
+        public void WhenCacheInterfaceDefaultTryRemoveKeyThrows()
+        {
+            var cache = new Mock<ICache<int, int>>();
+            cache.CallBase = true;
+
+            Action tryRemove = () => { cache.Object.TryRemove(1, out var value); };
+
+            tryRemove.Should().Throw<NotSupportedException>();
+        }
+
+        [Fact]
+        public void WhenCacheInterfaceDefaultTryRemoveKeyValueThrows()
+        {
+            var cache = new Mock<ICache<int, int>>();
+            cache.CallBase = true;
+
+            Action tryRemove = () => { cache.Object.TryRemove(new KeyValuePair<int, int>(1, 1)); };
+
+            tryRemove.Should().Throw<NotSupportedException>();
         }
 
         [Fact]

BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

@@ -582,12 +582,39 @@ public void WhenItemDoesNotExistUpdatedAddsItem()
         }
 
         [Fact]
-        public void WhenItemIsRemovedItIsRemoved()
+        public void WhenKeyExistsTryRemoveRemovesItem()
         {
             cache.GetOrAdd(1, k => k);
 
             cache.TryRemove(1).Should().BeTrue();
-            cache.TryGet(1, out var value).Should().BeFalse();
+            cache.TryGet(1, out _).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            cache.GetOrAdd(1, valueFactory.Create);
+
+            cache.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenItemExistsTryRemoveRemovesItem()
+        {
+            cache.GetOrAdd(1, k => k);
+
+            cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
+            cache.TryGet(1, out _).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenItemDoesntMatchTryRemoveDoesNotRemove()
+        {
+            cache.GetOrAdd(1, k => k);
+
+            cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
+            cache.TryGet(1, out var value).Should().BeTrue();
         }
 
         [Fact]

BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs

@@ -362,6 +362,33 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
             lru.TryGet(1, out var value).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be("1");
+        }
+
+        [Fact]
+        public void WhenItemExistsTryRemovesItemAndReturnsTrue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(new KeyValuePair<int, string>(1, "1")).Should().BeTrue();
+            lru.TryGet(1, out var value).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenTryRemoveKvpDoesntMatchItemNotRemovedAndReturnsFalse()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(new KeyValuePair<int, string>(1, "2")).Should().BeFalse();
+            lru.TryGet(1, out var value).Should().BeTrue();
+        }
+
         [Fact]
         public void WhenItemIsRemovedItIsDisposed()
         {

BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs

@@ -634,6 +634,15 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
 
             lru.TryRemove(1).Should().BeTrue();
             lru.TryGet(1, out var value).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be("1");
         }
 
         [Fact]
@@ -1196,6 +1205,26 @@ await Threaded.Run(4, () => {
             }
         }
 
+        [Fact]
+        public async Task WhenSoakConcurrentGetAndRemoveKvpCacheEndsInConsistentState()
+        {
+            for (int i = 0; i < 10; i++)
+            {
+                await Threaded.Run(4, () => {
+                    for (int i = 0; i < 100000; i++)
+                    {
+                        lru.TryRemove(new KeyValuePair<int, string>(i + 1, (i + 1).ToString()));
+                        lru.GetOrAdd(i + 1, i => i.ToString());
+                    }
+                });
+
+                this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}");
+                this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys));
+
+                RunIntegrityCheck();
+            }
+        }
+
         [Fact]
         public async Task WhenSoakConcurrentGetAndUpdateCacheEndsInConsistentState()
         {

BitFaster.Caching/Atomic/AtomicFactory.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
 
@@ -11,7 +12,7 @@ namespace BitFaster.Caching.Atomic
     /// <typeparam name="K">The type of the key.</typeparam>
     /// <typeparam name="V">The type of the value.</typeparam>
     [DebuggerDisplay("IsValueCreated={IsValueCreated}, Value={ValueIfCreated}")]
-    public sealed class AtomicFactory<K, V>
+    public sealed class AtomicFactory<K, V> : IEquatable<AtomicFactory<K, V>>
     {
         private Initializer initializer;
 
@@ -102,8 +103,36 @@ private V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : s
             }
 
             return value;
-        }
-
+        }
+
+        ///<inheritdoc/>
+        public override bool Equals(object obj)
+        {
+            return Equals(obj as AtomicFactory<K, V>);
+        }
+
+        ///<inheritdoc/>
+        public bool Equals(AtomicFactory<K, V> other)
+        {
+            if (other is null || !IsValueCreated || !other.IsValueCreated)
+            {
+                return false;
+            }
+
+            return EqualityComparer<V>.Default.Equals(ValueIfCreated, other.ValueIfCreated);
+        }
+
+        ///<inheritdoc/>
+        public override int GetHashCode()
+        {
+            if (!IsValueCreated)
+            {
+                return 0;
+            }
+
+            return ValueIfCreated.GetHashCode();
+        }
+
         private class Initializer
         {
             private readonly object syncLock = new();
@@ -129,6 +158,6 @@ public V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : st
                     return value;
                 }
             }
-        }
+        }
     }
 }

BitFaster.Caching/Atomic/AtomicFactoryCache.cs

@@ -2,7 +2,8 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
-
+using System.Linq.Expressions;
+
 namespace BitFaster.Caching.Atomic
 {
     /// <summary>
@@ -100,9 +101,38 @@ public bool TryGet(K key, out V value)
                 return true;
             }
 
+            value = default;
+            return false;
+        }
+
+        // backcompat: remove conditional compile
+#if NETCOREAPP3_0_OR_GREATER
+        ///<inheritdoc/>
+        ///<remarks>
+        ///If the value factory is still executing, returns false.
+        ///</remarks>
+        public bool TryRemove(KeyValuePair<K, V> item)
+        {
+            var kvp = new KeyValuePair<K, AtomicFactory<K, V>>(item.Key, new AtomicFactory<K, V>(item.Value));
+            return cache.TryRemove(kvp);
+        }
+
+        ///<inheritdoc/>
+        /// <remarks>
+        /// If the value factory is still executing, the default value will be returned.
+        /// </remarks>
+        public bool TryRemove(K key, out V value)
+        {
+            if (cache.TryRemove(key, out var atomic))
+            {
+                value = atomic.ValueIfCreated;
+                return true;
+            }
+
             value = default;
             return false;
         }
+#endif
 
         ///<inheritdoc/>
         public bool TryRemove(K key)