Merge pull request #4577 from hallacy/hallacy/fix_4571

update setupmethod behavior

CHANGES.rst

@@ -16,6 +16,10 @@ Unreleased
 
 -   Refactor ``register_error_handler`` to consolidate error checking.
     Rewrite some error messages to be more consistent. :issue:`4559`
+-   Use Blueprint decorators and functions intended for setup after
+    registering the blueprint will show a warning. In the next version,
+    this will become an error just like the application setup methods.
+    :issue:`4571`
 
 
 Version 2.1.2

src/flask/app.py

@@ -541,8 +541,17 @@ class Flask(Scaffold):
         # the app's commands to another CLI tool.
         self.cli.name = self.name
 
-    def _is_setup_finished(self) -> bool:
+    def _check_setup_finished(self, f_name: str) -> None:
-        return self.debug and self._got_first_request
+        if self._got_first_request:
+            raise AssertionError(
+                f"The setup method '{f_name}' can no longer be called"
+                " on the application. It has already handled its first"
+                " request, any changes will not be applied"
+                " consistently.\n"
+                "Make sure all imports, decorators, functions, etc."
+                " needed to set up the application are done before"
+                " running it."
+            )
 
     @locked_cached_property
     def name(self) -> str:  # type: ignore

src/flask/blueprints.py

@@ -6,6 +6,7 @@ from functools import update_wrapper
 from .scaffold import _endpoint_from_view_func
 from .scaffold import _sentinel
 from .scaffold import Scaffold
+from .scaffold import setupmethod
 from .typing import AfterRequestCallable
 from .typing import BeforeFirstRequestCallable
 from .typing import BeforeRequestCallable
@@ -162,7 +163,6 @@ class Blueprint(Scaffold):
     .. versionadded:: 0.7
     """
 
-    warn_on_modifications = False
     _got_registered_once = False
 
     #: Blueprint local JSON encoder class to use. Set to ``None`` to use
@@ -208,27 +208,33 @@ class Blueprint(Scaffold):
         self.cli_group = cli_group
         self._blueprints: t.List[t.Tuple["Blueprint", dict]] = []
 
-    def _is_setup_finished(self) -> bool:
+    def _check_setup_finished(self, f_name: str) -> None:
-        return self.warn_on_modifications and self._got_registered_once
+        if self._got_registered_once:
+            import warnings
 
+            warnings.warn(
+                f"The setup method '{f_name}' can no longer be called on"
+                f" the blueprint '{self.name}'. It has already been"
+                " registered at least once, any changes will not be"
+                " applied consistently.\n"
+                "Make sure all imports, decorators, functions, etc."
+                " needed to set up the blueprint are done before"
+                " registering it.\n"
+                "This warning will become an exception in Flask 2.3.",
+                UserWarning,
+                stacklevel=3,
+            )
+
+    @setupmethod
     def record(self, func: t.Callable) -> None:
         """Registers a function that is called when the blueprint is
         registered on the application.  This function is called with the
         state as argument as returned by the :meth:`make_setup_state`
         method.
         """
-        if self._got_registered_once and self.warn_on_modifications:
-            from warnings import warn
-
-            warn(
-                Warning(
-                    "The blueprint was already registered once but is"
-                    " getting modified now. These changes will not show"
-                    " up."
-                )
-            )
         self.deferred_functions.append(func)
 
+    @setupmethod
     def record_once(self, func: t.Callable) -> None:
         """Works like :meth:`record` but wraps the function in another
         function that will ensure the function is only called once.  If the
@@ -251,6 +257,7 @@ class Blueprint(Scaffold):
         """
         return BlueprintSetupState(self, app, options, first_registration)
 
+    @setupmethod
     def register_blueprint(self, blueprint: "Blueprint", **options: t.Any) -> None:
         """Register a :class:`~flask.Blueprint` on this blueprint. Keyword
         arguments passed to this method will override the defaults set
@@ -390,6 +397,7 @@ class Blueprint(Scaffold):
             bp_options["name_prefix"] = name
             blueprint.register(app, bp_options)
 
+    @setupmethod
     def add_url_rule(
         self,
         rule: str,
@@ -417,6 +425,7 @@ class Blueprint(Scaffold):
             )
         )
 
+    @setupmethod
     def app_template_filter(
         self, name: t.Optional[str] = None
     ) -> t.Callable[[TemplateFilterCallable], TemplateFilterCallable]:
@@ -433,6 +442,7 @@ class Blueprint(Scaffold):
 
         return decorator
 
+    @setupmethod
     def add_app_template_filter(
         self, f: TemplateFilterCallable, name: t.Optional[str] = None
     ) -> None:
@@ -449,6 +459,7 @@ class Blueprint(Scaffold):
 
         self.record_once(register_template)
 
+    @setupmethod
     def app_template_test(
         self, name: t.Optional[str] = None
     ) -> t.Callable[[TemplateTestCallable], TemplateTestCallable]:
@@ -467,6 +478,7 @@ class Blueprint(Scaffold):
 
         return decorator
 
+    @setupmethod
     def add_app_template_test(
         self, f: TemplateTestCallable, name: t.Optional[str] = None
     ) -> None:
@@ -485,6 +497,7 @@ class Blueprint(Scaffold):
 
         self.record_once(register_template)
 
+    @setupmethod
     def app_template_global(
         self, name: t.Optional[str] = None
     ) -> t.Callable[[TemplateGlobalCallable], TemplateGlobalCallable]:
@@ -503,6 +516,7 @@ class Blueprint(Scaffold):
 
         return decorator
 
+    @setupmethod
     def add_app_template_global(
         self, f: TemplateGlobalCallable, name: t.Optional[str] = None
     ) -> None:
@@ -521,6 +535,7 @@ class Blueprint(Scaffold):
 
         self.record_once(register_template)
 
+    @setupmethod
     def before_app_request(self, f: BeforeRequestCallable) -> BeforeRequestCallable:
         """Like :meth:`Flask.before_request`.  Such a function is executed
         before each request, even if outside of a blueprint.
@@ -530,6 +545,7 @@ class Blueprint(Scaffold):
         )
         return f
 
+    @setupmethod
     def before_app_first_request(
         self, f: BeforeFirstRequestCallable
     ) -> BeforeFirstRequestCallable:
@@ -548,6 +564,7 @@ class Blueprint(Scaffold):
         )
         return f
 
+    @setupmethod
     def teardown_app_request(self, f: TeardownCallable) -> TeardownCallable:
         """Like :meth:`Flask.teardown_request` but for a blueprint.  Such a
         function is executed when tearing down each request, even if outside of
@@ -558,6 +575,7 @@ class Blueprint(Scaffold):
         )
         return f
 
+    @setupmethod
     def app_context_processor(
         self, f: TemplateContextProcessorCallable
     ) -> TemplateContextProcessorCallable:
@@ -569,6 +587,7 @@ class Blueprint(Scaffold):
         )
         return f
 
+    @setupmethod
     def app_errorhandler(self, code: t.Union[t.Type[Exception], int]) -> t.Callable:
         """Like :meth:`Flask.errorhandler` but for a blueprint.  This
         handler is used for all requests, even if outside of the blueprint.
@@ -580,6 +599,7 @@ class Blueprint(Scaffold):
 
         return decorator
 
+    @setupmethod
     def app_url_value_preprocessor(
         self, f: URLValuePreprocessorCallable
     ) -> URLValuePreprocessorCallable:
@@ -589,6 +609,7 @@ class Blueprint(Scaffold):
         )
         return f
 
+    @setupmethod
     def app_url_defaults(self, f: URLDefaultCallable) -> URLDefaultCallable:
         """Same as :meth:`url_defaults` but application wide."""
         self.record_once(

src/flask/scaffold.py

@@ -27,8 +27,8 @@ from .typing import URLDefaultCallable
 from .typing import URLValuePreprocessorCallable
 
 if t.TYPE_CHECKING:  # pragma: no cover
-    from .wrappers import Response
     from .typing import ErrorHandlerCallable
+    from .wrappers import Response
 
 # a singleton sentinel value for parameter defaults
 _sentinel = object()
@@ -37,22 +37,10 @@ F = t.TypeVar("F", bound=t.Callable[..., t.Any])
 
 
 def setupmethod(f: F) -> F:
-    """Wraps a method so that it performs a check in debug mode if the
+    f_name = f.__name__
-    first request was already handled.
-    """
 
     def wrapper_func(self, *args: t.Any, **kwargs: t.Any) -> t.Any:
-        if self._is_setup_finished():
+        self._check_setup_finished(f_name)
-            raise AssertionError(
-                "A setup function was called after the first request "
-                "was handled. This usually indicates a bug in the"
-                " application where a module was not imported and"
-                " decorators or other functionality was called too"
-                " late.\nTo fix this make sure to import all your view"
-                " modules, database models, and everything related at a"
-                " central place before the application starts serving"
-                " requests."
-            )
         return f(self, *args, **kwargs)
 
     return t.cast(F, update_wrapper(wrapper_func, f))
@@ -239,7 +227,7 @@ class Scaffold:
     def __repr__(self) -> str:
         return f"<{type(self).__name__} {self.name!r}>"
 
-    def _is_setup_finished(self) -> bool:
+    def _check_setup_finished(self, f_name: str) -> None:
         raise NotImplementedError
 
     @property
@@ -376,6 +364,7 @@ class Scaffold:
 
         return self.route(rule, methods=[method], **options)
 
+    @setupmethod
     def get(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Shortcut for :meth:`route` with ``methods=["GET"]``.
 
@@ -383,6 +372,7 @@ class Scaffold:
         """
         return self._method_route("GET", rule, options)
 
+    @setupmethod
     def post(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Shortcut for :meth:`route` with ``methods=["POST"]``.
 
@@ -390,6 +380,7 @@ class Scaffold:
         """
         return self._method_route("POST", rule, options)
 
+    @setupmethod
     def put(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Shortcut for :meth:`route` with ``methods=["PUT"]``.
 
@@ -397,6 +388,7 @@ class Scaffold:
         """
         return self._method_route("PUT", rule, options)
 
+    @setupmethod
     def delete(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Shortcut for :meth:`route` with ``methods=["DELETE"]``.
 
@@ -404,6 +396,7 @@ class Scaffold:
         """
         return self._method_route("DELETE", rule, options)
 
+    @setupmethod
     def patch(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Shortcut for :meth:`route` with ``methods=["PATCH"]``.
 
@@ -411,6 +404,7 @@ class Scaffold:
         """
         return self._method_route("PATCH", rule, options)
 
+    @setupmethod
     def route(self, rule: str, **options: t.Any) -> t.Callable[[F], F]:
         """Decorate a view function to register it with the given URL
         rule and options. Calls :meth:`add_url_rule`, which has more
@@ -510,6 +504,7 @@ class Scaffold:
         """
         raise NotImplementedError
 
+    @setupmethod
     def endpoint(self, endpoint: str) -> t.Callable:
         """Decorate a view function to register it for the given
         endpoint. Used if a rule is added without a ``view_func`` with

tests/test_basic.py

@@ -329,6 +329,11 @@ def test_session_using_session_settings(app, client):
         flask.session["testing"] = 42
         return "Hello World"
 
+    @app.route("/clear")
+    def clear():
+        flask.session.pop("testing", None)
+        return "Goodbye World"
+
     rv = client.get("/", "http://www.example.com:8080/test/")
     cookie = rv.headers["set-cookie"].lower()
     assert "domain=.example.com" in cookie
@@ -337,11 +342,6 @@ def test_session_using_session_settings(app, client):
     assert "httponly" not in cookie
     assert "samesite" in cookie
 
-    @app.route("/clear")
-    def clear():
-        flask.session.pop("testing", None)
-        return "Goodbye World"
-
     rv = client.get("/clear", "http://www.example.com:8080/test/")
     cookie = rv.headers["set-cookie"].lower()
     assert "session=;" in cookie
@@ -1031,7 +1031,14 @@ def test_errorhandler_precedence(app, client):
     assert rv.data == b"E2"
 
 
-def test_trapping_of_bad_request_key_errors(app, client):
+@pytest.mark.parametrize(
+    ("debug", "trap", "expect_key", "expect_abort"),
+    [(False, None, True, True), (True, None, False, True), (False, True, False, False)],
+)
+def test_trap_bad_request_key_error(app, client, debug, trap, expect_key, expect_abort):
+    app.config["DEBUG"] = debug
+    app.config["TRAP_BAD_REQUEST_ERRORS"] = trap
+
     @app.route("/key")
     def fail():
         flask.request.form["missing_key"]
@@ -1040,26 +1047,23 @@ def test_trapping_of_bad_request_key_errors(app, client):
     def allow_abort():
         flask.abort(400)
 
-    rv = client.get("/key")
+    if expect_key:
-    assert rv.status_code == 400
+        rv = client.get("/key")
-    assert b"missing_key" not in rv.data
+        assert rv.status_code == 400
-    rv = client.get("/abort")
+        assert b"missing_key" not in rv.data
-    assert rv.status_code == 400
+    else:
+        with pytest.raises(KeyError) as exc_info:
+            client.get("/key")
 
-    app.debug = True
+        assert exc_info.errisinstance(BadRequest)
-    with pytest.raises(KeyError) as e:
+        assert "missing_key" in exc_info.value.get_description()
-        client.get("/key")
-    assert e.errisinstance(BadRequest)
-    assert "missing_key" in e.value.get_description()
-    rv = client.get("/abort")
-    assert rv.status_code == 400
 
-    app.debug = False
+    if expect_abort:
-    app.config["TRAP_BAD_REQUEST_ERRORS"] = True
+        rv = client.get("/abort")
-    with pytest.raises(KeyError):
+        assert rv.status_code == 400
-        client.get("/key")
+    else:
-    with pytest.raises(BadRequest):
+        with pytest.raises(BadRequest):
-        client.get("/abort")
+            client.get("/abort")
 
 
 def test_trapping_of_all_http_exceptions(app, client):
@@ -1661,7 +1665,7 @@ def test_nonascii_pathinfo(app, client):
     assert rv.data == b"Hello World!"
 
 
-def test_debug_mode_complains_after_first_request(app, client):
+def test_no_setup_after_first_request(app, client):
     app.debug = True
 
     @app.route("/")
@@ -1671,19 +1675,10 @@ def test_debug_mode_complains_after_first_request(app, client):
     assert not app.got_first_request
     assert client.get("/").data == b"Awesome"
 
-    with pytest.raises(AssertionError) as e:
+    with pytest.raises(AssertionError) as exc_info:
         app.add_url_rule("/foo", endpoint="late")
 
-    assert "A setup function was called" in str(e.value)
+    assert "setup method 'add_url_rule'" in str(exc_info.value)
-
-    app.debug = False
-
-    @app.route("/foo")
-    def working():
-        return "Meh"
-
-    assert client.get("/foo").data == b"Meh"
-    assert app.got_first_request
 
 
 def test_before_first_request_functions(app, client):
@@ -1720,28 +1715,23 @@ def test_before_first_request_functions_concurrent(app, client):
 
 
 def test_routing_redirect_debugging(monkeypatch, app, client):
-    @app.route("/foo/", methods=["GET", "POST"])
+    app.config["DEBUG"] = True
-    def foo():
-        return "success"
 
-    app.debug = False
+    @app.route("/user/", methods=["GET", "POST"])
-    rv = client.post("/foo", data={}, follow_redirects=True)
+    def user():
+        return flask.request.form["status"]
+
+    # default redirect code preserves form data
+    rv = client.post("/user", data={"status": "success"}, follow_redirects=True)
     assert rv.data == b"success"
 
-    app.debug = True
+    # 301 and 302 raise error
-
-    with client:
-        rv = client.post("/foo", data={}, follow_redirects=True)
-        assert rv.data == b"success"
-        rv = client.get("/foo", data={}, follow_redirects=True)
-        assert rv.data == b"success"
-
     monkeypatch.setattr(RequestRedirect, "code", 301)
 
-    with client, pytest.raises(AssertionError) as e:
+    with client, pytest.raises(AssertionError) as exc_info:
-        client.post("/foo", data={})
+        client.post("/user", data={"status": "error"}, follow_redirects=True)
 
-    assert "canonical URL 'http://localhost/foo/'" in str(e.value)
+    assert "canonical URL 'http://localhost/user/'" in str(exc_info.value)
 
 
 def test_route_decorator_custom_endpoint(app, client):