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

[dart-dio] multi-part file should accept filename #6671

Closed
ayushin opened this issue Jun 15, 2020 · 10 comments · Fixed by #9542
Closed

[dart-dio] multi-part file should accept filename #6671

ayushin opened this issue Jun 15, 2020 · 10 comments · Fixed by #9542

Comments

@ayushin
Copy link
Contributor

ayushin commented Jun 15, 2020

Right now multipart forms methods are generated with Uint8List type fields and hard-coded filename as the field name:

       Map<String, dynamic> formData = {};
               if (image != null) {
                   formData[r'image'] = MultipartFile.fromBytes(image, filename: r'image');
               }
       bodyData = FormData.fromMap(formData);

Which is incorrect, unsuitable for certain backends.

I think it is more suitable to generate these fields as MultipartFile and let the upstream handle the MultipartFile generation, just like dart generator does.

@ayushin
Copy link
Contributor Author

ayushin commented Jun 15, 2020

I don't seem to be able to generate dart-dio with master branch, but here are the diffs to 4.3.1 fix the above:

@ -51,7 +51,7 @@ class {{classname}} {
            {{/isFile}}
            {{#isFile}}
                if ({{paramName}} != null) {
-                    formData[r'{{baseName}}'] = MultipartFile.fromBytes({{paramName}}, filename: r'{{baseName}}');
+                    formData[r'{{baseName}}'] = {{paramName}};
                }
            {{/isFile}}
        {{/isMultipart}}
-        typeMapping.put("file", "Uint8List");
+       typeMapping.put("file", "MultipartFile");

@kuhnroyal
Copy link
Contributor

This should be working by now, can you check on master?

@wing328 wing328 closed this as completed Jan 22, 2021
@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

Hi, this is still not working, because the generated methods still take Uint8List so there is no way to pass the filename to them, like this:

      if (avatar != null) r'avatar': MultipartFile.fromBytes(avatar, filename: r'avatar'),

Here I would expect to pass MultipartFile and do fromBytes in my application with the filename.

@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

@kuhnroyal how this supposed to work?

@kuhnroyal
Copy link
Contributor

Yea looks like this needs more work, filename is always the same as the parameter name.

@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

Can we please re-open this issue?

@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

The fix is very easy - pass entire MultipartFile as argument not the Uint8List.

We can make it dynamic for backward compatibility and check type

@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

Here is a pull request with a breaking change: the generated api methods now accept MultipartFile instead of Uint8List which I think is more logical.

#9149

@kuhnroyal
Copy link
Contributor

This is your issue, you should be able to re-open it? It is not locked.
Need to think how we can handle the breaking change, dynamic is not a good solution.

@ayushin
Copy link
Contributor Author

ayushin commented Apr 1, 2021

@kuhnroyal I can't re-open it because I did not close it myself

kuhnroyal added a commit to kuhnroyal/openapi-generator that referenced this issue May 21, 2021
* add support for filenames in multipart requests by using  `MultipartFile` from dio directly
* add support for binary/file body data
* fixes OpenAPITools#6671
* fixes OpenAPITools#9079
wing328 pushed a commit that referenced this issue May 25, 2021
* [dart][dart-dio] Improve support for file uploads

* add support for filenames in multipart requests by using  `MultipartFile` from dio directly
* add support for binary/file body data
* fixes #6671
* fixes #9079

* Add and fix tests

* Only use MultipartFile for body/multipart parameters

* Fix test

* Actually fix tests
premiumitsolution pushed a commit to premiumitsolution/openapi-generator that referenced this issue May 26, 2021
…9542)

* [dart][dart-dio] Improve support for file uploads

* add support for filenames in multipart requests by using  `MultipartFile` from dio directly
* add support for binary/file body data
* fixes OpenAPITools#6671
* fixes OpenAPITools#9079

* Add and fix tests

* Only use MultipartFile for body/multipart parameters

* Fix test

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

Successfully merging a pull request may close this issue.

3 participants