Only single path added for Flask API MethodViews
Issue by mathewmarcus
Thursday Jan 18, 2018 at 21:14 GMT
Originally opened as https://github.com/marshmallow-code/apispec/issues/181
The Flask documentation gives a use case - for RESTful APIs - where a single method view can be added under multiple URL rules. The following code snippet is provided as an example.
class UserAPI(MethodView):
def get(self, user_id):
if user_id is None:
# return a list of users
pass
else:
# expose a single user
pass
def post(self):
# create a new user
pass
def delete(self, user_id):
# delete a single user
pass
def put(self, user_id):
# update a single user
pass
user_view = UserAPI.as_view('user_api')
app.add_url_rule('/users/', defaults={'user_id': None},
view_func=user_view, methods=['GET',])
app.add_url_rule('/users/', view_func=user_view, methods=['POST',])
app.add_url_rule('/users/<int:user_id>', view_func=user_view,
methods=['GET', 'PUT', 'DELETE'])
If we define the following apispec and add the above view function like so
spec = APISpec(
title='Swagger Petstore',
version='1.0.0',
plugins=[
'apispec.ext.flask',
'apispec.ext.marshmallow',
],
)
with app.test_request_context():
spec.add_path(view=user_view)
only one of the above paths would be included in the generated apispec.
Looking at the apispec.ext.flask module, it seems that this behavior is a result of line 92 in the _rule_for_view function.
# WARNING: Assume 1 rule per view function for now
rule = current_app.url_map._rules_by_endpoint[endpoint][0]
Given the comment, it seems like this behavior is expected. Is there any interest in/intent to modify this to enable all of the paths to be added to the apispec in the above situation?
Comment by lafrech
Thursday Jan 18, 2018 at 21:28 GMT
That would be an interesting enhancement, indeed. Would you like to take a stab at it?
Comment by mathewmarcus
Thursday Jan 18, 2018 at 21:37 GMT
If there's interest in adding it then yes definitely
Comment by mathewmarcus
Friday Jan 19, 2018 at 17:22 GMT
Just wanted to lay out what I see as two directions we could take.
- Having looked at #126 and #68, it seems that there is a desire to create an
add_pathsfunction or perhaps enable path helpers to return iterables which contain multiple instances ofapispec.core.Path. This seems like it would require significantly more code changes than option 2, but it would allow path registration to look like it does in the above example or like this:
with app.test_request_context():
spec.add_paths(view=user_view)
- The simpler option is to modify to the
apispec.ext.flask._rule_for_viewfunction to optionally select a rule based on matchingpathoroperationskwargs passed to theadd_pathmethod. Then, path registration would probably look more like.
with app.test_request_context():
spec.add_path(path='/users/', view=user_view, operations=dict(get={}))
spec.add_path(path='/users/', view=user_view, operations=dict(post={}))
spec.add_path(path='/users/{user_id}', view=user_view, operations=dict(get={}, put={}, delete={}))
Which option would you prefer? Or do you see a better way to accomplish this?
Comment by dankolbman
Thursday Jan 25, 2018 at 20:45 GMT
Currently running into this exact problem. One more thing to consider is how to handle docstrings.
The two rules GET /users, and GET /users/123, for example, both use the same get() in the MethodView, however, the first would presumably have no request schema, while the second would. How would these two be differentiated in the docstring? Would the path parameters in the docstring be mandatory, and try to resolve from there?
Comment by lafrech
Tuesday Apr 10, 2018 at 20:03 GMT
@mathewmarcus I've had a quick look at it today. I think the second option is reasonable.
It is not that verbose.
Those three calls could reduce to only two. Besides, if I understand correctly, the purpose of the flask helper is to parse the YAML, so you don't need to pass the whole operation descriptions, only the method names, right?
with app.test_request_context():
spec.add_path(path='/users/', view=user_view, methods=['get', 'post'])
spec.add_path(path='/users/{user_id}', view=user_view, methods=['get', 'put', 'delete'])
Regarding @dankolbman's question about GET, I have no idea. Would it be possible to put the docstrings in each individual function? This brings back a blurry memory of https://github.com/marshmallow-code/apispec/issues/85. Maybe that's the reason you passed the operations as dicts rather than just method names, but if you must parse the YAML yourself, the helper is not of much use.
I can't really remember and I'm not impacted because I don't use this helper: I don't use YAML docstrings, and I have another layer on top of apispec that calls add_path passing rules and operations as arguments: spec.add_path(app=app, rule=rule, operations={}).
I declare my CRUD resources using two MethodViews, Pet and PetById and I think it is simpler and clearer this way. Both get functions are totally different, I don't see the benefit of merging them and checking for an optional ID parameter. I guess it is a matter of opinion. Then I don't have to face the multiple rule per view function issue. I'm not saying the use case in the OP is not valid. But to me, the Pet/PetById way is the natural way, not a workaround.
Here's an example. It uses flask-rest-api, a layer over apispec, which provides all the decorators and calls spec.add_path, but never mind, the point is to illustrate the two MethodViews idea with a minified example.
blp = Blueprint('test', __name__, url_prefix='/test')
@blp.route('/')
class Resource(MethodView):
@blp.response(DocSchema(many=True))
def get(self):
return collection.items
@blp.arguments(DocSchema)
@blp.response(DocSchema)
def post(self, new_item):
return collection.post(new_item)
@blp.route('/<int:item_id>')
class ResourceById(MethodView):
@blp.response(DocSchema)
def get(self, item_id):
return collection.get_or_404(item_id)
@blp.arguments(DocSchema)
@blp.response(DocSchema)
def put(self, new_item, item_id):
collection.get_or_404(item_id)
return collection.put(item_id, new_item)
@blp.response(code=204)
def delete(self, item_id):
collection.get_or_404(item_id)
collection.delete(item_id)
I'm afraid I'm not being very helpful.
Comment by mathewmarcus
Tuesday Apr 24, 2018 at 01:09 GMT
@lafrech on the contrary, that definitely clears things up. And I agree that separating the routes into two MethodViews clarifies things. I only mentioned the single MethodView example because it was in the Flask docs.
In any case, I can move ahead on the second option.
Comment by ergo
Sunday Jun 24, 2018 at 11:27 GMT
I think I had similar problem in pyramid, maybe the approach I used there could help?
https://github.com/ergo/pyramid_apispec - one can register same view with different conditions for introspection.
Could this be added? We're working on a large scale API (which is already in production) and recently found out our swagger documentation is invalid due to this (quite silent warning):
WARNING: Assume 1 rule per view function for now
Could you at least add a logger statement which warns about this because it took us about 4 hours to debug (we're using flask-apispec to generate the docs, so this wasn't very apparent to us) :(
Has there been any additional thought/movement on this? Ever since the issue was migrated it seems it's gone stale.
This is a very common scenario for REST APIs, so it would be great to have this logic included.
Is there any update on this issue? Would come very handy.