diff --git a/falcon/hooks.py b/falcon/hooks.py index 38026d92d..82c050274 100644 --- a/falcon/hooks.py +++ b/falcon/hooks.py @@ -20,7 +20,7 @@ from falcon import COMBINED_METHODS from falcon.util.misc import get_argnames -from falcon.util.sync import set_coroutine_flag +from falcon.util.sync import _wrap_non_coroutine_unsafe _DECORABLE_METHOD_NAME = re.compile(r'^on_({})(_\w+)?$'.format( @@ -149,20 +149,16 @@ def _wrap_with_after(responder, action, action_args, action_kwargs): responder_argnames = get_argnames(responder) extra_argnames = responder_argnames[2:] # Skip req, resp - set_coroutine_flag(action) - if iscoroutinefunction(responder): + action = _wrap_non_coroutine_unsafe(action) + @wraps(responder) async def do_after(self, req, resp, *args, **kwargs): if args: _merge_responder_args(args, kwargs, extra_argnames) await responder(self, req, resp, **kwargs) - - if action._coroutine: - await action(req, resp, self, *action_args, **action_kwargs) - else: - action(req, resp, self, *action_args, **action_kwargs) + await action(req, resp, self, *action_args, **action_kwargs) else: @wraps(responder) def do_after(self, req, resp, *args, **kwargs): @@ -189,19 +185,15 @@ def _wrap_with_before(responder, action, action_args, action_kwargs): responder_argnames = get_argnames(responder) extra_argnames = responder_argnames[2:] # Skip req, resp - set_coroutine_flag(action) - if iscoroutinefunction(responder): + action = _wrap_non_coroutine_unsafe(action) + @wraps(responder) async def do_before(self, req, resp, *args, **kwargs): if args: _merge_responder_args(args, kwargs, extra_argnames) - if action._coroutine: - await action(req, resp, self, kwargs, *action_args, **action_kwargs) - else: - action(req, resp, self, kwargs, *action_args, **action_kwargs) - + await action(req, resp, self, kwargs, *action_args, **action_kwargs) await responder(self, req, resp, **kwargs) else: @wraps(responder) diff --git a/falcon/routing/compiled.py b/falcon/routing/compiled.py index e62d0638b..7b0af1434 100644 --- a/falcon/routing/compiled.py +++ b/falcon/routing/compiled.py @@ -248,7 +248,7 @@ def _check_coroutine_responders(self, method_map): # for WSGI and ASGI. It must be used with care to avoid # false-positive mismatch issues (for example, do not use # an async hook with a synchronous responder.) - should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ + should_wrap = 'FALCON_ASGI_WRAP_NON_COROUTINES' in os.environ for method, responder in method_map.items(): # NOTE(kgriffs): We don't simply wrap non-async functions diff --git a/falcon/util/sync.py b/falcon/util/sync.py index 5f215501f..c4a2e0d7e 100644 --- a/falcon/util/sync.py +++ b/falcon/util/sync.py @@ -1,11 +1,17 @@ +from functools import wraps import inspect +import os -__all__ = ['set_coroutine_flag'] +__all__ = [ + 'set_coroutine_flag', + 'sync_to_async_unsafe', + '_wrap_non_coroutine_unsafe', +] -def set_coroutine_flag(func_or_meth): - """Set a _coroutine attribute on the given func or method. +def set_coroutine_flag(func): + """Set a _coroutine attribute on the given function or method. This function will detect whether a function or method is a coroutine (i.e., one declared with async def), and set an @@ -15,9 +21,57 @@ def set_coroutine_flag(func_or_meth): The flag can be used to more quickly check whether the object is a coroutine vs. having to call :py:meth:`inspect.iscoroutinefunction` every time. + + Arguments: + func (callable): Function or method object to flag """ - if inspect.ismethod(func_or_meth): - func_or_meth.__func__._coroutine = inspect.iscoroutinefunction(func_or_meth) + if inspect.ismethod(func): + func.__func__._coroutine = inspect.iscoroutinefunction(func) else: - func_or_meth._coroutine = inspect.iscoroutinefunction(func_or_meth) + func._coroutine = inspect.iscoroutinefunction(func) + + +def sync_to_async_unsafe(func): + """Wrap a callable in a coroutine that executes the callable directly. + + This helper is only to be used for functions that do not perform any + blocking I/O or lengthy CPU-bound operations, since the entire async + loop will be blocked while the wrapped function is executed. + + For a safer, non-blocking alternative that runs the function in a + thread pool executor, use :func:~.sync_to_async instead. + + Arguments: + func (callable): Function, method, or other callable to wrap + """ + @wraps(func) + async def wrapper(*args, **kwargs): + return func(*args, **kwargs) + + return wrapper + + +def _wrap_non_coroutine_unsafe(func): + """Wrap a coroutine using ``sync_to_async_unsafe()`` for internal test cases. + + This method is intended for Falcon's own test sweet and should not be + used by apps themselves. It provides a convenient way to reuse sync + methods for ASGI test cases when it is safe to do so. + + Arguments: + func (callable): Function, method, or other callable to wrap + + Returns: + When not in test mode, this function simply returns the callable + unchanged. Otherwise, if the callable is not a coroutine function, + it will be wrapped using ``sync_to_async_unsafe()``. + """ + + if 'FALCON_ASGI_WRAP_NON_COROUTINES' not in os.environ: + return func + + if inspect.iscoroutinefunction(func): + return func + + return sync_to_async_unsafe(func) diff --git a/tests/_util.py b/tests/_util.py index 27859c5d6..5eb4d3697 100644 --- a/tests/_util.py +++ b/tests/_util.py @@ -1,3 +1,6 @@ +from contextlib import contextmanager +import os + import pytest import falcon @@ -71,3 +74,15 @@ async def wrapper(*args, **kwargs): def skipif_asgi_unsupported(): if not ASGI_SUPPORTED: pytest.skip('ASGI requires CPython or PyPy 3.6+') + + +@contextmanager +def disable_asgi_non_coroutine_wrapping(): + should_wrap = 'FALCON_ASGI_WRAP_NON_COROUTINES' in os.environ + if should_wrap: + del os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] + + yield + + if should_wrap: + os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] = 'Y' diff --git a/tests/asgi/test_hello_asgi.py b/tests/asgi/test_hello_asgi.py index b1b9ee68d..d512a6fc7 100644 --- a/tests/asgi/test_hello_asgi.py +++ b/tests/asgi/test_hello_asgi.py @@ -1,5 +1,4 @@ import io -import os import pytest @@ -7,6 +6,8 @@ from falcon import testing import falcon.asgi +from _util import disable_asgi_non_coroutine_wrapping # NOQA + @pytest.fixture def client(): @@ -252,14 +253,8 @@ def test_status_not_set(self, client): assert result.status_code == 200 def test_coroutine_required(self, client): - should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ - if should_wrap: - del os.environ['FALCON_ASGI_WRAP_RESPONDERS'] - - with pytest.raises(TypeError) as exinfo: - client.app.add_route('/', NonCoroutineResource()) - - assert 'responder must be a non-blocking async coroutine' in str(exinfo.value) + with disable_asgi_non_coroutine_wrapping(): + with pytest.raises(TypeError) as exinfo: + client.app.add_route('/', NonCoroutineResource()) - if should_wrap: - os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'Y' + assert 'responder must be a non-blocking async coroutine' in str(exinfo.value) diff --git a/tests/conftest.py b/tests/conftest.py index 99d3223f8..a3398257c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,12 +15,6 @@ def asgi(request): return is_asgi -@pytest.fixture(autouse=True, scope='session') -def set_env_variables(): - os.environ['FALCON_TESTING_SESSION'] = 'Y' - os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'Y' - - # NOTE(kgriffs): Some modules actually run a wsgiref server, so # to ensure we reset the detection for the other modules, we just # run this fixture before each one is tested. diff --git a/tests/test_before_hooks.py b/tests/test_before_hooks.py index 92c97f44d..0d62c98d3 100644 --- a/tests/test_before_hooks.py +++ b/tests/test_before_hooks.py @@ -7,7 +7,7 @@ import falcon import falcon.testing as testing -from _util import create_app, create_resp # NOQA +from _util import create_app, create_resp, disable_asgi_non_coroutine_wrapping # NOQA def validate(req, resp, resource, params): @@ -25,6 +25,10 @@ def validate_param(req, resp, resource, params, param_name, maxval=100): raise falcon.HTTPBadRequest('Out of Range', msg) +async def validate_param_async(*args, **kwargs): + validate_param(*args, **kwargs) + + class ResourceAwareValidateParam: def __call__(self, req, resp, resource, params): assert resource @@ -134,16 +138,6 @@ def on_get(self, req, resp, doc=None): self.doc = doc -class WrappedRespondersBodyParserAsyncResource: - - @falcon.before(validate_param, 'limit', 100) - @falcon.before(parse_body_async) - async def on_get(self, req, resp, doc=None): - self.req = req - self.resp = resp - self.doc = doc - - @falcon.before(bunnies) class WrappedClassResource: @@ -360,6 +354,15 @@ def test_parser_sync(body, doc): ] ) def test_parser_async(body, doc): + with disable_asgi_non_coroutine_wrapping(): + class WrappedRespondersBodyParserAsyncResource: + @falcon.before(validate_param_async, 'limit', 100) + @falcon.before(parse_body_async) + async def on_get(self, req, resp, doc=None): + self.req = req + self.resp = resp + self.doc = doc + app = create_app(asgi=True) resource = WrappedRespondersBodyParserAsyncResource() diff --git a/tests/test_validators.py b/tests/test_validators.py index 78fe121f2..036929533 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -183,10 +183,10 @@ def test_both_schemas_validation_failure(asgi): should_wrap = 'FALCON_ASGI_WRAP_RESPONDERS' in os.environ if should_wrap: - del os.environ['FALCON_ASGI_WRAP_RESPONDERS'] + del os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] client.app.add_route('/test', resource) if should_wrap: - os.environ['FALCON_ASGI_WRAP_RESPONDERS'] = 'y' + os.environ['FALCON_ASGI_WRAP_NON_COROUTINES'] = 'y' result = client.simulate_put('/test', json=_INVALID_MEDIA) assert result.status_code == 400 diff --git a/tox.ini b/tox.ini index 946a54df9..f5bb329ee 100644 --- a/tox.ini +++ b/tox.ini @@ -25,6 +25,8 @@ envlist = py38, [testenv] setenv = PIP_CONFIG_FILE={toxinidir}/pip.conf + FALCON_ASGI_WRAP_NON_COROUTINES=Y + FALCON_TESTING_SESSION=Y deps = -r{toxinidir}/requirements/tests commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon pytest tests []