Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add methods allowing retrieval of HttpRequestMessage from ResilienceContext #5460

Merged
merged 3 commits into from
Oct 1, 2024
+158 −15
Merged

Add methods allowing retrieval of HttpRequestMessage from ResilienceContext #5460

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5631d0d
Fixes #4957
iliar-turdushev Sep 30, 2024
69a412c
Fixes #4957
iliar-turdushev Sep 30, 2024
ca6a4e1
Fixes #4957
iliar-turdushev Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixes #4957 
Replaces usage of ResilienceKeys.RequestMessage variable with corresponding Get/Set methods
  • Loading branch information
@iliar-turdushev
iliar-turdushev committed Sep 30, 2024
commit 69a412c94051d08c8b41a2040bc2a7a042de451c
4 changes: 3 additions & 1 deletion .../Microsoft.Extensions.Http.Resilience/Hedging/Internals/RequestMessageSnapshotStrategy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand Down
2 changes: 1 addition & 1 deletion ...osoft.Extensions.Http.Resilience/Hedging/ResilienceHttpClientBuilderExtensions.Hedging.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
9 changes: 3 additions & 6 deletions src/Libraries/Microsoft.Extensions.Http.Resilience/Resilience/ResilienceHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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)
{
Expand Down
5 changes: 4 additions & 1 deletion ...raries/Microsoft.Extensions.Http.Resilience/Routing/Internal/RoutingResilienceStrategy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.");
}
Expand Down
12 changes: 11 additions & 1 deletion ...icrosoft.Extensions.Http.Resilience.Tests/Internal/RequestMessageSnapshotStrategyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand All @@ -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();
}
11 changes: 1 addition & 10 deletions ...osoft.Extensions.Http.Resilience.Tests/Resilience/HttpResilienceContextExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ public void GetRequestMessage_RequestMessageIsMissing_ReturnsNull()
var context = ResilienceContextPool.Shared.Get();

Assert.Null(context.GetRequestMessage());

ResilienceContextPool.Shared.Return(context);
}

[Fact]
Expand All @@ -35,8 +33,6 @@ public void GetRequestMessage_RequestMessageIsNull_ReturnsNull()
context.Properties.Set(ResilienceKeys.RequestMessage, null);

Assert.Null(context.GetRequestMessage());

ResilienceContextPool.Shared.Return(context);
}

[Fact]
Expand All @@ -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));
}

Expand All @@ -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]
Expand All @@ -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);
}
}
3 changes: 1 addition & 2 deletions .../Libraries/Microsoft.Extensions.Http.Resilience.Tests/Resilience/ResilienceHandlerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 });
});
Expand Down
13 changes: 11 additions & 2 deletions ...ries/Microsoft.Extensions.Http.Resilience.Tests/Routing/RoutingResilienceStrategyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
Expand Down
Loading