Closed Piedone closed 3 months ago
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.
Do you mean the 415? Maybe indeed.
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.
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 https://github.com/reactiveui/refit/issues/930#issuecomment-1297917546 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 https://github.com/reactiveui/refit/issues/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: https://github.com/OrchardCMS/OrchardCore/pull/16439
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 latestmain
is also affected.To Reproduce
c3714d55f360921e9c047850c19cb437ae2560e8
in Orchard Core Commerce.I tried to add Swagger to OC directly in
zl/swagger-test
but I got the same error as https://github.com/dotnet/aspnetcore/issues/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 therecipe
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 supportsIFormFile
. However, I don't know if we'd arrive at the exact same issue with that.Logs and screenshots