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

[Client Generation] Fix required handling for formdata properties #4994

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[Client Generation] Fix required handling for formdata properties #4994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33b2101
fix required handling for formdata properties
Oct 7, 2024
829b983
fix unit tests
Oct 24, 2024
20aff0d
Merge branch 'master' into fix-isrequired-for-formdata-parameters
KirkMunroSagent Nov 26, 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
20 changes: 18 additions & 2 deletions src/NSwag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/FileUploadController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,25 @@ public Task<IActionResult> 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")]
public Task<IActionResult> UploadAttachment2(
[FromForm][Required] CaseAttachmentModel2 model,
[Required] IFormFile contents)
{
return Task.FromResult<IActionResult>(Ok());
}

public class CaseAttachmentModel2
{
[Required]
public string Title { get; init; }

public int? MessageId { get; set; }
}
}
}
2 changes: 1 addition & 1 deletion ...Swag.Generation.AspNetCore.Tests.Web/Controllers/Parameters/HeaderParametersController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion ...Generation.AspNetCore.Tests.Web/Controllers/Parameters/SimpleQueryParametersController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
59 changes: 58 additions & 1 deletion src/NSwag.Generation.AspNetCore.Tests/Parameters/FormDataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,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
}
}
}
Expand Down Expand Up @@ -100,5 +106,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", ""));
}
}
}
14 changes: 8 additions & 6 deletions src/NSwag.Generation.AspNetCore.Tests/Parameters/QueryParametersTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,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]
Expand All @@ -75,10 +76,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]
Expand Down
14 changes: 11 additions & 3 deletions src/NSwag.Generation.AspNetCore/Processors/OperationParameterProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,18 @@ private static JsonSchema CreateOrGetFormDataSchema(OperationProcessorContext co
return value.Schema ??= new JsonSchema();
}

private static JsonSchemaProperty CreateFormDataProperty(OperationProcessorContext context, ExtendedApiParameterDescription extendedApiParameter, JsonSchema schema)
private JsonSchemaProperty CreateFormDataProperty(OperationProcessorContext context, ExtendedApiParameterDescription extendedApiParameter, JsonSchema schema)
{
return context.SchemaGenerator.GenerateWithReferenceAndNullability<JsonSchemaProperty>(
var formDataProperty = context.SchemaGenerator.GenerateWithReferenceAndNullability<JsonSchemaProperty>(
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)
Expand Down Expand Up @@ -520,7 +528,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
{
Expand Down
Loading