From 7249a689c80e0d81d514af723b23ee168d514c0b Mon Sep 17 00:00:00 2001 From: Iliar Turdushev Date: Fri, 20 Sep 2024 15:24:44 +0200 Subject: [PATCH 1/3] Fixes #2299 Adds logic handling a case when the value being requested is null --- src/Polly.Core/ResilienceProperties.cs | 14 ++++++++--- .../ResiliencePropertiesTests.cs | 23 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/Polly.Core/ResilienceProperties.cs b/src/Polly.Core/ResilienceProperties.cs index 466901436a8..e8cd9cfbb62 100644 --- a/src/Polly.Core/ResilienceProperties.cs +++ b/src/Polly.Core/ResilienceProperties.cs @@ -20,10 +20,18 @@ public sealed class ResilienceProperties /// True, if a property was retrieved. public bool TryGetValue(ResiliencePropertyKey key, [MaybeNullWhen(false)] out TValue value) { - if (Options.TryGetValue(key.Key, out object? val) && val is TValue typedValue) + if (Options.TryGetValue(key.Key, out object? val)) { - value = typedValue; - return true; + if (val is TValue typedValue) + { + value = typedValue; + return true; + } + else if (val == null) + { + value = default!; + return true; + } } value = default; diff --git a/test/Polly.Core.Tests/ResiliencePropertiesTests.cs b/test/Polly.Core.Tests/ResiliencePropertiesTests.cs index e4ef2c31ba0..45e97016b7f 100644 --- a/test/Polly.Core.Tests/ResiliencePropertiesTests.cs +++ b/test/Polly.Core.Tests/ResiliencePropertiesTests.cs @@ -14,6 +14,18 @@ public void TryGetValue_Ok() val.Should().Be(12345); } + [Fact] + public void TryGetValue_ValueIsNull_Ok() + { + var key = new ResiliencePropertyKey("dummy"); + var props = new ResilienceProperties(); + + props.Set(key, null); + + props.TryGetValue(key, out var val).Should().Be(true); + val.Should().Be(null); + } + [Fact] public void TryGetValue_NotFound_Ok() { @@ -34,6 +46,17 @@ public void GetValue_Ok() props.GetValue(key, default).Should().Be(12345); } + [Fact] + public void GetValue_ValueIsNull_Ok() + { + var key = new ResiliencePropertyKey("dummy"); + var props = new ResilienceProperties(); + + props.Set(key, null); + + props.GetValue(key, "default").Should().Be(null); + } + [Fact] public void GetValue_NotFound_EnsureDefault() { From 864e2ebc66a011dec2f8a15741b4b9ff30b8f02e Mon Sep 17 00:00:00 2001 From: Iliar Turdushev Date: Fri, 20 Sep 2024 19:31:19 +0200 Subject: [PATCH 2/3] Replaces "val == null" with "val is null" --- src/Polly.Core/ResilienceProperties.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Polly.Core/ResilienceProperties.cs b/src/Polly.Core/ResilienceProperties.cs index e8cd9cfbb62..6866be4a190 100644 --- a/src/Polly.Core/ResilienceProperties.cs +++ b/src/Polly.Core/ResilienceProperties.cs @@ -27,7 +27,7 @@ public bool TryGetValue(ResiliencePropertyKey key, [MaybeNullWhe value = typedValue; return true; } - else if (val == null) + else if (val is null) { value = default!; return true; From 1392bececfc803e6d364c06e118a143d65a1f4fa Mon Sep 17 00:00:00 2001 From: Iliar Turdushev Date: Thu, 26 Sep 2024 11:58:10 +0200 Subject: [PATCH 3/3] Added a comment describing the need to use "!" operator --- src/Polly.Core/ResilienceProperties.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Polly.Core/ResilienceProperties.cs b/src/Polly.Core/ResilienceProperties.cs index 6866be4a190..a5f43d54086 100644 --- a/src/Polly.Core/ResilienceProperties.cs +++ b/src/Polly.Core/ResilienceProperties.cs @@ -29,6 +29,12 @@ public bool TryGetValue(ResiliencePropertyKey key, [MaybeNullWhe } else if (val is null) { + // We have to use null-forgiving operator "!" here to suppress a null-state analysis warning. + // The reason is the following. The output type "TValue" doesn't have any type constraints as + // "notnull", "class" or "struct", therefore the analyzer considers "TValue" as non-nullable + // and warns us that we're assigning "null" to it. But that's not correct, because "TValue" + // could be a nullable type, e.g. "string?", and assigning "null" to it is correct. Therefore + // it is reasonable to use "!" here to suppress the warning. value = default!; return true; }