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.

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__':