From 3ca0887816e4cb06fae00ac35d408836dd32512e Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Mon, 8 May 2023 18:28:44 +0300 Subject: [PATCH 1/5] Remove last ! --- .../src/CacheEntryExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs index e8c72b3ec37d91..9ef1847818f6bc 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs @@ -174,7 +174,8 @@ public static ICacheEntry SetOptions(this ICacheEntry entry, MemoryCacheEntryOpt foreach (PostEvictionCallbackRegistration postEvictionCallback in options.PostEvictionCallbacks) { - entry.RegisterPostEvictionCallback(postEvictionCallback.EvictionCallback!, postEvictionCallback.State); + ArgumentNullException.ThrowIfNull(postEvictionCallback.EvictionCallback); + entry.RegisterPostEvictionCallback(postEvictionCallback.EvictionCallback, postEvictionCallback.State); } return entry; From f15934bece5d0086d1114ea225ef78560b766ca9 Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Tue, 9 May 2023 22:19:49 +0300 Subject: [PATCH 2/5] Update exception --- .../src/CacheEntryExtensions.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs index 9ef1847818f6bc..36872679a684de 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.Primitives; namespace Microsoft.Extensions.Caching.Memory @@ -93,7 +94,7 @@ public static ICacheEntry RegisterPostEvictionCallback( { ThrowHelper.ThrowIfNull(callback); - return entry.RegisterPostEvictionCallback(callback, state: null); + return entry.RegisterPostEvictionCallbackNoValidation(callback, state: null); } /// @@ -110,6 +111,14 @@ public static ICacheEntry RegisterPostEvictionCallback( { ThrowHelper.ThrowIfNull(callback); + return entry.RegisterPostEvictionCallbackNoValidation(callback, state); + } + + internal static ICacheEntry RegisterPostEvictionCallbackNoValidation( + this ICacheEntry entry, + PostEvictionDelegate callback, + object? state) + { entry.PostEvictionCallbacks.Add(new PostEvictionCallbackRegistration() { EvictionCallback = callback, @@ -172,13 +181,24 @@ public static ICacheEntry SetOptions(this ICacheEntry entry, MemoryCacheEntryOpt entry.AddExpirationToken(expirationToken); } - foreach (PostEvictionCallbackRegistration postEvictionCallback in options.PostEvictionCallbacks) + for (int i = 0; i < options.PostEvictionCallbacks.Count; i++) { - ArgumentNullException.ThrowIfNull(postEvictionCallback.EvictionCallback); - entry.RegisterPostEvictionCallback(postEvictionCallback.EvictionCallback, postEvictionCallback.State); + PostEvictionCallbackRegistration postEvictionCallback = options.PostEvictionCallbacks[i]; + if (postEvictionCallback.EvictionCallback is null) + ThrowNullCallback(i, nameof(options)); + + entry.RegisterPostEvictionCallbackNoValidation(postEvictionCallback.EvictionCallback, postEvictionCallback.State); } return entry; } + + [DoesNotReturn] + private static void ThrowNullCallback(int index, string paramName) + { + string message = + $"MemoryCacheEntryOptions.PostEvictionCallbacks contains a PostEvictionCallbackRegistration with a null EvictionCallback at index {index}."; + throw new ArgumentException(message, paramName); + } } } From 32b53e9f24e5c11720c01e6271b84b6e53cc204a Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Tue, 9 May 2023 22:19:53 +0300 Subject: [PATCH 3/5] Add test --- .../tests/MemoryCacheSetAndRemoveTests.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index e7ac1edb71ece7..1f84d7f20aae98 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -406,6 +406,29 @@ public void ClearClears() } } + [Fact] + public void SetNullCallback_NotAllowed_ArgumentException() + { + var cache = CreateCache(); + const string someKey = "test"; + var entry = cache.CreateEntry(someKey); + + var options = new MemoryCacheEntryOptions(); + + var notNullCallback = new PostEvictionCallbackRegistration() + { + EvictionCallback = (_, _ , _, _) => Console.WriteLine() + }; + + options.PostEvictionCallbacks.Add(notNullCallback); + + var nullCallback = new PostEvictionCallbackRegistration(); + + options.PostEvictionCallbacks.Add(nullCallback); + + Assert.Throws(() => entry.SetOptions(options)); + } + [Fact] public void RemoveRemovesAndInvokesCallback() { From 84ccaf3086ca50e3de1bf63f6a612027dc5cb89c Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Tue, 9 May 2023 22:49:58 +0300 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Eric Erhardt --- .../src/CacheEntryExtensions.cs | 2 +- .../tests/MemoryCacheSetAndRemoveTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs index 36872679a684de..14a5a7dc32de7f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/CacheEntryExtensions.cs @@ -114,7 +114,7 @@ public static ICacheEntry RegisterPostEvictionCallback( return entry.RegisterPostEvictionCallbackNoValidation(callback, state); } - internal static ICacheEntry RegisterPostEvictionCallbackNoValidation( + private static ICacheEntry RegisterPostEvictionCallbackNoValidation( this ICacheEntry entry, PostEvictionDelegate callback, object? state) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index 1f84d7f20aae98..12250fb20cb9a7 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -417,7 +417,7 @@ public void SetNullCallback_NotAllowed_ArgumentException() var notNullCallback = new PostEvictionCallbackRegistration() { - EvictionCallback = (_, _ , _, _) => Console.WriteLine() + EvictionCallback = (_, _ , _, _) => {} }; options.PostEvictionCallbacks.Add(notNullCallback); From f4b4bd27ae13d2c55ec5ae2b44eb38cab5ee76a7 Mon Sep 17 00:00:00 2001 From: hrrrrustic Date: Thu, 11 May 2023 22:53:40 +0300 Subject: [PATCH 5/5] Update src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs Co-authored-by: Christopher Watford <132389385+watfordkcf@users.noreply.github.com> --- .../tests/MemoryCacheSetAndRemoveTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index 12250fb20cb9a7..f1002232e4c18f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -417,7 +417,7 @@ public void SetNullCallback_NotAllowed_ArgumentException() var notNullCallback = new PostEvictionCallbackRegistration() { - EvictionCallback = (_, _ , _, _) => {} + EvictionCallback = (_, _, _, _) => {} }; options.PostEvictionCallbacks.Add(notNullCallback);