From f89ecca2123639c7cff36d2216a5d8a09f017f43 Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Tue, 15 Feb 2022 12:32:21 +0800 Subject: [PATCH 1/7] return value for single HttpContext parameter --- .../WebApplicationTests.cs | 22 +++++++++++++++++++ .../Builder/EndpointRouteBuilderExtensions.cs | 4 ++++ 2 files changed, 26 insertions(+) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 881ce71e78b9..2ea23d420a9d 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -31,6 +31,28 @@ namespace Microsoft.AspNetCore.Tests; public class WebApplicationTests { + [Fact] + public async Task WebApplication_SeparateMethodWithHttpContext() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + static async Task Fails(HttpContext context) => await Task.FromResult("response"); + + app.MapGet("/Fails", Fails); + + app.UseRouting(); + + await app.StartAsync(); + + var client = app.GetTestClient(); + + var result = await client.GetAsync("http://localhost/Fails"); + + Assert.Equal("response", await result.Content.ReadAsStringAsync()); + } + [Fact] public async Task WebApplicationBuilderConfiguration_IncludesCommandLineArguments() { diff --git a/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs index ab00f5ff2749..cd1c327888b2 100644 --- a/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs @@ -38,6 +38,10 @@ public static IEndpointConventionBuilder MapGet( string pattern, RequestDelegate requestDelegate) { + if (requestDelegate.Method.ReturnType.IsGenericType) + { + return MapMethods(endpoints, pattern, GetVerb, requestDelegate as Delegate); + } return MapMethods(endpoints, pattern, GetVerb, requestDelegate); } From 5501bf4cee937182d95285f18013b7d40ea28a07 Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Mon, 21 Feb 2022 16:00:36 +0800 Subject: [PATCH 2/7] make the changes @davidfowl recommended --- .../WebApplicationTests.cs | 22 ------------------- .../Builder/EndpointRouteBuilderExtensions.cs | 3 ++- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 2ea23d420a9d..881ce71e78b9 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -31,28 +31,6 @@ namespace Microsoft.AspNetCore.Tests; public class WebApplicationTests { - [Fact] - public async Task WebApplication_SeparateMethodWithHttpContext() - { - var builder = WebApplication.CreateBuilder(); - builder.WebHost.UseTestServer(); - await using var app = builder.Build(); - - static async Task Fails(HttpContext context) => await Task.FromResult("response"); - - app.MapGet("/Fails", Fails); - - app.UseRouting(); - - await app.StartAsync(); - - var client = app.GetTestClient(); - - var result = await client.GetAsync("http://localhost/Fails"); - - Assert.Equal("response", await result.Content.ReadAsStringAsync()); - } - [Fact] public async Task WebApplicationBuilderConfiguration_IncludesCommandLineArguments() { diff --git a/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs index cd1c327888b2..4c1cb9b09904 100644 --- a/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRouteBuilderExtensions.cs @@ -38,7 +38,8 @@ public static IEndpointConventionBuilder MapGet( string pattern, RequestDelegate requestDelegate) { - if (requestDelegate.Method.ReturnType.IsGenericType) + var returnType = requestDelegate.Method.ReturnType; + if (returnType is { IsGenericType: true } && returnType.GetGenericTypeDefinition() == typeof(Task<>)) { return MapMethods(endpoints, pattern, GetVerb, requestDelegate as Delegate); } From d055f959c6caaebe8b869ae544b539e47ccf3cc1 Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Wed, 23 Feb 2022 12:01:13 +0800 Subject: [PATCH 3/7] add unit test --- ...egateEndpointRouteBuilderExtensionsTest.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index cf2e1cca9f43..78487021910d 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -69,6 +69,27 @@ public void MapEndpoint_StringPattern_BuildsEndpoint() Assert.Equal("/", endpointBuilder1.RoutePattern.RawText); } + [Fact] + public void MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() + { + // Arrange + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + static async Task GenericTypeTaskDelegate(HttpContext context) => await Task.FromResult("response"); + + // Act + var endpointBuilder = builder.MapGet("/", GenericTypeTaskDelegate); + + // Assert + var endpointBuilder1 = GetRouteEndpointBuilder(builder); + var target = endpointBuilder1.RequestDelegate.Target; + Assert.NotNull(target); + var handlerField = target.GetType().GetField("handler"); + Assert.NotNull(handlerField); + var handler = handlerField.GetValue(target) as Delegate; + Assert.NotNull(handler); + Assert.True(handler.Method.ReturnType.IsGenericType); + } + [Fact] public void MapEndpoint_TypedPattern_BuildsEndpoint() { From 68a81aaed4dcf1f1e4cefd065a894915a47bfd8c Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Mon, 28 Feb 2022 13:37:33 +0800 Subject: [PATCH 4/7] make the changes @davidfowl recommended --- ...egateEndpointRouteBuilderExtensionsTest.cs | 124 ++++++++++++++++-- 1 file changed, 115 insertions(+), 9 deletions(-) diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index 78487021910d..a3672ee674ce 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -1,10 +1,15 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO.Pipelines; using System.Linq.Expressions; +using System.Text; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Moq; namespace Microsoft.AspNetCore.Builder; @@ -70,24 +75,27 @@ public void MapEndpoint_StringPattern_BuildsEndpoint() } [Fact] - public void MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() + public async void MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() { + var httpContext = CreateHttpContext(); + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + // Arrange var builder = new DefaultEndpointRouteBuilder(Mock.Of()); - static async Task GenericTypeTaskDelegate(HttpContext context) => await Task.FromResult("response"); + static async Task GenericTypeTaskDelegate(HttpContext context) => await Task.FromResult("String Test"); // Act var endpointBuilder = builder.MapGet("/", GenericTypeTaskDelegate); // Assert var endpointBuilder1 = GetRouteEndpointBuilder(builder); - var target = endpointBuilder1.RequestDelegate.Target; - Assert.NotNull(target); - var handlerField = target.GetType().GetField("handler"); - Assert.NotNull(handlerField); - var handler = handlerField.GetValue(target) as Delegate; - Assert.NotNull(handler); - Assert.True(handler.Method.ReturnType.IsGenericType); + var requestDelegate = endpointBuilder1.RequestDelegate; + await requestDelegate(httpContext); + + var responseBody = Encoding.UTF8.GetString(responseBodyStream.ToArray()); + + Assert.Equal("String Test", responseBody); } [Fact] @@ -238,4 +246,102 @@ private class Attribute1 : Attribute private class Attribute2 : Attribute { } + + private DefaultHttpContext CreateHttpContext() + { + var responseFeature = new TestHttpResponseFeature(); + + return new() + { + RequestServices = new ServiceCollection().BuildServiceProvider(), + Features = + { + [typeof(IHttpResponseFeature)] = responseFeature, + [typeof(IHttpResponseBodyFeature)] = responseFeature, + [typeof(IHttpRequestLifetimeFeature)] = new TestHttpRequestLifetimeFeature(), + } + }; + } + + private class TestHttpRequestLifetimeFeature : IHttpRequestLifetimeFeature + { + private readonly CancellationTokenSource _requestAbortedCts = new(); + + public CancellationToken RequestAborted { get => _requestAbortedCts.Token; set => throw new NotImplementedException(); } + + public void Abort() + { + _requestAbortedCts.Cancel(); + } + } + + private class TestHttpResponseFeature : IHttpResponseFeature, IHttpResponseBodyFeature + { + public int StatusCode { get; set; } = 200; + public string? ReasonPhrase { get; set; } + public IHeaderDictionary Headers { get; set; } = new HeaderDictionary(); + + public bool HasStarted { get; private set; } + + // Assume any access to the response Body/Stream/Writer is writing for test purposes. + public Stream Body + { + get + { + HasStarted = true; + return Stream.Null; + } + set + { + } + } + + public Stream Stream + { + get + { + HasStarted = true; + return Stream.Null; + } + } + + public PipeWriter Writer + { + get + { + HasStarted = true; + return PipeWriter.Create(Stream.Null); + } + } + + public Task StartAsync(CancellationToken cancellationToken = default) + { + HasStarted = true; + return Task.CompletedTask; + } + + public Task CompleteAsync() + { + HasStarted = true; + return Task.CompletedTask; + } + + public Task SendFileAsync(string path, long offset, long? count, CancellationToken cancellationToken = default) + { + HasStarted = true; + return Task.CompletedTask; + } + + public void DisableBuffering() + { + } + + public void OnStarting(Func callback, object state) + { + } + + public void OnCompleted(Func callback, object state) + { + } + } } From 673d11ac1dbc40a75ebf305ac401accf819797f4 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 27 Feb 2022 22:04:23 -0800 Subject: [PATCH 5/7] Update src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs --- .../RequestDelegateEndpointRouteBuilderExtensionsTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index a3672ee674ce..97507d130b07 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -75,7 +75,7 @@ public void MapEndpoint_StringPattern_BuildsEndpoint() } [Fact] - public async void MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() + public async Task MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() { var httpContext = CreateHttpContext(); var responseBodyStream = new MemoryStream(); From 970eacbdc89f1a8797df32199e00fba849b86c04 Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Mon, 28 Feb 2022 20:52:11 +0800 Subject: [PATCH 6/7] remove codes --- ...egateEndpointRouteBuilderExtensionsTest.cs | 91 ------------------- 1 file changed, 91 deletions(-) diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index a3672ee674ce..0e14fc463d99 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -249,99 +249,8 @@ private class Attribute2 : Attribute private DefaultHttpContext CreateHttpContext() { - var responseFeature = new TestHttpResponseFeature(); - return new() { - RequestServices = new ServiceCollection().BuildServiceProvider(), - Features = - { - [typeof(IHttpResponseFeature)] = responseFeature, - [typeof(IHttpResponseBodyFeature)] = responseFeature, - [typeof(IHttpRequestLifetimeFeature)] = new TestHttpRequestLifetimeFeature(), - } }; } - - private class TestHttpRequestLifetimeFeature : IHttpRequestLifetimeFeature - { - private readonly CancellationTokenSource _requestAbortedCts = new(); - - public CancellationToken RequestAborted { get => _requestAbortedCts.Token; set => throw new NotImplementedException(); } - - public void Abort() - { - _requestAbortedCts.Cancel(); - } - } - - private class TestHttpResponseFeature : IHttpResponseFeature, IHttpResponseBodyFeature - { - public int StatusCode { get; set; } = 200; - public string? ReasonPhrase { get; set; } - public IHeaderDictionary Headers { get; set; } = new HeaderDictionary(); - - public bool HasStarted { get; private set; } - - // Assume any access to the response Body/Stream/Writer is writing for test purposes. - public Stream Body - { - get - { - HasStarted = true; - return Stream.Null; - } - set - { - } - } - - public Stream Stream - { - get - { - HasStarted = true; - return Stream.Null; - } - } - - public PipeWriter Writer - { - get - { - HasStarted = true; - return PipeWriter.Create(Stream.Null); - } - } - - public Task StartAsync(CancellationToken cancellationToken = default) - { - HasStarted = true; - return Task.CompletedTask; - } - - public Task CompleteAsync() - { - HasStarted = true; - return Task.CompletedTask; - } - - public Task SendFileAsync(string path, long offset, long? count, CancellationToken cancellationToken = default) - { - HasStarted = true; - return Task.CompletedTask; - } - - public void DisableBuffering() - { - } - - public void OnStarting(Func callback, object state) - { - } - - public void OnCompleted(Func callback, object state) - { - } - } } From efda3d272f80f6cb272cf13686ae75a1f157af86 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 28 Feb 2022 07:46:59 -0800 Subject: [PATCH 7/7] Apply suggestions from code review --- .../RequestDelegateEndpointRouteBuilderExtensionsTest.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs index f4a41fe6cc61..a76b96a19be5 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RequestDelegateEndpointRouteBuilderExtensionsTest.cs @@ -77,7 +77,7 @@ public void MapEndpoint_StringPattern_BuildsEndpoint() [Fact] public async Task MapEndpoint_ReturnGenericTypeTask_GeneratedDelegate() { - var httpContext = CreateHttpContext(); + var httpContext = new DefaultHttpContext(); var responseBodyStream = new MemoryStream(); httpContext.Response.Body = responseBodyStream; @@ -246,11 +246,4 @@ private class Attribute1 : Attribute private class Attribute2 : Attribute { } - - private DefaultHttpContext CreateHttpContext() - { - return new() - { - }; - } }