Add The Ability To Specify A Different Content Type #14
Conversation
|
I think this idea is good! https://www.iana.org/assignments/media-types/media-types.xhtml#multipart here is the list of IANA multipart variants right now. |
juna-nb
changed the title
juna-nb
changed the title
| class Middleware < Faraday::Request::UrlEncoded | ||
| DEFAULT_BOUNDARY_PREFIX = '-----------RubyMultipartPost' | ||
|
|
||
| self.mime_type = 'multipart/form-data' |
There was a problem hiding this comment.
This needed to be changed from a class variable to an instance variable otherwise it was causing race conditions in the RSpec tests (and presumably likely in people's code as well).
lib/faraday/multipart/middleware.rb
Outdated
| type = request_type(env) | ||
| env.body && (type.empty? || (type == mime_type)) | ||
| end | ||
|
|
There was a problem hiding this comment.
And because we changed the mime_type from a class variable to an instance variable we also had to do so across this object, included in places where we once inherited from UrlEncoded. So this code is copied from that object but with the mime_type now being referenced from an instance variable instead.
| type = type.split(';', 2).first if type.index(';') | ||
| type | ||
| end | ||
|
|
There was a problem hiding this comment.
Copied over from UrlEncoded to remove that class as a dependancy.
| else | ||
| 'multipart/form-data' | ||
| end | ||
| end |
There was a problem hiding this comment.
I figured as long as the Content-Type started with multipart that is mainly what we cared about and it keeps this middleware flexible.
|
@olleolleolle is there anything else you need me to do to expedite getting this change merged and released? Thanks! |
| # @return [String] | ||
| def request_type(env) | ||
| type = env.request_headers[CONTENT_TYPE].to_s | ||
| type = type.split(';', 2).first if type.index(';') |
There was a problem hiding this comment.
Unrelated to this PR: I'll try fixing the "using index to check if the string contains a character" minor issue. There are other methods, like String#include?. Let's keep this code "as copied over".
|
@juna-nb Ah, also, use Rubocop, some of the fixes can be -A -a auto-corrected. There seemed to be a duplicate method which perhaps needs more reading. Good luck! |
|
Thanks @olleolleolle I'm hoping to get back to this today or tomorrow. |
There are many different mime types for multipart requests and this change provides the option to specify a different one. If a multipart mime type is not specified the middleware will default to multipart/form-data. This change also changes the structure of the middleware a bit. It changes the parent from UrlEncoded to inherit from Middleware directly instead. This is because we were overriding most of the UrlEncoded methods anyway so bringing last remaining methods over and removing that link in the dependency chain seemed to make sense. Another structural code change was to move the methods that didn't need to be public under the private keyword. The intent of this was to help future developers know what they can change without worrying breaking any external dependencies.
|
Thanks for the feedback @olleolleolle and for the catch on the fix for the older version of Ruby. I've addressed them now, as well as the Rubocop linting issues, and some stray changes (due to my Markdown auto-formatters). |
When trying to use faraday-multipart for Publishing Flows Tableau API's I was giving an error because Tableau's APIs multipart APIs only allow a
Content-Type: multipart/mixed.As noted in the comment below by @olleolleolle there are many different accepted mime types for multipart requests and this change provides the option to specify a different one.
The new option for this middleware provided in this PR:
If a multipart mime type is not specified the middleware will default to multipart/form-data.
This change also changes the structure of the middleware a bit.
It changes the parent from UrlEncoded to inherit from Middleware directly instead. This is because we were overriding most of the UrlEncoded methods anyway so bringing last remaining methods over and removing that link in the dependency chain seemed to make sense.
Another structural code change was to move the methods that didn't need to be public under the private keyword. The intent of this was to help future developers know what they can change without worrying about breaking any external dependencies.