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.

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

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

Open
Piedone opened this issue Jul 11, 2024 · 3 comments
Open

Comments

@Piedone
Copy link
Member

Piedone commented Jul 11, 2024

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 zl/swagger-test 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

image

@MikeAlhayek
Copy link
Member

MikeAlhayek commented Jul 11, 2024

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
Copy link
Member Author

Piedone commented Jul 12, 2024

Do you mean the 415? Maybe indeed.

@Piedone
Copy link
Member Author

Piedone commented Jul 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants