From fb37aa2a3990aabb36df41210613d7023034a2bb Mon Sep 17 00:00:00 2001 From: Alasdair Date: Wed, 3 May 2023 21:29:31 +0100 Subject: [PATCH] Unwrap ValueTask return types (#4374) * Unwrap ValueTask return types, same as Task, unwrap (Value)Task> and unify unwrapping for consistency #4373 * lahma suggestion for performance --- .../Responses/WrappedResponseController.cs | 57 +++++++++++++ .../Responses/WrappedResponseTests.cs | 40 +++++++++ .../WrappedResponseTests.cs | 84 +++++++++++++++++++ .../GenericResultWrapperTypes.cs | 25 ++++++ .../OpenApiSchemaGenerator.cs | 29 ++----- .../OperationResponseProcessorBase.cs | 5 +- 6 files changed, 216 insertions(+), 24 deletions(-) create mode 100644 src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Responses/WrappedResponseController.cs create mode 100644 src/NSwag.Generation.AspNetCore.Tests/Responses/WrappedResponseTests.cs create mode 100644 src/NSwag.Generation.WebApi.Tests/WrappedResponseTests.cs create mode 100644 src/NSwag.Generation/GenericResultWrapperTypes.cs diff --git a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Responses/WrappedResponseController.cs b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Responses/WrappedResponseController.cs new file mode 100644 index 0000000000..1f9ac39d0e --- /dev/null +++ b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Responses/WrappedResponseController.cs @@ -0,0 +1,57 @@ +using System; +using System.Threading.Tasks; + +using Microsoft.AspNetCore.Mvc; + +using NSwag.Annotations; + +namespace NSwag.Generation.AspNetCore.Tests.Web.Controllers.Responses +{ + [ApiController] + [Route( "api/wrappedresponse" )] + public class WrappedResponseController : Controller + { + + [HttpGet( "task" )] + public async Task Task() + { + throw new NotImplementedException(); + } + + [HttpGet( "int" )] + public int Int() + { + throw new NotImplementedException(); + } + + [HttpGet( "taskofint" )] + public async Task TaskOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet( "valuetaskofint" )] + public async ValueTask ValueTaskOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet( "actionresultofint" )] + public ActionResult ActionResultOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet( "taskofactionresultofint" )] + public async Task> TaskOfActionResultOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet( "valuetaskofactionresultofint" )] + public async ValueTask> ValueTaskOfActionResultOfInt() + { + throw new NotImplementedException(); + } + } +} diff --git a/src/NSwag.Generation.AspNetCore.Tests/Responses/WrappedResponseTests.cs b/src/NSwag.Generation.AspNetCore.Tests/Responses/WrappedResponseTests.cs new file mode 100644 index 0000000000..b11a22144d --- /dev/null +++ b/src/NSwag.Generation.AspNetCore.Tests/Responses/WrappedResponseTests.cs @@ -0,0 +1,40 @@ +using System; +using System.Linq; +using System.Threading.Tasks; + +using Xunit; + +using NJsonSchema; + +using NSwag.Generation.AspNetCore.Tests.Web.Controllers.Responses; + +namespace NSwag.Generation.AspNetCore.Tests.Responses +{ + public class WrappedResponseTests : AspNetCoreTestsBase + { + [Fact] + public async Task When_response_is_wrapped_in_certain_generic_result_types_then_discard_the_wrapper_type() + { + // Arrange + var settings = new AspNetCoreOpenApiDocumentGeneratorSettings(); + + // Act + var document = await GenerateDocumentAsync(settings, typeof(WrappedResponseController)); + + // Assert + OpenApiResponse GetOperationResponse(String ActionName) + => document.Operations.Where(op => op.Operation.OperationId == $"{nameof(WrappedResponseController).Substring(0, nameof(WrappedResponseController).Length - "Controller".Length )}_{ActionName}").Single().Operation.ActualResponses.Single().Value; + JsonObjectType GetOperationResponseSchemaType( String ActionName ) + => GetOperationResponse( ActionName ).Schema.Type; + var IntType = JsonSchema.FromType().Type; + + Assert.Null(GetOperationResponse(nameof(WrappedResponseController.Task)).Schema); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.Int))); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.TaskOfInt))); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.ValueTaskOfInt))); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.ActionResultOfInt))); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.TaskOfActionResultOfInt))); + Assert.Equal(IntType, GetOperationResponseSchemaType(nameof( WrappedResponseController.ValueTaskOfActionResultOfInt))); + } + } +} \ No newline at end of file diff --git a/src/NSwag.Generation.WebApi.Tests/WrappedResponseTests.cs b/src/NSwag.Generation.WebApi.Tests/WrappedResponseTests.cs new file mode 100644 index 0000000000..94097c8a04 --- /dev/null +++ b/src/NSwag.Generation.WebApi.Tests/WrappedResponseTests.cs @@ -0,0 +1,84 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using System.Web.Http; +using System.Web.Http.Results; + +using CoreMvc = Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using NJsonSchema; + + +namespace NSwag.Generation.WebApi.Tests +{ + [TestClass] + public class WrappedResponseTests + { + public class WrappedResponseController : ApiController + { + [HttpGet, Route( "task" )] + public Task Task() + { + throw new NotImplementedException(); + } + + [HttpGet, Route( "int" )] + public int Int() + { + throw new NotImplementedException(); + } + + [HttpGet, Route( "taskofint" )] + public Task TaskOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet, Route( "valuetaskofint" )] + public ValueTask ValueTaskOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet, Route( "jsonresultofint" )] + public JsonResult JsonResultOfInt() + { + throw new NotImplementedException(); + } + + [HttpGet, Route( "actionresultofint" )] + public CoreMvc.ActionResult ActionResultOfInt() + { + throw new NotImplementedException(); + } + + } + + [TestMethod] + public async Task When_response_is_wrapped_in_certain_generic_result_types_then_discard_the_wrapper_type() + { + // Arrange + var generator = new WebApiOpenApiDocumentGenerator( new WebApiOpenApiDocumentGeneratorSettings() ); + + // Act + var document = await generator.GenerateForControllerAsync(); + + // Assert + OpenApiResponse GetOperationResponse( String ActionName ) + => document.Operations.Where( op => op.Operation.OperationId == $"{nameof(WrappedResponseController).Substring(0,nameof(WrappedResponseController).Length - "Controller".Length )}_{ActionName}" ).Single().Operation.ActualResponses.Single().Value; + JsonObjectType GetOperationResponseSchemaType( String ActionName ) + => GetOperationResponse( ActionName ).Schema.Type; + var IntType = JsonSchema.FromType().Type; + + Assert.IsNull( GetOperationResponse( nameof( WrappedResponseController.Task ) ).Schema ); + Assert.AreEqual( IntType, GetOperationResponseSchemaType( nameof( WrappedResponseController.Int ) ) ); + Assert.AreEqual( IntType, GetOperationResponseSchemaType( nameof( WrappedResponseController.TaskOfInt ) ) ); + Assert.AreEqual( IntType, GetOperationResponseSchemaType( nameof( WrappedResponseController.ValueTaskOfInt ) ) ); + Assert.AreEqual( IntType, GetOperationResponseSchemaType( nameof( WrappedResponseController.JsonResultOfInt ) ) ); + Assert.AreEqual( IntType, GetOperationResponseSchemaType( nameof( WrappedResponseController.ActionResultOfInt ) ) ); + + } + } +} \ No newline at end of file diff --git a/src/NSwag.Generation/GenericResultWrapperTypes.cs b/src/NSwag.Generation/GenericResultWrapperTypes.cs new file mode 100644 index 0000000000..a8791f0f1e --- /dev/null +++ b/src/NSwag.Generation/GenericResultWrapperTypes.cs @@ -0,0 +1,25 @@ +using System; +using System.Linq; + +namespace NSwag.Generation +{ + internal static class GenericResultWrapperTypes + { + internal static bool IsGenericWrapperType( string typeName ) + => + typeName == "Task`1" || + typeName == "ValueTask`1" || + typeName == "JsonResult`1" || + typeName == "ActionResult`1" + ; + + internal static void RemoveGenericWrapperTypes(ref T o, Func getName, Func unwrap) + { + // We iterate because a common signature is public async Task> Action() + while (IsGenericWrapperType(getName(o))) + { + o = unwrap(o); + } + } + } +} diff --git a/src/NSwag.Generation/OpenApiSchemaGenerator.cs b/src/NSwag.Generation/OpenApiSchemaGenerator.cs index 1c6665579c..c6bb2a9809 100644 --- a/src/NSwag.Generation/OpenApiSchemaGenerator.cs +++ b/src/NSwag.Generation/OpenApiSchemaGenerator.cs @@ -51,29 +51,18 @@ protected override void GenerateObject(JsonSchema schema, JsonTypeDescription ty } } - /// Generetes a schema directly or referenced for the requested schema type; also adds nullability if required. - /// The resulted schema type which may reference the actual schema. - /// The type of the schema to generate. - /// Specifies whether the property, parameter or requested schema type is nullable. - /// The schema resolver. - /// An action to transform the resulting schema (e.g. property or parameter) before the type of reference is determined (with $ref or allOf/oneOf). - /// The requested schema object. - public override TSchemaType GenerateWithReferenceAndNullability( + /// Generetes a schema directly or referenced for the requested schema type; also adds nullability if required. + /// The resulted schema type which may reference the actual schema. + /// The type of the schema to generate. + /// Specifies whether the property, parameter or requested schema type is nullable. + /// The schema resolver. + /// An action to transform the resulting schema (e.g. property or parameter) before the type of reference is determined (with $ref or allOf/oneOf). + /// The requested schema object. + public override TSchemaType GenerateWithReferenceAndNullability( ContextualType contextualType, bool isNullable, JsonSchemaResolver schemaResolver, Action transformation = null) { - if (contextualType.TypeName == "Task`1") - { - contextualType = contextualType.OriginalGenericArguments[0]; - } - else if (contextualType.TypeName == "JsonResult`1") - { - contextualType = contextualType.OriginalGenericArguments[0]; - } - else if (contextualType.TypeName == "ActionResult`1") - { - contextualType = contextualType.OriginalGenericArguments[0]; - } + GenericResultWrapperTypes.RemoveGenericWrapperTypes (ref contextualType,t=>t.TypeName,t=>t.OriginalGenericArguments[0]); if (IsFileResponse(contextualType)) { diff --git a/src/NSwag.Generation/Processors/OperationResponseProcessorBase.cs b/src/NSwag.Generation/Processors/OperationResponseProcessorBase.cs index 8534d40242..0b57477d0b 100644 --- a/src/NSwag.Generation/Processors/OperationResponseProcessorBase.cs +++ b/src/NSwag.Generation/Processors/OperationResponseProcessorBase.cs @@ -268,10 +268,7 @@ private void LoadDefaultSuccessResponse(ParameterInfo returnParameter, string su returnType = typeof(void); } - while (returnType.Name == "Task`1" || returnType.Name == "ActionResult`1") - { - returnType = returnType.GenericTypeArguments[0]; - } + GenericResultWrapperTypes.RemoveGenericWrapperTypes (ref returnType,t=>t.Name,t=>t.GenericTypeArguments[0]); if (IsVoidResponse(returnType)) {