Add ConcurrentDictionary.TryRemove(KeyValuePair) (dotnet/corefx#41597)
authorStephen Toub <stoub@microsoft.com>
Mon, 7 Oct 2019 18:19:57 +0000 (14:19 -0400)
committerGitHub <noreply@github.com>
Mon, 7 Oct 2019 18:19:57 +0000 (14:19 -0400)
Commit migrated from https://github.com/dotnet/corefx/commit/a62579502cb9e86b761c2249d0d04af3d827b3a3

src/libraries/System.Collections.Concurrent/ref/System.Collections.Concurrent.cs
src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
src/libraries/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionaryTests.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/FailedProxyCache.cs
src/libraries/System.Net.ServicePoint/src/System/Net/ServicePointManager.cs

index de2a131..c6db222 100644 (file)
@@ -121,6 +121,7 @@ namespace System.Collections.Concurrent
         public System.Collections.Generic.KeyValuePair<TKey, TValue>[] ToArray() { throw null; }
         public bool TryAdd(TKey key, TValue value) { throw null; }
         public bool TryGetValue(TKey key, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TValue value) { throw null; }
+        public bool TryRemove(System.Collections.Generic.KeyValuePair<TKey, TValue> item) { throw null; }
         public bool TryRemove(TKey key, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TValue value) { throw null; }
         public bool TryUpdate(TKey key, TValue newValue, TValue comparisonValue) { throw null; }
     }
index 9947ae5..410e0e5 100644 (file)
@@ -342,6 +342,31 @@ namespace System.Collections.Concurrent
             return TryRemoveInternal(key, out value, false, default);
         }
 
+        /// <summary>Removes a key and value from the dictionary.</summary>
+        /// <param name="item">The <see cref="KeyValuePair{TKey,TValue}"/> representing the key and value to remove.</param>
+        /// <returns>
+        /// true if the key and value represented by <paramref name="item"/> are successfully
+        /// found and removed; otherwise, false.
+        /// </returns>
+        /// <remarks>
+        /// Both the specifed key and value must match the entry in the dictionary for it to be removed.
+        /// The key is compared using the dictionary's comparer (or the default comparer for <typeparamref name="TKey"/>
+        /// if no comparer was provided to the dictionary when it was constructed).  The value is compared using the
+        /// default comparer for <typeparamref name="TValue"/>.
+        /// </remarks>
+        /// <exception cref="System.ArgumentNullException">
+        /// The <see cref="KeyValuePair{TKey, TValue}.Key"/> property of <paramref name="item"/> is a null reference.
+        /// </exception>
+        public bool TryRemove(KeyValuePair<TKey, TValue> item)
+        {
+            if (item.Key is null)
+            {
+                throw new ArgumentNullException(nameof(item), SR.ConcurrentDictionary_ItemKeyIsNull);
+            }
+
+            return TryRemoveInternal(item.Key, out _, matchValue: true, item.Value);
+        }
+
         /// <summary>
         /// Removes the specified key from the dictionary if it exists and returns its associated value.
         /// If matchValue flag is set, the key will be removed only if is associated with a particular
@@ -1421,13 +1446,8 @@ namespace System.Collections.Concurrent
         /// found and removed; otherwise, false.</returns>
         /// <exception cref="System.ArgumentNullException">The Key property of <paramref
         /// name="keyValuePair"/> is a null reference (Nothing in Visual Basic).</exception>
-        bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> keyValuePair)
-        {
-            if (keyValuePair.Key == null) throw new ArgumentNullException(nameof(keyValuePair), SR.ConcurrentDictionary_ItemKeyIsNull);
-
-            TValue throwAwayValue;
-            return TryRemoveInternal(keyValuePair.Key, out throwAwayValue, true, keyValuePair.Value);
-        }
+        bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> keyValuePair) =>
+            TryRemove(keyValuePair);
 
         #endregion
 
index 44adf95..e2bfe6c 100644 (file)
@@ -468,6 +468,42 @@ namespace System.Collections.Concurrent.Tests
         }
 
         [Fact]
+        public static void TryRemove_KeyValuePair_ArgumentValidation()
+        {
+            AssertExtensions.Throws<ArgumentNullException>("item", () => new ConcurrentDictionary<string, int>().TryRemove(new KeyValuePair<string, int>(null, 42)));
+            new ConcurrentDictionary<int, int>().TryRemove(new KeyValuePair<int, int>(0, 0)); // no error when using default value type
+            new ConcurrentDictionary<int?, int>().TryRemove(new KeyValuePair<int?, int>(0, 0)); // or nullable
+        }
+
+        [Fact]
+        public static void TryRemove_KeyValuePair_RemovesSuccessfullyAsAppropriate()
+        {
+            var dict = new ConcurrentDictionary<string, int>();
+
+            for (int i = 0; i < 2; i++)
+            {
+                Assert.False(dict.TryRemove(KeyValuePair.Create("key", 42)));
+                Assert.Equal(0, dict.Count);
+                Assert.True(dict.TryAdd("key", 42));
+                Assert.Equal(1, dict.Count);
+                Assert.True(dict.TryRemove(KeyValuePair.Create("key", 42)));
+                Assert.Equal(0, dict.Count);
+            }
+
+            Assert.True(dict.TryAdd("key", 42));
+            Assert.False(dict.TryRemove(KeyValuePair.Create("key", 43))); // value doesn't match
+        }
+
+        [Fact]
+        public static void TryRemove_KeyValuePair_MatchesKeyWithDefaultComparer()
+        {
+            var dict = new ConcurrentDictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+            dict.TryAdd("key", "value");
+            Assert.False(dict.TryRemove(KeyValuePair.Create("key", "VALUE")));
+            Assert.True(dict.TryRemove(KeyValuePair.Create("KEY", "value")));
+        }
+
+        [Fact]
         public static void TestGetOrAdd()
         {
             TestGetOrAddOrUpdate(1, 1, 1, 10000, true);
index 14cfb61..7d56346 100644 (file)
@@ -85,11 +85,8 @@ namespace System.Net.Http
         /// </summary>
         /// <param name="uri">The <paramref name="uri"/> of the proxy to renew.</param>
         /// <param name="renewTicks">The current renewal time for the proxy. If the value has changed from this, the proxy will not be renewed.</param>
-        public bool TryRenewProxy(Uri uri, long renewTicks)
-        {
-            var collection = (ICollection<KeyValuePair<Uri, long>>)_failedProxies;
-            return collection.Remove(new KeyValuePair<Uri, long>(uri, renewTicks));
-        }
+        public bool TryRenewProxy(Uri uri, long renewTicks) =>
+            _failedProxies.TryRemove(new KeyValuePair<Uri, long>(uri, renewTicks));
 
         /// <summary>
         /// Cleans up any old proxies that should no longer be marked as failing.
index 29d9df1..95ff221 100644 (file)
@@ -137,16 +137,14 @@ namespace System.Net
                 // to scavenge the table looking for entries that have lost their service point and removing them.
                 foreach (KeyValuePair<string, WeakReference<ServicePoint>> entry in s_servicePointTable)
                 {
-                    ServicePoint ignored;
-                    if (!entry.Value.TryGetTarget(out ignored))
+                    if (!entry.Value.TryGetTarget(out _))
                     {
-                        // We use the IDictionary.Remove method rather than TryRemove as it will only
-                        // remove the entry from the table if both the key/value in the pair match.
+                        // Remove the entry from the table if both the key/value in the pair match.
                         // This avoids a race condition where another thread concurrently sets a new
                         // weak reference value for the same key, and is why when adding the new
                         // service point below, we don't use any weak reference object we may already
                         // have from the initial retrieval above.
-                        ((IDictionary<string, WeakReference<ServicePoint>>)s_servicePointTable).Remove(entry);
+                        s_servicePointTable.TryRemove(entry);
                     }
                 }