Improve #192 by using inspect module methods

seratch · seratch · commit 3eabe68f9ffa · 2021-01-13T07:07:37.000+09:00
diff --git a/slack_bolt/kwargs_injection/async_utils.py b/slack_bolt/kwargs_injection/async_utils.py
@@ -1,4 +1,5 @@
 # pytype: skip-file
+import inspect
 import logging
 from typing import Callable, Dict, Optional, Any, Sequence
 
@@ -25,6 +26,7 @@ def build_async_required_kwargs(
     request: AsyncBoltRequest,
     response: Optional[BoltResponse],
     next_func: Callable[[], None] = None,
+    this_func: Optional[Callable] = None,
 ) -> Dict[str, Any]:
     all_available_args = {
         "logger": logger,
@@ -73,8 +75,12 @@ def build_async_required_kwargs(
         if first_arg_name in {"self", "cls"}:
             required_arg_names.pop(0)
         elif first_arg_name not in all_available_args.keys():
-            logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
-            required_arg_names.pop(0)
+            if this_func is None:
+                logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
+                required_arg_names.pop(0)
+            elif inspect.ismethod(this_func):
+                # We are sure that we should skip manipulating this arg
+                required_arg_names.pop(0)
 
     kwargs: Dict[str, Any] = {
         k: v for k, v in all_available_args.items() if k in required_arg_names
diff --git a/slack_bolt/kwargs_injection/utils.py b/slack_bolt/kwargs_injection/utils.py
@@ -1,4 +1,5 @@
 # pytype: skip-file
+import inspect
 import logging
 from typing import Callable, Dict, Optional, Any, Sequence
 
@@ -25,6 +26,7 @@ def build_required_kwargs(
     request: BoltRequest,
     response: Optional[BoltResponse],
     next_func: Callable[[], None] = None,
+    this_func: Optional[Callable] = None,
 ) -> Dict[str, Any]:
     all_available_args = {
         "logger": logger,
@@ -73,8 +75,12 @@ def build_required_kwargs(
         if first_arg_name in {"self", "cls"}:
             required_arg_names.pop(0)
         elif first_arg_name not in all_available_args.keys():
-            logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
-            required_arg_names.pop(0)
+            if this_func is None:
+                logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
+                required_arg_names.pop(0)
+            elif inspect.ismethod(this_func):
+                # We are sure that we should skip manipulating this arg
+                required_arg_names.pop(0)
 
     kwargs: Dict[str, Any] = {
         k: v for k, v in all_available_args.items() if k in required_arg_names
diff --git a/slack_bolt/lazy_listener/async_internals.py b/slack_bolt/lazy_listener/async_internals.py
@@ -23,6 +23,7 @@ async def request_wired_wrapper() -> None:
                     required_arg_names=arg_names,
                     request=request,
                     response=None,
+                    this_func=internal_func,
                 )
             )
         except Exception as e:
diff --git a/slack_bolt/lazy_listener/internals.py b/slack_bolt/lazy_listener/internals.py
@@ -23,6 +23,7 @@ def request_wired_func_wrapper() -> None:
                     required_arg_names=arg_names,
                     request=request,
                     response=None,
+                    this_func=func,
                 )
             )
         except Exception as e:
diff --git a/slack_bolt/listener/async_listener.py b/slack_bolt/listener/async_listener.py
@@ -117,6 +117,7 @@ async def run_ack_function(
                 required_arg_names=self.arg_names,
                 request=request,
                 response=response,
+                this_func=self.ack_function,
             )
         )
 
diff --git a/slack_bolt/listener/custom_listener.py b/slack_bolt/listener/custom_listener.py
@@ -52,5 +52,6 @@ def run_ack_function(
                 required_arg_names=self.arg_names,
                 request=request,
                 response=response,
+                this_func=self.ack_function,
             )
         )
diff --git a/slack_bolt/listener_matcher/async_builtins.py b/slack_bolt/listener_matcher/async_builtins.py
@@ -14,5 +14,6 @@ async def async_matches(self, req: AsyncBoltRequest, resp: BoltResponse) -> bool
                 required_arg_names=self.arg_names,
                 request=req,
                 response=resp,
+                this_func=self.func,
             )
         )
diff --git a/slack_bolt/listener_matcher/async_listener_matcher.py b/slack_bolt/listener_matcher/async_listener_matcher.py
@@ -45,6 +45,7 @@ async def async_matches(self, req: AsyncBoltRequest, resp: BoltResponse) -> bool
                 required_arg_names=self.arg_names,
                 request=req,
                 response=resp,
+                this_func=self.func,
             )
         )
 
diff --git a/slack_bolt/listener_matcher/builtins.py b/slack_bolt/listener_matcher/builtins.py
@@ -50,6 +50,7 @@ def matches(self, req: BoltRequest, resp: BoltResponse) -> bool:
                 required_arg_names=self.arg_names,
                 request=req,
                 response=resp,
+                this_func=self.func,
             )
         )
 
diff --git a/slack_bolt/listener_matcher/custom_listener_matcher.py b/slack_bolt/listener_matcher/custom_listener_matcher.py
@@ -28,5 +28,6 @@ def matches(self, req: BoltRequest, resp: BoltResponse) -> bool:
                 required_arg_names=self.arg_names,
                 request=req,
                 response=resp,
+                this_func=self.func,
             )
         )
diff --git a/slack_bolt/middleware/async_custom_middleware.py b/slack_bolt/middleware/async_custom_middleware.py
@@ -39,6 +39,7 @@ async def async_process(
                 request=req,
                 response=resp,
                 next_func=next,
+                this_func=self.func,
             )
         )
 
diff --git a/slack_bolt/middleware/custom_middleware.py b/slack_bolt/middleware/custom_middleware.py
@@ -35,6 +35,7 @@ def process(
                 request=req,
                 response=resp,
                 next_func=next,
+                this_func=self.func,
             )
         )
 
diff --git a/tests/scenario_tests/test_app_using_methods_in_class.py b/tests/scenario_tests/test_app_using_methods_in_class.py
@@ -1,3 +1,4 @@
+import inspect
 import json
 from time import time, sleep
 from typing import Callable
@@ -31,6 +32,29 @@ def teardown_method(self):
         cleanup_mock_web_api_server(self)
         restore_os_env(self.old_os_env)
 
+    def test_inspect_behaviors(self):
+        def f():
+            pass
+
+        assert inspect.ismethod(f) is False
+
+        class A:
+            def b(self):
+                pass
+
+            @classmethod
+            def c(cls):
+                pass
+
+            @staticmethod
+            def d():
+                pass
+
+        a = A()
+        assert inspect.ismethod(a.b) is True
+        assert inspect.ismethod(A.c) is True
+        assert inspect.ismethod(A.d) is False
+
     def run_app_and_verify(self, app: App):
         payload = {
             "type": "message_action",
@@ -119,12 +143,24 @@ def test_instance_methods_uncommon_name(self):
         app.shortcut("test-shortcut")(awesome.instance_method2)
         self.run_app_and_verify(app)
 
+    def test_instance_methods_uncommon_name_3(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret)
+        awesome = AwesomeClass("Slackbot")
+        app.use(awesome.instance_middleware)
+        app.shortcut("test-shortcut")(awesome.instance_method3)
+        self.run_app_and_verify(app)
+
     def test_static_methods(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret)
         app.use(AwesomeClass.static_middleware)
         app.shortcut("test-shortcut")(AwesomeClass.static_method)
         self.run_app_and_verify(app)
 
+    def test_invalid_arg_in_func(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret)
+        app.shortcut("test-shortcut")(top_level_function)
+        self.run_app_and_verify(app)
+
 
 class AwesomeClass:
     def __init__(self, name: str):
@@ -159,7 +195,20 @@ def instance_method2(whatever, context: BoltContext, say: Say, ack: Ack):
         ack()
         say(f"Hello <@{context.user_id}>! My name is {whatever.name}")
 
+    text = "hello world"
+
+    def instance_method3(this, ack, logger, say):
+        ack()
+        logger.debug(this.text)
+        say(f"Hi there!")
+
     @staticmethod
     def static_method(context: BoltContext, say: Say, ack: Ack):
         ack()
         say(f"Hello <@{context.user_id}>!")
+
+
+def top_level_function(invalid_arg, ack, say):
+    assert invalid_arg is None
+    ack()
+    say("Hi")
diff --git a/tests/scenario_tests_async/test_app_using_methods_in_class.py b/tests/scenario_tests_async/test_app_using_methods_in_class.py
@@ -1,4 +1,5 @@
 import asyncio
+import inspect
 import json
 from time import time
 from typing import Callable
@@ -41,6 +42,29 @@ def event_loop(self):
         finally:
             restore_os_env(old_os_env)
 
+    def test_inspect_behaviors(self):
+        async def f():
+            pass
+
+        assert inspect.ismethod(f) is False
+
+        class A:
+            async def b(self):
+                pass
+
+            @classmethod
+            async def c(cls):
+                pass
+
+            @staticmethod
+            async def d():
+                pass
+
+        a = A()
+        assert inspect.ismethod(a.b) is True
+        assert inspect.ismethod(A.c) is True
+        assert inspect.ismethod(A.d) is False
+
     async def run_app_and_verify(self, app: AsyncApp):
         payload = {
             "type": "message_action",
@@ -126,20 +150,34 @@ async def test_instance_methods(self):
         await self.run_app_and_verify(app)
 
     @pytest.mark.asyncio
-    async def test_instance_methods_uncommon_name(self):
+    async def test_instance_methods_uncommon_name_1(self):
         app = AsyncApp(client=self.web_client, signing_secret=self.signing_secret)
         awesome = AwesomeClass("Slackbot")
         app.use(awesome.instance_middleware)
         app.shortcut("test-shortcut")(awesome.instance_method2)
         await self.run_app_and_verify(app)
 
+    @pytest.mark.asyncio
+    async def test_instance_methods_uncommon_name_2(self):
+        app = AsyncApp(client=self.web_client, signing_secret=self.signing_secret)
+        awesome = AwesomeClass("Slackbot")
+        app.use(awesome.instance_middleware)
+        app.shortcut("test-shortcut")(awesome.instance_method3)
+        await self.run_app_and_verify(app)
+
     @pytest.mark.asyncio
     async def test_static_methods(self):
         app = AsyncApp(client=self.web_client, signing_secret=self.signing_secret)
         app.use(AwesomeClass.static_middleware)
         app.shortcut("test-shortcut")(AwesomeClass.static_method)
         await self.run_app_and_verify(app)
 
+    @pytest.mark.asyncio
+    async def test_invalid_arg_in_func(self):
+        app = AsyncApp(client=self.web_client, signing_secret=self.signing_secret)
+        app.shortcut("test-shortcut")(top_level_function)
+        await self.run_app_and_verify(app)
+
 
 class AwesomeClass:
     def __init__(self, name: str):
@@ -182,7 +220,20 @@ async def instance_method2(
         await ack()
         await say(f"Hello <@{context.user_id}>! My name is {whatever.name}")
 
+    text = "hello world"
+
+    async def instance_method3(this, ack, logger, say):
+        await ack()
+        logger.debug(this.text)
+        await say(f"Hi there!")
+
     @staticmethod
     async def static_method(context: AsyncBoltContext, say: AsyncSay, ack: AsyncAck):
         await ack()
         await say(f"Hello <@{context.user_id}>!")
+
+
+async def top_level_function(invalid_arg, ack, say):
+    assert invalid_arg is None
+    await ack()
+    await say("Hi")

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ async def request_wired_wrapper() -> None:`
`23`	`23`	`required_arg_names=arg_names,`
`24`	`24`	`request=request,`
`25`	`25`	`response=None,`
	`26`	`+ this_func=internal_func,`
`26`	`27`	`)`
`27`	`28`	`)`
`28`	`29`	`except Exception as e:`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ def request_wired_func_wrapper() -> None:`
`23`	`23`	`required_arg_names=arg_names,`
`24`	`24`	`request=request,`
`25`	`25`	`response=None,`
	`26`	`+ this_func=func,`
`26`	`27`	`)`
`27`	`28`	`)`
`28`	`29`	`except Exception as e:`
Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,7 @@ async def run_ack_function(`
`117`	`117`	`required_arg_names=self.arg_names,`
`118`	`118`	`request=request,`
`119`	`119`	`response=response,`
	`120`	`+ this_func=self.ack_function,`
`120`	`121`	`)`
`121`	`122`	`)`
`122`	`123`
Original file line number	Diff line number	Diff line change
`@@ -52,5 +52,6 @@ def run_ack_function(`
`52`	`52`	`required_arg_names=self.arg_names,`
`53`	`53`	`request=request,`
`54`	`54`	`response=response,`
	`55`	`+ this_func=self.ack_function,`
`55`	`56`	`)`
`56`	`57`	`)`
Original file line number	Diff line number	Diff line change
`@@ -14,5 +14,6 @@ async def async_matches(self, req: AsyncBoltRequest, resp: BoltResponse) -> bool`
`14`	`14`	`required_arg_names=self.arg_names,`
`15`	`15`	`request=req,`
`16`	`16`	`response=resp,`
	`17`	`+ this_func=self.func,`
`17`	`18`	`)`
`18`	`19`	`)`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ async def async_matches(self, req: AsyncBoltRequest, resp: BoltResponse) -> bool`
`45`	`45`	`required_arg_names=self.arg_names,`
`46`	`46`	`request=req,`
`47`	`47`	`response=resp,`
	`48`	`+ this_func=self.func,`
`48`	`49`	`)`
`49`	`50`	`)`
`50`	`51`
Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,7 @@ def matches(self, req: BoltRequest, resp: BoltResponse) -> bool:`
`50`	`50`	`required_arg_names=self.arg_names,`
`51`	`51`	`request=req,`
`52`	`52`	`response=resp,`
	`53`	`+ this_func=self.func,`
`53`	`54`	`)`
`54`	`55`	`)`
`55`	`56`
Original file line number	Diff line number	Diff line change
`@@ -28,5 +28,6 @@ def matches(self, req: BoltRequest, resp: BoltResponse) -> bool:`
`28`	`28`	`required_arg_names=self.arg_names,`
`29`	`29`	`request=req,`
`30`	`30`	`response=resp,`
	`31`	`+ this_func=self.func,`
`31`	`32`	`)`
`32`	`33`	`)`