before_first_request removed as of Flask 2.3

[bezborodow]

flask_menu is now broken as of Flask 2.3.0 (pallets/flask#4605):

The app.before_first_request and bp.before_app_first_request decorators are removed.

Causes: inveniosoftware/flask-breadcrumbs#56.

[bezborodow]

See __init__.py, line 369:

357     def menu_decorator(f):
358         """Decorator of a view function that should be included in the menu."""
359         if isinstance(app, Blueprint):
360             endpoint = app.name + '.' + f.__name__
361             before_first_request = app.before_app_first_request
362         else:
363             endpoint = f.__name__
364             before_first_request = app.before_first_request
365 
366         expected = inspect.getfullargspec(f).args if PY3 else \
367             inspect.getargspec(f).args
368 
369         @before_first_request #      <---------------------------------
370         def _register_menu_item():

[RRGTHWAR]

For what it's worth, this is actually super easy to fix. In init.py, change this:

        if isinstance(app, Blueprint):
            endpoint = app.name + '.' + f.__name__
            before_first_request = app.before_app_first_request
        else:
            endpoint = f.__name__
            before_first_request = app.before_first_request

to this:

        if isinstance(app, Blueprint):
            endpoint = app.name + '.' + f.__name__
        else:
            endpoint = f.__name__

And this:

        @before_first_request
        def _register_menu_item():

to this:

        @app.record_once
        def _register_menu_item(func):

Then in classy.py, do the same:

    if isinstance(app, Blueprint):
        endpoint_prefix = app.name + '.'
    else:
        endpoint_prefix = ''

    @app.record_once
    def _register_menu_items(func):
        for meth_str in dir(classy_view):
            meth = getattr(classy_view, meth_str)

And presto! No more deprecation warnings, and compatibility with Flask 2.3.

The "func" thing is in there because record_once expects to pass a function to the function it's calling, so you get an error if it's not included.

[taimurrabuske]

I think that a cleaner way to do that would be to avoid the @app.record_once decorator altogether, as it is only valid if app is an instance of Blueprint, and fails otherwise. What about just call _register_menu_item() after it is defined locally, instead of leveraging it to the app/blueprint as a decorator? My implementation (that seems to work) follows:

    def menu_decorator(f):
        """Decorator of a view function that should be included in the menu."""
        if isinstance(app, Blueprint):
            endpoint = app.name + '.' + f.__name__
        else:
            endpoint = f.__name__

        expected = inspect.getfullargspec(f).args if PY3 else \
            inspect.getargspec(f).args

        # @before_first_request
        def _register_menu_item():
            # str(path) allows path to be a string-convertible object
            # that may be useful for delayed evaluation of path
            item = current_menu.submenu(str(path))
            item.register(
                endpoint,
                text,
                order,
                endpoint_arguments_constructor=endpoint_arguments_constructor,
                dynamic_list_constructor=dynamic_list_constructor,
                active_when=active_when,
                visible_when=visible_when,
                expected_args=expected,
                **kwargs)
        _register_menu_item()
        return f

[djast]

I've tried both of these approaches, and for each of them I seem to be getting "Working outside of application context" errors at item = current_menu.submenu(str(path)) under at least some circumstances I have yet to nail down. Some of these can be worked around by registering my blueprints within a with app.app_context(): block, but this was not needed with the old code.

A compromise between the two approaches that seems to work better than either alone (at least for me) is to change:

    if isinstance(app, Blueprint):
        endpoint = app.name + '.' + f.__name__
        before_first_request = app.before_app_first_request
    else:
        endpoint = f.__name__
        before_first_request = app.before_first_request

to:

    if isinstance(app, Blueprint):
        endpoint = app.name + '.' + f.__name__
        before_first_request = app.record_once
    else:
        endpoint = f.__name__
        before_first_request = lambda x : x()

and changing def _register_menu_item(): to def _register_menu_item(*args):. This uses record_once for blueprints as with RRGTHWAR's fix, but executes _register_menu_item() immediately for applications as with taimurrabuske's fix.

However, even with this change, I still need to move my app.register_blueprint() calls to a with app.app_context(): block for the app to run, which is not ideal.

[djast]

I've tried to investigate this further. I have what may be a naive approach that appears to work (at least for me on a development server), but it's extraordinarily ugly.

One complication I ran into is that blueprints that register menu items may be imported either before or after Menu.init_app() has been called.

The attached patch attempts to deal with that by deferring the _register_menu_item() call for blueprints to init_app() if the app has not yet been initialized, or calling it immediately if it has, in each case within a with app.app_context(): block.

I resorted to global variables to store the list of functions to be deferred to init_app() and the app instance to be used thereafter; I don't know specifically what havoc this will wreak, but it's clearly inelegant, and I am not remotely confident that this doesn't break something catastrophically.

Any recommendations on how to improve this by eliminating the _app and _defer globals from this patch would be welcome. Likewise if the entire approach is misguided and a better solution can be suggested.

flask_menu.txt

[djast]

A slightly cleaner version of the same patch:

• as far as I can tell, there's no need to define before_first_request() differently for blueprints vs. apps; adding the function to the _defer list to be called in init_app() should work just as well as invoking the function immediately.
• I made _app and _defer class variables for the Menu class, which is perhaps only slightly better than making them global variables. I still don't know whether there's a better approach, but it doesn't feel clean.

I don't use Flask-Classy, so this patch doesn't cover required changes to classy.py, but a similar if not identical definition of before_first_request (and importing Menu in addition to current_menu from flask_menu) might be sufficient for that too; maybe someone who uses Flask-Classy could try that out and report back.

flask_menu.txt

[djast]

Not sure if anyone's still paying attention to this, but I'm still looking at improved ways to address this.

One thing I tripped over while looking at the code is the fact that it assumes that the endpoints for a blueprint are associated with the blueprint name, which is not necessarily correct if the blueprint is registered with a name= argument; e.g., in the current code, this fails:

from flask import Blueprint
from flask_menu import register_menu
bp = Blueprint('bp', __name__, template_folder='templates')
@bp.route("/demo")
@register_menu(bp, 'top', "Demo")
def demo():
    return "test"
app.register_blueprint(bp, url_prefix="/alt", name="alt")

This was always the case, but fixing this bug may be an opportunity to address this unrelated deficiency, which can be addressed by getting the endpoint when the blueprint is registered rather than directly from the Blueprint object.

The attached patch takes an approach closer to my first patch, with separate before_first_request wrappers for blueprints vs. applications, but uses @app.record for blueprints to register them when the blueprint is registered. It also eliminates the ugly globlal/class variables from my previous patch attempts.

Note that if a blueprint is registered multiple times, e.g.,

app.register_blueprint(bp, url_prefix="/alt1", name="alt1")
app.register_blueprint(bp, url_prefix="/alt2", name="alt2")

then the menu items will be registered multiple times, and the URLs for the menu paths will depend on the order the blueprints are registered. It might be helpful to add functionality to @register_menu to be able to customize the menu path based on the blueprint name, so that menus could have links to routes in multiple instances of the blueprint; however, that's beyond the scope of this issue.

There may be more elegant ways to code this patch, and I still have made no attempt to deal with classy.py with this version of it. As with my first attempts, I'm not confident that there aren't cases that this code breaks.

flask_menu.txt

[utnapischtim]

@djast i am working on a fix in cooperation with CERN. i will go through your thoughts to see if i missed something. Have you cloned the repository and pushed your changes to a branch on github? That would make it easier for me to see your changes.

The PR i am working on is #85

[djast]

Unfortunately, I'm new to github and don't use it for development, and am also new to Flask development.

The changes to init.py in my patch above can be applied to your decorators.py fairly easily. The broad strokes are:

• use app.record instead of app.before_app_first_request for blueprints, and add a parameter to _register_menu_item() for the BlueprintSetupState object that's passed when it's invoked;
• if it's not a blueprint, use before_first_request = lambda x : x() to call the function immediately instead of registering it to be invoked later (this is admittedly not very clean at all--see below)
• defer the definition of the endpoint to be registered so it's done from _register_menu_item() rather than menu_decorator(), so that it can use the name of the blueprint as registered by the app
• if it's a blueprint, create an application context to do the item.register().

I think my patch can be simplified further by:

• changing ctx = None to ctx = app.context in the if state is None path (so an application context can be used regardless of whether it's an application or a blueprint);
• changing def _doitem(): to with ctx(): and eliminating the if ctx: block at the bottom.

I've attached a patch that incorporates that simplification and can be applied to decorators.py in your utnapischtim:add-init-functions branch.

This could perhaps be improved further by having menu_decorator() register the function (to be called by init_app()) rather than invoking it immediately, but I'm not sure of the appropriate way to do that.

decorators-patch.txt

[sfister5]

Is this to be fixed or is this a dead project?

[djast]

I still don't really know my way around github development, but I've cloned the repo and created a branch with my patch as requested by utnapischtim (nearly a year ago).

It applies the changes to decorators.py I attached in my previous comment and removes the Flask<2.3.0 dependency in setup.cfg.

I haven't created a pull request, as I have no idea how backward-compatible the changes are.

[ggyczew]

This project is DEAD for sure. I have made in 2022 PR of fix to be used with flask-breadcrumbs. Today I have updated Flask versions in project and ended up with this BUG.