jmcarp / flask-apispec

MIT License
652 stars 154 forks source link

@marshal_with(None) don't work for function-based views #89

Open NotJustPizza opened 6 years ago

NotJustPizza commented 6 years ago

In docs (https://flask-apispec.readthedocs.io/en/latest/usage.html) we can find this example:

from flask import make_response
from flask_apispec.views import MethodResource

class PetResource(MethodResource):

    @marshal_with(PetSchema)
    def get(self, pet_id):
        return Pet.query.filter(Pet.id == pet_id).one()

    @use_kwargs(PetSchema)
    @marshal_with(PetSchema, code=201)
    def post(self, **kwargs):
        return Pet(**kwargs)

    @use_kwargs(PetSchema)
    @marshal_with(PetSchema)
    def put(self, pet_id, **kwargs):
        pet = Pet.query.filter(Pet.id == pet_id).one()
        pet.__dict__.update(**kwargs)
        return pet

    @marshal_with(None, code=204)
    def delete(self, pet_id):
        pet = Pet.query.filter(Pet.id == pet_id).one()
        pet.delete()
        return make_response('', 204)

I tried same, but using function-based views, examples:

@app.route("/pet", methods=["DELETE"])
@marshal_with(None, code=204)
def delete_pet():
    return None, 204

@app.route("/pet", methods=["POST"])
@marshal_with(None, code=204)
def upade_pet():
    return None, 204
 * Running on http://localhost:5000/ (Press CTRL+C to quit)
127.0.0.1 - - [08/Apr/2018 03:47:00] "DELETE /pet HTTP/1.1" 500 -
Error on request:
Traceback (most recent call last):
  File "C:<path to project>\venv\lib\site-packages\werkzeug\serving.py", line 270, in run_wsgi
    execute(self.server.app)
  File "C:<path to project>\venv\lib\site-packages\werkzeug\serving.py", line 258, in execute
    application_iter = app(environ, start_response)
  File "C:<path to project>\venv\lib\site-packages\werkzeug\wsgi.py", line 826, in __call__
    return app(environ, start_response)
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1997, in __call__
    return self.wsgi_app(environ, start_response)
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1985, in wsgi_app
    response = self.handle_exception(e)
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1540, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "C:<path to project>\venv\lib\site-packages\flask\_compat.py", line 33, in reraise
    raise value
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1982, in wsgi_app
    response = self.full_dispatch_request()
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1614, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1517, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "C:<path to project>\venv\lib\site-packages\flask\_compat.py", line 33, in reraise
    raise value
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "C:<path to project>\venv\lib\site-packages\flask\app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "C:<path to project>\venv\lib\site-packages\flask_apispec\annotations.py", line 117, in wrapped
    return wrapper(*args, **kwargs)
  File "C:<path to project>\venv\lib\site-packages\flask_apispec\wrapper.py", line 34, in __call__
    return self.marshal_result(unpacked, status_code)
  File "C:<path to project>\venv\lib\site-packages\flask_apispec\wrapper.py", line 58, in marshal_result
    dumped = schema.dump(unpacked[0])
AttributeError: 'dict' object has no attribute 'dump'

It works with "@marshal_with(MyShema, code=204)", but never with None as first parameter.

//Edit

Workaround is to define empty schema:

class EmptySchema(Schema):
    pass

and use:

@marshal_with(EmptySchema, code=204)
sloria commented 6 years ago

Looks like a bug to me. PRs welcome!

rphes commented 6 years ago

This can be solved by replacing

schemas = utils.merge_recursive(annotation.options)
schema = schemas.get(status_code, schemas.get('default'))
if schema and annotation.apply is not False:
    schema = utils.resolve_schema(schema['schema'], request=flask.request)
    dumped = schema.dump(unpacked[0])
    output = dumped.data if MARSHMALLOW_VERSION_INFO[0] < 3 else dumped
else:
    output = unpacked[0]
return format_output((format_response(output),) + unpacked[1:])

from wrapper.py, with

options = utils.merge_recursive(annotation.options)
option = options.get(status_code, options.get('default'))
if option:
    schema = utils.resolve_schema(option['schema'], request=flask.request)
else:
    schema = None

if schema and annotation.apply is not False:
    dumped = schema.dump(unpacked[0])
    output = dumped.data if MARSHMALLOW_VERSION_INFO[0] < 3 else dumped
else:
    output = unpacked[0]
return format_output((format_response(output), ) + unpacked[1:])

The problem is that the code only checks whether there is a response defined for the current request, and not whether this definition actually contains a valid schema.

I'd submit a PR, but I'm having trouble adding a test for this code as Webtest complains about having a 204 response without a content-length header.

Let me know what you think.

keithly commented 5 years ago

It seems like #101 fixes this, but it hasn't been merged.