Add support for Flask MethodView to apispec.ext.flask

[djanderson]

Addresses issues #85 and #125.

There are only two slight differences when using a MethodView function:

1. the method ("get", "post", etc) is inferred and is not included in the docstring
2. the path is added with spec.add_path(method_view=view)

Here's the patch in action:

from flask import Flask
from flask.views import MethodView

app = Flask(__name__)

class GistApi(MethodView):
    def get(self):
       """Gist view
       ---
       responses:
           200:
               schema:
                   $ref: '#/definitions/Gist'
       """

app.test_request_context().push()
method_view = GistApi.as_view('gists')
app.add_url_rule("/gists", view_func=method_view)
spec.add_path(method_view=method_view)
print(spec.to_dict()['paths'])
# {'/gists', {'get': {'responses': {200: {'schema': {'$ref': '#/definitions/Gist'}}}}}}

Please review!

[yoichi]

@djanderson your code uses parameter name "method_view" != "view" in path_helper_function, then it cannot be used with apispec.ext.marshmallow (cause TypeError).
You can solve it by switching behavior like follows, instead of registering 2nd path_helper_function.

if hasattr(view, 'view_class'):
    if issubclass(view_func.view_class, MethodView):
        ....

I think MethodView support is useful. Thanks!

[yoichi]

One more change is required to use it with apispec.ext.marshmallow. Currently operations constructed in the preceding extension is not passed to the subsequent extensions.

[djanderson]

Hi @yoichi, thanks for the feedback.

I hate to ask, but could you give me an example of something that throws a TypeError? Like maybe propose a test case I can add that shows the error?

I'm currently using this with apispec.ext.marshmallow and it seems to be working. In my case, I'm registering the marshmallow schemas with spec.definition instead of spec.add_path(urlspec.... If you're already registering your paths with marshmallow UrlSpec path helper, what benefit does adding it again with the MethodView give you?

[yoichi]

I'm sorry for lack of explanation.
If we pass

 plugins=[
     'apispec.ext.flask',
     'apispec.ext.marshmallow'
]

to APISpec constructor, TypeError is occured and ignored in add_path(method_view=method_view) (not thrown to outside) when calling on apispec.ext.marshmallow.schema_path_helper and the method never executed since argument 'view' is not given.

As I noted, there was another problem (#138). While I fixing it, I found tornado extension uses 'urlspec' (!= 'view') argument and have the same problem as above, and I had to make 'view' argument of apispec.ext.marshmallow.schema_path_helper optional for backward compatibility.
Therefore, there is no problem in the behavior with your patch.

On the other hand, I think it is better that the implementation will not rely on the fact one of followings cause TypeError internally and not called:

• apispec.ext.flask.path_from_view (cause TypeError when 'view' is not given)
• apispec.ext.flask.path_from_method_view (cause TypeError when 'method_view' is not given)

I prefer an explicit conditional branch in path helper function instead.

Sincelery,

[djanderson]

I see, thanks for the clarification.

I don't disagree with you, but I probably won't address it in this PR since I don't think I'm abusing the TypeError in a way that's not currently intended.

I appreciate the review, though!

[yoichi]

I've also put comments in the code to clarify what I mean.
But I will respect your choice if you decide that you will not deal with it.

Thanks,

[yoichi]

Is it necessary to register multiple path helper functions in one extension? I think this breaks the locality of the conditional branch whether the view is derived from MethodView or not.

[yoichi]

Using 'method_view' (not 'view' as used in existing helper function) forces user to distinguish the view is derived from MethodView or not.

[tangram]

I would love to see this merged and released. Any chance you could find the time to resolve the last few things, @djanderson, @yoichi?

[djanderson]

@tangram, thanks for the encouragement. I think this implementation was faithful to the "ask forgiveness, not permission" style of the existing code, but @yoichi has raised some fair criticisms and his modifications in #139 look to provide the desired capability as well as an improved user experience. I'm closing this PR so that we can focus on landing #139. @yoichi, thanks again for the critical eye and improvements.