From 26654973e5362ba54940ce62fd74c8a3a74303c4 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Fri, 18 Aug 2017 16:59:55 +0530 Subject: [PATCH 1/5] bots: Use user 'id' instead of 'name' in is_private function. Names are not guaranteed to be unique, user ids should be used when comparing if the sender and receiver are the same. --- zulip_bots/zulip_bots/lib.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 11c8dd0b3..4aec921bc 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -79,6 +79,7 @@ def __init__(self, client, root_dir): self._client = client self._root_dir = root_dir try: + self.user_id = user_profile['user_id'] self.full_name = user_profile['full_name'] self.email = user_profile['email'] except KeyError: @@ -176,10 +177,10 @@ def extract_query_without_mention(message, client): def is_private(message, client): # type: (Dict[str, Any], ExternalBotHandler) -> bool - # bot will not reply if the sender name is the same as the bot name + # bot will not reply if the sender id is the same as the bot id # to prevent infinite loop if message['type'] == 'private': - return client.full_name != message['sender_full_name'] + return client.user_id != message['sender_id'] return False def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name): From 979767cdb8108839dc946182d9bf41f24705cd17 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Fri, 18 Aug 2017 22:11:37 +0530 Subject: [PATCH 2/5] bots: Modify is_private function to take 'id' as parameter. This is done so that Embedded bot system can also make use of this function directly, as only id is needed in this function. Also, there would be ambiguity as what type of BotHandler is passed in the arguments: EmbeddedBotHandler or ExternalBotHandler. --- zulip_bots/zulip_bots/lib.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 4aec921bc..c70e51d59 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -175,12 +175,18 @@ def extract_query_without_mention(message, client): query_without_mention = message['content'][len(start_with_mention.group()):] return query_without_mention.lstrip() -def is_private(message, client): - # type: (Dict[str, Any], ExternalBotHandler) -> bool - # bot will not reply if the sender id is the same as the bot id - # to prevent infinite loop +def is_private(message, at_mention_bot_id): + # type: (Dict[str, Any], int) -> bool + """ + This function is to ensure that the bot doesn't go into infinite loop if the message sender id is + the same as the id of the bot which is called. This function makes the bot not reply to itself. + + This function is being leveraged by two systems; external bot system and embedded bot system, + any change/modification in the structure of this should be reflected at other places accordingly. + For details read "extract_query_without_mention" function docstring. + """ if message['type'] == 'private': - return client.user_id != message['sender_id'] + return at_mention_bot_id != message['sender_id'] return False def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name): @@ -212,7 +218,7 @@ def handle_message(message): # is_mentioned is true if the bot is mentioned at ANY position (not necessarily # the first @mention in the message). is_mentioned = message['is_mentioned'] - is_private_message = is_private(message, restricted_client) + is_private_message = is_private(message, restricted_client.user_id) # Strip at-mention botname from the message if is_mentioned: From a01584c357756bd017315e1bb133bd9c6a5dd72e Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Sat, 19 Aug 2017 17:43:19 +0530 Subject: [PATCH 3/5] bots: Modify 'extract_query_without_mention' to take bot name as input. This is done so that Embedded bot system can also make use of this function directly, as only client name is needed in this function, and the type of client need not be ExternalBotHandler. Also, there would be ambiguity as what type of BotHandler is passed in the arguments: EmbeddedBotHandler or ExternalBotHandler. --- zulip_bots/zulip_bots/lib.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index c70e51d59..5fa27cb05 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -162,13 +162,23 @@ def state(self, default): yield new_state self.set_state(new_state) -def extract_query_without_mention(message, client): - # type: (Dict[str, Any], ExternalBotHandler) -> str +def extract_query_without_mention(message, at_mention_bot_name): + # type: (Dict[str, Any], str) -> str """ If the bot is the first @mention in the message, then this function returns the message with the bot's @mention removed. Otherwise, it returns None. + This function is being leveraged by two systems; external bot system and embedded bot system. + This function is being called by: + 1. 'run_message_handler_for_bot' function (zulip_bots/lib.py file in zulip/python-zulip-api + repository) that executes/runs/calls external bots. + 2. 'consume' function in EmbeddedBotWorker class (zerver/worker/queue_processors.py + file in zulip/zulip repository) that executes/runs/calls embedded bots. + + Since, this is a general utility function for any working bot, it is planned to be an independent + function for now. Any refactoring should correctly be reflected in all the bot systems using this + function. """ - bot_mention = r'^@(\*\*{0}\*\*)'.format(client.full_name) + bot_mention = r'^@(\*\*{0}\*\*)'.format(at_mention_bot_name) start_with_mention = re.compile(bot_mention).match(message['content']) if start_with_mention is None: return None @@ -224,7 +234,7 @@ def handle_message(message): if is_mentioned: # message['content'] will be None when the bot's @-mention is not at the beginning. # In that case, the message shall not be handled. - message['content'] = extract_query_without_mention(message=message, client=restricted_client) + message['content'] = extract_query_without_mention(message=message, at_mention_bot_name=restricted_client.full_name) if message['content'] is None: return From 3befbc31496247dbcc986b65e97815c8247ed920 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Sat, 19 Aug 2017 18:44:37 +0530 Subject: [PATCH 4/5] bots: Create an independent funtion to check and remove at-mention. Extract code and make an independent function to check and remove at-mention string (if the message is not a private message). This is done so that Embedded bots can also use this piece of code. --- zulip_bots/zulip_bots/lib.py | 50 +++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 5fa27cb05..395a73e0a 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -199,6 +199,39 @@ def is_private(message, at_mention_bot_id): return at_mention_bot_id != message['sender_id'] return False +def get_message_content_if_bot_is_called(message, at_mention_bot_name, at_mention_bot_id): + # type: (Dict[str, Any], str) -> Any + """ + Check if the bot is called or not; a bot can be called by 2 ways: @mention-botname or private message + to the bot. Once it is confirmed if a bot is called or not, then we move to the second part of the + function. + If the bot is privately messaged, then the message content need not be modified and the bot can directly + process the entire message content. + If the bot is called by @mention-botname, then we need to remove @mention-botname for the bot to + process the rest of the message content. + + This function is being leveraged by two systems; external bot system and embedded bot system, + any change/modification in the structure of this should be reflected at other places accordingly. + For details read "extract_query_without_mention" function docstring. + """ + # is_mentioned is true if the bot is mentioned at ANY position (not necessarily + # the first @mention in the message). + is_mentioned = message['is_mentioned'] + is_private_message = is_private(message=message, at_mention_bot_id=at_mention_bot_id) + + # Strip at-mention botname from the message + if is_mentioned: + # message['content'] will be None when the bot's @-mention is not at the beginning. + # In that case, the message shall not be handled. + message['content'] = extract_query_without_mention(message=message, + at_mention_bot_name=at_mention_bot_name) + if message['content'] is None: + return + + if (is_private_message or is_mentioned): + return message['content'] + return None + def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name): # type: (Any, bool, str) -> Any # @@ -225,20 +258,13 @@ def handle_message(message): # type: (Dict[str, Any]) -> None logging.info('waiting for next message') - # is_mentioned is true if the bot is mentioned at ANY position (not necessarily - # the first @mention in the message). - is_mentioned = message['is_mentioned'] - is_private_message = is_private(message, restricted_client.user_id) + message_content_if_bot_is_called = get_message_content_if_bot_is_called(message=message, + at_mention_bot_name=restricted_client.full_name, + at_mention_bot_id=restricted_client.user_id) - # Strip at-mention botname from the message - if is_mentioned: - # message['content'] will be None when the bot's @-mention is not at the beginning. - # In that case, the message shall not be handled. - message['content'] = extract_query_without_mention(message=message, at_mention_bot_name=restricted_client.full_name) - if message['content'] is None: - return + if message_content_if_bot_is_called: + message['content'] = message_content_if_bot_is_called - if is_private_message or is_mentioned: message_handler.handle_message( message=message, bot_handler=restricted_client, From aa3c4b505c22e989e6c7c7a1354a2c53aaf22847 Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Sun, 20 Aug 2017 19:29:17 +0530 Subject: [PATCH 5/5] bots: Extract code to create a function 'initialize_config_bot'. This function needs to be called by bots using additional specific configurations stored either in a file (in the case of External bots) or in the database (in case of Embedded bots). Extracting this piece of code to make it usable independently. --- zulip_bots/zulip_bots/lib.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 395a73e0a..d42f6d5cc 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -199,6 +199,19 @@ def is_private(message, at_mention_bot_id): return at_mention_bot_id != message['sender_id'] return False +def initialize_config_bot(message_handler, bot_handler): + # type: (Any, Any) -> None + """ + If a bot has bot-specific configuration settings (both public or private) to be set, then this + function calls the 'initialize' function which in turn calls 'get_config_info' for bots. + + This function is being leveraged by two systems; external bot system and embedded bot system, + any change/modification in the structure of this should be reflected at other places accordingly. + For details read "extract_query_without_mention" function docstring. + """ + if hasattr(message_handler, 'initialize'): + message_handler.initialize(bot_handler=bot_handler) + def get_message_content_if_bot_is_called(message, at_mention_bot_name, at_mention_bot_id): # type: (Dict[str, Any], str) -> Any """ @@ -246,8 +259,7 @@ def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name): restricted_client = ExternalBotHandler(client, bot_dir) message_handler = lib_module.handler_class() - if hasattr(message_handler, 'initialize'): - message_handler.initialize(bot_handler=restricted_client) + initialize_config_bot(message_handler=message_handler, bot_handler=restricted_client) state_handler = StateHandler()