From acb669e5ea4c8b840e5f45681aa0bbb47a38ad73 Mon Sep 17 00:00:00 2001 From: ZLoo Date: Tue, 23 Jul 2024 01:24:07 +0300 Subject: [PATCH 1/4] Fix warning CA1062#CacheSyntax --- src/Polly/Caching/CacheSyntax.cs | 40 ++++++++++++++++++++----- test/Polly.Specs/Caching/CacheSpecs.cs | 41 ++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/Polly/Caching/CacheSyntax.cs b/src/Polly/Caching/CacheSyntax.cs index 221bd288587..8a4a1d1a0f4 100644 --- a/src/Polly/Caching/CacheSyntax.cs +++ b/src/Polly/Caching/CacheSyntax.cs @@ -1,7 +1,6 @@ #nullable enable namespace Polly; -#pragma warning disable CA1062 // Validate arguments of public methods public partial class Policy { /// @@ -46,8 +45,19 @@ public static CachePolicy Cache(ISyncCacheProvider cacheProvider, ITtlStrategy t /// The policy instance. /// Thrown when is . /// Thrown when is . - public static CachePolicy Cache(ISyncCacheProvider cacheProvider, TimeSpan ttl, ICacheKeyStrategy cacheKeyStrategy, Action? onCacheError = null) => - Cache(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheError); + public static CachePolicy Cache( + ISyncCacheProvider cacheProvider, + TimeSpan ttl, + ICacheKeyStrategy cacheKeyStrategy, + Action? onCacheError = null) + { + if (cacheKeyStrategy is null) + { + throw new ArgumentNullException(nameof(cacheKeyStrategy)); + } + + return Cache(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheError); + } /// /// Builds a that will function like a result cache for delegate executions returning a result. @@ -222,8 +232,16 @@ public static CachePolicy Cache( Action onCacheMiss, Action onCachePut, Action? onCacheGetError, - Action? onCachePutError) => - Cache(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError); + Action? onCachePutError) + { + if (cacheKeyStrategy is null) + { + throw new ArgumentNullException(nameof(cacheKeyStrategy)); + } + + return Cache(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, + onCachePut, onCacheGetError, onCachePutError); + } /// /// Builds a that will function like a result cache for delegate executions returning a result. @@ -254,8 +272,16 @@ public static CachePolicy Cache( Action onCacheMiss, Action onCachePut, Action? onCacheGetError, - Action? onCachePutError) => - Cache(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, onCacheGetError, onCachePutError); + Action? onCachePutError) + { + if (cacheKeyStrategy is null) + { + throw new ArgumentNullException(nameof(cacheKeyStrategy)); + } + + return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, + onCacheGetError, onCachePutError); + } /// /// Builds a that will function like a result cache for delegate executions returning a result. diff --git a/test/Polly.Specs/Caching/CacheSpecs.cs b/test/Polly.Specs/Caching/CacheSpecs.cs index e1e06fe6cf4..78160ce240b 100644 --- a/test/Polly.Specs/Caching/CacheSpecs.cs +++ b/test/Polly.Specs/Caching/CacheSpecs.cs @@ -26,9 +26,44 @@ public void Should_throw_when_ttl_strategy_is_null() public void Should_throw_when_cache_key_strategy_is_null() { ISyncCacheProvider cacheProvider = new StubCacheProvider(); - Func cacheKeyStrategy = null!; - Action action = () => Policy.Cache(cacheProvider, TimeSpan.MaxValue, cacheKeyStrategy); - action.Should().Throw().And.ParamName.Should().Be("cacheKeyStrategy"); + var ttl = TimeSpan.MaxValue; + ITtlStrategy ttlStrategy = new ContextualTtl(); + ICacheKeyStrategy cacheKeyStrategy = null!; + Func cacheKeyStrategyFunc = null!; + Action onCacheGet = (_, _) => { }; + Action onCacheMiss = (_, _) => { }; + Action onCachePut = (_, _) => { }; + Action? onCacheGetError = null; + Action? onCachePutError = null; + const string expected = "cacheKeyStrategy"; + + Action action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy); + action.Should().Throw().And.ParamName.Should().Be(expected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc); + action.Should().Throw().And.ParamName.Should().Be(expected); + + action = () => Policy.Cache( + cacheProvider, + ttl, + cacheKeyStrategy, + onCacheGet, + onCacheMiss, + onCachePut, + onCacheGetError, + onCachePutError); + action.Should().Throw().And.ParamName.Should().Be(expected); + + action = () => Policy.Cache( + cacheProvider, + ttlStrategy, + cacheKeyStrategy, + onCacheGet, + onCacheMiss, + onCachePut, + onCacheGetError, + onCachePutError); + action.Should().Throw().And.ParamName.Should().Be(expected); } #endregion From 430ee4b3eddb8815c8c5583888bb34954fc4f6b6 Mon Sep 17 00:00:00 2001 From: ZLoo Date: Tue, 23 Jul 2024 01:36:18 +0300 Subject: [PATCH 2/4] Fix PR --- test/Polly.Specs/Caching/CacheSpecs.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Polly.Specs/Caching/CacheSpecs.cs b/test/Polly.Specs/Caching/CacheSpecs.cs index 78160ce240b..ad2e32d8f33 100644 --- a/test/Polly.Specs/Caching/CacheSpecs.cs +++ b/test/Polly.Specs/Caching/CacheSpecs.cs @@ -35,13 +35,13 @@ public void Should_throw_when_cache_key_strategy_is_null() Action onCachePut = (_, _) => { }; Action? onCacheGetError = null; Action? onCachePutError = null; - const string expected = "cacheKeyStrategy"; + const string CacheKeyStrategyExpected = "cacheKeyStrategy"; Action action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy); - action.Should().Throw().And.ParamName.Should().Be(expected); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc); - action.Should().Throw().And.ParamName.Should().Be(expected); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); action = () => Policy.Cache( cacheProvider, @@ -52,7 +52,7 @@ public void Should_throw_when_cache_key_strategy_is_null() onCachePut, onCacheGetError, onCachePutError); - action.Should().Throw().And.ParamName.Should().Be(expected); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); action = () => Policy.Cache( cacheProvider, @@ -63,7 +63,7 @@ public void Should_throw_when_cache_key_strategy_is_null() onCachePut, onCacheGetError, onCachePutError); - action.Should().Throw().And.ParamName.Should().Be(expected); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); } #endregion From 461c58f9d1012cc5d8c33e00acf0d08a0a3450fc Mon Sep 17 00:00:00 2001 From: ZLoo Date: Tue, 23 Jul 2024 08:51:49 +0300 Subject: [PATCH 3/4] Add tests --- test/Polly.Specs/Caching/CacheSpecs.cs | 225 ++++++++++++++++++++++--- 1 file changed, 204 insertions(+), 21 deletions(-) diff --git a/test/Polly.Specs/Caching/CacheSpecs.cs b/test/Polly.Specs/Caching/CacheSpecs.cs index ad2e32d8f33..1f89159c787 100644 --- a/test/Polly.Specs/Caching/CacheSpecs.cs +++ b/test/Polly.Specs/Caching/CacheSpecs.cs @@ -9,8 +9,49 @@ public class CacheSpecs : IDisposable public void Should_throw_when_cache_provider_is_null() { ISyncCacheProvider cacheProvider = null!; - Action action = () => Policy.Cache(cacheProvider, TimeSpan.MaxValue); - action.Should().Throw().And.ParamName.Should().Be("cacheProvider"); + var ttl = TimeSpan.MaxValue; + ITtlStrategy ttlStrategy = new ContextualTtl(); + ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]); + Func cacheKeyStrategyFunc = (_) => string.Empty; + Action onCache = (_, _) => { }; + Action? onCacheError = null; + const string CacheProviderExpected = "cacheProvider"; + + Action action = () => Policy.Cache(cacheProvider, ttl, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttl, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheProviderExpected); } [Fact] @@ -18,8 +59,29 @@ public void Should_throw_when_ttl_strategy_is_null() { ISyncCacheProvider cacheProvider = new StubCacheProvider(); ITtlStrategy ttlStrategy = null!; - Action action = () => Policy.Cache(cacheProvider, ttlStrategy); - action.Should().Throw().And.ParamName.Should().Be("ttlStrategy"); + ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]); + Func cacheKeyStrategyFunc = (_) => string.Empty; + Action onCache = (_, _) => { }; + Action? onCacheError = null; + const string TtlStrategyExpected = "ttlStrategy"; + + Action action = () => Policy.Cache(cacheProvider, ttlStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCache, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(TtlStrategyExpected); } [Fact] @@ -30,40 +92,161 @@ public void Should_throw_when_cache_key_strategy_is_null() ITtlStrategy ttlStrategy = new ContextualTtl(); ICacheKeyStrategy cacheKeyStrategy = null!; Func cacheKeyStrategyFunc = null!; - Action onCacheGet = (_, _) => { }; - Action onCacheMiss = (_, _) => { }; - Action onCachePut = (_, _) => { }; - Action? onCacheGetError = null; - Action? onCachePutError = null; + Action onCache = (_, _) => { }; + Action? onCacheError = null; const string CacheKeyStrategyExpected = "cacheKeyStrategy"; - Action action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy); + Action action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCacheError); action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); - action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc); + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCacheError); action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); action = () => Policy.Cache( cacheProvider, ttl, cacheKeyStrategy, - onCacheGet, - onCacheMiss, - onCachePut, - onCacheGetError, - onCachePutError); + onCache, + onCache, + onCache, + onCacheError, + onCacheError); action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); action = () => Policy.Cache( cacheProvider, ttlStrategy, cacheKeyStrategy, - onCacheGet, - onCacheMiss, - onCachePut, - onCacheGetError, - onCachePutError); + onCache, + onCache, + onCache, + onCacheError, + onCacheError); action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); + + action = () => Policy.Cache( + cacheProvider, + ttl, + cacheKeyStrategyFunc, + onCache, + onCache, + onCache, + onCacheError, + onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); + + action = () => Policy.Cache( + cacheProvider, + ttlStrategy, + cacheKeyStrategyFunc, + onCache, + onCache, + onCache, + onCacheError, + onCacheError); + action.Should().Throw().And.ParamName.Should().Be(CacheKeyStrategyExpected); + } + + [Fact] + public void Should_throw_when_on_cache_get_is_null() + { + ISyncCacheProvider cacheProvider = new StubCacheProvider(); + var ttl = TimeSpan.MaxValue; + ITtlStrategy ttlStrategy = new ContextualTtl(); + ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]); + Func cacheKeyStrategyFunc = (_) => string.Empty; + Action onCacheGet = null!; + Action onCache = (_, _) => { }; + Action? onCacheError = null; + const string OnCacheGetExpected = "onCacheGet"; + + Action action = () => Policy.Cache(cacheProvider, ttl, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCacheGet, onCache, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheGetExpected); + } + + [Fact] + public void Should_throw_when_on_cache_miss_is_null() + { + ISyncCacheProvider cacheProvider = new StubCacheProvider(); + var ttl = TimeSpan.MaxValue; + ITtlStrategy ttlStrategy = new ContextualTtl(); + ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]); + Func cacheKeyStrategyFunc = (_) => string.Empty; + Action onCacheMiss = null!; + Action onCache = (_, _) => { }; + Action? onCacheError = null; + const string OnCacheMissExpected = "onCacheMiss"; + + Action action = () => Policy.Cache(cacheProvider, ttl, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCache, onCacheMiss, onCache, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCacheMissExpected); + } + + [Fact] + public void Should_throw_when_on_cache_put_is_null() + { + ISyncCacheProvider cacheProvider = new StubCacheProvider(); + var ttl = TimeSpan.MaxValue; + ITtlStrategy ttlStrategy = new ContextualTtl(); + ICacheKeyStrategy cacheKeyStrategy = new StubCacheKeyStrategy(context => context.OperationKey + context["id"]); + Func cacheKeyStrategyFunc = (_) => string.Empty; + Action onCachePut = null!; + Action onCache = (_, _) => { }; + Action? onCacheError = null; + const string OnCachePutExpected = "onCachePut"; + + Action action = () => Policy.Cache(cacheProvider, ttl, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategy, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); + + action = () => Policy.Cache(cacheProvider, ttl, cacheKeyStrategyFunc, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); + + action = () => Policy.Cache(cacheProvider, ttlStrategy, cacheKeyStrategyFunc, onCache, onCache, onCachePut, onCacheError, onCacheError); + action.Should().Throw().And.ParamName.Should().Be(OnCachePutExpected); } #endregion From 92e5d10975931bab3a171301d8a79b0ab4615392 Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Tue, 23 Jul 2024 08:14:11 +0100 Subject: [PATCH 4/4] Apply suggestions from code review --- src/Polly/Caching/CacheSyntax.cs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Polly/Caching/CacheSyntax.cs b/src/Polly/Caching/CacheSyntax.cs index 8a4a1d1a0f4..7babe72907f 100644 --- a/src/Polly/Caching/CacheSyntax.cs +++ b/src/Polly/Caching/CacheSyntax.cs @@ -239,8 +239,15 @@ public static CachePolicy Cache( throw new ArgumentNullException(nameof(cacheKeyStrategy)); } - return Cache(cacheProvider, new RelativeTtl(ttl), cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, - onCachePut, onCacheGetError, onCachePutError); + return Cache( + cacheProvider, + new RelativeTtl(ttl), + cacheKeyStrategy.GetCacheKey, + onCacheGet, + onCacheMiss, + onCachePut, + onCacheGetError, + onCachePutError); } /// @@ -279,8 +286,15 @@ public static CachePolicy Cache( throw new ArgumentNullException(nameof(cacheKeyStrategy)); } - return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, onCacheGet, onCacheMiss, onCachePut, - onCacheGetError, onCachePutError); + return Cache( + cacheProvider, + ttlStrategy, + cacheKeyStrategy.GetCacheKey, + onCacheGet, + onCacheMiss, + onCachePut, + onCacheGetError, + onCachePutError); } ///