From 4ba78a5325e71691d0ec0ebb71b5d8614ffe4e1c Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Sun, 7 Feb 2021 19:56:39 +0900 Subject: [PATCH] Fix #232 Unmatched message listener middleware can be called --- slack_bolt/app/app.py | 2 +- slack_bolt/app/async_app.py | 2 +- tests/scenario_tests/test_message.py | 58 ++++++++++++++++++-- tests/scenario_tests_async/test_message.py | 62 ++++++++++++++++++++-- 4 files changed, 116 insertions(+), 8 deletions(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index fa5ec277e..fcbbe1c51 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -517,7 +517,7 @@ def __call__(*args, **kwargs): # By contrast, messages posted using class app's bot token still have the subtype. constraints = {"type": "message", "subtype": (None, "bot_message")} primary_matcher = builtin_matchers.event(constraints=constraints) - middleware.append(MessageListenerMatches(keyword)) + middleware.insert(0, MessageListenerMatches(keyword)) return self._register_listener( list(functions), primary_matcher, matchers, middleware, True ) diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index f28a4b7ff..c82412cf5 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -567,7 +567,7 @@ def __call__(*args, **kwargs): primary_matcher = builtin_matchers.event( constraints=constraints, asyncio=True ) - middleware.append(AsyncMessageListenerMatches(keyword)) + middleware.insert(0, AsyncMessageListenerMatches(keyword)) return self._register_listener( list(functions), primary_matcher, matchers, middleware, True ) diff --git a/tests/scenario_tests/test_message.py b/tests/scenario_tests/test_message.py index e750bfd40..fe16f8245 100644 --- a/tests/scenario_tests/test_message.py +++ b/tests/scenario_tests/test_message.py @@ -5,6 +5,7 @@ from slack_sdk.signature import SignatureVerifier from slack_sdk.web import WebClient +from slack_bolt import BoltResponse from slack_bolt.app import App from slack_bolt.request import BoltRequest from tests.mock_web_api_server import ( @@ -46,13 +47,15 @@ def build_headers(self, timestamp: str, body: str): "x-slack-request-timestamp": [timestamp], } - def build_request(self) -> BoltRequest: + def build_request_from_body(self, message_body: dict) -> BoltRequest: timestamp, body = str(int(time.time())), json.dumps(message_body) return BoltRequest(body=body, headers=self.build_headers(timestamp, body)) + def build_request(self) -> BoltRequest: + return self.build_request_from_body(message_body) + def build_request2(self) -> BoltRequest: - timestamp, body = str(int(time.time())), json.dumps(message_body2) - return BoltRequest(body=body, headers=self.build_headers(timestamp, body)) + return self.build_request_from_body(message_body2) def test_string_keyword(self): app = App( @@ -136,6 +139,55 @@ def test_regexp_keyword_unmatched(self): assert response.status == 404 assert_auth_test_count(self, 1) + # https://github.com/slackapi/bolt-python/issues/232 + def test_issue_232_message_listener_middleware(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + called = { + "first": False, + "second": False, + } + + def this_should_be_skipped(): + return BoltResponse(status=500, body="failed") + + @app.message("first", middleware=[this_should_be_skipped]) + def first(): + called["first"] = True + + @app.message("second", middleware=[]) + def second(): + called["second"] = True + + request = self.build_request_from_body( + { + "token": "verification_token", + "team_id": "T111", + "enterprise_id": "E111", + "api_app_id": "A111", + "event": { + "client_msg_id": "a8744611-0210-4f85-9f15-5faf7fb225c8", + "type": "message", + "text": "This message should match the second listener only", + "user": "W111", + "ts": "1596183880.004200", + "team": "T111", + "channel": "C111", + "event_ts": "1596183880.004200", + "channel_type": "channel", + }, + "type": "event_callback", + "event_id": "Ev111", + "event_time": 1596183880, + } + ) + response = app.dispatch(request) + assert response.status == 200 + assert called["first"] == False + assert called["second"] == True + message_body = { "token": "verification_token", diff --git a/tests/scenario_tests_async/test_message.py b/tests/scenario_tests_async/test_message.py index b69e849f6..b1504d550 100644 --- a/tests/scenario_tests_async/test_message.py +++ b/tests/scenario_tests_async/test_message.py @@ -7,6 +7,7 @@ from slack_sdk.signature import SignatureVerifier from slack_sdk.web.async_client import AsyncWebClient +from slack_bolt import BoltResponse from slack_bolt.app.async_app import AsyncApp from slack_bolt.request.async_request import AsyncBoltRequest from tests.mock_web_api_server import ( @@ -52,13 +53,15 @@ def build_headers(self, timestamp: str, body: str): "x-slack-request-timestamp": [timestamp], } - def build_request(self) -> AsyncBoltRequest: + def build_request_from_body(self, message_body: dict) -> AsyncBoltRequest: timestamp, body = str(int(time())), json.dumps(message_body) return AsyncBoltRequest(body=body, headers=self.build_headers(timestamp, body)) + def build_request(self) -> AsyncBoltRequest: + return self.build_request_from_body(message_body) + def build_request2(self) -> AsyncBoltRequest: - timestamp, body = str(int(time())), json.dumps(message_body2) - return AsyncBoltRequest(body=body, headers=self.build_headers(timestamp, body)) + return self.build_request_from_body(message_body2) @pytest.mark.asyncio async def test_string_keyword(self): @@ -148,6 +151,59 @@ async def test_regexp_keyword_unmatched(self): assert response.status == 404 await assert_auth_test_count_async(self, 1) + # https://github.com/slackapi/bolt-python/issues/232 + @pytest.mark.asyncio + async def test_issue_232_message_listener_middleware(self): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + + called = { + "first": False, + "second": False, + } + + async def this_should_be_skipped(): + return BoltResponse(status=500, body="failed") + + @app.message("first", middleware=[this_should_be_skipped]) + async def first(): + called["first"] = True + + @app.message("second", middleware=[]) + async def second(): + called["second"] = True + + request = self.build_request_from_body( + { + "token": "verification_token", + "team_id": "T111", + "enterprise_id": "E111", + "api_app_id": "A111", + "event": { + "client_msg_id": "a8744611-0210-4f85-9f15-5faf7fb225c8", + "type": "message", + "text": "This message should match the second listener only", + "user": "W111", + "ts": "1596183880.004200", + "team": "T111", + "channel": "C111", + "event_ts": "1596183880.004200", + "channel_type": "channel", + }, + "type": "event_callback", + "event_id": "Ev111", + "event_time": 1596183880, + } + ) + response = await app.async_dispatch(request) + assert response.status == 200 + + await asyncio.sleep(0.3) + assert called["first"] == False + assert called["second"] == True + message_body = { "token": "verification_token",