Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blueprint and arg fixes #70

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

revmischa
Copy link

Fixes three problems:

  1. Don't auto-register ResourceMeta-based views that have multiple URLs registered with view_funcs. This allows the standard way of building resourceful APIs with flask MethodViews to work nicely. See http://flask.pocoo.org/docs/0.12/views/#method-views-for-apis

  2. Uses the wrong endpoint name when splitting on blueprint name

  3. When using a view as in 1), there may be multiple methods mapped to the same view_func, as in the common case of GET /foo and GET /foo/123. The param is incorrectly mapped to the first version because they are both GET methods on the same view func. This fixes that incorrect behavior. Before it would add the path param to the endpoint without any params.

@henryfjordan
Copy link
Contributor

Thanks for these fixes @revmischa. I'm a little surprised anyone even found the register_existing_resources function, I left it undocumented because I only wrote it as a workaround for not being able to get flask-apispec to pick up blueprints and this was clearly not a great long-term solution.

I'll install this and give it a test soon with a few of the apps we have that use it. I think this might be the root of some subtle malformed swagger issues we've had.

That being said, you seem more familiar with the internals of Flask than I so if you can solve the original problem of blueprints not being caught then maybe we should remove this function.

Example:

"""Doesn't work"""
docs = FlaskApiSpec(app)
app.register_blueprint(any_bp, url_prefix='/test')

"""Works"""
app.register_blueprint(any_bp, url_prefix='/test')
docs = FlaskApiSpec(app)
docs.register_existing_resources()

@revmischa
Copy link
Author

I used metaclasses to auto-register my class-based views. It works nicely. I don't like having to call docs.register() for every function or class. It shouldn't be necessary.
I'd like to post the module I wrote for class-based views, but it needs some cleaning up before it can be open sourced.

@codecov-io
Copy link

Codecov Report

Merging #70 into master will decrease coverage by 0.42%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
- Coverage   97.64%   97.21%   -0.43%     
==========================================
  Files           8        8              
  Lines         339      323      -16     
==========================================
- Hits          331      314      -17     
- Misses          8        9       +1
Impacted Files Coverage Δ
flask_apispec/paths.py 100% <ø> (ø) ⬆️
flask_apispec/extension.py 95.58% <57.14%> (-4.42%) ⬇️
flask_apispec/wrapper.py 97.91% <0%> (-0.13%) ⬇️
flask_apispec/annotations.py 90.24% <0%> (ø) ⬆️
flask_apispec/apidoc.py 98.24% <0%> (+1.18%) ⬆️
flask_apispec/utils.py 100% <0%> (+1.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 535fc9d...d8d7770. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants