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

Nested blueprint should be able to have same name #4069

Closed
MaicoTimmerman opened this issue May 17, 2021 · 9 comments · Fixed by #4074
Closed

Nested blueprint should be able to have same name #4069

MaicoTimmerman opened this issue May 17, 2021 · 9 comments · Fixed by #4074
Milestone

Comments

@MaicoTimmerman
Copy link

Nested blueprints should be able to have the same name. Currently all blueprints are registered under their own name, however for a nested blueprint id expected it to use the <parent_name>.<child_name>

from pprint import pprint
import flask


app = flask.Flask(__name__)
view_bp = flask.Blueprint("home", __name__)
api_bp = flask.Blueprint("api", __name__, url_prefix="/api")
home_api_bp = flask.Blueprint("home", __name__)

@view_bp.route("/")
@home_api_bp.route("/")
def view():
    print(flask.request, flask.request.endpoint)

api_bp.register_blueprint(home_api_bp)

app.register_blueprint(view_bp)
app.register_blueprint(api_bp)
pprint(app.url_map)
Traceback (most recent call last):
  File "/home/maico/test.py", line 18, in <module>
    app.register_blueprint(api_bp)
  File "/home/maico/.local/lib/python3.9/site-packages/flask/scaffold.py", line 54, in wrapper_func
    return f(self, *args, **kwargs)
  File "/home/maico/.local/lib/python3.9/site-packages/flask/app.py", line 1023, in register_blueprint
    blueprint.register(self, options)
  File "/home/maico/.local/lib/python3.9/site-packages/flask/blueprints.py", line 359, in register
    blueprint.register(app, bp_options)
  File "/home/maico/.local/lib/python3.9/site-packages/flask/blueprints.py", line 275, in register
    assert app.blueprints[self.name] is self, (
AssertionError: A name collision occurred between blueprints <Blueprint 'home'> and <Blueprint 'home'>. Both share the same name 'home'. Blueprints that are created on the fly need unique names.

Environment:

  • Python version: 3.9.1
  • Flask version: 2.1.0dev0: 9039534
@ThiefMaster
Copy link
Member

Should this really be possible? having home and api.home seems like a really easy way to confuse developers...

@MaicoTimmerman
Copy link
Author

MaicoTimmerman commented May 17, 2021

The me the current situation was even more confusing, the error both uses the name home and there was no way to narrow down which blueprint is which.

Additionally, say this limitation stays in place, I'd need to rename the home_api_bp to something like api_home which results in the endpoints becoming:

Map([
 <Rule '/' (GET, HEAD, OPTIONS) -> home.view>,
 <Rule '/' (GET, HEAD, OPTIONS) -> api.api_home.view>,
 <Rule '/static/<filename>' (GET, HEAD, OPTIONS) -> static>])

That seems rather odd, having api.api_home.view as argument for methods like url_for. I feel having the logic of reusing parent naming: api.home.view, is better.
edit: typo

@MaicoTimmerman
Copy link
Author

If you agree on this behavior, I'd be willing to create a PR for this.

@pgjones
Copy link
Member

pgjones commented May 17, 2021

In my view each instance of a blueprint must have a unique name, as this allows flask to know which blueprint's methods to invoke.

If you deleted the home_api_bp and used view_bp everywhere in its place things would work as you'd expect - could this be the solution you desire?

@MaicoTimmerman
Copy link
Author

That would defeat the purpose of nested blueprints, as then to get api.home we have to directly register with the app and can no longer use the power of nested blueprints.

I just did some inspection to see the logic here, but it seems my proposal was already included in the original https://github.com/pallets/flask/pull/3923/files#diff-30595964c383760a7295e59705f71f85766068b8231d8227ee34b5f5752a41e9R327. Nested blueprints already build their endpoint based on their parent blueprints, just not register in app.blueprints, therefore I think this can be considered a bug.

@pgjones
Copy link
Member

pgjones commented May 17, 2021

I think I understand what you are asking now. I think you'd like something like an endpoint argument to the Blueprint constructor, much like there is to the add_url_rule method, whereby the endpoint is used instead of the name to construct the route endpoints. I envision it working like,

from pprint import pprint
import flask


app = flask.Flask(__name__)
view_bp = flask.Blueprint("home", __name__)
api_bp = flask.Blueprint("api", __name__, url_prefix="/api")
home_api_bp = flask.Blueprint("api_home", __name__, endpoint="home")

@view_bp.route("/")
@home_api_bp.route("/")
def view():
    print(flask.request, flask.request.endpoint)

api_bp.register_blueprint(home_api_bp)

app.register_blueprint(view_bp)
app.register_blueprint(api_bp)
pprint(app.url_map)

giving this output,

Map([
 <Rule '/' (GET, HEAD, OPTIONS) -> home.view>,
 <Rule '/' (GET, HEAD, OPTIONS) -> api.home.view>,
 <Rule '/static/<filename>' (GET, HEAD, OPTIONS) -> static>])

Sadly the names must be unique and the name used in the endpoint must match the name used in the endpoint. It isn't clear to me how this could be altered as these conditions allow the endpoint to be split by . into the constituent blueprint names which are then used to lookup the actual blueprint instances see #4059.

@davidism
Copy link
Member

This is getting into the territory of #1091.

@MaicoTimmerman
Copy link
Author

It isn't clear to me how this could be altered as these conditions allow the endpoint to be split by . into the constituent blueprint names which are then used to lookup the actual blueprint instances see #4059.

Given the comments the PR, would it be an option to have blueprints be initialized with a name keeping the requirement that the name shouldn't contain . and during registration be upgraded to a fully qualified name in app.blueprints based on the parent blueprints:

parent = flask.Blueprint("parent", __name__, url_prefix="/parent")
child = flask.Blueprint("child", __name__, url_prefix="/child")
grandchild = flask.Blueprint("grandchild", __name__, url_prefix="/grandchild")

@parent.route("/")
def parent_index():
    print(request.endpoint)  # parent.parent_index
    print(request.blueprint)  # parent
    print(request.blueprints)  # ['parent']
    return "Parent"

@child.route("/")
def child_index():
    print(request.endpoint)  # parent.child.child_index
    print(request.blueprint) # parent.child
    print(request.blueprints) # ['parent.child', 'parent']
    return "Child"

@grandchild.route("/")
def grandchild_index():
    print(request.endpoint) # parent.child.grandchild.grandchild_index
    print(request.blueprint) # parent.child.grandchild
    print(request.blueprints) # ['parent.child.grandchild', 'parent.child', 'parent']
    return "Grandchild"

where app.url_map would be:

Map([<Rule '/parent/child/grandchild/' (GET, HEAD, OPTIONS) -> parent.child.grandchild.grandchild_index>,
 <Rule '/parent/child/' (GET, HEAD, OPTIONS) -> parent.child.child_index>,
 <Rule '/parent/' (GET, HEAD, OPTIONS) -> parent.parent_index>,
 <Rule '/static/<filename>' (GET, HEAD, OPTIONS) -> static>])

Using this the instances of parent can still be found by iteratively spliting on . from the right, while also maintaining the ability to directly lookup blueprints based on the fully-qualified name.

@davidism
Copy link
Member

@pgjones and I have discussed this on Discord and know how we're going to solve it. That's why that PR was closed, another will be submitted.

Basically request.blueprint is the key that the blueprint was registered with in app.blueprints. It should be the dot-separated path of blueprint names. The full paths must still be unique. That will solve this issue. Next, we will make the uniqueness check more strict, and require unique names even for the same blueprint registered twice, by allowing name= in register_blueprint the same as url_prefix=. This will solve #1091.

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

Successfully merging a pull request may close this issue.

4 participants