From 192e9e101d934a3171677ada511a11c26899cc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20H=C3=B6nig?= Date: Sat, 10 Feb 2018 23:24:37 +0100 Subject: [PATCH] bots: Add magic method validate_config(). This method allows bots to validate their config info in a standardized way. Adding the method to a bot is optional, but recommended for bots with config options that can be invalid, like api keys. The giphy bot serves as an example. The primary reason behind this is to allow the zulip backend to validate config data for embedded bots. The backend does not have a permanent bot class instance, so validate_config() must be a static method. --- zulip_bots/zulip_bots/bots/giphy/giphy.py | 15 ++++++++++----- zulip_bots/zulip_bots/bots/giphy/test_giphy.py | 16 ++++++++++------ zulip_bots/zulip_bots/custom_exceptions.py | 11 +++++++++++ zulip_bots/zulip_bots/lib.py | 9 +++++++++ zulip_bots/zulip_bots/test_lib.py | 13 +++++++++++++ 5 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 zulip_bots/zulip_bots/custom_exceptions.py diff --git a/zulip_bots/zulip_bots/bots/giphy/giphy.py b/zulip_bots/zulip_bots/bots/giphy/giphy.py index e2fd19c..91f7d98 100644 --- a/zulip_bots/zulip_bots/bots/giphy/giphy.py +++ b/zulip_bots/zulip_bots/bots/giphy/giphy.py @@ -7,6 +7,8 @@ import os import re from requests.exceptions import HTTPError, ConnectionError +from zulip_bots.custom_exceptions import ConfigValidationError + GIPHY_TRANSLATE_API = 'http://api.giphy.com/v1/gifs/translate' class GiphyHandler(object): @@ -24,21 +26,24 @@ class GiphyHandler(object): The bot responds also to private messages. ''' - def initialize(self, bot_handler: Any) -> None: - self.config_info = bot_handler.get_config_info('giphy') + @staticmethod + def validate_config(config_info: Dict[str, str]) -> None: query = {'s': 'Hello', - 'api_key': self.config_info['key']} + 'api_key': config_info['key']} try: data = requests.get(GIPHY_TRANSLATE_API, params=query) data.raise_for_status() except ConnectionError as e: - bot_handler.quit(str(e)) + raise ConfigValidationError(str(e)) except HTTPError as e: error_message = str(e) if (data.status_code == 403): error_message += ('This is likely due to an invalid key.\n' 'Follow the instructions in doc.md for setting an API key.') - bot_handler.quit(error_message) + raise ConfigValidationError(error_message) + + def initialize(self, bot_handler: Any) -> None: + self.config_info = bot_handler.get_config_info('giphy') def handle_message(self, message: Dict[str, str], bot_handler: Any) -> None: bot_response = get_bot_giphy_response( diff --git a/zulip_bots/zulip_bots/bots/giphy/test_giphy.py b/zulip_bots/zulip_bots/bots/giphy/test_giphy.py index b3bfea9..09bcaba 100755 --- a/zulip_bots/zulip_bots/bots/giphy/test_giphy.py +++ b/zulip_bots/zulip_bots/bots/giphy/test_giphy.py @@ -29,18 +29,22 @@ class TestGiphyBot(BotTestCase): 'Sorry, I don\'t have a GIF for "world without zulip"! :astonished:', ) - def test_403(self) -> None: + def test_invalid_config(self) -> None: bot = get_bot_message_handler(self.bot_name) bot_handler = StubBotHandler() + with self.mock_http_conversation('test_403'): + self.validate_invalid_config({'key': '12345678'}, + "This is likely due to an invalid key.\n") - with self.mock_config_info({'key': '12345678'}), \ - self.mock_http_conversation('test_403'), \ - self.assertRaises(bot_handler.BotQuitException): - bot.initialize(bot_handler) + def test_valid_config(self) -> None: + bot = get_bot_message_handler(self.bot_name) + bot_handler = StubBotHandler() + with self.mock_http_conversation('test_normal'): + self.validate_valid_config({'key': '12345678'}) def test_connection_error_while_running(self) -> None: with self.mock_config_info({'key': '12345678'}), \ - patch('requests.get', side_effect=[MagicMock(), ConnectionError()]), \ + patch('requests.get', side_effect=[ConnectionError()]), \ patch('logging.exception'): self.verify_reply( 'world without chocolate', diff --git a/zulip_bots/zulip_bots/custom_exceptions.py b/zulip_bots/zulip_bots/custom_exceptions.py new file mode 100644 index 0000000..7b7f92b --- /dev/null +++ b/zulip_bots/zulip_bots/custom_exceptions.py @@ -0,0 +1,11 @@ +# This file implements some custom exceptions that can +# be used by all bots. +# We avoid adding these exceptions to lib.py, because the +# current architecture works by lib.py importing bots, not +# the other way around. + +class ConfigValidationError(Exception): + ''' + Raise if the config data passed to a bot's validate_config() + is invalid (e.g. wrong API key, invalid email, etc.). + ''' diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 4bf4494..9c4928a 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -18,6 +18,7 @@ from typing import Any, Optional, List, Dict, IO, Text, Set from types import ModuleType from zulip import Client, ZulipError +from zulip_bots.custom_exceptions import ConfigValidationError class NoBotConfigException(Exception): pass @@ -272,6 +273,14 @@ def run_message_handler_for_bot(lib_module, quiet, config_file, bot_config_file, restricted_client = ExternalBotHandler(client, bot_dir, bot_details, bot_config_file) message_handler = lib_module.handler_class() + if hasattr(message_handler, 'validate_config'): + config_data = restricted_client.get_config_info(bot_name) + try: + lib_module.handler_class.validate_config(config_data) + except ConfigValidationError as e: + print("There was a problem validating your config file:\n\n{}".format(e)) + sys.exit(1) + if hasattr(message_handler, 'initialize'): message_handler.initialize(bot_handler=restricted_client) diff --git a/zulip_bots/zulip_bots/test_lib.py b/zulip_bots/zulip_bots/test_lib.py index bc44197..24a9f64 100755 --- a/zulip_bots/zulip_bots/test_lib.py +++ b/zulip_bots/zulip_bots/test_lib.py @@ -3,6 +3,10 @@ from unittest import TestCase from typing import List, Dict, Any, Tuple +from zulip_bots.custom_exceptions import ( + ConfigValidationError, +) + from zulip_bots.request_test_lib import ( mock_http_conversation, ) @@ -149,6 +153,15 @@ class BotTestCase(TestCase): response = bot_handler.unique_response() self.assertEqual(expected_response, response['content']) + def validate_invalid_config(self, config_data: Dict[str, str], error_regexp: str) -> None: + bot_class = type(get_bot_message_handler(self.bot_name)) + with self.assertRaisesRegexp(ConfigValidationError, error_regexp): + bot_class.validate_config(config_data) + + def validate_valid_config(self, config_data: Dict[str, str]) -> None: + bot_class = type(get_bot_message_handler(self.bot_name)) + bot_class.validate_config(config_data) + def test_bot_usage(self): # type: () -> None bot = get_bot_message_handler(self.bot_name)