From 77ddf816fda195ce4166a22b7d748d5fdcd0858a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 15:47:24 +0100 Subject: [PATCH 1/6] Add return values to changes in allowlist to determine if action was successful --- simplematrixbotlib/config.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/simplematrixbotlib/config.py b/simplematrixbotlib/config.py index dda852e..9c41671 100644 --- a/simplematrixbotlib/config.py +++ b/simplematrixbotlib/config.py @@ -85,29 +85,39 @@ def allowlist(self, value: Set[str]) -> None: return self._allowlist = checked - def add_allowlist(self, value: Set[str]) -> None: + def add_allowlist(self, value: Set[str]) -> bool: """ Parameters ---------- value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be added to allowlist. + Returns + ------- + bool + A bool indicating if the action was successful """ checked = self._check_set_regex(value) if checked is None: - return + return False self._allowlist = self._allowlist.union(checked) + return True - def remove_allowlist(self, value: Set[str]) -> None: + def remove_allowlist(self, value: Set[str]) -> bool: """ Parameters ---------- value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be removed from allowlist. + Returns + ------- + bool + A bool indicating if the action was successful """ checked = self._check_set_regex(value) if checked is None: - return + return False self._allowlist = self._allowlist - checked + return True @property def blocklist(self) -> Set[re.Pattern]: From c2fd52d3cfbbd42c2b467f24509cf0fe73c8ecbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 22:18:46 +0100 Subject: [PATCH 2/6] Remove return values instead throw error if regex patterns not valid --- simplematrixbotlib/config.py | 74 ++++++++++++------------------------ 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/simplematrixbotlib/config.py b/simplematrixbotlib/config.py index 9c41671..a769f30 100644 --- a/simplematrixbotlib/config.py +++ b/simplematrixbotlib/config.py @@ -1,29 +1,21 @@ from dataclasses import dataclass, field import toml import re -from typing import Set, Union +from typing import Set @dataclass class Config: _join_on_invite: bool = True _allowlist: Set[re.Pattern] = field( - default_factory=set) #TODO: default to bot's homeserver + default_factory=set) # TODO: default to bot's homeserver _blocklist: Set[re.Pattern] = field(default_factory=set) - def _check_set_regex(self, - value: Set[str]) -> Union[Set[re.Pattern], None]: - new_list = set() - for v in value: - try: - tmp = re.compile(v) - except re.error: - print( - f"{v} is not a valid regular expression. Ignoring your list update." - ) - return None - new_list.add(tmp) - return new_list + @staticmethod + def _check_set_regex(value: Set[str]): + """Checks if the patterns in value are valid or throws an error""" + [re.compile(v) for v in value] # Fails for invalid regex + def _load_config_dict(self, config_dict: dict) -> None: for key, value in config_dict.items(): @@ -80,44 +72,32 @@ def allowlist(self) -> Set[re.Pattern]: @allowlist.setter def allowlist(self, value: Set[str]) -> None: - checked = self._check_set_regex(value) - if checked is None: - return - self._allowlist = checked + self._check_set_regex(value) + self._allowlist = value - def add_allowlist(self, value: Set[str]) -> bool: + def add_allowlist(self, value: Set[str]): """ + Adds all regex to the allowlist if valid, else throws error + Parameters ---------- value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be added to allowlist. - Returns - ------- - bool - A bool indicating if the action was successful """ - checked = self._check_set_regex(value) - if checked is None: - return False - self._allowlist = self._allowlist.union(checked) - return True + self._check_set_regex(value) + self._allowlist = self._allowlist.union(value) def remove_allowlist(self, value: Set[str]) -> bool: """ + Removes all in the value set from the allowlist or throws an error + Parameters ---------- value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be removed from allowlist. - Returns - ------- - bool - A bool indicating if the action was successful """ - checked = self._check_set_regex(value) - if checked is None: - return False - self._allowlist = self._allowlist - checked - return True + self._check_set_regex(value) + self._allowlist = self._allowlist - value @property def blocklist(self) -> Set[re.Pattern]: @@ -132,10 +112,8 @@ def blocklist(self) -> Set[re.Pattern]: @blocklist.setter def blocklist(self, value: Set[str]) -> None: - checked = self._check_set_regex(value) - if checked is None: - return - self._blocklist = checked + self._check_set_regex(value) + self._blocklist = value def add_blocklist(self, value: Set[str]) -> None: """ @@ -144,10 +122,8 @@ def add_blocklist(self, value: Set[str]) -> None: value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be added to blocklist. """ - checked = self._check_set_regex(value) - if checked is None: - return - self._blocklist = self._blocklist.union(checked) + self._check_set_regex(value) + self._blocklist = self._blocklist.union(value) def remove_blocklist(self, value: Set[str]) -> None: """ @@ -156,7 +132,5 @@ def remove_blocklist(self, value: Set[str]) -> None: value : Set[str] A set of strings which represent Matrix IDs or a regular expression matching Matrix IDs to be removed from blocklist. """ - checked = self._check_set_regex(value) - if checked is None: - return - self._blocklist = self._blocklist - checked + self._check_set_regex(value) + self._blocklist = self._blocklist - value From 4483dbdf57b705fc4a4ca1ba7839f9ce623c64ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 22:34:04 +0100 Subject: [PATCH 3/6] Fix type annotation --- simplematrixbotlib/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplematrixbotlib/config.py b/simplematrixbotlib/config.py index a769f30..d67a969 100644 --- a/simplematrixbotlib/config.py +++ b/simplematrixbotlib/config.py @@ -87,7 +87,7 @@ def add_allowlist(self, value: Set[str]): self._check_set_regex(value) self._allowlist = self._allowlist.union(value) - def remove_allowlist(self, value: Set[str]) -> bool: + def remove_allowlist(self, value: Set[str]): """ Removes all in the value set from the allowlist or throws an error From 673cfc11e2262cc335402dcda17045ed3262a1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 23:35:48 +0100 Subject: [PATCH 4/6] Add tests and slightly more informative error message --- simplematrixbotlib/config.py | 8 ++++++-- tests/config/test_config.py | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/simplematrixbotlib/config.py b/simplematrixbotlib/config.py index d67a969..69ce348 100644 --- a/simplematrixbotlib/config.py +++ b/simplematrixbotlib/config.py @@ -14,7 +14,11 @@ class Config: @staticmethod def _check_set_regex(value: Set[str]): """Checks if the patterns in value are valid or throws an error""" - [re.compile(v) for v in value] # Fails for invalid regex + for v in value: + try: + re.compile(v) # Fails for invalid regex + except re.error: + raise re.error(f"{v} is not a valid regex.") def _load_config_dict(self, config_dict: dict) -> None: @@ -71,7 +75,7 @@ def allowlist(self) -> Set[re.Pattern]: return self._allowlist @allowlist.setter - def allowlist(self, value: Set[str]) -> None: + def allowlist(self, value: Set[str]) -> None: self._check_set_regex(value) self._allowlist = value diff --git a/tests/config/test_config.py b/tests/config/test_config.py index ac42796..8adba1d 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -69,3 +69,14 @@ def test_manual_set(): assert config.blocklist == set(map(re.compile, ['@test:matrix\\.org'])) config.blocklist = {'*:example\\.org'} assert config.blocklist == set(map(re.compile, ['@test:matrix\\.org'])) + + +def test_botlib(): + config = botlib.Config() + with pytest.raises(re.error): + config._check_set_regex(set(r"[.")) + # Valid patterns + config._check_set_regex(set(r"@testuser:example.org")) + + with pytest.raises(re.error): + config.load_toml(os.path.join(sample_config_path, 'config5.toml')) From b96610eb3a67e34de854323f7f16cfa2e2bc3d92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 23:38:23 +0100 Subject: [PATCH 5/6] Add tests and slightly more informative error message --- tests/config/sample_config_files/config5.toml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tests/config/sample_config_files/config5.toml diff --git a/tests/config/sample_config_files/config5.toml b/tests/config/sample_config_files/config5.toml new file mode 100644 index 0000000..e0be816 --- /dev/null +++ b/tests/config/sample_config_files/config5.toml @@ -0,0 +1,3 @@ +[simplematrixbotlib.config] +allowlist = ["[."] +blocklist = ["@test2:example\\.org"] From 9fe0cf815d2e1ef9e0c9c06fa073369c67147ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian-Samuel=20Geb=C3=BChr?= Date: Thu, 17 Feb 2022 23:42:49 +0100 Subject: [PATCH 6/6] Fix code style errors --- simplematrixbotlib/config.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/simplematrixbotlib/config.py b/simplematrixbotlib/config.py index 69ce348..5902730 100644 --- a/simplematrixbotlib/config.py +++ b/simplematrixbotlib/config.py @@ -16,11 +16,10 @@ def _check_set_regex(value: Set[str]): """Checks if the patterns in value are valid or throws an error""" for v in value: try: - re.compile(v) # Fails for invalid regex + re.compile(v) # Fails for invalid regex except re.error: raise re.error(f"{v} is not a valid regex.") - def _load_config_dict(self, config_dict: dict) -> None: for key, value in config_dict.items(): if key == 'join_on_invite': @@ -75,7 +74,7 @@ def allowlist(self) -> Set[re.Pattern]: return self._allowlist @allowlist.setter - def allowlist(self, value: Set[str]) -> None: + def allowlist(self, value: Set[str]) -> None: self._check_set_regex(value) self._allowlist = value