TenantApiController.Setup has supposedly unneeded [FromForm], failing Swagger

[Piedone]

Describe the bug

Swagger throws an exception for this problem in TenantApiController.Setup if you add it to an Orchard Core-using app (using v6.6.2, v6.5.0 doesn't have this). This completely prevents using Swagger if the Tenants feature is enabled.

Orchard Core version

2.0.0-preview-18259 from the 2nd July but the latest main is also affected.

To Reproduce

1. Checkout c3714d55f360921e9c047850c19cb437ae2560e8 in Orchard Core Commerce.
2. Enable the Tenants feature.
3. Visit https://localhost:44349/swagger/v1/swagger.json.

I tried to add Swagger to OC directly in zl/swagger-test but I got the same error as dotnet/aspnetcore#14370. Applying the workaround there got rid of the build error but I got 404 under /swagger/v1/swagger.json. And adding it to the root Web project instead gives the "System.InvalidOperationException: No constructor for type 'Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator' can be instantiated using services from the service container and default values." build error...

Expected behavior

Swagger working.

However, simply removing [FromForm] breaks the API, and until the recipe parameter is there, I couldn't craft a request with Refit that wouldn't fail with HTTP 415 (Unsupported Media Type).

Perhaps the real solution is to migrate this ApiController to minimal API too, since that also supports IFormFile. However, I don't know if we'd arrive at the exact same issue with that.

Logs and screenshots

[MikeAlhayek]

I am wondering if converting this API to a MinimalAPI will change the behavior. Maybe we have a middleware that causes this behavior. Something to try if you are trying to fix it.

[Piedone]

Do you mean the 415? Maybe indeed.

[Piedone]

BTW this Refit API method does work:

[Post("/api/tenants/setup")]
[Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
Task<ApiResponse<string>> SetupAsync(
    [Body(buffered: true)] TenantSetupApiModel setupTenantParameters,
    [AliasAs("recipe")] StreamPart recipe = null);

With:

[HttpPost]
[Route("setup")]
public async Task<ActionResult> Setup(SetupApiViewModel model, IFormFile recipe = null)

But only for SetupApiViewModel. I couldn't figure out a way to pass a file to recipe. This is supposed to be the way to upload a file:

    [Post("/api/tenants/setup")]
    [Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
    [Multipart]
    Task<ApiResponse<string>> SetupAsync(
        TenantSetupApiModel setupTenantParameters,
        [AliasAs("recipe")] StreamPart recipe = null);

But then I arrive at the HTTP 415 even though the request, as seen in Fiddler, seems fine to me. Using model for the first parameter doesn't help. Multipart should be automatically supported by ApiControllers though.

Trying to change the API action to this so it's fully a form with two parts:

public async Task<ActionResult> Setup([FromForm] SetupApiViewModel model, IFormFile recipe = null)

Results in no data being captured in SetupApiViewModel with this:

    [Post("/api/tenants/setup")]
    [Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
    [Multipart]
    Task<ApiResponse<string>> SetupAsync(
        [AliasAs("model")] TenantSetupApiModel setupTenantParameters,
        [AliasAs("recipe")] StreamPart recipe = null);

Most possibly due to Refit not supporting `form-data.

And this causes Refit to mistakenly try to serialize the file stream:

    [Post("/api/tenants/setup")]
    [Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
    Task<ApiResponse<string>> SetupAsync(
        [AliasAs("model")][Body(buffered: true)] TenantSetupApiModel setupTenantParameters,
        [AliasAs("recipe")] StreamPart recipe = null);

Sending the view model in the query also just yields the HTTP 415:

    [Post("/api/tenants/setup")]
    [Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
    [Multipart]
    Task<ApiResponse<string>> SetupAsync(
        [Query] TenantSetupApiModel setupTenantParameters,
        [AliasAs("recipe")] StreamPart recipe = null);

As does sending the recipe as a JSON as the body:

    [Post("/api/tenants/setup")]
    [Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
    Task<ApiResponse<string>> SetupAsync(
        [Query] TenantSetupApiModel setupTenantParameters,
        [Body(buffered: true)][AliasAs("recipe")] string recipe = null);

The only option I see is something like this:

public async Task<ActionResult> Setup([FromQuery] SetupApiViewModel model, [FromBody] string recipeJson = null)

But that would be breaking, and I still couldn't get this one working.

I give this up for now. It looks like such a simple thing, but I wasted too much time on this with only cryptic error messages.

[Piedone]

I discovered Refitter in the meantime, which is a life-saver, since it generate Refit code from OpenAPI specs. This is what I found:

This looks invalid, since Swagger doesn't generate a way to post the recipe at all:

[HttpPost]
[Route("setup")]
public async Task<ActionResult> Setup(SetupApiViewModel model, IFormFile recipe = null)

Adding an IFormFile Recipe property to the model and changing the signature as below works:

public async Task<ActionResult> Setup(SetupApiViewModel model)

However, this moves all setup parameters to query strings, resulting in the below ugly Refit code (though a class can be used instead too). More importantly, this would put the password into the query string too, which is a security issue, it should also be in the body.

[Multipart]
[Post("/api/tenants/setup")]
Task<IApiResponse> Setup([Query, AliasAs("Name")] string name, [Query, AliasAs("SiteName")] string siteName, [Query, AliasAs("DatabaseProvider")] string databaseProvider, [Query, AliasAs("ConnectionString")] string connectionString, [Query, AliasAs("TablePrefix")] string tablePrefix, [Query, AliasAs("UserName")] string userName, [Query, AliasAs("Email")] string email, [Query, AliasAs("Password")] string password, [Query, AliasAs("RecipeName")] string recipeName, [Query, AliasAs("SiteTimeZone")] string siteTimeZone, [Query, AliasAs("Schema")] string schema, [AliasAs("Recipe")] StreamPart recipe);

This is the solution from reactiveui/refit#930 (comment) too.

Trying [FromBody]:

public async Task<ActionResult> Setup([FromBody] SetupApiViewModel model)

This generates the following API:

[Post("/api/tenants/setup")]
Task<IApiResponse> Setup([Body] SetupApiViewModel body);

[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.0.8.0 (NJsonSchema v11.0.1.0 (Newtonsoft.Json v13.0.0.0))")]
public partial class SetupApiViewModel
{

    [JsonPropertyName("name")]
    [System.ComponentModel.DataAnnotations.Required]
    public string Name { get; set; }

    [JsonPropertyName("siteName")]
    [System.ComponentModel.DataAnnotations.Required]
    public string SiteName { get; set; }

    [JsonPropertyName("databaseProvider")]
    public string DatabaseProvider { get; set; }

    [JsonPropertyName("connectionString")]
    public string ConnectionString { get; set; }

    [JsonPropertyName("tablePrefix")]
    public string TablePrefix { get; set; }

    [JsonPropertyName("userName")]
    [System.ComponentModel.DataAnnotations.Required]
    public string UserName { get; set; }

    [JsonPropertyName("email")]
    [System.ComponentModel.DataAnnotations.Required]
    public string Email { get; set; }

    [JsonPropertyName("password")]
    public string Password { get; set; }

    [JsonPropertyName("recipeName")]
    public string RecipeName { get; set; }

    [JsonPropertyName("recipe")]
    public byte[] Recipe { get; set; }

    [JsonPropertyName("siteTimeZone")]
    public string SiteTimeZone { get; set; }

    [JsonPropertyName("schema")]
    public string Schema { get; set; }
}

This looks OK, though I'd prefer not to have the Recipe as byte[], since it can be arbitrarily large; StreamPart would be better. It doesn't work anyway, since it results in an HTTP 400 with "One or more validation errors occurred.","status":400,"errors":{"$.recipe":["The JSON value could not be converted to Microsoft.AspNetCore.Http.IFormFile. Path: $.recipe | LineNumber: 0 | BytePositionInLine: 70944."

Changing it to StreamPart causes the property to be attempted to be serialized, resulting in "System.InvalidOperationException: 'Timeouts are not supported on this stream.'".

Changing it to string results in an HTTP 400 too with "The JSON value could not be converted to Microsoft.AspNetCore.Http.IFormFile. Path: $.recipe | LineNumber: 0 | BytePositionInLine: 72694."

Trying [FromForm]:

public async Task<ActionResult> Setup([FromForm] SetupApiViewModel model)

Generates faulty code so I assume is invalid.

BTW combining the original API with [FromBody]:

public async Task<ActionResult> Setup([FromBody] SetupApiViewModel model, IFormFile recipe = null)

Yields no schema generated for the recipe, so I presume it's invalid.

Trying a multipart form with [FromForm] again:

public async Task<ActionResult> Setup([FromForm] SetupApiViewModel model, IFormFile recipe = null)

Generates faulty code:

[Multipart]
[Post("/api/tenants/setup")]
Task<IApiResponse> Setup( StreamPart recipe);

But this is again the reactiveui/refit#930 issue, so this should, but won't work, since the first part of the multipart will be the ViewModel in a JSON, what OC won't understand (since it expects form data):

[Post("/api/tenants/setup")]
[Headers(AuthorizationBearer, RequestedWithXmlHttpRequest)]
[Multipart]
Task<ApiResponse<string>> SetupAsync(TenantSetupApiModel model, StreamPart recipe);

But we wouldn't want this anyway, since this API endpoint should be able to accept JSON.

So... With the original API in main that Swagger trips on, the requests that work have only a single JSON body, with the recipe JSON encoded in a property:

{"name":"ApiClientTenant638565081195196341","siteName":"Api Client Tenant Site","databaseProvider":"Sqlite","connectionString":"","tablePrefix":"apiclienttenant638565081195196341","userName":"admin","email":"admin@example.com","password":"Password1!","recipeName":null,"Recipe":"{\r\n  \u0022name\u0022: \u0022Agency\u0022,}\r\n","siteTimeZone":"Europe/Budapest"}

It's all a string. So, we can just have a string:

public async Task<ActionResult> Setup(SetupApiViewModel model, string recipe = null)

This works, but requires the recipe to be passed into the query string. But moving it to the ViewModel works, and keeps the API surface the same! And it also satisfied Swagger. I've submitted a PR for this: #16439