From 0a0ad5199d5fada3b199f55552852556df58e199 Mon Sep 17 00:00:00 2001 From: Florian Fischer Date: Fri, 11 Aug 2023 12:34:46 +0200 Subject: remove global variables using the environment Previously the geldschieberbot module used global variables retrieved from the environment for its configuration. This is not very flexible and can lead to problems when using the module from other python code. Therefore make all configuration Geldschieberbot specific and pass the values to the constructor. If no values are provided fallback to the environment. This change is fully backwards compatible. --- geldschieberbot.py | 52 ++++++++++++++++++++++++++++++++++------------------ test.py | 40 +++++++++++++++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/geldschieberbot.py b/geldschieberbot.py index 2853379..c58887b 100644 --- a/geldschieberbot.py +++ b/geldschieberbot.py @@ -10,14 +10,6 @@ import subprocess import sys import typing as T -# Path where our data is stored persistent on disk -STATE_FILE = os.environ["GSB_STATE_FILE"] - -GROUP_ID = os.environ["GSB_GROUP_ID"] - -SEND_CMD = os.environ["GSB_SEND_CMD"] -GROUP_SEND_CMD = SEND_CMD + GROUP_ID - @dataclass class MessageContext: @@ -131,8 +123,9 @@ class Geldschieberbot: STATE_KEYS = ("balance", "name2num", "num2name", "available_cars", "scheduled_cmds", "changes", "aliases") - def load_state(self, state_path=STATE_FILE): + def load_state(self, state_path=''): """Load state from disk""" + state_path = state_path or self.state_path if os.path.isfile(state_path): with open(state_path, 'r', encoding='utf-8') as state_f: self.state = json.load(state_f) @@ -186,8 +179,9 @@ class Geldschieberbot: self.changes = self.state["changes"] self.aliases = self.state["aliases"] - def save_state(self, state_path=STATE_FILE): + def save_state(self, state_path=''): """Load state from disk""" + state_path = state_path or self.state_path with open(state_path, 'w', encoding='utf-8') as state_file: json.dump(self.state, state_file, cls=GeldschieberbotJSONEncoder) @@ -220,9 +214,10 @@ class Geldschieberbot: def send(self, msg, attachment=None, - cmd=SEND_CMD, + cmd='', quote: T.Optional[Quote] = None): """Send a message with optional attachment""" + cmd = cmd or self.send_cmd if not self.quiet: if attachment: cmd += f' -a {attachment}' @@ -949,14 +944,13 @@ class Geldschieberbot: return {'msg': out} - @classmethod - def export_state(cls, msg: MessageContext) -> dict[str, str]: + def export_state(self, msg: MessageContext) -> dict[str, str]: """Send the state file as attachment""" if not msg.sender: return {'err': 'you must register first'} out = f'State from {datetime.now().date().isoformat()}' - return {'msg': out, 'attachment': STATE_FILE} + return {'msg': out, 'attachment': self.state_path} def schedule(self, msg: MessageContext) -> dict[str, str]: """Schedule a command for periodic execution""" @@ -1088,13 +1082,26 @@ class Geldschieberbot: self.aliases[alias] = users return {'msg': f'New alias "{alias}" registered'} - def __init__(self, dry_run=False, quote_cmd=True): + def __init__(self, + state_path='', + group_id='', + send_cmd='', + dry_run=False, + quote_cmd=True): + self.state_path = state_path or os.environ['GSB_STATE_FILE'] + + self.group_id = group_id or os.environ["GSB_GROUP_ID"] + + self.send_cmd = send_cmd or os.environ["GSB_SEND_CMD"] + self.group_send_cmd = self.send_cmd + self.group_id + self.dry_run = dry_run # Run without changing the stored state self.quote_cmd = quote_cmd # Quote the message causing the reply - self.load_state() self.quiet = False # Run without sending messages self.record_changes = True # Should changes be recorded + self.load_state() + # Command dispatch table self.cmds = { 'reg': self.register, @@ -1166,7 +1173,8 @@ class Geldschieberbot: return message = envelope["dataMessage"] - if message["groupInfo"] and message["groupInfo"]["groupId"] != GROUP_ID: + if message["groupInfo"] and message["groupInfo"][ + "groupId"] != self.group_id: return body = [l.strip() for l in message["message"].lower().splitlines()] @@ -1251,6 +1259,10 @@ def main(): '--dry-run', help='do not persist changes', action='store_true') + parser.add_argument('-f', '--state-file', help='the state file') + parser.add_argument('-g', '--group-id', help='the group id to listen to') + parser.add_argument('--send-cmd', + help='the shell command used to send messages') parser.add_argument('-nq', '--no-quote', help='not quote the message causing the reply', @@ -1260,7 +1272,11 @@ def main(): if args.dry_run: print("Dry Run no changes will apply!") - bot = Geldschieberbot(dry_run=args.dry_run, quote_cmd=not args.no_quote) + bot = Geldschieberbot(state_path=args.state_file, + group_id=args.group_id, + send_cmd=args.send_cmd, + dry_run=args.dry_run, + quote_cmd=not args.no_quote) # Read cmds from stdin for line in sys.stdin.read().splitlines(): diff --git a/test.py b/test.py index 71d6d34..c1a5423 100755 --- a/test.py +++ b/test.py @@ -8,6 +8,8 @@ from string import Template import subprocess import unittest +from geldschieberbot import Geldschieberbot, GeldschieberbotJSONEncoder + alice, bob, charlie = "alice", "bob", "charlie" num = {alice: "+49123456", bob: "+49654321", charlie: "+49615243"} os.environ["GSB_GROUP_ID"] = "test" @@ -96,11 +98,19 @@ def reset_state_string(string): json.dump(json.loads(string), f) -def compare_state(comp_state): - with open(comp_state, "r", encoding='utf-8') as csf, \ - open(os.environ["GSB_STATE_FILE"], "r", encoding='utf-8') as sf: +def compare_state(expected_state_path, + state=None, + state_path=os.environ["GSB_STATE_FILE"]) -> bool: + s = '' + if state is None: + with open(state_path, "r", encoding='utf-8') as sf: + s = sf.read() + else: + s = json.dumps(state, cls=GeldschieberbotJSONEncoder) + + with open(expected_state_path, "r", encoding='utf-8') as csf: cs = csf.read() - s = sf.read() + return cs == s @@ -1375,7 +1385,27 @@ class TestConvertState(unittest.TestCase): res = run_bot(self, num[alice], "!thanks") self.assertEqual( res.stdout, - f"You are welcome. It is a pleasure to work with you, {num['alice']}.") + f"You are welcome. It is a pleasure to work with you, {num['alice']}." + ) + + +class TestStateLoadStore(unittest.TestCase): + + def test_default_init(self): + bot = Geldschieberbot() + self.assertTrue( + compare_state(os.environ['GSB_STATE_FILE'], state=bot.state)) + + def test_explicit_init(self): + sp = "test/state_3users.json" + bot = Geldschieberbot(sp) + self.assertTrue(compare_state(sp, state=bot.state)) + + def test_explicit_load(self): + sp = "test/state_3users.json" + bot = Geldschieberbot() + bot.load_state(sp) + self.assertTrue(compare_state(sp, state=bot.state)) if __name__ == '__main__': -- cgit v1.2.3