Fix #561 matchers can be called even when app.message keyword does not match (#577)

* Fix #561 matchers can be called even when app.message keyword does not match

* Fix a bug

Author: seratch

slack_bolt/app/app.py

@@ -810,7 +810,9 @@ def __call__(*args, **kwargs):
                     "thread_broadcast",
                 ),
             }
-            primary_matcher = builtin_matchers.event(constraints=constraints)
+            primary_matcher = builtin_matchers.message_event(
+                keyword=keyword, constraints=constraints
+            )
             middleware.insert(0, MessageListenerMatches(keyword))
             return self._register_listener(
                 list(functions), primary_matcher, matchers, middleware, True

slack_bolt/app/async_app.py

@@ -860,8 +860,8 @@ def __call__(*args, **kwargs):
                     "thread_broadcast",
                 ),
             }
-            primary_matcher = builtin_matchers.event(
-                constraints=constraints, asyncio=True
+            primary_matcher = builtin_matchers.message_event(
+                constraints=constraints, keyword=keyword, asyncio=True
             )
             middleware.insert(0, AsyncMessageListenerMatches(keyword))
             return self._register_listener(

slack_bolt/listener_matcher/builtins.py

@@ -1,5 +1,6 @@
 # pytype: skip-file
 import inspect
+import re
 import sys
 
 from slack_bolt.error import BoltError
@@ -95,37 +96,10 @@ def func(body: Dict[str, Any]) -> bool:
 
         def func(body: Dict[str, Any]) -> bool:
             if is_event(body):
-                event = body["event"]
-                if not _matches(constraints["type"], event["type"]):
-                    return False
-                if "subtype" in constraints:
-                    expected_subtype: Union[
-                        str, Sequence[Optional[Union[str, Pattern]]]
-                    ] = constraints["subtype"]
-                    if expected_subtype is None:
-                        # "subtype" in constraints is intentionally None for this pattern
-                        return "subtype" not in event
-                    elif isinstance(expected_subtype, (str, Pattern)):
-                        return "subtype" in event and _matches(
-                            expected_subtype, event["subtype"]
-                        )
-                    elif isinstance(expected_subtype, Sequence):
-                        subtypes: Sequence[
-                            Optional[Union[str, Pattern]]
-                        ] = expected_subtype
-                        for expected in subtypes:
-                            actual: Optional[str] = event.get("subtype")
-                            if expected is None:
-                                if actual is None:
-                                    return True
-                            elif actual is not None and _matches(expected, actual):
-                                return True
-                        return False
-                    else:
-                        return "subtype" in event and _matches(
-                            expected_subtype, event["subtype"]
-                        )
-                return True
+                return _check_event_subtype(
+                    event_payload=body["event"],
+                    constraints=constraints,
+                )
             return False
 
         return build_listener_matcher(func, asyncio)
@@ -135,6 +109,64 @@ def func(body: Dict[str, Any]) -> bool:
     )
 
 
+def message_event(
+    constraints: Dict[str, Union[str, Sequence[Optional[Union[str, Pattern]]]]],
+    keyword: Union[str, Pattern],
+    asyncio: bool = False,
+) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
+    if "type" in constraints and keyword is not None:
+        _verify_message_event_type(constraints["type"])
+
+        def func(body: Dict[str, Any]) -> bool:
+            if is_event(body):
+                is_valid_subtype = _check_event_subtype(
+                    event_payload=body["event"],
+                    constraints=constraints,
+                )
+                if is_valid_subtype is True:
+                    # Check keyword matching
+                    text = body.get("event", {}).get("text", "")
+                    match_result = re.findall(keyword, text)
+                    if match_result is not None and match_result != []:
+                        return True
+            return False
+
+        return build_listener_matcher(func, asyncio)
+
+    raise BoltError(f"event ({constraints}: {type(constraints)}) must be dict")
+
+
+def _check_event_subtype(event_payload: dict, constraints: dict) -> bool:
+    if not _matches(constraints["type"], event_payload["type"]):
+        return False
+    if "subtype" in constraints:
+        expected_subtype: Union[
+            str, Sequence[Optional[Union[str, Pattern]]]
+        ] = constraints["subtype"]
+        if expected_subtype is None:
+            # "subtype" in constraints is intentionally None for this pattern
+            return "subtype" not in event_payload
+        elif isinstance(expected_subtype, (str, Pattern)):
+            return "subtype" in event_payload and _matches(
+                expected_subtype, event_payload["subtype"]
+            )
+        elif isinstance(expected_subtype, Sequence):
+            subtypes: Sequence[Optional[Union[str, Pattern]]] = expected_subtype
+            for expected in subtypes:
+                actual: Optional[str] = event_payload.get("subtype")
+                if expected is None:
+                    if actual is None:
+                        return True
+                elif actual is not None and _matches(expected, actual):
+                    return True
+            return False
+        else:
+            return "subtype" in event_payload and _matches(
+                expected_subtype, event_payload["subtype"]
+            )
+    return True
+
+
 def _verify_message_event_type(event_type: str) -> None:
     if isinstance(event_type, str) and event_type.startswith("message."):
         raise ValueError(error_message_event_type(event_type))

tests/scenario_tests/test_message.py

@@ -205,6 +205,24 @@ def second():
         assert called["first"] == False
         assert called["second"] == True
 
+    def test_issue_561_matchers(self):
+        app = App(
+            client=self.web_client,
+            signing_secret=self.signing_secret,
+        )
+
+        def just_fail():
+            raise "This matcher should not be called!"
+
+        @app.message("xxx", matchers=[just_fail])
+        def just_ack():
+            raise "This listener should not be called!"
+
+        request = self.build_request()
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert_auth_test_count(self, 1)
+
 
 message_body = {
     "token": "verification_token",

tests/scenario_tests_async/test_message.py

@@ -219,8 +219,27 @@ async def second():
         assert response.status == 200
 
         await asyncio.sleep(0.3)
-        assert called["first"] == False
-        assert called["second"] == True
+        assert called["first"] is False
+        assert called["second"] is True
+
+    @pytest.mark.asyncio
+    async def test_issue_561_matchers(self):
+        app = AsyncApp(
+            client=self.web_client,
+            signing_secret=self.signing_secret,
+        )
+
+        async def just_fail():
+            raise "This matcher should not be called!"
+
+        @app.message("xxx", matchers=[just_fail])
+        async def just_ack():
+            raise "This listener should not be called!"
+
+        request = self.build_request()
+        response = await app.async_dispatch(request)
+        assert response.status == 404
+        await assert_auth_test_count_async(self, 1)
 
 
 message_body = {