Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Re-type config paths in ConfigErrors to be StrSequences (#15615)
Browse files Browse the repository at this point in the history
Part of #14809.

Signed-off-by: Sean Quah <seanq@matrix.org>
  • Loading branch information
squahtx committed May 18, 2023
1 parent e15aa00 commit 68dcd2c
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 26 deletions.
1 change: 1 addition & 0 deletions changelog.d/15615.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Re-type config paths in `ConfigError`s to be `StrSequence`s instead of `Iterable[str]`s.
3 changes: 2 additions & 1 deletion synapse/config/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
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__)
Expand All @@ -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

Expand Down
3 changes: 2 additions & 1 deletion synapse/config/_base.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions synapse/config/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions synapse/config/oembed.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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
Expand Down Expand Up @@ -80,7 +80,7 @@ def _parse_and_validate_providers(
)

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(
Expand Down Expand Up @@ -112,7 +112,7 @@ def _parse_and_validate_provider(
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.
Expand Down
4 changes: 2 additions & 2 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
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

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions synapse/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,15 @@

# 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
Expand Down
24 changes: 9 additions & 15 deletions synapse/util/module_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -92,17 +88,15 @@ 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
parsing part of the config, and we now need to set it in context.
"""
path = prefix
if e.path:
path = itertools.chain(prefix, e.path)
path = tuple(prefix) + tuple(e.path)

e1 = ConfigError(msg, path)

Expand Down

0 comments on commit 68dcd2c

Please sign in to comment.