-
Notifications
You must be signed in to change notification settings - Fork 76
Description
@lafrech if you have a moment, please drop a note here if you support addressing the documented mixed multipart limitation in principle. If you do, I'll prepare a formal PR from the code below that @claudius-kienle contributed, unless he volunteers to do so
No hard feelings either way, and no rush whatsoever. I agree with your initial reaction, that this did not seem to be needing prioritization. It's an edge case, and a pattern that can be worked around easily in most cases
Background
Issue #46 was originally created as an ask for supporting multiple files in multipart requests. Today, multiple files are indeed supported and this is well documented 🙏
At some point, that issue was taken a little off-course with questions about other multipart scenarios- specifically mixed file and variable requests
I created this issue specifically for consideration of changes to the (documented) behavior/limitation for mixed multipart requests
The Problem
Currently, multipart requests that contain both files and variables work (that is, the decorated function gets what it should) but the OpenAPI documentation is not emitted fully. This is well-known and made clear to users, despite it being considered a reasonably rare pattern/edge-case
The Solution
The following was informally contributed by @claudius-kienle via the aforementioned issue
It has been used but not fully reviewed or properly tested
It parses mixed multipart form fields (files and variables) and generates the correct documentation
It can be added to the ArgumentsMixin class and then used in place of the existing ArgumentsMixin::arguments method which, in practice, is applied as a decorator after being mixed in to the Blueprint class
Note, I did not write this code, but noticed that having it implemented this way (as an additional method, rather than a patch to the existing arguments method) allows it to function as "opt-in", which reduces the risk of breaking current users
def multipart_arguments(self,
schema,
*,
content_type=None,
required=True,
description=None,
example=None,
examples=None,
**kwargs):
# At this stage, put schema instance in doc dictionary. Il will be
# replaced later on by $ref or json.
parameters = {
'in': 'files',
'required': required,
'schema': schema,
}
if content_type is not None:
parameters['content_type'] = content_type
if example is not None:
parameters['example'] = example
if examples is not None:
parameters['examples'] = examples
if description is not None:
parameters['description'] = description
error_status_code = kwargs.get('error_status_code', self.ARGUMENTS_PARSER.DEFAULT_VALIDATION_STATUS)
def decorator(func):
arg_parser = self.ARGUMENTS_PARSER.use_args(schema, location='files', **kwargs)
@wraps(func)
def wrapper(*f_args, **f_kwargs):
req = self.ARGUMENTS_PARSER.get_default_request()
from werkzeug.datastructures import ImmutableMultiDict
data = []
orig_files = req.files
orig_form = req.form
for key in req.files:
data.append((key, req.files[key]))
for key in req.form:
data.append((key, req.form[key]))
req.form = None
req.files = ImmutableMultiDict(data)
try:
result = arg_parser(func)(*f_args, **f_kwargs)
except Exception as e:
req.files = orig_files
req.form = orig_form
raise e
req.files = orig_files
req.form = orig_form
return result
# Add parameter to parameters list in doc info in function object
# The deepcopy avoids modifying the wrapped function doc
wrapper._apidoc = deepcopy(getattr(wrapper, '_apidoc', {}))
docs = wrapper._apidoc.setdefault('arguments', {})
docs.setdefault('parameters', []).append(parameters)
docs.setdefault('responses', {})[error_status_code] = http.HTTPStatus(error_status_code).name
# Call use_args (from webargs) to inject params in function
return wrapper
return decoratorOriginally posted by @claudius-kienle in #46 (comment)
As far as I know, there was no PR created for this. As such, there is no proposed documentation update, no test-cases, no formal or informal code review
However, I would love to get rid of the ugly git+https://... from my project require statement at some point. I would also like to not meed to worry about occasionally checking if any sync is needed from upstream
Thanks for your help on this @lafrech, regardless of where it goes from here