Re-type config paths in `ConfigError`s to be `StrSequence`s (#15615)

Part of #14809.

Signed-off-by: Sean Quah <seanq@matrix.org>

8 files changed, 31 insertions, 26 deletions

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)