Skip to content

Fix WebhookHandler.add to able to corp with handler function with *args in its parameters #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
loloicci wants to merge 2 commits into from
Closed

Fix WebhookHandler.add to able to corp with handler function with *args in its parameters #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c25f22
Add tests to add wrapped function to webhook handler
loloicci Apr 15, 2020
ec635ca
Fix WebhookHandler.__get_args_count to enable to corp with functions …
loloicci Apr 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions linebot/webhook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,16 @@ def __add_handler(self, func, event, message=None):

@staticmethod
def __get_args_count(func):
HANDLER_ARGSIZE_MAXIMAM = 2
Copy link
Member

@cryeo cryeo Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[imo]
How about returning the flag that function has varadic arguments or not in addition to the number of non-varadic arguments?

In this way, we should modify

line-bot-sdk-python/linebot/webhook.py

Lines 256 to 262 in ec635ca

args_count = self.__get_args_count(func)
if args_count == 0:
func()
elif args_count == 1:
func(event)
else:
func(event, payload.destination)

to following, but I think that it is more clear.

has_varargs, args_count = self.__get_args_count(func)
if has_varargs or args_count >= 2:
    func(event, payload.destination)
elif args_count == 1:
    func(event)
else:
    func()

if PY3:
arg_spec = inspect.getfullargspec(func)
if arg_spec.varargs is not None:
return HANDLER_ARGSIZE_MAXIMAM
return len(arg_spec.args)
else:
arg_spec = inspect.getargspec(func)
if arg_spec.varargs is not None:
return HANDLER_ARGSIZE_MAXIMAM
return len(arg_spec.args)

@staticmethod
Expand Down
93 changes: 93 additions & 0 deletions tests/test_webhook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os
import unittest
from builtins import open
import inspect

from linebot import (
SignatureValidator, WebhookParser, WebhookHandler
Expand All @@ -29,6 +30,7 @@
LocationMessage, StickerMessage, FileMessage,
SourceUser, SourceRoom, SourceGroup,
DeviceLink, DeviceUnlink, ScenarioResult, ActionResult)
from linebot.utils import PY3


class TestSignatureValidator(unittest.TestCase):
Expand Down Expand Up @@ -527,5 +529,96 @@ def test_handler(self):
self.handler.handle(body, 'signature')


def wrap(func):
def wrapper(*args):
if PY3:
arg_spec = inspect.getfullargspec(func)
else:
arg_spec = inspect.getargspec(func)
return func(*args[0:len(arg_spec.args)])
return wrapper


class TestWebhookHandlerWithWrappedFunction(unittest.TestCase):
def setUp(self):
self.handler = WebhookHandler('channel_secret')

@self.handler.add(MessageEvent, message=TextMessage)
@wrap
def message_text(event, destination):
self.assertEqual('message', event.type)
self.assertEqual('text', event.message.type)
self.assertEqual('U123', destination)

@self.handler.add(MessageEvent,
message=(ImageMessage, VideoMessage, AudioMessage))
@wrap
def message_content(event):
self.assertEqual('message', event.type)
self.assertIn(
event.message.type,
['image', 'video', 'audio']
)

@self.handler.add(MessageEvent, message=StickerMessage)
@wrap
def message_sticker(event):
self.assertEqual('message', event.type)
self.assertEqual('sticker', event.message.type)

@self.handler.add(MessageEvent)
@wrap
def message(event):
self.assertEqual('message', event.type)
self.assertNotIn(
event.message.type,
['text', 'image', 'video', 'audio', 'sticker']
)

@self.handler.add(FollowEvent)
@wrap
def follow(event, destination):
self.assertEqual('follow', event.type)
self.assertEqual('U123', destination)

@self.handler.add(JoinEvent)
@wrap
def join(event):
self.assertEqual('join', event.type)

@self.handler.add(PostbackEvent)
@wrap
def postback(event):
self.assertEqual('postback', event.type)

@self.handler.add(BeaconEvent)
@wrap
def beacon(event):
self.assertEqual('beacon', event.type)

@self.handler.add(AccountLinkEvent)
@wrap
def account_link(event):
self.assertEqual('accountLink', event.type)

@self.handler.default()
def default(event):
self.assertNotIn(
event.type,
['message', 'follow', 'join', 'postback', 'beacon', 'accountLink']
)

def test_handler_with_wrapped_function(self):
file_dir = os.path.dirname(__file__)
webhook_sample_json_path = os.path.join(file_dir, 'text', 'webhook.json')
with open(webhook_sample_json_path) as fp:
body = fp.read()

# mock
self.handler.parser.signature_validator.validate = lambda a, b: True

self.handler.handle(body, 'signature')


Comment on lines +532 to +622
Copy link
Member

@cryeo cryeo Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[imo]
Test seems good, but it seems redundant to makeTestWebhookHandlerWithWrappedFunction.

If we separate the part which invokes function from WebhookHandler.handle like following and test it, the test will be more clear and simple I think.

    def __invoke_func(self, func, event, payload):
        has_varargs, args_count = self.__get_args_count(func)
        if has_varargs or args_count >= 2:
            func(event, payload.destination)
        elif args_count == 1:
            func(event)
        else:
            func()

if __name__ == '__main__':
unittest.main()