From d8a208a58b4053e3f7a5f49b2e3756269cb78db3 Mon Sep 17 00:00:00 2001 From: blissful Date: Tue, 24 Oct 2023 21:20:17 -0400 Subject: [PATCH] parse config with descriptive errors --- rose/config.py | 145 ++++++++++++++++++++++---- rose/config_test.py | 247 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 327 insertions(+), 65 deletions(-) diff --git a/rose/config.py b/rose/config.py index a02d20d..c4a446d 100644 --- a/rose/config.py +++ b/rose/config.py @@ -24,6 +24,10 @@ class MissingConfigKeyError(RoseError): pass +class InvalidConfigValueError(RoseError): + pass + + @dataclass(frozen=True) class Config: music_source_dir: Path @@ -49,32 +53,131 @@ def read(cls, config_path_override: Path | None = None) -> Config: with cfgpath.open("rb") as fp: data = tomllib.load(fp) except FileNotFoundError as e: - raise ConfigNotFoundError(f"Configuration file not found ({CONFIG_PATH})") from e + raise ConfigNotFoundError(f"Configuration file not found ({cfgpath})") from e + + try: + music_source_dir = Path(data["music_source_dir"]).expanduser() + except KeyError as e: + raise MissingConfigKeyError( + f"Missing key music_source_dir in configuration file ({cfgpath})" + ) from e + except (ValueError, TypeError) as e: + raise InvalidConfigValueError( + f"Invalid value for music_source_dir in configuration file ({cfgpath}): " + "must be a path" + ) from e + + try: + fuse_mount_dir = Path(data["fuse_mount_dir"]).expanduser() + except KeyError as e: + raise MissingConfigKeyError( + f"Missing key fuse_mount_dir in configuration file ({cfgpath})" + ) from e + except (ValueError, TypeError) as e: + raise InvalidConfigValueError( + f"Invalid value for fuse_mount_dir in configuration file ({cfgpath}): " + "must be a path" + ) from e - cache_dir = CACHE_PATH - if "cache_dir" in data: + try: cache_dir = Path(data["cache_dir"]).expanduser() + except KeyError: + cache_dir = CACHE_PATH + except (TypeError, ValueError) as e: + raise InvalidConfigValueError( + f"Invalid value for cache_dir in configuration file ({cfgpath}): must be a path" + ) from e cache_dir.mkdir(exist_ok=True) + try: + max_proc = int(data["max_proc"]) + if max_proc <= 0: + raise ValueError(f"max_proc must be a positive integer: got {max_proc}") + except KeyError: + max_proc = max(1, multiprocessing.cpu_count() // 2) + except ValueError as e: + raise InvalidConfigValueError( + f"Invalid value for max_proc in configuration file ({cfgpath}): " + "must be a positive integer" + ) from e + artist_aliases_map: dict[str, list[str]] = defaultdict(list) artist_aliases_parents_map: dict[str, list[str]] = defaultdict(list) - for parent, subs in data.get("artist_aliases", []): - artist_aliases_map[parent] = subs - for s in subs: - artist_aliases_parents_map[s].append(parent) + try: + for parent, subs in data.get("artist_aliases", []): + artist_aliases_map[parent] = subs + if not isinstance(subs, list): + raise ValueError(f"Aliases must be of type list[str]: got {type(subs)}") + for s in subs: + if not isinstance(s, str): + raise ValueError(f"Each alias must be of type str: got {type(s)}") + artist_aliases_parents_map[s].append(parent) + except (ValueError, TypeError) as e: + raise InvalidConfigValueError( + f"Invalid value for artist_aliases in configuration file ({cfgpath}): " + "must be a list of (parent: str, aliases: list[str]) tuples" + ) from e try: - return cls( - music_source_dir=Path(data["music_source_dir"]).expanduser(), - fuse_mount_dir=Path(data["fuse_mount_dir"]).expanduser(), - cache_dir=cache_dir, - cache_database_path=cache_dir / "cache.sqlite3", - max_proc=data.get("max_proc", max(1, multiprocessing.cpu_count() // 2)), - artist_aliases_map=artist_aliases_map, - artist_aliases_parents_map=artist_aliases_parents_map, - fuse_hide_artists=data.get("fuse_hide_artists", []), - fuse_hide_genres=data.get("fuse_hide_genres", []), - fuse_hide_labels=data.get("fuse_hide_labels", []), - ) - except KeyError as e: - raise MissingConfigKeyError(f"Missing key in configuration file: {e}") from e + fuse_hide_artists = data["fuse_hide_artists"] + if not isinstance(fuse_hide_artists, list): + raise ValueError( + f"fuse_hide_artists must be a list[str]: got {type(fuse_hide_artists)}" + ) + for s in fuse_hide_artists: + if not isinstance(s, str): + raise ValueError(f"Each artist must be of type str: got {type(s)}") + except KeyError: + fuse_hide_artists = [] + except ValueError as e: + raise InvalidConfigValueError( + f"Invalid value for fuse_hide_artists in configuration file ({cfgpath}): " + "must be a list of strings" + ) from e + + try: + fuse_hide_genres = data["fuse_hide_genres"] + if not isinstance(fuse_hide_genres, list): + raise ValueError( + f"fuse_hide_genres must be a list[str]: got {type(fuse_hide_genres)}" + ) + for s in fuse_hide_genres: + if not isinstance(s, str): + raise ValueError(f"Each genre must be of type str: got {type(s)}") + except KeyError: + fuse_hide_genres = [] + except ValueError as e: + raise InvalidConfigValueError( + f"Invalid value for fuse_hide_genres in configuration file ({cfgpath}): " + "must be a list of strings" + ) from e + + try: + fuse_hide_labels = data["fuse_hide_labels"] + if not isinstance(fuse_hide_labels, list): + raise ValueError( + f"fuse_hide_labels must be a list[str]: got {type(fuse_hide_labels)}" + ) + for s in fuse_hide_labels: + if not isinstance(s, str): + raise ValueError(f"Each label must be of type str: got {type(s)}") + except KeyError: + fuse_hide_labels = [] + except ValueError as e: + raise InvalidConfigValueError( + f"Invalid value for fuse_hide_labels in configuration file ({cfgpath}): " + "must be a list of strings" + ) from e + + return cls( + music_source_dir=music_source_dir, + fuse_mount_dir=fuse_mount_dir, + cache_dir=cache_dir, + cache_database_path=cache_dir / "cache.sqlite3", + max_proc=max_proc, + artist_aliases_map=artist_aliases_map, + artist_aliases_parents_map=artist_aliases_parents_map, + fuse_hide_artists=fuse_hide_artists, + fuse_hide_genres=fuse_hide_genres, + fuse_hide_labels=fuse_hide_labels, + ) diff --git a/rose/config_test.py b/rose/config_test.py index fe634ed..a70be07 100644 --- a/rose/config_test.py +++ b/rose/config_test.py @@ -1,10 +1,9 @@ -import os import tempfile from pathlib import Path import pytest -from rose.config import Config, ConfigNotFoundError, MissingConfigKeyError +from rose.config import Config, ConfigNotFoundError, InvalidConfigValueError, MissingConfigKeyError def test_config_minimal() -> None: @@ -12,59 +11,66 @@ def test_config_minimal() -> None: path = Path(tmpdir) / "config.toml" with path.open("w") as fp: fp.write( - """\ - music_source_dir = "~/.music-src" - fuse_mount_dir = "~/music" - """ + """ + music_source_dir = "~/.music-src" + fuse_mount_dir = "~/music" + """ ) c = Config.read(config_path_override=path) - assert str(c.music_source_dir) == os.environ["HOME"] + "/.music-src" - assert str(c.fuse_mount_dir) == os.environ["HOME"] + "/music" + assert c.music_source_dir == Path.home() / ".music-src" + assert c.fuse_mount_dir == Path.home() / "music" def test_config_full() -> None: with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "config.toml" + cache_dir = Path(tmpdir) / "cache" with path.open("w") as fp: fp.write( - """\ - music_source_dir = "~/.music-src" - fuse_mount_dir = "~/music" - artist_aliases = [ - ["Abakus", ["Cinnamon Chasers"]], - ["tripleS", ["EVOLution", "LOVElution", "+(KR)ystal Eyes", "Acid Angel From Asia", "Acid Eyes"]], - ] - fuse_hide_artists = [ "xxx" ] - fuse_hide_genres = [ "yyy" ] - fuse_hide_labels = [ "zzz" ] + f""" + music_source_dir = "~/.music-src" + fuse_mount_dir = "~/music" + cache_dir = "{cache_dir}" + max_proc = 8 + artist_aliases = [ + ["Abakus", ["Cinnamon Chasers"]], + ["tripleS", ["EVOLution", "LOVElution", "+(KR)ystal Eyes", "Acid Angel From Asia", "Acid Eyes"]], + ] + fuse_hide_artists = [ "xxx" ] + fuse_hide_genres = [ "yyy" ] + fuse_hide_labels = [ "zzz" ] """ # noqa: E501 ) - c = Config.read(config_path_override=path) - assert str(c.music_source_dir) == os.environ["HOME"] + "/.music-src" - assert str(c.fuse_mount_dir) == os.environ["HOME"] + "/music" - assert c.artist_aliases_map == { - "Abakus": ["Cinnamon Chasers"], - "tripleS": [ - "EVOLution", - "LOVElution", - "+(KR)ystal Eyes", - "Acid Angel From Asia", - "Acid Eyes", - ], - } - assert c.artist_aliases_parents_map == { - "Cinnamon Chasers": ["Abakus"], - "EVOLution": ["tripleS"], - "LOVElution": ["tripleS"], - "+(KR)ystal Eyes": ["tripleS"], - "Acid Angel From Asia": ["tripleS"], - "Acid Eyes": ["tripleS"], - } - assert c.fuse_hide_artists == ["xxx"] - assert c.fuse_hide_genres == ["yyy"] - assert c.fuse_hide_labels == ["zzz"] + assert Config.read(config_path_override=path) == Config( + music_source_dir=Path.home() / ".music-src", + fuse_mount_dir=Path.home() / "music", + cache_dir=cache_dir, + cache_database_path=cache_dir / "cache.sqlite3", + max_proc=8, + artist_aliases_map={ + "Abakus": ["Cinnamon Chasers"], + "tripleS": [ + "EVOLution", + "LOVElution", + "+(KR)ystal Eyes", + "Acid Angel From Asia", + "Acid Eyes", + ], + }, + artist_aliases_parents_map={ + "Cinnamon Chasers": ["Abakus"], + "EVOLution": ["tripleS"], + "LOVElution": ["tripleS"], + "+(KR)ystal Eyes": ["tripleS"], + "Acid Angel From Asia": ["tripleS"], + "Acid Eyes": ["tripleS"], + }, + fuse_hide_artists=["xxx"], + fuse_hide_genres=["yyy"], + fuse_hide_labels=["zzz"], + ) def test_config_not_found() -> None: @@ -74,10 +80,163 @@ def test_config_not_found() -> None: Config.read(config_path_override=path) -def test_config_missing_key() -> None: +def test_config_missing_key_validation() -> None: with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "config.toml" path.touch() + + def append(x: str) -> None: + with path.open("a") as fp: + fp.write("\n" + x) + + append('music_source_dir = "/"') with pytest.raises(MissingConfigKeyError) as excinfo: Config.read(config_path_override=path) - assert str(excinfo.value) == "Missing key in configuration file: 'music_source_dir'" + assert str(excinfo.value) == f"Missing key fuse_mount_dir in configuration file ({path})" + + +def test_config_value_validation() -> None: + config = "" + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "config.toml" + path.touch() + + def write(x: str) -> None: + with path.open("w") as fp: + fp.write(x) + + # music_source_dir + write("music_source_dir = 123") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for music_source_dir in configuration file ({path}): must be a path" + ) + config += '\nmusic_source_dir = "~/.music-src"' + + # fuse_mount_dir + write(config + "\nfuse_mount_dir = 123") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_mount_dir in configuration file ({path}): must be a path" + ) + config += '\nfuse_mount_dir = "~/music"' + + # cache_dir + write(config + "\ncache_dir = 123") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for cache_dir in configuration file ({path}): must be a path" + ) + config += '\ncache_dir = "~/.cache/rose"' + + # max_proc + write(config + '\nmax_proc = "lalala"') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for max_proc in configuration file ({path}): must be a positive integer" # noqa + ) + config += "\nmax_proc = 8" + + # artist_aliases + write(config + '\nartist_aliases = "lalala"') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + write(config + '\nartist_aliases = ["lalala"]') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + write(config + '\nartist_aliases = [["lalala"]]') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + write(config + '\nartist_aliases = [["lalala", "lalala"]]') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + write(config + '\nartist_aliases = [["lalala", "lalala", "lalala"]]') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + write(config + '\nartist_aliases = [["lalala", [123]]]') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for artist_aliases in configuration file ({path}): must be a list of (parent: str, aliases: list[str]) tuples" # noqa + ) + config += '\nartist_aliases = [["tripleS", ["EVOLution"]]]' + + # fuse_hide_artists + write(config + '\nfuse_hide_artists = "lalala"') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_artists in configuration file ({path}): must be a list of strings" # noqa + ) + write(config + "\nfuse_hide_artists = [123]") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_artists in configuration file ({path}): must be a list of strings" # noqa + ) + config += '\nfuse_hide_artists = ["xxx"]' + + # fuse_hide_genres + write(config + '\nfuse_hide_genres = "lalala"') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_genres in configuration file ({path}): must be a list of strings" # noqa + ) + write(config + "\nfuse_hide_genres = [123]") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_genres in configuration file ({path}): must be a list of strings" # noqa + ) + config += '\nfuse_hide_genres = ["xxx"]' + + # fuse_hide_labels + write(config + '\nfuse_hide_labels = "lalala"') + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_labels in configuration file ({path}): must be a list of strings" # noqa + ) + write(config + "\nfuse_hide_labels = [123]") + with pytest.raises(InvalidConfigValueError) as excinfo: + Config.read(config_path_override=path) + assert ( + str(excinfo.value) + == f"Invalid value for fuse_hide_labels in configuration file ({path}): must be a list of strings" # noqa + ) + config += '\nfuse_hide_labels = ["xxx"]'