From 1ff98a2d21f6800b7b97195e0f9c90776ffb8f9b Mon Sep 17 00:00:00 2001 From: Elad Moshe Date: Sun, 24 Feb 2019 23:54:57 +0200 Subject: [PATCH 1/2] wait until app ctx is ready before matching url `RequestContext.match_request` is moved from `__init__` to `push`. This causes matching to happen later, when the app context is available. This enables URL converters that use things such as the database. --- src/flask/ctx.py | 6 +++--- tests/test_basic.py | 20 -------------------- tests/test_converters.py | 38 ++++++++++++++++++++++++++++++++++++++ tests/test_testing.py | 1 - 4 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 tests/test_converters.py diff --git a/src/flask/ctx.py b/src/flask/ctx.py index c23f7d19..8f040b34 100644 --- a/src/flask/ctx.py +++ b/src/flask/ctx.py @@ -314,9 +314,6 @@ class RequestContext(object): # functions. self._after_request_functions = [] - if self.url_adapter is not None: - self.match_request() - @property def g(self): return _app_ctx_stack.top.g @@ -384,6 +381,9 @@ class RequestContext(object): _request_ctx_stack.push(self) + if self.url_adapter is not None: + self.match_request() + # Open the session at the moment that the request context is available. # This allows a custom open_session method to use the request context. # Only open a new session if this is the first time the request was diff --git a/tests/test_basic.py b/tests/test_basic.py index 63f23a8d..37e8d5c4 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -1384,26 +1384,6 @@ def test_url_for_passes_special_values_to_build_error_handler(app): flask.url_for("/") -def test_custom_converters(app, client): - from werkzeug.routing import BaseConverter - - class ListConverter(BaseConverter): - def to_python(self, value): - return value.split(",") - - def to_url(self, value): - base_to_url = super(ListConverter, self).to_url - return ",".join(base_to_url(x) for x in value) - - app.url_map.converters["list"] = ListConverter - - @app.route("/") - def index(args): - return "|".join(args) - - assert client.get("/1,2,3").data == b"1|2|3" - - def test_static_files(app, client): rv = client.get("/static/index.html") assert rv.status_code == 200 diff --git a/tests/test_converters.py b/tests/test_converters.py new file mode 100644 index 00000000..53fd6cf1 --- /dev/null +++ b/tests/test_converters.py @@ -0,0 +1,38 @@ +from flask.globals import _app_ctx_stack + + +def test_custom_converters(app, client): + from werkzeug.routing import BaseConverter + + class ListConverter(BaseConverter): + def to_python(self, value): + return value.split(",") + + def to_url(self, value): + base_to_url = super(ListConverter, self).to_url + return ",".join(base_to_url(x) for x in value) + + app.url_map.converters["list"] = ListConverter + + @app.route("/") + def index(args): + return "|".join(args) + + assert client.get("/1,2,3").data == b"1|2|3" + + +def test_model_converters(app, client): + from werkzeug.routing import BaseConverter + + class ModelConverterTester(BaseConverter): + def to_python(self, value): + assert _app_ctx_stack.top is not None + return value + + app.url_map.converters["model"] = ModelConverterTester + + @app.route("/") + def index(user_name): + return user_name, 200 + + client.get("/admin").data diff --git a/tests/test_testing.py b/tests/test_testing.py index a22f1146..2ea5b416 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -153,7 +153,6 @@ def test_blueprint_with_subdomain(): ctx = app.test_request_context("/", subdomain="xxx") assert ctx.request.url == "http://xxx.example.com:1234/foo/" - assert ctx.request.blueprint == bp.name rv = client.get("/", subdomain="xxx") assert rv.data == b"http://xxx.example.com:1234/foo/" From c65863912ba3c93d014378e21402c97babca272b Mon Sep 17 00:00:00 2001 From: David Lord Date: Thu, 13 Jun 2019 12:40:01 -0700 Subject: [PATCH 2/2] move url matching after opening session --- CHANGES.rst | 4 ++++ src/flask/ctx.py | 10 +++++----- tests/test_converters.py | 26 ++++++++++++++------------ tests/test_testing.py | 3 +++ 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index a727f947..0f0b5b76 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -71,6 +71,10 @@ Unreleased - The ``flask run`` command no longer fails if Python is not built with SSL support. Using the ``--cert`` option will show an appropriate error message. :issue:`3211` +- URL matching now occurs after the request context is pushed, rather + than when it's created. This allows custom URL converters to access + the app and request contexts, such as to query a database for an id. + :issue:`3088` .. _#2935: https://github.com/pallets/flask/issues/2935 .. _#2957: https://github.com/pallets/flask/issues/2957 diff --git a/src/flask/ctx.py b/src/flask/ctx.py index 8f040b34..a2944628 100644 --- a/src/flask/ctx.py +++ b/src/flask/ctx.py @@ -347,8 +347,8 @@ class RequestContext(object): of the request. """ try: - url_rule, self.request.view_args = self.url_adapter.match(return_rule=True) - self.request.url_rule = url_rule + result = self.url_adapter.match(return_rule=True) + self.request.url_rule, self.request.view_args = result except HTTPException as e: self.request.routing_exception = e @@ -381,9 +381,6 @@ class RequestContext(object): _request_ctx_stack.push(self) - if self.url_adapter is not None: - self.match_request() - # Open the session at the moment that the request context is available. # This allows a custom open_session method to use the request context. # Only open a new session if this is the first time the request was @@ -395,6 +392,9 @@ class RequestContext(object): if self.session is None: self.session = session_interface.make_null_session(self.app) + if self.url_adapter is not None: + self.match_request() + def pop(self, exc=_sentinel): """Pops the request context and unbinds it by doing that. This will also trigger the execution of functions registered by the diff --git a/tests/test_converters.py b/tests/test_converters.py index 53fd6cf1..dd6c4d68 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -1,9 +1,10 @@ -from flask.globals import _app_ctx_stack +from werkzeug.routing import BaseConverter + +from flask import has_request_context +from flask import url_for def test_custom_converters(app, client): - from werkzeug.routing import BaseConverter - class ListConverter(BaseConverter): def to_python(self, value): return value.split(",") @@ -20,19 +21,20 @@ def test_custom_converters(app, client): assert client.get("/1,2,3").data == b"1|2|3" + with app.test_request_context(): + assert url_for("index", args=[4, 5, 6]) == "/4,5,6" -def test_model_converters(app, client): - from werkzeug.routing import BaseConverter - class ModelConverterTester(BaseConverter): +def test_context_available(app, client): + class ContextConverter(BaseConverter): def to_python(self, value): - assert _app_ctx_stack.top is not None + assert has_request_context() return value - app.url_map.converters["model"] = ModelConverterTester + app.url_map.converters["ctx"] = ContextConverter - @app.route("/") - def index(user_name): - return user_name, 200 + @app.route("/") + def index(name): + return name - client.get("/admin").data + assert client.get("/admin").data == b"admin" diff --git a/tests/test_testing.py b/tests/test_testing.py index 2ea5b416..1490840c 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -154,6 +154,9 @@ def test_blueprint_with_subdomain(): ctx = app.test_request_context("/", subdomain="xxx") assert ctx.request.url == "http://xxx.example.com:1234/foo/" + with ctx: + assert ctx.request.blueprint == bp.name + rv = client.get("/", subdomain="xxx") assert rv.data == b"http://xxx.example.com:1234/foo/"