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

Should Flask-Classy honor url_prefix where registering Blueprints? #13

Closed
maxcountryman opened this issue Jan 15, 2013 · 9 comments
Closed
Assignees

Comments

@maxcountryman
Copy link
Contributor

I'm not entirely sure if this by design, but it seems as though the register method of FlaskView does not honor url_prefix as defined by calls to the register_blueprint method.

e.g. should something like this work?

from flask import Blueprint, Flask
from flask.ext.classy import FlaskView

bp = Blueprint('bp', __name__)


class ClassyView(FlaskView):
    def index(self):
        return 'foobar'


ClassyView.register(bp)
app = Flask(__name__)
app.register_blueprint(bp, url_prefix='/bp')

if __name__ == '__main__':
    app.run()

Basically I would expect to now be able to navigate to http://localhost:5000/bp/ and see the "foobar" response. However this doesn't seem to be the case.

@apiguy
Copy link
Owner

apiguy commented Jan 15, 2013

In my opinion, it totally should. I need to get this turned into a failing test case so I can get a fix worked out.

@maxcountryman
Copy link
Contributor Author

Awesome! Would love to see this added.

@ghost ghost assigned apiguy Jan 16, 2013
@maxcountryman
Copy link
Contributor Author

Here's a test case for you:

https://gist.github.com/4552573

diff --git a/test_classy/test_blueprints.py b/test_classy/test_blueprints.py
index 6ea0284..71f079c 100644
--- a/test_classy/test_blueprints.py
+++ b/test_classy/test_blueprints.py
@@ -65,8 +65,10 @@ def test_bp_custom_http_method():
     resp = client.post("/basic/route3/")
     eq_("Custom HTTP Method", resp.data)

+def test_bp_url_prefix():
+    foo = Blueprint('foo', __name__)
+    BasicView.register(foo)
+    app.register_blueprint(foo, url_prefix='/foo')

-
-
-
-
+    resp = client.get('/foo/')
+    eq_('Index', resp.data)

@apiguy
Copy link
Owner

apiguy commented Feb 4, 2013

Looks like it does work actually. I think you forgot that BasicView doesn't have a default route_base set and so the inferred route_base will be /basic/. If you set the route_base when you register the BasicView it works fine:

def test_bp_url_prefix():
    foo = Blueprint('foo', __name__)
    BasicView.register(foo, route_base="/")
    app.register_blueprint(foo, url_prefix='/foo')

    resp = client.get('/foo/')
    eq_('Index', resp.data)

@maxcountryman
Copy link
Contributor Author

Right, and this is how we're working around this issue but I don't think that you should require setting a route_base = '/': this idiom is confusing and unnecessarily verbose . Because I've set a url_prefix I think that should trump the FlaskView default otherwise it seems it's changing Flask's behavior in a surprising way.

@apiguy
Copy link
Owner

apiguy commented Feb 4, 2013

Ahhhh I think I understand now. So what you're saying is that when you set a url_prefix for a Blueprint that it should have the same effect as setting route_base on a FlaskView? I don't think I agree with that, as you'll end up with some confusing behavior with FlaskViews that do have route_view specified. Should one trump the other? If so which one? What if some views define a route_view and some dont? At present the route's composition can cleanly be described as:

[Blueprint url_prefix]/[FlaskView route_base]/[view method]

It's consistent and works the same wether you use a Blueprint or not.

Aside from that though, I think that using the Blueprint's url_prefix presents a fairly significant challenge. Flask-Classy doesn't know anything about this parameter since it's part of the app's register_blueprint method (which subsequently calls the Blueprint.register method). Somehow we'd have to be able to get notified of this event, then check for any registered FlaskViews and then re-process any routes that were created by those FlaskViews. Since there is no signal, our only options would be:

  1. Monkeypatch the Blueprint at runtime. This is undesirable, but would have minimal impact on the api.
  2. Create a custom Blueprint subclass or mixin. This could create conflicts with existing code or other plugins.
  3. Create a custom Flask subclass or mixin. Same issues as above.

I think it might make more sense to make the default route_base behavior configurable. The present behavior is to use the class name, minus the 'View' suffix. I could provide some configuration that would allow you set your own behavior for this, maybe using your own callable or even just a string value. Do you think that might solve the issue you're having?

@maxcountryman
Copy link
Contributor Author

What I'm saying is that Flask's API shouldn't be trumped by Flask-Classy. In other words if I pass in url_prefix that should serve as the route_base in effect. In cases where url_prefix isn't passed in then it's probably fine to use whatever default behavior you like.

I think the fundamental problem here is Flask-Classy is changing the behavior of Flask Blueprints in a non-obvious and surprising way. But I don't think the intention is to break the Flask API. In my view, as it is currently, Flask-Classy is breaking the Flask API (in fact you can see this by my unit test which should pass if the Blueprint API weren't being overloaded later on by Flask-Classy's automatic routing behavior). The reason this is important is because url_prefix is an explicit flag that informs the Blueprint to behave in a certain way but, uh oh, Flask-Classy doesn't honor that and clobbers the base_route by default. While it's possible to hack around this by being verbose with route_base this doesn't feel like the right API to solve this problem with.

Perhaps one possibility might simply be to not automatically route to a base other than '/', so that the API becomes explicit instead of implicit, which seems to be the source of the problems here. Now it might not be practical to change the current behavior on FlaskView so maybe another View class could work here instead? Basically the same logic but with all this magic turned off by default. In this way working with Blueprints could become more natural and we might avoid altering Flask unintentionally.

@apiguy
Copy link
Owner

apiguy commented Feb 4, 2013

Flask-Classy doesn't change the behavior of Flask or Blueprints at all (take a look at the code, seriously no modification is made to Flask or Blueprints). I think you may be misunderstanding that all FlaskViews have an implied route_base. For example...

Assume you have this view:

class BasicView(FlaskView):
    def my_method(self):
        return "Hello"

If you register this view with a Flask app directly like

BasicView.register(app)

You'll the route to BasicView.my_method is going to be:

/basic/my_method

If you take that exact same view and register it with a Blueprint using url_prefix like

bp = Blueprint()
BasicView.register(bp)
app.register_blueprint(bp, url_prefix="/my_bp")

Then the route to that same Blueprint.my_method simply has the url_prefix prepended to it. This behavior is managed by Flask, and not by Flask-Classy. It would look like:

/my_bp/basic/my_method

What you're asking is that when you specify a url_prefix for a Blueprint instance that has any FlaskViews registered that we use the url_prefix and ignore FlaskViews built in route_base behavior. Here is just one example of a problem that would cause. Imagine you had these two FlaskViews:

class AwesomeView(FlaskView):
    def my_method(self):
        return "Woah! Awesome!"

class RadicalView(FlaskView):
    def my_method(self):
        return "Totally rad."

Both of these views have a view method called my_method and Flask-Classy supports this by setting a default route_base of awesome and radical to make sure they don't collide. But if we just took 'url_prefix' as you're asking:

bp = Blueprint()
AwesomeView.register(bp)
RadicalView.register(bp)
app.register_blueprint(bp, url_prefix='/my_bp')

What would you expect to happen if I then visited

/my_bp/my_method

Which view wins in this case, should it be AwesomeView.my_method or RadicalView.my_method?

That's why route_base exists. By default, Flask-Classy handles this collision problem for you (to an extent) but allows you to define which one should reside at the root level by specifying route_base = '/' either in the view itself or during registration. If you choose to use url_prefix when registering a Blueprint that prefix will be prepended onto whatever routes have been defined on that Blueprint object, wether they are routes from Flask-Classy or standard Flask routes.

@undergroundtheater
Copy link

I think the best solution for the OP is what I did:

class MyFlaskView(FlaskView):
    route_base = '/'

And inherit from that. Don't inherit from it if you want the default behavior from FlaskView, or if you're going to use multiple FlaskView classes on a single blueprint.

Yes, I know this is old, but it seemed like it needed an answer. :)

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

No branches or pull requests

3 participants