Only single path added for Flask API MethodViews

[lafrech]

Issue by mathewmarcus
Thursday Jan 18, 2018 at 21:14 GMT
Originally opened as marshmallow-code/apispec#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?

[lafrech]

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?

[lafrech]

Comment by mathewmarcus
Thursday Jan 18, 2018 at 21:37 GMT

---

If there's interest in adding it then yes definitely

[lafrech]

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.

1. Having looked at Bump flake8-bugbear from 23.12.2 to 24.1.16 #126 and Revert "add pycharm project dir to gitignore" #68, it seems that there is a desire to create an add_paths function or perhaps enable path helpers to return iterables which contain multiple instances of apispec.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)

2. The simpler option is to modify to the apispec.ext.flask._rule_for_view function to optionally select a rule based on matching path or operations kwargs passed to the add_path method.
   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?

[lafrech]

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?

[lafrech]

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 marshmallow-code/apispec#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.

[lafrech]

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.

[lafrech]

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.

[Paradoxis]

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) :(