Fix CA1062 warnings (#2239)

Fix CA1062 warnings for `AsyncCachePolicy`.
App-vNext · Jul 23, 2024 · eecf683 · eecf683
1 parent c332b15
commit eecf683
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 8 deletions.
diff --git a/src/Polly/Caching/AsyncCachePolicy.cs b/src/Polly/Caching/AsyncCachePolicy.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 AsyncCachePolicy : AsyncPolicy
 {
     private readonly IAsyncCacheProvider _asyncCacheProvider;
@@ -43,13 +42,31 @@ protected override Task ImplementationAsync(
         Func<Context, CancellationToken, Task> action,
         Context context,
         CancellationToken cancellationToken,
-        bool continueOnCapturedContext) => action(context, cancellationToken); // Pass-through/NOOP policy action, for void-returning executions through the cache policy.
+        bool continueOnCapturedContext)
+    {
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        // Pass-through/NOOP policy action, for void-returning executions through the cache policy.
+        return action(context, cancellationToken);
+    }
 
     /// <inheritdoc/>
     [DebuggerStepThrough]
-    protected override Task<TResult> ImplementationAsync<TResult>(Func<Context, CancellationToken, Task<TResult>> action, Context context, CancellationToken cancellationToken,
-        bool continueOnCapturedContext) =>
-        AsyncCacheEngine.ImplementationAsync<TResult>(
+    protected override Task<TResult> ImplementationAsync<TResult>(
+        Func<Context, CancellationToken, Task<TResult>> action,
+        Context context,
+        CancellationToken cancellationToken,
+        bool continueOnCapturedContext)
+    {
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        return AsyncCacheEngine.ImplementationAsync<TResult>(
             _asyncCacheProvider.AsyncFor<TResult>(),
             _ttlStrategy.For<TResult>(),
             _cacheKeyStrategy,
@@ -62,6 +79,7 @@ protected override Task<TResult> ImplementationAsync<TResult>(Func<Context, Canc
             _onCacheGetError,
             _onCachePutError,
             cancellationToken);
+    }
 }
 
 /// <summary>
@@ -104,9 +122,18 @@ internal AsyncCachePolicy(
 
     /// <inheritdoc/>
     [DebuggerStepThrough]
-    protected override Task<TResult> ImplementationAsync(Func<Context, CancellationToken, Task<TResult>> action, Context context, CancellationToken cancellationToken,
-        bool continueOnCapturedContext) =>
-        AsyncCacheEngine.ImplementationAsync<TResult>(
+    protected override Task<TResult> ImplementationAsync(
+        Func<Context, CancellationToken, Task<TResult>> action,
+        Context context,
+        CancellationToken cancellationToken,
+        bool continueOnCapturedContext)
+    {
+        if (action is null)
+        {
+            throw new ArgumentNullException(nameof(action));
+        }
+
+        return AsyncCacheEngine.ImplementationAsync<TResult>(
             _asyncCacheProvider,
             _ttlStrategy,
             _cacheKeyStrategy,
@@ -119,5 +146,6 @@ protected override Task<TResult> ImplementationAsync(Func<Context, CancellationT
             _onCacheGetError,
             _onCachePutError,
             cancellationToken);
+    }
 }
 
diff --git a/test/Polly.Specs/Caching/CacheAsyncSpecs.cs b/test/Polly.Specs/Caching/CacheAsyncSpecs.cs
@@ -5,6 +5,59 @@ public class CacheAsyncSpecs : IDisposable
 {
     #region Configuration
 
+    [Fact]
+    public void Should_throw_when_action_is_null()
+    {
+        var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+        Func<Context, CancellationToken, Task<EmptyStruct>> action = null!;
+        Func<Context, CancellationToken, Task> actionVoid = null!;
+
+        IAsyncCacheProvider asyncCacheProvider = 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(AsyncCachePolicy),
+            flags,
+            null,
+            [
+                asyncCacheProvider,
+                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: "ImplementationAsync", ReturnType.Name: "Task`1" });
+        var generic = methodInfo.MakeGenericMethod(typeof(EmptyStruct));
+
+        var func = () => generic.Invoke(instance, [action, new Context(), CancellationToken.None, false]);
+
+        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: "ImplementationAsync", ReturnType.Name: "Task" });
+
+        func = () => methodInfo.Invoke(instance, [actionVoid, new Context(), CancellationToken.None, false]);
+
+        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/CacheTResultAsyncSpecs.cs b/test/Polly.Specs/Caching/CacheTResultAsyncSpecs.cs
@@ -5,6 +5,48 @@ public class CacheTResultAsyncSpecs : IDisposable
 {
     #region Configuration
 
+    [Fact]
+    public void Should_throw_when_action_is_null()
+    {
+        var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+        Func<Context, CancellationToken, Task<EmptyStruct>> action = null!;
+
+        IAsyncCacheProvider<EmptyStruct> asyncCacheProvider = new StubCacheProvider().AsyncFor<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(AsyncCachePolicy<EmptyStruct>),
+            flags,
+            null,
+            [
+                asyncCacheProvider,
+                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: "ImplementationAsync", ReturnType.Name: "Task`1" });
+
+        var func = () => methodInfo.Invoke(instance, [action, new Context(), CancellationToken.None, false]);
+
+        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()
     {