Invalid order of optional params included by operationProcessors after updating to NSwag v14.3.0

[piano-marcelo]

Describe the bug

The order of optional params in generated dotnet client added via OperationProcessor is invalid, as they should be added after all required params. Error CS1737

Version of NSwag toolchain, computer and .NET runtime used

Dotnet 9, NSwag v14.3.0 (there's no issue in v14.2.0)

To Reproduce

OperationProcessor

public sealed class TestOptionalParamOperationProcessor : IOperationProcessor
{
    public bool Process(OperationProcessorContext context)
    {
        context.OperationDescription.Operation.Parameters.Add(
            new OpenApiParameter
            {
                Description =
                    "Optional param",
                Schema = new OpenApiHeader
                {
                    Default = null,
                    IsNullableRaw = false,
                    Type = JsonObjectType.String,
                    AllowEmptyValue = true
                },
                Kind = OpenApiParameterKind.Header,
                Name = "x-test-optional-parameter",
                OriginalName = "optionalParam"
            });
        return true;
    }
}

Controller

[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
    private static readonly string[] Summaries =
    [
        "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
    ];

    [HttpGet("{cityId:guid}")]
    public WeatherForecast GetById(Guid cityId) => new()
    {
        Date = DateOnly.FromDateTime(DateTime.Now.AddDays(1)),
        TemperatureC = Random.Shared.Next(-20, 55),
        Summary = Summaries[Random.Shared.Next(Summaries.Length)],
        CityId = cityId
    };
}

Configuration in Program.cs

...
builder.Services.AddOpenApiDocument(config =>
{
    config.Title = "My API";
    config.Version = "v1";
    config.DocumentName = "test";
    config.OperationProcessors.Add(new TestOptionalParamOperationProcessor());
});
...

WebApi.csproj

<Project Sdk="Microsoft.NET.Sdk.Web">

    <PropertyGroup>
        <TargetFramework>net9.0</TargetFramework>
        <Nullable>enable</Nullable>
        <ImplicitUsings>enable</ImplicitUsings>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.3"/>
        <PackageReference Include="NSwag.AspNetCore" Version="14.3.0" />
        <PackageReference Include="NSwag.MSBuild" Version="14.3.0" PrivateAssets="All" />
    </ItemGroup>

    <PropertyGroup>
        <RunPostBuildEvent>OnBuildSuccess</RunPostBuildEvent>
    </PropertyGroup>

    <Target Name="NSwag" AfterTargets="PostBuildEvent" Condition=" '$(Configuration)' == 'Debug' ">
        <Exec WorkingDirectory="$(ProjectDir)" EnvironmentVariables="ASPNETCORE_ENVIRONMENT=Development" Command="$(NSwagExe_Net90) run nswag.json /variables:Configuration=$(Configuration)" />
    </Target>

</Project>

nswag.json

{
  "runtime": "Net90",
  "defaultVariables": null,
  "documentGenerator": {
    "aspNetCoreToOpenApi": {
      "project": "web-api.csproj",
      "documentName": "test",
      "msBuildProjectExtensionsPath": null,
      "configuration": null,
      "runtime": null,
      "targetFramework": null,
      "noBuild": true,
      "msBuildOutputPath": null,
      "verbose": true,
      "workingDirectory": null,
      "aspNetCoreEnvironment": null,
      "output": "openapi-test.json",
      "newLineBehavior": "Auto"
    }
  },
  "codeGenerators": {
    "openApiToCSharpClient": {
      "clientBaseClass": null,
      "configurationClass": null,
      "generateClientClasses": true,
      "suppressClientClassesOutput": false,
      "generateClientInterfaces": true,
      "suppressClientInterfacesOutput": false,
      "clientBaseInterface": null,
      "injectHttpClient": true,
      "disposeHttpClient": false,
      "protectedMethods": [],
      "generateExceptionClasses": true,
      "exceptionClass": "ApiException",
      "wrapDtoExceptions": true,
      "useHttpClientCreationMethod": false,
      "httpClientType": "System.Net.Http.HttpClient",
      "useHttpRequestMessageCreationMethod": false,
      "useBaseUrl": false,
      "generateBaseUrlProperty": false,
      "generateSyncMethods": false,
      "generatePrepareRequestAndProcessResponseAsAsyncMethods": false,
      "exposeJsonSerializerSettings": false,
      "clientClassAccessModifier": "public",
      "typeAccessModifier": "public",
      "propertySetterAccessModifier": "",
      "generateNativeRecords": false,
      "generateContractsOutput": false,
      "contractsNamespace": null,
      "contractsOutputFilePath": null,
      "parameterDateTimeFormat": "s",
      "parameterDateFormat": "yyyy-MM-dd",
      "generateUpdateJsonSerializerSettingsMethod": true,
      "useRequestAndResponseSerializationSettings": false,
      "serializeTypeInformation": false,
      "queryNullValue": "",
      "className": "{controller}Api",
      "operationGenerationMode": "MultipleClientsFromOperationId",
      "additionalNamespaceUsages": null,
      "additionalContractNamespaceUsages": [],
      "generateOptionalParameters": true,
      "generateJsonMethods": false,
      "enforceFlagEnums": false,
      "parameterArrayType": "IEnumerable",
      "parameterDictionaryType": "IDictionary",
      "responseArrayType": "List",
      "responseDictionaryType": "Dictionary",
      "wrapResponses": false,
      "wrapResponseMethods": [],
      "generateResponseClasses": false,
      "responseClass": "SwaggerResponse",
      "namespace": "Test.Api.Client",
      "requiredPropertiesMustBeDefined": true,
      "dateType": "System.DateTimeOffset",
      "jsonConverters": null,
      "anyType": "object",
      "dateTimeType": "System.DateTimeOffset",
      "timeType": "System.TimeSpan",
      "timeSpanType": "System.TimeSpan",
      "arrayType": "System.Collections.Generic.ICollection",
      "arrayInstanceType": "System.Collections.ObjectModel.Collection",
      "dictionaryType": "System.Collections.Generic.IDictionary",
      "dictionaryInstanceType": "System.Collections.Generic.Dictionary",
      "arrayBaseType": "System.Collections.ObjectModel.Collection",
      "dictionaryBaseType": "System.Collections.Generic.Dictionary",
      "classStyle": "Poco",
      "jsonLibrary": "SystemTextJson",
      "generateDefaultValues": true,
      "generateDataAnnotations": true,
      "excludedTypeNames": [],
      "excludedParameterNames": [
        ""
      ],
      "handleReferences": false,
      "generateImmutableArrayProperties": false,
      "generateImmutableDictionaryProperties": false,
      "jsonSerializerSettingsTransformationMethod": "JsonSerializerHelper.ConfigureOptions",
      "inlineNamedArrays": false,
      "inlineNamedDictionaries": false,
      "inlineNamedTuples": true,
      "inlineNamedAny": false,
      "generateDtoTypes": false,
      "generateOptionalPropertiesAsNullable": false,
      "generateNullableReferenceTypes": false,
      "templateDirectory": null,
      "serviceHost": null,
      "serviceSchemes": null,
      "output": "ApiClient.cs",
      "newLineBehavior": "Auto"
    }
  }
}

Additional context

It's just working totally fine when switching to NSwag 14.2.0.

Expected behavior

AS IS:

public partial interface IWeatherForecastApi
    {
        System.Threading.Tasks.Task<WeatherForecast> GetByIdAsync(string optionalParam = null, System.Guid cityId, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
    }

TO BE:

public partial interface IWeatherForecastApi
    {
        System.Threading.Tasks.Task<WeatherForecast> GetByIdAsync(System.Guid cityId, string optionalParam = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
    }

[CRidge]

We're seeing the same in generated Typescript code. Our code base has hundreds of API methods, most of which now have parameters in different order to what our client code expects - and we get errors due to optional being before required also in Typescript.

[msrouchou]

We also get the order issue moving from 14.2.0 to 14.3.0. We do not make use of operationProcessors, yet, the parameters order is broken with the optional parameters superseding the mandatory ones. in the config file, we are using "generateOptionalParameters": true.

[kescherCode]

This does indeed seem to be broken in the generation of OpenAPI schemas.

[Wind010]

Same.

[lahma]

If no one is willing to investigate further and some resolution is still expected, maybe creating a most simple reproduction without the need to copy and paste things to a some solution would help out. Ideally a direct test case against NSwag repo but that might be a hard ask.

[RicoSuter]

Looking through the merged PRs, I couldn't identify a possible PR directly... maybe it's easier to just debug it and see where the order breaks? I assume it's not happening always, so would be good if someone could provide a simple repro...

[xC0dex]

I created a minimal repo here https://github.com/xC0dex/NSwagSample.

I think this code is responsible for the correct order:

14.3.0

NSwag/src/NSwag.CodeGeneration.CSharp/Models/CSharpOperationModel.cs

Lines 56 to 68 in 62ab9f3

	if (generator is CSharpControllerGenerator)
	{
	parameters = [.. parameters
	.OrderBy(p => p.Position ?? 0)
	.ThenBy(p => !p.IsRequired)
	.ThenBy(p => p.Default == null)];
	}
	else
	{
	parameters = [.. parameters
	.OrderBy(p => p.Position ?? 0)
	.ThenBy(p => !p.IsRequired)];
	}

The only change from 14.2.0 to 14.3.0 in this code is the usage of collection expression:

14.2.0

NSwag/src/NSwag.CodeGeneration.CSharp/Models/CSharpOperationModel.cs

Lines 58 to 71 in 2888363

	if (generator is CSharpControllerGenerator)
	{
	parameters = parameters
	.OrderBy(p => p.Position ?? 0)
	.OrderBy(p => !p.IsRequired)
	.ThenBy(p => p.Default == null).ToList();
	}
	else
	{
	parameters = parameters
	.OrderBy(p => p.Position ?? 0)
	.OrderBy(p => !p.IsRequired)
	.ToList();
	}

[lahma]

Thank you @xC0dex , that example was really helpful and especially the investigation you did which lead to the incorrect change I've made. I've created #5135 which should explain the problem and resolve this issue.