diff --git a/docs/_newsfragments/2343.breakingchange.rst b/docs/_newsfragments/2343.breakingchange.rst new file mode 100644 index 000000000..ba2284e25 --- /dev/null +++ b/docs/_newsfragments/2343.breakingchange.rst @@ -0,0 +1,5 @@ +Removed ``is_async`` argument from :meth:`~falcon.media.validators.jsonschema.validate` +and the hooks :meth:`~falcon.before` and :meth:`~falcon.after` since it's +no longer needed. +Cython from 3.0 will correctly mark ``asnyc def`` as coroutine, making +this argument no longer useful. diff --git a/falcon/hooks.py b/falcon/hooks.py index 024766751..5866824b1 100644 --- a/falcon/hooks.py +++ b/falcon/hooks.py @@ -27,7 +27,6 @@ cast, Dict, List, - Protocol, Tuple, TYPE_CHECKING, TypeVar, @@ -38,6 +37,12 @@ from falcon.util.misc import get_argnames from falcon.util.sync import _wrap_non_coroutine_unsafe +try: + from typing import Concatenate, ParamSpec +except ImportError: # pragma: no cover + from typing_extensions import Concatenate + from typing_extensions import ParamSpec # type: ignore[assignment] + if TYPE_CHECKING: import falcon as wsgi from falcon import asgi @@ -46,62 +51,8 @@ from falcon.typing import Responder from falcon.typing import ResponderMethod - -# TODO: if is_async is removed these protocol would no longer be needed, since -# ParamSpec could be used together with Concatenate to use a simple Callable -# to type the before and after functions. This approach was prototyped in -# https://github.com/falconry/falcon/pull/2234 -class SyncBeforeFn(Protocol): - def __call__( - self, - req: wsgi.Request, - resp: wsgi.Response, - resource: Resource, - params: Dict[str, Any], - *args: Any, - **kwargs: Any, - ) -> None: ... - - -class AsyncBeforeFn(Protocol): - def __call__( - self, - req: asgi.Request, - resp: asgi.Response, - resource: Resource, - params: Dict[str, Any], - *args: Any, - **kwargs: Any, - ) -> Awaitable[None]: ... - - -BeforeFn = Union[SyncBeforeFn, AsyncBeforeFn] - - -class SyncAfterFn(Protocol): - def __call__( - self, - req: wsgi.Request, - resp: wsgi.Response, - resource: Resource, - *args: Any, - **kwargs: Any, - ) -> None: ... - - -class AsyncAfterFn(Protocol): - def __call__( - self, - req: asgi.Request, - resp: asgi.Response, - resource: Resource, - *args: Any, - **kwargs: Any, - ) -> Awaitable[None]: ... - - -AfterFn = Union[SyncAfterFn, AsyncAfterFn] _R = TypeVar('_R', bound=Union['Responder', 'Resource']) +_FN = ParamSpec('_FN') _DECORABLE_METHOD_NAME = re.compile( @@ -110,7 +61,18 @@ def __call__( def before( - action: BeforeFn, *args: Any, is_async: bool = False, **kwargs: Any + action: Union[ + Callable[ + Concatenate[wsgi.Request, wsgi.Response, Resource, Dict[str, Any], _FN], + None, + ], + Callable[ + Concatenate[asgi.Request, asgi.Response, Resource, Dict[str, Any], _FN], + Awaitable[None], + ], + ], + *args: _FN.args, + **kwargs: _FN.kwargs, ) -> Callable[[_R], _R]: """Execute the given action function *before* the responder. @@ -142,21 +104,6 @@ def do_something(req, resp, resource, params): and *params* arguments. Keyword Args: - is_async (bool): Set to ``True`` for ASGI apps to provide a hint that - the decorated responder is a coroutine function (i.e., that it - is defined with ``async def``) or that it returns an awaitable - coroutine object. - - Normally, when the function source is declared using ``async def``, - the resulting function object is flagged to indicate it returns a - coroutine when invoked, and this can be automatically detected. - However, it is possible to use a regular function to return an - awaitable coroutine object, in which case a hint is required to let - the framework know what to expect. Also, a hint is always required - when using a cythonized coroutine function, since Cython does not - flag them in a way that can be detected in advance, even when the - function is declared using ``async def``. - **kwargs: Any additional keyword arguments will be passed through to *action*. """ @@ -168,9 +115,7 @@ def _before(responder_or_resource: _R) -> _R: ): if _DECORABLE_METHOD_NAME.match(responder_name): responder = cast('Responder', responder) - do_before_all = _wrap_with_before( - responder, action, args, kwargs, is_async - ) + do_before_all = _wrap_with_before(responder, action, args, kwargs) setattr(responder_or_resource, responder_name, do_before_all) @@ -178,7 +123,7 @@ def _before(responder_or_resource: _R) -> _R: else: responder = cast('Responder', responder_or_resource) - do_before_one = _wrap_with_before(responder, action, args, kwargs, is_async) + do_before_one = _wrap_with_before(responder, action, args, kwargs) return cast(_R, do_before_one) @@ -186,7 +131,14 @@ def _before(responder_or_resource: _R) -> _R: def after( - action: AfterFn, *args: Any, is_async: bool = False, **kwargs: Any + action: Union[ + Callable[Concatenate[wsgi.Request, wsgi.Response, Resource, _FN], None], + Callable[ + Concatenate[asgi.Request, asgi.Response, Resource, _FN], Awaitable[None] + ], + ], + *args: _FN.args, + **kwargs: _FN.kwargs, ) -> Callable[[_R], _R]: """Execute the given action function *after* the responder. @@ -201,21 +153,6 @@ def after( arguments. Keyword Args: - is_async (bool): Set to ``True`` for ASGI apps to provide a hint that - the decorated responder is a coroutine function (i.e., that it - is defined with ``async def``) or that it returns an awaitable - coroutine object. - - Normally, when the function source is declared using ``async def``, - the resulting function object is flagged to indicate it returns a - coroutine when invoked, and this can be automatically detected. - However, it is possible to use a regular function to return an - awaitable coroutine object, in which case a hint is required to let - the framework know what to expect. Also, a hint is always required - when using a cythonized coroutine function, since Cython does not - flag them in a way that can be detected in advance, even when the - function is declared using ``async def``. - **kwargs: Any additional keyword arguments will be passed through to *action*. """ @@ -227,9 +164,7 @@ def _after(responder_or_resource: _R) -> _R: ): if _DECORABLE_METHOD_NAME.match(responder_name): responder = cast('Responder', responder) - do_after_all = _wrap_with_after( - responder, action, args, kwargs, is_async - ) + do_after_all = _wrap_with_after(responder, action, args, kwargs) setattr(responder_or_resource, responder_name, do_after_all) @@ -237,7 +172,7 @@ def _after(responder_or_resource: _R) -> _R: else: responder = cast('Responder', responder_or_resource) - do_after_one = _wrap_with_after(responder, action, args, kwargs, is_async) + do_after_one = _wrap_with_after(responder, action, args, kwargs) return cast(_R, do_after_one) @@ -251,10 +186,9 @@ def _after(responder_or_resource: _R) -> _R: def _wrap_with_after( responder: Responder, - action: AfterFn, + action: Callable[..., Union[None, Awaitable[None]]], action_args: Any, action_kwargs: Any, - is_async: bool, ) -> Responder: """Execute the given action function after a responder method. @@ -264,24 +198,16 @@ def _wrap_with_after( method, taking the form ``func(req, resp, resource)``. action_args: Additional positional arguments to pass to *action*. action_kwargs: Additional keyword arguments to pass to *action*. - is_async: Set to ``True`` for cythonized responders that are - actually coroutine functions, since such responders can not - be auto-detected. A hint is also required for regular functions - that happen to return an awaitable coroutine object. """ responder_argnames = get_argnames(responder) extra_argnames = responder_argnames[2:] # Skip req, resp do_after_responder: Responder - if is_async or iscoroutinefunction(responder): - # NOTE(kgriffs): I manually verified that the implicit "else" branch - # is actually covered, but coverage isn't tracking it for - # some reason. - if not is_async: # pragma: nocover - async_action = cast('AsyncAfterFn', _wrap_non_coroutine_unsafe(action)) - else: - async_action = cast('AsyncAfterFn', action) + if iscoroutinefunction(responder): + async_action = cast( + Callable[..., Awaitable[None]], _wrap_non_coroutine_unsafe(action) + ) async_responder = cast('AsgiResponderMethod', responder) @wraps(async_responder) @@ -300,7 +226,7 @@ async def do_after( do_after_responder = cast('AsgiResponderMethod', do_after) else: - sync_action = cast('SyncAfterFn', action) + sync_action = cast(Callable[..., None], action) sync_responder = cast('ResponderMethod', responder) @wraps(sync_responder) @@ -323,10 +249,9 @@ def do_after( def _wrap_with_before( responder: Responder, - action: BeforeFn, + action: Callable[..., Union[None, Awaitable[None]]], action_args: Tuple[Any, ...], action_kwargs: Dict[str, Any], - is_async: bool, ) -> Responder: """Execute the given action function before a responder method. @@ -336,24 +261,16 @@ def _wrap_with_before( method, taking the form ``func(req, resp, resource, params)``. action_args: Additional positional arguments to pass to *action*. action_kwargs: Additional keyword arguments to pass to *action*. - is_async: Set to ``True`` for cythonized responders that are - actually coroutine functions, since such responders can not - be auto-detected. A hint is also required for regular functions - that happen to return an awaitable coroutine object. """ responder_argnames = get_argnames(responder) extra_argnames = responder_argnames[2:] # Skip req, resp do_before_responder: Responder - if is_async or iscoroutinefunction(responder): - # NOTE(kgriffs): I manually verified that the implicit "else" branch - # is actually covered, but coverage isn't tracking it for - # some reason. - if not is_async: # pragma: nocover - async_action = cast('AsyncBeforeFn', _wrap_non_coroutine_unsafe(action)) - else: - async_action = cast('AsyncBeforeFn', action) + if iscoroutinefunction(responder): + async_action = cast( + Callable[..., Awaitable[None]], _wrap_non_coroutine_unsafe(action) + ) async_responder = cast('AsgiResponderMethod', responder) @wraps(async_responder) @@ -372,7 +289,7 @@ async def do_before( do_before_responder = cast('AsgiResponderMethod', do_before) else: - sync_action = cast('SyncBeforeFn', action) + sync_action = cast(Callable[..., None], action) sync_responder = cast('ResponderMethod', responder) @wraps(sync_responder) diff --git a/falcon/media/validators/jsonschema.py b/falcon/media/validators/jsonschema.py index 561fcfdf1..69be06345 100644 --- a/falcon/media/validators/jsonschema.py +++ b/falcon/media/validators/jsonschema.py @@ -20,7 +20,7 @@ def validate( - req_schema: Schema = None, resp_schema: Schema = None, is_async: bool = False + req_schema: Schema = None, resp_schema: Schema = None ) -> Callable[[ResponderMethod], ResponderMethod]: """Validate ``req.media`` using JSON Schema. @@ -51,20 +51,6 @@ def validate( resp_schema (dict): A dictionary that follows the JSON Schema specification. The response will be validated against this schema. - is_async (bool): Set to ``True`` for ASGI apps to provide a hint that - the decorated responder is a coroutine function (i.e., that it - is defined with ``async def``) or that it returns an awaitable - coroutine object. - - Normally, when the function source is declared using ``async def``, - the resulting function object is flagged to indicate it returns a - coroutine when invoked, and this can be automatically detected. - However, it is possible to use a regular function to return an - awaitable coroutine object, in which case a hint is required to let - the framework know what to expect. Also, a hint is always required - when using a cythonized coroutine function, since Cython does not - flag them in a way that can be detected in advance, even when the - function is declared using ``async def``. Example: @@ -96,23 +82,10 @@ async def on_post(self, req, resp): # -- snip -- - .. tab-item:: ASGI (Cythonized App) - - .. code:: python - - from falcon.media.validators import jsonschema - - # -- snip -- - - @jsonschema.validate(my_post_schema, is_async=True) - async def on_post(self, req, resp): - - # -- snip -- - """ def decorator(func: ResponderMethod) -> ResponderMethod: - if iscoroutinefunction(func) or is_async: + if iscoroutinefunction(func): return _validate_async(func, req_schema, resp_schema) return _validate(func, req_schema, resp_schema) diff --git a/pyproject.toml b/pyproject.toml index ebccd4505..0e38a07c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,9 @@ requires = [ name = "falcon" readme = {file = "README.rst", content-type = "text/x-rst"} dynamic = ["version"] -dependencies = [] +dependencies = [ + "typing-extensions >= 4.2.0; python_version<'3.10'", +] requires-python = ">=3.8" description = "The ultra-reliable, fast ASGI+WSGI framework for building data plane APIs at scale." authors = [ diff --git a/tests/asgi/_cythonized.pyx b/tests/asgi/_cythonized.pyx index 091e07358..c08603cf1 100644 --- a/tests/asgi/_cythonized.pyx +++ b/tests/asgi/_cythonized.pyx @@ -8,22 +8,22 @@ from falcon.media.validators.jsonschema import validate _MESSAGE_SCHEMA = { - 'definitions': {}, - '$schema': 'http://json-schema.org/draft-07/schema#', - '$id': 'http://example.com/root.json', - 'type': 'object', - 'title': 'The Root Schema', - 'required': ['message'], - 'properties': { - 'message': { - '$id': '#/properties/message', - 'type': 'string', - 'title': 'The Message Schema', - 'default': '', - 'examples': ['hello world'], - 'pattern': '^(.*)$' - } - } + 'definitions': {}, + '$schema': 'http://json-schema.org/draft-07/schema#', + '$id': 'http://example.com/root.json', + 'type': 'object', + 'title': 'The Root Schema', + 'required': ['message'], + 'properties': { + 'message': { + '$id': '#/properties/message', + 'type': 'string', + 'title': 'The Message Schema', + 'default': '', + 'examples': ['hello world'], + 'pattern': '^(.*)$' + } + } } @@ -44,19 +44,9 @@ class NOPClass: class TestResourceWithValidation: - @validate(resp_schema=_MESSAGE_SCHEMA, is_async=True) - async def on_get(self, req, resp): - resp.media = { - 'message': 'hello world' - } - - -class TestResourceWithValidationNoHint: @validate(resp_schema=_MESSAGE_SCHEMA) async def on_get(self, req, resp): - resp.media = { - 'message': 'hello world' - } + resp.media = {'message': 'hello world'} class TestResourceWithScheduledJobs: @@ -85,7 +75,7 @@ class TestResourceWithScheduledJobsAsyncRequired: pass # NOTE(kgriffs): This will fail later since we can't detect - # up front that it isn't a coroutine function. + # up front that it isn't a coroutine function. resp.schedule(background_job_sync) @@ -99,13 +89,6 @@ async def my_after_hook(req, resp, resource): class TestResourceWithHooks: - @falcon.before(my_before_hook, is_async=True) - @falcon.after(my_after_hook, is_async=True) - async def on_get(self, req, resp): - pass - - -class TestResourceWithHooksNoHint: @falcon.before(my_before_hook) @falcon.after(my_after_hook) async def on_get(self, req, resp): diff --git a/tests/asgi/test_cythonized_asgi.py b/tests/asgi/test_cythonized_asgi.py index 67d7499b7..06e1da4d7 100644 --- a/tests/asgi/test_cythonized_asgi.py +++ b/tests/asgi/test_cythonized_asgi.py @@ -31,6 +31,7 @@ else: _CYTHON_FUNC_TEST_TYPES = [] +pytestmark = pytest.mark.skipif(not pyximport, reason='Cython not installed') # NOTE(vytas): Cython 3.0+ now correctly marks cythonized coroutines as such, # however, the relevant protocol is only available in Python 3.10+. @@ -59,7 +60,6 @@ async def nop_method_async(self): pass -@pytest.mark.skipif(not pyximport, reason='Cython not installed') @pytest.mark.parametrize('func', _CYTHON_FUNC_TEST_TYPES) def test_is_cython_func(func): assert not is_python_func(func) @@ -80,23 +80,20 @@ def test_not_cython_func(func): assert is_python_func(func) -@pytest.mark.skipif(not pyximport, reason='Cython not installed') def test_jsonchema_validator(client, util): with util.disable_asgi_non_coroutine_wrapping(): if CYTHON_COROUTINE_HINT: - client.app.add_route('/', _cythonized.TestResourceWithValidationNoHint()) + client.app.add_route('/', _cythonized.TestResourceWithValidation()) else: with pytest.raises(TypeError): client.app.add_route( - '/wowsuchfail', _cythonized.TestResourceWithValidationNoHint() + '/wowsuchfail', _cythonized.TestResourceWithValidation() ) - - client.app.add_route('/', _cythonized.TestResourceWithValidation()) + return client.simulate_get() -@pytest.mark.skipif(not pyximport, reason='Cython not installed') def test_scheduled_jobs(client): resource = _cythonized.TestResourceWithScheduledJobs() client.app.add_route('/', resource) @@ -107,7 +104,6 @@ def test_scheduled_jobs(client): assert resource.counter['backround:on_get:sync'] == 40 -@pytest.mark.skipif(not pyximport, reason='Cython not installed') def test_scheduled_jobs_type_error(client): client.app.add_route( '/wowsuchfail', _cythonized.TestResourceWithScheduledJobsAsyncRequired() @@ -123,16 +119,15 @@ def test_scheduled_jobs_type_error(client): client.simulate_get('/wowsuchfail') -@pytest.mark.skipif(not pyximport, reason='Cython not installed') def test_hooks(client, util): with util.disable_asgi_non_coroutine_wrapping(): if CYTHON_COROUTINE_HINT: - client.app.add_route('/', _cythonized.TestResourceWithHooksNoHint()) + client.app.add_route('/', _cythonized.TestResourceWithHooks()) else: with pytest.raises(TypeError): - client.app.add_route('/', _cythonized.TestResourceWithHooksNoHint()) + client.app.add_route('/', _cythonized.TestResourceWithHooks()) - client.app.add_route('/', _cythonized.TestResourceWithHooks()) + return result = client.simulate_get() assert result.headers['x-answer'] == '42' diff --git a/tests/test_after_hooks.py b/tests/test_after_hooks.py index f6e53769f..341d5c7d5 100644 --- a/tests/test_after_hooks.py +++ b/tests/test_after_hooks.py @@ -128,12 +128,12 @@ def on_post(self, req, resp): class WrappedRespondersResourceAsync: @falcon.after(serialize_body_async) - @falcon.after(validate_output, is_async=False) + @falcon.after(validate_output) async def on_get(self, req, resp): self.req = req self.resp = resp - @falcon.after(serialize_body_async, is_async=True) + @falcon.after(serialize_body_async) async def on_put(self, req, resp): self.req = req self.resp = resp diff --git a/tests/test_before_hooks.py b/tests/test_before_hooks.py index 40e6ba7a6..174586a58 100644 --- a/tests/test_before_hooks.py +++ b/tests/test_before_hooks.py @@ -338,12 +338,12 @@ def test_parser_async(body, doc, util): with util.disable_asgi_non_coroutine_wrapping(): class WrappedRespondersBodyParserAsyncResource: - @falcon.before(validate_param_async, 'limit', 100, is_async=True) + @falcon.before(validate_param_async, 'limit', 100) @falcon.before(parse_body_async) async def on_get(self, req, resp, doc=None): self.doc = doc - @falcon.before(parse_body_async, is_async=False) + @falcon.before(parse_body_async) async def on_put(self, req, resp, doc=None): self.doc = doc