From 4a4105df1f96c5b4ae702864292e3ca05f517c9a Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Wed, 7 Jul 2021 17:14:28 +0900 Subject: [PATCH] Fix #370 by adding an alias of `next` arg (`next_`) in middleware arguments --- slack_bolt/kwargs_injection/args.py | 6 ++ slack_bolt/kwargs_injection/async_args.py | 3 + slack_bolt/kwargs_injection/async_utils.py | 1 + slack_bolt/kwargs_injection/utils.py | 1 + slack_bolt/listener/listener.py | 4 +- slack_bolt/logger/messages.py | 2 +- .../middleware/async_custom_middleware.py | 3 + slack_bolt/middleware/async_middleware.py | 11 +++ .../async_multi_teams_authorization.py | 3 + .../async_single_team_authorization.py | 3 + .../single_team_authorization.py | 3 + slack_bolt/middleware/custom_middleware.py | 3 + .../async_message_listener_matches.py | 3 + .../message_listener_matches.py | 3 + slack_bolt/middleware/middleware.py | 11 +++ .../async_request_verification.py | 3 + .../request_verification.py | 3 + .../middleware/ssl_check/async_ssl_check.py | 3 + slack_bolt/middleware/ssl_check/ssl_check.py | 3 + .../url_verification/url_verification.py | 3 + slack_bolt/workflows/step/step_middleware.py | 3 + tests/scenario_tests/test_middleware.py | 68 +++++++++++++++++++ tests/scenario_tests_async/test_middleware.py | 55 +++++++++++++++ 23 files changed, 198 insertions(+), 3 deletions(-) diff --git a/slack_bolt/kwargs_injection/args.py b/slack_bolt/kwargs_injection/args.py index ff43cffe9..71e427a37 100644 --- a/slack_bolt/kwargs_injection/args.py +++ b/slack_bolt/kwargs_injection/args.py @@ -72,6 +72,8 @@ def handle_buttons(ack, respond, logger, context, body, client): # middleware next: Callable[[], None] """`next()` utility function, which tells the middleware chain that it can continue with the next one""" + next_: Callable[[], None] + """An alias of `next()` for avoiding the Python built-in method overrides in middleware functions""" def __init__( self, @@ -93,6 +95,9 @@ def __init__( ack: Ack, say: Say, respond: Respond, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], None], **kwargs # noqa ): @@ -116,3 +121,4 @@ def __init__( self.say: Say = say self.respond: Respond = respond self.next: Callable[[], None] = next + self.next_: Callable[[], None] = next diff --git a/slack_bolt/kwargs_injection/async_args.py b/slack_bolt/kwargs_injection/async_args.py index 7c9273f43..8982605d0 100644 --- a/slack_bolt/kwargs_injection/async_args.py +++ b/slack_bolt/kwargs_injection/async_args.py @@ -71,6 +71,8 @@ async def handle_buttons(ack, respond, logger, context, body, client): # middleware next: Callable[[], Awaitable[None]] """`next()` utility function, which tells the middleware chain that it can continue with the next one""" + next_: Callable[[], Awaitable[None]] + """An alias of `next()` for avoiding the Python built-in method overrides in middleware functions""" def __init__( self, @@ -115,3 +117,4 @@ def __init__( self.say: AsyncSay = say self.respond: AsyncRespond = respond self.next: Callable[[], Awaitable[None]] = next + self.next_: Callable[[], Awaitable[None]] = next diff --git a/slack_bolt/kwargs_injection/async_utils.py b/slack_bolt/kwargs_injection/async_utils.py index dae473eea..a6ee86f96 100644 --- a/slack_bolt/kwargs_injection/async_utils.py +++ b/slack_bolt/kwargs_injection/async_utils.py @@ -52,6 +52,7 @@ def build_async_required_kwargs( "respond": request.context.respond, # middleware "next": next_func, + "next_": next_func, # for the middleware using Python's built-in `next()` function } all_available_args["payload"] = ( all_available_args["options"] diff --git a/slack_bolt/kwargs_injection/utils.py b/slack_bolt/kwargs_injection/utils.py index 2685ba342..94cbc3c64 100644 --- a/slack_bolt/kwargs_injection/utils.py +++ b/slack_bolt/kwargs_injection/utils.py @@ -52,6 +52,7 @@ def build_required_kwargs( "respond": request.context.respond, # middleware "next": next_func, + "next_": next_func, # for the middleware using Python's built-in `next()` function } all_available_args["payload"] = ( all_available_args["options"] diff --git a/slack_bolt/listener/listener.py b/slack_bolt/listener/listener.py index 37fa8d93a..cc1f584e4 100644 --- a/slack_bolt/listener/listener.py +++ b/slack_bolt/listener/listener.py @@ -45,10 +45,10 @@ def run_middleware( for m in self.middleware: middleware_state = {"next_called": False} - def next(): + def next_(): middleware_state["next_called"] = True - resp = m.process(req=req, resp=resp, next=next) + resp = m.process(req=req, resp=resp, next=next_) if not middleware_state["next_called"]: # next() was not called in this middleware return (resp, True) diff --git a/slack_bolt/logger/messages.py b/slack_bolt/logger/messages.py index 12302631e..6321f2feb 100644 --- a/slack_bolt/logger/messages.py +++ b/slack_bolt/logger/messages.py @@ -99,7 +99,7 @@ def warning_unhandled_by_global_middleware( # type: ignore name: str, req: Union[BoltRequest, "AsyncBoltRequest"] # type: ignore ) -> str: # type: ignore return ( - f"A global middleware ({name}) skipped calling `next()` " + f"A global middleware ({name}) skipped calling either `next()` or `next_()` " f"without providing a response for the request ({req.body})" ) diff --git a/slack_bolt/middleware/async_custom_middleware.py b/slack_bolt/middleware/async_custom_middleware.py index ca50a23f9..d967f188e 100644 --- a/slack_bolt/middleware/async_custom_middleware.py +++ b/slack_bolt/middleware/async_custom_middleware.py @@ -31,6 +31,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: return await self.func( diff --git a/slack_bolt/middleware/async_middleware.py b/slack_bolt/middleware/async_middleware.py index 7392e92fc..163def40a 100644 --- a/slack_bolt/middleware/async_middleware.py +++ b/slack_bolt/middleware/async_middleware.py @@ -14,6 +14,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> Optional[BoltResponse]: """Processes a request data before other middleware and listeners. @@ -24,6 +27,14 @@ async def simple_middleware(req, resp, next): # do something here await next() + This `async_process(req, resp, next)` method is supposed to be invoked only inside bolt-python. + If you want to avoid the name `next()` in your middleware functions, you can use `next_()` method instead. + + @app.middleware + async def simple_middleware(req, resp, next_): + # do something here + await next_() + Args: req: The incoming request resp: The response diff --git a/slack_bolt/middleware/authorization/async_multi_teams_authorization.py b/slack_bolt/middleware/authorization/async_multi_teams_authorization.py index 57814996f..3d6f2e16b 100644 --- a/slack_bolt/middleware/authorization/async_multi_teams_authorization.py +++ b/slack_bolt/middleware/authorization/async_multi_teams_authorization.py @@ -28,6 +28,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: if _is_no_auth_required(req): diff --git a/slack_bolt/middleware/authorization/async_single_team_authorization.py b/slack_bolt/middleware/authorization/async_single_team_authorization.py index 8db49432e..0c9ff9cd9 100644 --- a/slack_bolt/middleware/authorization/async_single_team_authorization.py +++ b/slack_bolt/middleware/authorization/async_single_team_authorization.py @@ -22,6 +22,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: if _is_no_auth_required(req): diff --git a/slack_bolt/middleware/authorization/single_team_authorization.py b/slack_bolt/middleware/authorization/single_team_authorization.py index 3307f926a..084f21141 100644 --- a/slack_bolt/middleware/authorization/single_team_authorization.py +++ b/slack_bolt/middleware/authorization/single_team_authorization.py @@ -30,6 +30,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: if _is_no_auth_required(req): diff --git a/slack_bolt/middleware/custom_middleware.py b/slack_bolt/middleware/custom_middleware.py index bff3a4f93..6b52b5897 100644 --- a/slack_bolt/middleware/custom_middleware.py +++ b/slack_bolt/middleware/custom_middleware.py @@ -27,6 +27,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: return self.func( diff --git a/slack_bolt/middleware/message_listener_matches/async_message_listener_matches.py b/slack_bolt/middleware/message_listener_matches/async_message_listener_matches.py index 6a76c6a5b..2b3093d48 100644 --- a/slack_bolt/middleware/message_listener_matches/async_message_listener_matches.py +++ b/slack_bolt/middleware/message_listener_matches/async_message_listener_matches.py @@ -16,6 +16,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: text = req.body.get("event", {}).get("text", "") diff --git a/slack_bolt/middleware/message_listener_matches/message_listener_matches.py b/slack_bolt/middleware/message_listener_matches/message_listener_matches.py index 14b4091eb..51e7cafb8 100644 --- a/slack_bolt/middleware/message_listener_matches/message_listener_matches.py +++ b/slack_bolt/middleware/message_listener_matches/message_listener_matches.py @@ -16,6 +16,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: text = req.body.get("event", {}).get("text", "") diff --git a/slack_bolt/middleware/middleware.py b/slack_bolt/middleware/middleware.py index 2e86bba5e..560499d6c 100644 --- a/slack_bolt/middleware/middleware.py +++ b/slack_bolt/middleware/middleware.py @@ -14,6 +14,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> Optional[BoltResponse]: """Processes a request data before other middleware and listeners. @@ -24,6 +27,14 @@ def simple_middleware(req, resp, next): # do something here next() + This `process(req, resp, next)` method is supposed to be invoked only inside bolt-python. + If you want to avoid the name `next()` in your middleware functions, you can use `next_()` method instead. + + @app.middleware + def simple_middleware(req, resp, next_): + # do something here + next_() + Args: req: The incoming request resp: The response diff --git a/slack_bolt/middleware/request_verification/async_request_verification.py b/slack_bolt/middleware/request_verification/async_request_verification.py index afb584d12..68484bde0 100644 --- a/slack_bolt/middleware/request_verification/async_request_verification.py +++ b/slack_bolt/middleware/request_verification/async_request_verification.py @@ -18,6 +18,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: if self._can_skip(req.mode, req.body): diff --git a/slack_bolt/middleware/request_verification/request_verification.py b/slack_bolt/middleware/request_verification/request_verification.py index aa8d0516d..ebbf803e3 100644 --- a/slack_bolt/middleware/request_verification/request_verification.py +++ b/slack_bolt/middleware/request_verification/request_verification.py @@ -26,6 +26,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: if self._can_skip(req.mode, req.body): diff --git a/slack_bolt/middleware/ssl_check/async_ssl_check.py b/slack_bolt/middleware/ssl_check/async_ssl_check.py index a8a62c5e3..5b806a4bc 100644 --- a/slack_bolt/middleware/ssl_check/async_ssl_check.py +++ b/slack_bolt/middleware/ssl_check/async_ssl_check.py @@ -12,6 +12,9 @@ async def async_process( *, req: AsyncBoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], Awaitable[BoltResponse]], ) -> BoltResponse: if self._is_ssl_check_request(req.body): diff --git a/slack_bolt/middleware/ssl_check/ssl_check.py b/slack_bolt/middleware/ssl_check/ssl_check.py index 3f7754c4f..33be2bf63 100644 --- a/slack_bolt/middleware/ssl_check/ssl_check.py +++ b/slack_bolt/middleware/ssl_check/ssl_check.py @@ -23,6 +23,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: if self._is_ssl_check_request(req.body): diff --git a/slack_bolt/middleware/url_verification/url_verification.py b/slack_bolt/middleware/url_verification/url_verification.py index 1d1f7b2b4..591838e31 100644 --- a/slack_bolt/middleware/url_verification/url_verification.py +++ b/slack_bolt/middleware/url_verification/url_verification.py @@ -19,6 +19,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> BoltResponse: if self._is_url_verification_request(req.body): diff --git a/slack_bolt/workflows/step/step_middleware.py b/slack_bolt/workflows/step/step_middleware.py index f43fcbe7b..9abd804be 100644 --- a/slack_bolt/workflows/step/step_middleware.py +++ b/slack_bolt/workflows/step/step_middleware.py @@ -21,6 +21,9 @@ def process( *, req: BoltRequest, resp: BoltResponse, + # As this method is not supposed to be invoked by bolt-python users, + # the naming conflict with the built-in one affects + # only the internals of this method next: Callable[[], BoltResponse], ) -> Optional[BoltResponse]: diff --git a/tests/scenario_tests/test_middleware.py b/tests/scenario_tests/test_middleware.py index 3d623fc02..aa16bf620 100644 --- a/tests/scenario_tests/test_middleware.py +++ b/tests/scenario_tests/test_middleware.py @@ -87,6 +87,53 @@ def test_next_call(self): assert response.body == "acknowledged!" assert_auth_test_count(self, 1) + def test_decorator_next_call(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + + @app.middleware + def just_next(next): + next() + + app.shortcut("test-shortcut")(just_ack) + + response = app.dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + assert_auth_test_count(self, 1) + + def test_next_call_(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.use(just_next_) + app.shortcut("test-shortcut")(just_ack) + + response = app.dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + assert_auth_test_count(self, 1) + + def test_decorator_next_call_(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + + @app.middleware + def just_next_(next_): + next_() + + app.shortcut("test-shortcut")(just_ack) + + response = app.dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + assert_auth_test_count(self, 1) + def test_class_call(self): class NextClass: def __call__(self, next): @@ -104,6 +151,23 @@ def __call__(self, next): assert response.body == "acknowledged!" assert_auth_test_count(self, 1) + def test_class_call_(self): + class NextUnderscoreClass: + def __call__(self, next_): + next_() + + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.use(NextUnderscoreClass()) + app.shortcut("test-shortcut")(just_ack) + + response = app.dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + assert_auth_test_count(self, 1) + def just_ack(ack): ack("acknowledged!") @@ -115,3 +179,7 @@ def no_next(): def just_next(next): next() + + +def just_next_(next_): + next_() diff --git a/tests/scenario_tests_async/test_middleware.py b/tests/scenario_tests_async/test_middleware.py index 176d89fb2..10311087e 100644 --- a/tests/scenario_tests_async/test_middleware.py +++ b/tests/scenario_tests_async/test_middleware.py @@ -16,6 +16,7 @@ from tests.utils import remove_os_env_temporarily, restore_os_env +# Note that async middleware system does not support instance methods n a class. class TestAsyncMiddleware: signing_secret = "secret" valid_token = "xoxb-valid" @@ -95,6 +96,56 @@ async def test_next_call(self): assert response.body == "acknowledged!" await assert_auth_test_count_async(self, 1) + @pytest.mark.asyncio + async def test_decorator_next_call(self): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + + @app.middleware + async def just_next(next): + await next() + + app.shortcut("test-shortcut")(just_ack) + + response = await app.async_dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + await assert_auth_test_count_async(self, 1) + + @pytest.mark.asyncio + async def test_next_underscore_call(self): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.use(just_next_) + app.shortcut("test-shortcut")(just_ack) + + response = await app.async_dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + await assert_auth_test_count_async(self, 1) + + @pytest.mark.asyncio + async def test_decorator_next_underscore_call(self): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + + @app.middleware + async def just_next_(next_): + await next_() + + app.shortcut("test-shortcut")(just_ack) + + response = await app.async_dispatch(self.build_request()) + assert response.status == 200 + assert response.body == "acknowledged!" + await assert_auth_test_count_async(self, 1) + async def just_ack(ack): await ack("acknowledged!") @@ -106,3 +157,7 @@ async def no_next(): async def just_next(next): await next() + + +async def just_next_(next_): + await next_()