From 68dcd2cbcb3c01787ade9cf3725486712a7cafda Mon Sep 17 00:00:00 2001 From: Sean Quah <8349537+squahtx@users.noreply.github.com> Date: Thu, 18 May 2023 11:11:30 +0100 Subject: [PATCH] Re-type config paths in `ConfigError`s to be `StrSequence`s (#15615) Part of #14809. Signed-off-by: Sean Quah --- changelog.d/15615.misc | 1 + synapse/config/_base.py | 3 ++- synapse/config/_base.pyi | 3 ++- synapse/config/_util.py | 8 ++++---- synapse/config/oembed.py | 6 +++--- synapse/config/server.py | 4 ++-- synapse/types/__init__.py | 8 ++++++++ synapse/util/module_loader.py | 24 +++++++++--------------- 8 files changed, 31 insertions(+), 26 deletions(-) create mode 100644 changelog.d/15615.misc diff --git a/changelog.d/15615.misc b/changelog.d/15615.misc new file mode 100644 index 0000000000..a39fd0a098 --- /dev/null +++ b/changelog.d/15615.misc @@ -0,0 +1 @@ +Re-type config paths in `ConfigError`s to be `StrSequence`s instead of `Iterable[str]`s. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 2ce60610ca..1d268a1817 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -44,6 +44,7 @@ import jinja2 import pkg_resources import yaml +from synapse.types import StrSequence from synapse.util.templates import _create_mxc_to_http_filter, _format_ts_filter logger = logging.getLogger(__name__) @@ -58,7 +59,7 @@ class ConfigError(Exception): the problem lies. """ - def __init__(self, msg: str, path: Optional[Iterable[str]] = None): + def __init__(self, msg: str, path: Optional[StrSequence] = None): self.msg = msg self.path = path diff --git a/synapse/config/_base.pyi b/synapse/config/_base.pyi index b5cec132b4..fc51aed234 100644 --- a/synapse/config/_base.pyi +++ b/synapse/config/_base.pyi @@ -61,9 +61,10 @@ from synapse.config import ( # noqa: F401 voip, workers, ) +from synapse.types import StrSequence class ConfigError(Exception): - def __init__(self, msg: str, path: Optional[Iterable[str]] = None): + def __init__(self, msg: str, path: Optional[StrSequence] = None): self.msg = msg self.path = path diff --git a/synapse/config/_util.py b/synapse/config/_util.py index dfc5d12210..acccca413b 100644 --- a/synapse/config/_util.py +++ b/synapse/config/_util.py @@ -11,17 +11,17 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, Iterable, Type, TypeVar +from typing import Any, Dict, Type, TypeVar import jsonschema from pydantic import BaseModel, ValidationError, parse_obj_as from synapse.config._base import ConfigError -from synapse.types import JsonDict +from synapse.types import JsonDict, StrSequence def validate_config( - json_schema: JsonDict, config: Any, config_path: Iterable[str] + json_schema: JsonDict, config: Any, config_path: StrSequence ) -> None: """Validates a config setting against a JsonSchema definition @@ -45,7 +45,7 @@ def validate_config( def json_error_to_config_error( - e: jsonschema.ValidationError, config_path: Iterable[str] + e: jsonschema.ValidationError, config_path: StrSequence ) -> ConfigError: """Converts a json validation error to a user-readable ConfigError diff --git a/synapse/config/oembed.py b/synapse/config/oembed.py index 0d32aba70a..d7959639ee 100644 --- a/synapse/config/oembed.py +++ b/synapse/config/oembed.py @@ -19,7 +19,7 @@ from urllib import parse as urlparse import attr import pkg_resources -from synapse.types import JsonDict +from synapse.types import JsonDict, StrSequence from ._base import Config, ConfigError from ._util import validate_config @@ -80,7 +80,7 @@ class OembedConfig(Config): ) def _parse_and_validate_provider( - self, providers: List[JsonDict], config_path: Iterable[str] + self, providers: List[JsonDict], config_path: StrSequence ) -> Iterable[OEmbedEndpointConfig]: # Ensure it is the proper form. validate_config( @@ -112,7 +112,7 @@ class OembedConfig(Config): api_endpoint, patterns, endpoint.get("formats") ) - def _glob_to_pattern(self, glob: str, config_path: Iterable[str]) -> Pattern: + def _glob_to_pattern(self, glob: str, config_path: StrSequence) -> Pattern: """ Convert the glob into a sane regular expression to match against. The rules followed will be slightly different for the domain portion vs. diff --git a/synapse/config/server.py b/synapse/config/server.py index 386c3194b8..64201238d6 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -27,7 +27,7 @@ from netaddr import AddrFormatError, IPNetwork, IPSet from twisted.conch.ssh.keys import Key from synapse.api.room_versions import KNOWN_ROOM_VERSIONS -from synapse.types import JsonDict +from synapse.types import JsonDict, StrSequence from synapse.util.module_loader import load_module from synapse.util.stringutils import parse_and_validate_server_name @@ -73,7 +73,7 @@ def _6to4(network: IPNetwork) -> IPNetwork: def generate_ip_set( ip_addresses: Optional[Iterable[str]], extra_addresses: Optional[Iterable[str]] = None, - config_path: Optional[Iterable[str]] = None, + config_path: Optional[StrSequence] = None, ) -> IPSet: """ Generate an IPSet from a list of IP addresses or CIDRs. diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index 325219656a..42baf8ac6b 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -84,7 +84,15 @@ JsonSerializable = object # Collection[str] that does not include str itself; str being a Sequence[str] # is very misleading and results in bugs. +# +# StrCollection is an unordered collection of strings. If ordering is important, +# StrSequence can be used instead. StrCollection = Union[Tuple[str, ...], List[str], AbstractSet[str]] +# Sequence[str] that does not include str itself; str being a Sequence[str] +# is very misleading and results in bugs. +# +# Unlike StrCollection, StrSequence is an ordered collection of strings. +StrSequence = Union[Tuple[str, ...], List[str]] # Note that this seems to require inheriting *directly* from Interface in order diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 5a638c6e9a..e3a54df48b 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -14,17 +14,17 @@ import importlib import importlib.util -import itertools from types import ModuleType -from typing import Any, Iterable, Tuple, Type +from typing import Any, Tuple, Type import jsonschema from synapse.config._base import ConfigError from synapse.config._util import json_error_to_config_error +from synapse.types import StrSequence -def load_module(provider: dict, config_path: Iterable[str]) -> Tuple[Type, Any]: +def load_module(provider: dict, config_path: StrSequence) -> Tuple[Type, Any]: """Loads a synapse module with its config Args: @@ -39,9 +39,7 @@ def load_module(provider: dict, config_path: Iterable[str]) -> Tuple[Type, Any]: modulename = provider.get("module") if not isinstance(modulename, str): - raise ConfigError( - "expected a string", path=itertools.chain(config_path, ("module",)) - ) + raise ConfigError("expected a string", path=tuple(config_path) + ("module",)) # We need to import the module, and then pick the class out of # that, so we split based on the last dot. @@ -55,19 +53,17 @@ def load_module(provider: dict, config_path: Iterable[str]) -> Tuple[Type, Any]: try: provider_config = provider_class.parse_config(module_config) except jsonschema.ValidationError as e: - raise json_error_to_config_error( - e, itertools.chain(config_path, ("config",)) - ) + raise json_error_to_config_error(e, tuple(config_path) + ("config",)) except ConfigError as e: raise _wrap_config_error( "Failed to parse config for module %r" % (modulename,), - prefix=itertools.chain(config_path, ("config",)), + prefix=tuple(config_path) + ("config",), e=e, ) except Exception as e: raise ConfigError( "Failed to parse config for module %r" % (modulename,), - path=itertools.chain(config_path, ("config",)), + path=tuple(config_path) + ("config",), ) from e else: provider_config = module_config @@ -92,9 +88,7 @@ def load_python_module(location: str) -> ModuleType: return mod -def _wrap_config_error( - msg: str, prefix: Iterable[str], e: ConfigError -) -> "ConfigError": +def _wrap_config_error(msg: str, prefix: StrSequence, e: ConfigError) -> "ConfigError": """Wrap a relative ConfigError with a new path This is useful when we have a ConfigError with a relative path due to a problem @@ -102,7 +96,7 @@ def _wrap_config_error( """ path = prefix if e.path: - path = itertools.chain(prefix, e.path) + path = tuple(prefix) + tuple(e.path) e1 = ConfigError(msg, path)