Fixes dotnet#4957

Replaces usage of ResilienceKeys.RequestMessage variable with corresponding Get/Set methods
iliar-turdushev · Sep 30, 2024 · 69a412c · 69a412c
1 parent 5631d0d
commit 69a412c
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 24 deletions.
diff --git a/.../Microsoft.Extensions.Http.Resilience/Hedging/Internals/RequestMessageSnapshotStrategy.cs b/.../Microsoft.Extensions.Http.Resilience/Hedging/Internals/RequestMessageSnapshotStrategy.cs
@@ -20,7 +20,9 @@ protected override async ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState
         ResilienceContext context,
         TState state)
     {
-        if (!context.Properties.TryGetValue(ResilienceKeys.RequestMessage, out var request) || request is null)
+        HttpRequestMessage? request = context.GetRequestMessage();
+
+        if (request is null)
         {
             Throw.InvalidOperationException("The HTTP request message was not found in the resilience context.");
         }

diff --git a/...osoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs b/...osoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs
@@ -94,7 +94,7 @@ public static IStandardHedgingHandlerBuilder AddStandardHedgingHandler(this IHtt
                 requestMessage.SetResilienceContext(args.ActionContext);
 
                 // replace the request message
-                args.ActionContext.Properties.Set(ResilienceKeys.RequestMessage, requestMessage);
+                args.ActionContext.SetRequestMessage(requestMessage);
 
                 if (args.PrimaryContext.Properties.TryGetValue(ResilienceKeys.RoutingStrategy, out var routingPipeline))
                 {

diff --git a/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHandler.cs b/src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHandler.cs
@@ -58,7 +58,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
         ResilienceContext context = GetOrSetResilienceContext(request, cancellationToken, out bool created);
         TrySetRequestMetadata(context, request);
-        SetRequestMessage(context, request);
+        context.SetRequestMessage(request);
 
         try
         {
@@ -117,7 +117,7 @@ protected override HttpResponseMessage Send(HttpRequestMessage request, Cancella
 
         ResilienceContext context = GetOrSetResilienceContext(request, cancellationToken, out bool created);
         TrySetRequestMetadata(context, request);
-        SetRequestMessage(context, request);
+        context.SetRequestMessage(request);
 
         try
         {
@@ -165,11 +165,8 @@ private static void TrySetRequestMetadata(ResilienceContext context, HttpRequest
         }
     }
 
-    private static void SetRequestMessage(ResilienceContext context, HttpRequestMessage request)
-        => context.Properties.Set(ResilienceKeys.RequestMessage, request);
-
     private static HttpRequestMessage GetRequestMessage(ResilienceContext context, HttpRequestMessage request)
-        => context.Properties.GetValue(ResilienceKeys.RequestMessage, request);
+        => context.GetRequestMessage() ?? request;
 
     private static void RestoreResilienceContext(ResilienceContext context, HttpRequestMessage request, bool created)
     {

diff --git a/...raries/Microsoft.Extensions.Http.Resilience/Routing/Internal/RoutingResilienceStrategy.cs b/...raries/Microsoft.Extensions.Http.Resilience/Routing/Internal/RoutingResilienceStrategy.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Net.Http;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Http.Resilience.Internal;
 using Microsoft.Shared.Diagnostics;
@@ -26,7 +27,9 @@ protected override async ValueTask<Outcome<TResult>> ExecuteCore<TResult, TState
         ResilienceContext context,
         TState state)
     {
-        if (!context.Properties.TryGetValue(ResilienceKeys.RequestMessage, out var request))
+        HttpRequestMessage? request = context.GetRequestMessage();
+
+        if (request is null)
         {
             Throw.InvalidOperationException("The HTTP request message was not found in the resilience context.");
         }

diff --git a/...icrosoft.Extensions.Http.Resilience.Tests/Internal/RequestMessageSnapshotStrategyTests.cs b/...icrosoft.Extensions.Http.Resilience.Tests/Internal/RequestMessageSnapshotStrategyTests.cs
@@ -20,7 +20,7 @@ public async Task ExecuteAsync_EnsureSnapshotAttached()
         var strategy = Create();
         var context = ResilienceContextPool.Shared.Get();
         using var request = new HttpRequestMessage();
-        context.Properties.Set(ResilienceKeys.RequestMessage, request);
+        context.SetRequestMessage(request);
 
         using var response = await strategy.ExecuteAsync(
             context =>
@@ -39,5 +39,15 @@ public void ExecuteAsync_RequestMessageNotFound_Throws()
         strategy.Invoking(s => s.Execute(() => { })).Should().Throw<InvalidOperationException>();
     }
 
+    [Fact]
+    public void ExecuteAsync_RequestMessageIsNull_Throws()
+    {
+        var strategy = Create();
+        var context = ResilienceContextPool.Shared.Get();
+        context.SetRequestMessage(null);
+
+        strategy.Invoking(s => s.Execute(_ => { }, context)).Should().Throw<InvalidOperationException>();
+    }
+
     private static ResiliencePipeline Create() => new ResiliencePipelineBuilder().AddStrategy(_ => new RequestMessageSnapshotStrategy(), Mock.Of<ResilienceStrategyOptions>()).Build();
 }
diff --git a/...osoft.Extensions.Http.Resilience.Tests/Resilience/HttpResilienceContextExtensionsTests.cs b/...osoft.Extensions.Http.Resilience.Tests/Resilience/HttpResilienceContextExtensionsTests.cs
@@ -24,8 +24,6 @@ public void GetRequestMessage_RequestMessageIsMissing_ReturnsNull()
         var context = ResilienceContextPool.Shared.Get();
 
         Assert.Null(context.GetRequestMessage());
-
-        ResilienceContextPool.Shared.Return(context);
     }
 
     [Fact]
@@ -35,8 +33,6 @@ public void GetRequestMessage_RequestMessageIsNull_ReturnsNull()
         context.Properties.Set(ResilienceKeys.RequestMessage, null);
 
         Assert.Null(context.GetRequestMessage());
-
-        ResilienceContextPool.Shared.Return(context);
     }
 
     [Fact]
@@ -47,15 +43,14 @@ public void GetRequestMessage_RequestMessageIsPresent_ReturnsRequestMessage()
         context.Properties.Set(ResilienceKeys.RequestMessage, request);
 
         Assert.Same(request, context.GetRequestMessage());
-
-        ResilienceContextPool.Shared.Return(context);
     }
 
     [Fact]
     public void SetRequestMessage_ResilienceContextIsNull_Throws()
     {
         ResilienceContext context = null!;
         using var request = new HttpRequestMessage();
+
         Assert.Throws<ArgumentNullException>(() => context.SetRequestMessage(request));
     }
 
@@ -67,8 +62,6 @@ public void SetRequestMessage_RequestMessageIsNull_SetsNullRequestMessage()
 
         Assert.True(context.Properties.TryGetValue(ResilienceKeys.RequestMessage, out HttpRequestMessage? request));
         Assert.Null(request);
-
-        ResilienceContextPool.Shared.Return(context);
     }
 
     [Fact]
@@ -80,7 +73,5 @@ public void SetRequestMessage_RequestMessageIsNotNull_SetsRequestMessage()
 
         Assert.True(context.Properties.TryGetValue(ResilienceKeys.RequestMessage, out HttpRequestMessage? actualRequest));
         Assert.Same(request, actualRequest);
-
-        ResilienceContextPool.Shared.Return(context);
     }
 }
diff --git a/.../Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/ResilienceHandlerTest.cs b/.../Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/ResilienceHandlerTest.cs
@@ -8,7 +8,6 @@
 using System.Threading.Tasks;
 using FluentAssertions;
 using Microsoft.Extensions.Http.Diagnostics;
-using Microsoft.Extensions.Http.Resilience.Internal;
 using Microsoft.Extensions.Http.Resilience.Test.Helpers;
 using Polly;
 using Xunit;
@@ -108,7 +107,7 @@ public async Task Send_EnsureInvoker(bool executionContextSet, bool asynchronous
         handler.InnerHandler = new TestHandlerStub((r, _) =>
         {
             r.GetResilienceContext().Should().NotBeNull();
-            r.GetResilienceContext()!.Properties.GetValue(ResilienceKeys.RequestMessage, null!).Should().BeSameAs(r);
+            r.GetResilienceContext()!.GetRequestMessage().Should().BeSameAs(r);
 
             return Task.FromResult(new HttpResponseMessage { StatusCode = HttpStatusCode.Created });
         });

diff --git a/...ries/Microsoft.Extensions.Http.Resilience.Tests/Routing/RoutingResilienceStrategyTests.cs b/...ries/Microsoft.Extensions.Http.Resilience.Tests/Routing/RoutingResilienceStrategyTests.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Net.Http;
 using FluentAssertions;
-using Microsoft.Extensions.Http.Resilience.Internal;
 using Microsoft.Extensions.Http.Resilience.Routing.Internal;
 using Moq;
 using Polly;
@@ -22,14 +21,24 @@ public void NoRequestMessage_Throws()
         strategy.Invoking(s => s.Execute(() => { })).Should().Throw<InvalidOperationException>().WithMessage("The HTTP request message was not found in the resilience context.");
     }
 
+    [Fact]
+    public void RequestMessageIsNull_Throws()
+    {
+        var strategy = Create(() => Mock.Of<RequestRoutingStrategy>());
+        var context = ResilienceContextPool.Shared.Get();
+        context.SetRequestMessage(null);
+
+        strategy.Invoking(s => s.Execute(_ => { }, context)).Should().Throw<InvalidOperationException>().WithMessage("The HTTP request message was not found in the resilience context.");
+    }
+
     [Fact]
     public void NoRoutingProvider_Ok()
     {
         using var request = new HttpRequestMessage();
 
         var strategy = Create(null);
         var context = ResilienceContextPool.Shared.Get();
-        context.Properties.Set(ResilienceKeys.RequestMessage, request);
+        context.SetRequestMessage(request);
 
         strategy.Invoking(s => s.Execute(_ => { }, context)).Should().NotThrow();
     }