From 85cd19d356c211b4921e814a2de24e13a6c7d4e9 Mon Sep 17 00:00:00 2001
From: ZLoo <kozlov.s.v.1992@gmail.com>
Date: Tue, 23 Jul 2024 10:09:56 +0300
Subject: [PATCH] Fix CA1062 warnings (#2238)

Fix CA1062 warnings for `CachePolicy`.
---
 src/Polly/Caching/CachePolicy.cs              | 36 ++++++++++---
 test/Polly.Specs/Caching/CacheSpecs.cs        | 53 +++++++++++++++++++
 test/Polly.Specs/Caching/CacheTResultSpecs.cs | 42 +++++++++++++++
 3 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/src/Polly/Caching/CachePolicy.cs b/src/Polly/Caching/CachePolicy.cs
index e36ccf0d072..e1eed65bd77 100644
--- a/src/Polly/Caching/CachePolicy.cs
+++ b/src/Polly/Caching/CachePolicy.cs
@@ -4,7 +4,6 @@ namespace Polly.Caching;
 /// <summary>
 /// A cache policy that can be applied to the results of delegate executions.
 /// </summary>
-#pragma warning disable CA1062 // Validate arguments of public methods
 public class CachePolicy : Policy, ICachePolicy
 {
     private readonly ISyncCacheProvider _syncCacheProvider;
@@ -39,14 +38,27 @@ internal CachePolicy(
     }
 
     /// <inheritdoc/>
-    protected override void Implementation(Action<Context, CancellationToken> action, Context context, CancellationToken cancellationToken) // Pass-through/NOOP policy action, for void-returning calls through a cache policy.
-        =>
-            action(context, cancellationToken);
+    protected override void Implementation(Action<Context, CancellationToken> action, Context context, CancellationToken cancellationToken)
+    {
+        // Pass-through/NOOP policy action, for void-returning calls through a cache policy.
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        action(context, cancellationToken);
+    }
 
     /// <inheritdoc/>
     [DebuggerStepThrough]
-    protected override TResult Implementation<TResult>(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken) =>
-        CacheEngine.Implementation<TResult>(
+    protected override TResult Implementation<TResult>(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken)
+    {
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        return CacheEngine.Implementation<TResult>(
             _syncCacheProvider.For<TResult>(),
             _ttlStrategy.For<TResult>(),
             _cacheKeyStrategy,
@@ -58,6 +70,7 @@ protected override TResult Implementation<TResult>(Func<Context, CancellationTok
             _onCacheGetError,
             _onCachePutError,
             cancellationToken);
+    }
 }
 
 /// <summary>
@@ -99,8 +112,14 @@ internal CachePolicy(
 
     /// <inheritdoc/>
     [DebuggerStepThrough]
-    protected override TResult Implementation(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken) =>
-        CacheEngine.Implementation(
+    protected override TResult Implementation(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken)
+    {
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        return CacheEngine.Implementation(
             _syncCacheProvider,
             _ttlStrategy,
             _cacheKeyStrategy,
@@ -112,4 +131,5 @@ protected override TResult Implementation(Func<Context, CancellationToken, TResu
             _onCacheGetError,
             _onCachePutError,
             cancellationToken);
+    }
 }
diff --git a/test/Polly.Specs/Caching/CacheSpecs.cs b/test/Polly.Specs/Caching/CacheSpecs.cs
index 87d763508d8..9836b24a4f7 100644
--- a/test/Polly.Specs/Caching/CacheSpecs.cs
+++ b/test/Polly.Specs/Caching/CacheSpecs.cs
@@ -5,6 +5,59 @@ public class CacheSpecs : IDisposable
 {
     #region Configuration
 
+    [Fact]
+    public void Should_throw_when_action_is_null()
+    {
+        var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+        Func<Context, CancellationToken, EmptyStruct> action = null!;
+        Action<Context, CancellationToken> actionVoid = null!;
+
+        ISyncCacheProvider syncCacheProvider = new StubCacheProvider();
+        ITtlStrategy ttlStrategy = new ContextualTtl();
+        Func<Context, string> cacheKeyStrategy = (_) => string.Empty;
+        Action<Context, string> onCacheGet = (_, _) => { };
+        Action<Context, string> onCacheMiss = (_, _) => { };
+        Action<Context, string> onCachePut = (_, _) => { };
+        Action<Context, string, Exception>? onCacheGetError = null;
+        Action<Context, string, Exception>? onCachePutError = null;
+
+        var instance = Activator.CreateInstance(
+            typeof(CachePolicy),
+            flags,
+            null,
+            [
+                syncCacheProvider,
+                ttlStrategy,
+                cacheKeyStrategy,
+                onCacheGet,
+                onCacheMiss,
+                onCachePut,
+                onCacheGetError,
+                onCachePutError,
+            ],
+            null)!;
+        var instanceType = instance.GetType();
+        var methods = instanceType.GetMethods(flags);
+        var methodInfo = methods.First(method => method is { Name: "Implementation", ReturnType.Name: "TResult" });
+        var generic = methodInfo.MakeGenericMethod(typeof(EmptyStruct));
+
+        var func = () => generic.Invoke(instance, [action, new Context(), CancellationToken.None]);
+
+        var exceptionAssertions = func.Should().Throw<TargetInvocationException>();
+        exceptionAssertions.And.Message.Should().Be("Exception has been thrown by the target of an invocation.");
+        exceptionAssertions.And.InnerException.Should().BeOfType<ArgumentNullException>()
+            .Which.ParamName.Should().Be("action");
+
+        methodInfo = methods.First(method => method is { Name: "Implementation", ReturnType.Name: "Void" });
+
+        func = () => methodInfo.Invoke(instance, [actionVoid, new Context(), CancellationToken.None]);
+
+        exceptionAssertions = func.Should().Throw<TargetInvocationException>();
+        exceptionAssertions.And.Message.Should().Be("Exception has been thrown by the target of an invocation.");
+        exceptionAssertions.And.InnerException.Should().BeOfType<ArgumentNullException>()
+            .Which.ParamName.Should().Be("action");
+    }
+
     [Fact]
     public void Should_throw_when_cache_provider_is_null()
     {
diff --git a/test/Polly.Specs/Caching/CacheTResultSpecs.cs b/test/Polly.Specs/Caching/CacheTResultSpecs.cs
index ab4227531a9..15e7f278ce0 100644
--- a/test/Polly.Specs/Caching/CacheTResultSpecs.cs
+++ b/test/Polly.Specs/Caching/CacheTResultSpecs.cs
@@ -5,6 +5,48 @@ public class CacheTResultSpecs : IDisposable
 {
     #region Configuration
 
+    [Fact]
+    public void Should_throw_when_action_is_null()
+    {
+        var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+        Func<Context, CancellationToken, EmptyStruct> action = null!;
+
+        ISyncCacheProvider<EmptyStruct> syncCacheProvider = new StubCacheProvider().For<EmptyStruct>();
+        ITtlStrategy<EmptyStruct> ttlStrategy = new ContextualTtl().For<EmptyStruct>();
+        Func<Context, string> cacheKeyStrategy = (_) => string.Empty;
+        Action<Context, string> onCacheGet = (_, _) => { };
+        Action<Context, string> onCacheMiss = (_, _) => { };
+        Action<Context, string> onCachePut = (_, _) => { };
+        Action<Context, string, Exception>? onCacheGetError = null;
+        Action<Context, string, Exception>? onCachePutError = null;
+
+        var instance = Activator.CreateInstance(
+            typeof(CachePolicy<EmptyStruct>),
+            flags,
+            null,
+            [
+                syncCacheProvider,
+                ttlStrategy,
+                cacheKeyStrategy,
+                onCacheGet,
+                onCacheMiss,
+                onCachePut,
+                onCacheGetError,
+                onCachePutError,
+            ],
+            null)!;
+        var instanceType = instance.GetType();
+        var methods = instanceType.GetMethods(flags);
+        var methodInfo = methods.First(method => method is { Name: "Implementation", ReturnType.Name: "EmptyStruct" });
+
+        var func = () => methodInfo.Invoke(instance, [action, new Context(), CancellationToken.None]);
+
+        var exceptionAssertions = func.Should().Throw<TargetInvocationException>();
+        exceptionAssertions.And.Message.Should().Be("Exception has been thrown by the target of an invocation.");
+        exceptionAssertions.And.InnerException.Should().BeOfType<ArgumentNullException>()
+            .Which.ParamName.Should().Be("action");
+    }
+
     [Fact]
     public void Should_throw_when_cache_provider_is_null()
     {