-
Notifications
You must be signed in to change notification settings - Fork 208
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
multipart/form-data request content type is not handled properly #220
Comments
Some example to test https://docs.microsoft.com/en-us/graph/api/section-post-pages?view=graph-rest-1.0 |
when implemented, we should clear the appsettings.json to be able to ship self-contained executable |
OpenAPI specification example of multipart forms. In OpenAPI 3.0, JSON Schema isn't rich enough to describe multipart forms, so you may need a combination of schema object and encoding object. However, the encoding object is rarely used, so I suggest we wait until we add support for OpenAPI 3.1 as then all the information that is needed is in the JSON Schema. This will avoid us having to add support for the encoding object. We should still be able to support multipart/form-data media types with scalar parts without support for the encoding object. |
This one is a bit more complex to handle since multipart are effectively a composition of other serialization formats. This new serializer/deserializer would effectively need to be a composition of others (configurable, should be easily done). It'd also be a break of our promise that models being generated are serialization format agnostic. @darrelmiller to think about that further and come up with guidance. |
Another interesting point that @ddyett brought up is the fact that there's probably not even a way to describe the sub parts mime type in open API today. At the end of the day we're most likely blocked by that dependency besides giving customers access to the stream directly and some task design to compose the request body like we do for batches today. |
@andrueastman @rkodev I've finally been able to make progress on that... in terms of usage here is what it looks like var mpBody = new MultipartBody();
mpBody.AddOrReplacePart("Presentation", "text/html", htmlPayload);
mpBody.AddOrReplacePart("imageBlock1", "image/png", imgStream);
await apiClient.Users["id"].OneNote.Pages.PostAsync(mpBody); I've considered for a while trying to put all the parts as parameters of PostAsync, but that'd mean that adding a part in the schema would be binary breaking. Not ideal. Plus there are cases where we don't know all the parts in advance (e.g. one note, the number of images in any given page is variable). Andrew: would you mind sharing what API surface v4 was offering until now please? |
@baywet var multipartContent = new MultipartFormDataContent();
var htmlString =
"<!DOCTYPE html>\n<html>\n <head>\n <title>A page with <i>rendered</i> images and an <b>attached</b> file</title>\n <meta name=\"created\" content=\"2015-07-22T09:00:00-08:00\" />\n </head>\n <body>\n <p>Here's an image from an online source:</p>\n <img src=\"https://...\" alt=\"an image on the page\" width=\"500\" />\n <p>Here's an image uploaded as binary data:</p>\n <img src=\"name:imageBlock1\" alt=\"an image on the page\" width=\"300\" />\n <p>Here's a file attachment:</p>\n <object data-attachment=\"FileName.pdf\" data=\"name:fileBlock1\" type=\"application/pdf\" />\n </body>\n</html>";
var presentation = new StringContent(htmlString, Encoding.UTF8, "text/html");
multipartContent.Add(presentation,"Presentation");//needs a name
await graphClient.Users["id"].OneNote.Pages.Request().AddAsync(multipartContent); |
Thanks for sharing the information. I've also created an issue in the conversion library so we can emit the right metadata. |
onenote example '/users/{user-id}/onenote/pages':
post:
parameters:
- name: user-id
in: path
description: The unique identifier of user
required: true
style: simple
schema:
type: string
x-ms-docs-key-type: user
description: Create a new page in the specified section.
tags:
- pages.create
operationId: create_page
requestBody:
content:
'multipart/form-data':
schema:
properties:
Presentation:
type: string
encoding:
Presentation:
contentType: text/html
responses:
'204':
description: No Content |
Where can we track the progress for this in the dotnet client generator? |
@billybraga this is already implemented? |
Really? I have something like this: ...
"requestBody": {
"content": {
"multipart/form-data": {
"required": [
"file",
"metadata"
],
"type": "object",
"properties": {
"metadata": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"nullable": true
},
"file": {
"type": "string",
"format": "binary"
}
},
"additionalProperties": false
}
},
"required": true
}
... And I get something like this: public partial class CreateFeatureDocumentRequest : IParsable
{
/// <summary>The file property</summary>
public string File { get; set; } |
Have you tried using the MultipartBody ? |
I'm not sure how I would, the generated client method looks like this: public async Task PostAsync(CreateFeatureDocumentRequest body //... Also, I did not find any documentation for using |
Can you start a new issue so we can continue the conversation please? |
Sorry for disturbing again, but I'll post my solution here in case someone ends up in the same situation: I needed to add "encoding" in my open api spec: ...
"requestBody": {
"content": {
"multipart/form-data": {
"required": [
"file",
"metadata"
],
"type": "object",
"properties": {
"metadata": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"nullable": true
},
"file": {
"type": "string",
"format": "binary"
}
},
"additionalProperties": false,
// this is what was missing
"encoding": {
"file": {
"style": "form"
}
}
}
},
"required": true
}
... Without this, kiota doesn't generate MultipartBody. https://github.com/microsoft/kiota/blob/main/src/Kiota.Builder/KiotaBuilder.cs#L1483 |
The content serialization won't work as we don't have a serializer for it. Adding a serializer should be trivial using the natively available libraries on the platform to build the body so encoding is handled for us.
More reading
Originally reported
AB#9788
The text was updated successfully, but these errors were encountered: