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

Fix CA1062 warnings for `CacheTResultSyntax`.
---
 src/Polly/Caching/CacheTResultSyntax.cs       | 122 ++++++++++++++++--
 test/Polly.Specs/Caching/CacheTResultSpecs.cs |  63 ++++++++-
 2 files changed, 168 insertions(+), 17 deletions(-)

diff --git a/src/Polly/Caching/CacheTResultSyntax.cs b/src/Polly/Caching/CacheTResultSyntax.cs
index b3c571e5fa2..bd2d9009380 100644
--- a/src/Polly/Caching/CacheTResultSyntax.cs
+++ b/src/Polly/Caching/CacheTResultSyntax.cs
@@ -1,7 +1,6 @@
 #nullable enable
 namespace Polly;
 
-#pragma warning disable CA1062 // Validate arguments of public methods
 public partial class Policy
 {
     /// <summary>
@@ -70,6 +69,11 @@ public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider cacheProvid
             throw new ArgumentNullException(nameof(cacheProvider));
         }
 
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
         return Cache<TResult>(cacheProvider.For<TResult>(), new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheError);
     }
 
@@ -88,13 +92,22 @@ public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider cacheProvid
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheProvider"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="ttlStrategy"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheKeyStrategy"/> is <see langword="null"/>.</exception>
-    public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider cacheProvider, ITtlStrategy ttlStrategy, ICacheKeyStrategy cacheKeyStrategy, Action<Context, string, Exception>? onCacheError = null)
+    public static CachePolicy<TResult> Cache<TResult>(
+        ISyncCacheProvider cacheProvider,
+        ITtlStrategy ttlStrategy,
+        ICacheKeyStrategy cacheKeyStrategy,
+        Action<Context, string, Exception>? onCacheError = null)
     {
         if (cacheProvider == null)
         {
             throw new ArgumentNullException(nameof(cacheProvider));
         }
 
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
         return Cache<TResult>(cacheProvider.For<TResult>(), ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheError);
     }
 
@@ -256,6 +269,11 @@ public static CachePolicy<TResult> Cache<TResult>(
             throw new ArgumentNullException(nameof(cacheProvider));
         }
 
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
         return Cache<TResult>(cacheProvider.For<TResult>(), new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError);
     }
 
@@ -296,6 +314,11 @@ public static CachePolicy<TResult> Cache<TResult>(
             throw new ArgumentNullException(nameof(cacheProvider));
         }
 
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
         return Cache<TResult>(cacheProvider.For<TResult>(), ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError);
     }
 
@@ -439,8 +462,19 @@ public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider<TResult> ca
     /// <returns>The policy instance.</returns>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheProvider"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheKeyStrategy"/> is <see langword="null"/>.</exception>
-    public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider<TResult> cacheProvider, TimeSpan ttl, ICacheKeyStrategy cacheKeyStrategy, Action<Context, string, Exception>? onCacheError = null) =>
-        Cache<TResult>(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheError);
+    public static CachePolicy<TResult> Cache<TResult>(
+        ISyncCacheProvider<TResult> cacheProvider,
+        TimeSpan ttl,
+        ICacheKeyStrategy cacheKeyStrategy,
+        Action<Context, string, Exception>? onCacheError = null)
+    {
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
+        return Cache<TResult>(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheError);
+    }
 
     /// <summary>
     /// <para>Builds a <see cref="Policy{TResult}" /> that will function like a result cache for delegate executions returning a <typeparamref name="TResult"/>.</para>
@@ -457,12 +491,28 @@ public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider<TResult> ca
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheProvider"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="ttlStrategy"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheKeyStrategy"/> is <see langword="null"/>.</exception>
-    public static CachePolicy<TResult> Cache<TResult>(ISyncCacheProvider<TResult> cacheProvider, ITtlStrategy ttlStrategy, ICacheKeyStrategy cacheKeyStrategy, Action<Context, string, Exception>? onCacheError = null)
+    public static CachePolicy<TResult> Cache<TResult>(
+        ISyncCacheProvider<TResult> cacheProvider,
+        ITtlStrategy ttlStrategy,
+        ICacheKeyStrategy cacheKeyStrategy,
+        Action<Context, string, Exception>? onCacheError = null)
     {
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
         Action<Context, string> emptyDelegate = (_, _) => { };
 
-        return Cache<TResult>(cacheProvider, ttlStrategy.For<TResult>(), cacheKeyStrategy.GetCacheKey,
-            emptyDelegate, emptyDelegate, emptyDelegate, onCacheError, onCacheError);
+        return Cache<TResult>(
+            cacheProvider,
+            ttlStrategy.For<TResult>(),
+            cacheKeyStrategy.GetCacheKey,
+            emptyDelegate,
+            emptyDelegate,
+            emptyDelegate,
+            onCacheError,
+            onCacheError);
     }
 
     /// <summary>
@@ -670,9 +720,23 @@ public static CachePolicy<TResult> Cache<TResult>(
         Action<Context, string> onCacheMiss,
         Action<Context, string> onCachePut,
         Action<Context, string, Exception>? onCacheGetError,
-        Action<Context, string, Exception>? onCachePutError) =>
-        Cache<TResult>(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss,
-            onCachePut, onCacheGetError, onCachePutError);
+        Action<Context, string, Exception>? onCachePutError)
+    {
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
+        return Cache<TResult>(
+            cacheProvider,
+            new RelativeTtl(ttl),
+            cacheKeyStrategy.GetCacheKey,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+    }
 
     /// <summary>
     /// <para>Builds a <see cref="Policy{TResult}" /> that will function like a result cache for delegate executions returning a <typeparamref name="TResult"/>.</para>
@@ -704,8 +768,23 @@ public static CachePolicy<TResult> Cache<TResult>(
         Action<Context, string> onCacheMiss,
         Action<Context, string> onCachePut,
         Action<Context, string, Exception>? onCacheGetError,
-        Action<Context, string, Exception>? onCachePutError) =>
-        Cache<TResult>(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError);
+        Action<Context, string, Exception>? onCachePutError)
+    {
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
+        return Cache<TResult>(
+            cacheProvider,
+            ttlStrategy,
+            cacheKeyStrategy.GetCacheKey,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+    }
 
     /// <summary>
     /// <para>Builds a <see cref="Policy{TResult}" /> that will function like a result cache for delegate executions returning a <typeparamref name="TResult"/>.</para>
@@ -737,8 +816,23 @@ public static CachePolicy<TResult> Cache<TResult>(
         Action<Context, string> onCacheMiss,
         Action<Context, string> onCachePut,
         Action<Context, string, Exception>? onCacheGetError,
-        Action<Context, string, Exception>? onCachePutError) =>
-        Cache<TResult>(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError);
+        Action<Context, string, Exception>? onCachePutError)
+    {
+        if (cacheKeyStrategy is null)
+        {
+            throw new ArgumentNullException(nameof(cacheKeyStrategy));
+        }
+
+        return Cache<TResult>(
+            cacheProvider,
+            ttlStrategy,
+            cacheKeyStrategy.GetCacheKey,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+    }
 
     /// <summary>
     /// <para>Builds a <see cref="Policy{TResult}" /> that will function like a result cache for delegate executions returning a <typeparamref name="TResult"/>.</para>
diff --git a/test/Polly.Specs/Caching/CacheTResultSpecs.cs b/test/Polly.Specs/Caching/CacheTResultSpecs.cs
index 15e7f278ce0..360e330c1e6 100644
--- a/test/Polly.Specs/Caching/CacheTResultSpecs.cs
+++ b/test/Polly.Specs/Caching/CacheTResultSpecs.cs
@@ -51,8 +51,13 @@ public void Should_throw_when_action_is_null()
     public void Should_throw_when_cache_provider_is_null()
     {
         ISyncCacheProvider cacheProvider = null!;
+        ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]);
+
         Action action = () => Policy.Cache<ResultPrimitive>(cacheProvider, TimeSpan.MaxValue);
         action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("cacheProvider");
+
+        action = () => Policy.Cache<ResultPrimitive>(cacheProvider, new ContextualTtl(), cacheKeyStrategy);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("cacheProvider");
     }
 
     [Fact]
@@ -68,9 +73,61 @@ public void Should_throw_when_ttl_strategy_is_null()
     public void Should_throw_when_cache_key_strategy_is_null()
     {
         ISyncCacheProvider cacheProvider = new StubCacheProvider();
-        Func<Context, string> cacheKeyStrategy = null!;
-        Action action = () => Policy.Cache<ResultPrimitive>(cacheProvider, TimeSpan.MaxValue, cacheKeyStrategy);
-        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("cacheKeyStrategy");
+        var ttl = TimeSpan.MaxValue;
+        ITtlStrategy ttlStrategy = new ContextualTtl();
+        ICacheKeyStrategy cacheKeyStrategy = null!;
+        Func<Context, string> cacheKeyStrategyFunc = null!;
+        Action<Context, string> onCacheGet = (_, _) => { };
+        Action<Context, string> onCacheMiss = (_, _) => { };
+        Action<Context, string> onCachePut = (_, _) => { };
+        Action<Context, string, Exception>? onCacheGetError = null;
+        Action<Context, string, Exception>? onCachePutError = null;
+        const string CacheKeyStrategyExpected = "cacheKeyStrategy";
+
+        Action action = () => Policy.Cache<ResultPrimitive>(cacheProvider, ttlStrategy, cacheKeyStrategy, onCacheGetError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(cacheProvider, ttl, cacheKeyStrategyFunc);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(cacheProvider.For<ResultPrimitive>(), ttl, cacheKeyStrategy, onCacheGetError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(cacheProvider.For<ResultPrimitive>(), ttlStrategy, cacheKeyStrategy, onCacheGetError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(
+            cacheProvider.For<ResultPrimitive>(),
+            ttl,
+            cacheKeyStrategy,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(
+            cacheProvider.For<ResultPrimitive>(),
+            ttlStrategy,
+            cacheKeyStrategy,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
+
+        action = () => Policy.Cache<ResultPrimitive>(
+            cacheProvider.For<ResultPrimitive>(),
+            ttlStrategy.For<ResultPrimitive>(),
+            cacheKeyStrategy,
+            onCacheGet,
+            onCacheMiss,
+            onCachePut,
+            onCacheGetError,
+            onCachePutError);
+        action.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be(CacheKeyStrategyExpected);
     }
 
     #endregion