From 33b21018ea4cd33caba2e611189f17fbff15dd0b Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Mon, 7 Oct 2024 13:03:22 -0300 Subject: [PATCH 1/2] fix required handling for formdata properties --- .../Parameters/FileUploadController.cs | 16 ++++++ .../Parameters/FormDataTests.cs | 51 +++++++++++++++++++ .../Processors/OperationParameterProcessor.cs | 15 ++++-- 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs index d06203a66a..49ddb6d8a0 100644 --- a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs +++ b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs @@ -36,5 +36,21 @@ public class CaseAttachmentModel public IFormFile Contents { get; set; } } + + [HttpPost("UploadAttachment2")] + public Task UploadAttachment2( + [FromForm][Required] CaseAttachmentModel2 model, + [Required] IFormFile contents) + { + return Task.FromResult(Ok()); + } + + public class CaseAttachmentModel2 + { + [Required] + public string Title { get; init; } + + public int? MessageId { get; set; } + } } } \ No newline at end of file diff --git a/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs b/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs index ee27012b5f..2166874f70 100644 --- a/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs +++ b/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs @@ -102,5 +102,56 @@ public async Task WhenOperationHasFormDataComplex_ThenItIsInRequestBody() } },".Replace("\r", ""), json.Replace("\r", "")); } + + [Fact] + public async Task WhenOperationHasFormDataComplexWithRequiredProperties_ThenItIsInRequestBody() + { + // Arrange + var settings = new AspNetCoreOpenApiDocumentGeneratorSettings + { + SchemaSettings = new NewtonsoftJsonSchemaGeneratorSettings + { + SchemaType = SchemaType.OpenApi3 + } + }; + + // Act + var document = await GenerateDocumentAsync(settings, typeof(FileUploadController)); + var json = document.ToJson(); + + // Assert + var operation = document.Operations.First(o => o.Operation.OperationId == "FileUpload_UploadAttachment2").Operation; + + Assert.NotNull(operation); + Assert.Contains(@"""requestBody"": { + ""content"": { + ""multipart/form-data"": { + ""schema"": { + ""type"": ""object"", + ""required"": [ + ""Title"", + ""contents"" + ], + ""properties"": { + ""Title"": { + ""type"": ""string"", + ""nullable"": false + }, + ""MessageId"": { + ""type"": ""integer"", + ""format"": ""int32"", + ""nullable"": true + }, + ""contents"": { + ""type"": ""string"", + ""format"": ""binary"", + ""nullable"": false + } + } + } + } + } + },".Replace("\r", ""), json.Replace("\r", "")); + } } } \ No newline at end of file diff --git a/src/NSwag.Generation.AspNetCore/Processors/OperationParameterProcessor.cs b/src/NSwag.Generation.AspNetCore/Processors/OperationParameterProcessor.cs index 7cf08d1806..53dbd391f9 100644 --- a/src/NSwag.Generation.AspNetCore/Processors/OperationParameterProcessor.cs +++ b/src/NSwag.Generation.AspNetCore/Processors/OperationParameterProcessor.cs @@ -348,10 +348,18 @@ private JsonSchema CreateOrGetFormDataSchema(OperationProcessorContext context) return requestBody.Content[MultipartFormData].Schema; } - private static JsonSchemaProperty CreateFormDataProperty(OperationProcessorContext context, ExtendedApiParameterDescription extendedApiParameter, JsonSchema schema) + private JsonSchemaProperty CreateFormDataProperty(OperationProcessorContext context, ExtendedApiParameterDescription extendedApiParameter, JsonSchema schema) { - return context.SchemaGenerator.GenerateWithReferenceAndNullability( + var formDataProperty = context.SchemaGenerator.GenerateWithReferenceAndNullability( extendedApiParameter.ApiParameter.Type.ToContextualType(extendedApiParameter.Attributes), context.SchemaResolver); + + var contextualPropertyType = extendedApiParameter.ParameterType.ToContextualType(); + var typeDescription = _settings.SchemaSettings.ReflectionService.GetDescription(contextualPropertyType, _settings.SchemaSettings); + var isRequired = extendedApiParameter.IsRequired(_settings.RequireParametersWithoutDefault); + formDataProperty.IsRequired = isRequired; + formDataProperty.IsNullableRaw = _settings.AllowNullableBodyParameters && !isRequired && typeDescription.IsNullable; + + return formDataProperty; } private bool IsFileArray(Type type, JsonTypeDescription typeInfo) @@ -527,7 +535,7 @@ public bool IsRequired(bool requireParametersWithoutDefault) // available in asp.net core >= 2.2 if (ApiParameter.HasProperty("IsRequired")) { - isRequired = ApiParameter.TryGetPropertyValue("IsRequired", false); + isRequired = ApiParameter.TryGetPropertyValue("IsRequired", false) || ApiParameter.ModelMetadata?.IsRequired == true; } else { @@ -538,7 +546,6 @@ public bool IsRequired(bool requireParametersWithoutDefault) } else if (ApiParameter.ModelMetadata != null && ApiParameter.ModelMetadata.IsBindingRequired) - { isRequired = true; } From 829b983588d989aa50f0648ccd1ae75f0de30620 Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Thu, 24 Oct 2024 09:44:21 -0300 Subject: [PATCH 2/2] fix unit tests --- .../Controllers/Parameters/FileUploadController.cs | 4 ++-- .../Parameters/HeaderParametersController.cs | 2 +- .../Parameters/SimpleQueryParametersController.cs | 2 +- .../Parameters/FormDataTests.cs | 8 +++++++- .../Parameters/QueryParametersTests.cs | 14 ++++++++------ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs index 49ddb6d8a0..7ac69dd5dd 100644 --- a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs +++ b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs @@ -32,9 +32,9 @@ public Task UploadAttachment( public class CaseAttachmentModel { - public string Description { get; set; } + public string? Description { get; set; } - public IFormFile Contents { get; set; } + public IFormFile? Contents { get; set; } } [HttpPost("UploadAttachment2")] diff --git a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/HeaderParametersController.cs b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/HeaderParametersController.cs index 932542324e..0cedb1f514 100644 --- a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/HeaderParametersController.cs +++ b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/HeaderParametersController.cs @@ -7,7 +7,7 @@ namespace NSwag.Generation.AspNetCore.Tests.Web.Controllers.Parameters public class HeaderParametersController : Controller { [HttpGet] - public ActionResult MyAction([FromHeader] string required, [FromHeader] string optional = null) + public ActionResult MyAction([FromHeader] string required, [FromHeader] string? optional = null) { return Ok(); } diff --git a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/SimpleQueryParametersController.cs b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/SimpleQueryParametersController.cs index 97c93f95ba..25093331ca 100644 --- a/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/SimpleQueryParametersController.cs +++ b/src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/SimpleQueryParametersController.cs @@ -8,7 +8,7 @@ namespace NSwag.Generation.AspNetCore.Tests.Web.Controllers.Parameters public class SimpleQueryParametersController : Controller { [HttpGet] - public ActionResult GetList(int? required, int optional = 10, [BindRequired, FromQuery] string requiredString = null) + public ActionResult GetList(int requiredBecauseNonNullable, int? optionalBecauseNullable, int? optionalBecauseNullableWithDefaultValue = 10, [BindRequired, FromQuery] string? requiredBecauseBindRequired = null) { return Ok(); } diff --git a/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs b/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs index 2166874f70..2a6ed9bf17 100644 --- a/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs +++ b/src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs @@ -43,16 +43,22 @@ public async Task WhenOperationHasFormDataFile_ThenItIsInRequestBody() ""content"": { ""multipart/form-data"": { ""schema"": { + ""required"": [ + ""files"", + ""test"" + ], ""properties"": { ""files"": { ""type"": ""array"", + ""nullable"": false, ""items"": { ""type"": ""string"", ""format"": ""binary"" } }, ""test"": { - ""type"": ""string"" + ""type"": ""string"", + ""nullable"": false } } } diff --git a/src/NSwag.Generation.AspNetCore.Tests/Parameters/QueryParametersTests.cs b/src/NSwag.Generation.AspNetCore.Tests/Parameters/QueryParametersTests.cs index cf83460910..ec61d0cbd4 100644 --- a/src/NSwag.Generation.AspNetCore.Tests/Parameters/QueryParametersTests.cs +++ b/src/NSwag.Generation.AspNetCore.Tests/Parameters/QueryParametersTests.cs @@ -54,10 +54,11 @@ public async Task When_simple_query_parameters_are_nullable_and_set_to_null_they // Assert var operation = document.Operations.First().Operation; - Assert.Equal(3, operation.ActualParameters.Count); + Assert.Equal(4, operation.ActualParameters.Count); Assert.True(operation.ActualParameters.Skip(0).First().IsRequired); - Assert.False(operation.ActualParameters.Skip(1).First().IsRequired); - Assert.True(operation.ActualParameters.Skip(2).First().IsRequired); + Assert.True(operation.ActualParameters.Skip(1).First().IsRequired); + Assert.False(operation.ActualParameters.Skip(2).First().IsRequired); + Assert.True(operation.ActualParameters.Skip(3).First().IsRequired); } [Fact] @@ -76,10 +77,11 @@ public async Task When_simple_query_parameter_has_BindRequiredAttribute_then_it_ // Assert var operation = document.Operations.First().Operation; - Assert.Equal(3, operation.ActualParameters.Count); - Assert.False(operation.ActualParameters.Skip(0).First().IsRequired); + Assert.Equal(4, operation.ActualParameters.Count); + Assert.True(operation.ActualParameters.Skip(0).First().IsRequired); Assert.False(operation.ActualParameters.Skip(1).First().IsRequired); - Assert.True(operation.ActualParameters.Skip(2).First().IsRequired); + Assert.False(operation.ActualParameters.Skip(2).First().IsRequired); + Assert.True(operation.ActualParameters.Skip(3).First().IsRequired); } [Fact]