(Work-in-progress) Update config file reading and validation

.pre-commit-config.yaml

@@ -31,7 +31,7 @@ repos:
     hooks:
       - id: mypy
         exclude: (docs/.*)
-        additional_dependencies: ["types-filelock", "types-freezegun", "types-pkg_resources"]
+        additional_dependencies: ["attrs", "types-filelock", "types-freezegun", "types-pkg_resources"]
 
   - repo: https://github.com/PyCQA/flake8
     rev: 7.0.0

CHANGES.md

@@ -8,6 +8,11 @@
 
 - Updated documentation for `[mirror]` configuration options `PR #1669`
 
+## Bug Fixes
+
+- Don't write a diff file unless the 'diff-file' option is set in the configuration file `PR #1694`
+- Correctly interpret references with surrounding text when using custom reference syntax in 'diff-file' `PR #1694`
+
 # 6.5.0
 
 ## New Features

src/bandersnatch/config/__init__.py

@@ -0,0 +1,4 @@
+# flake8: noqa
+from .core import BandersnatchConfig
+from .errors import ConfigurationError, InvalidValueError, MissingOptionError
+from .mirror_options import MirrorOptions

src/bandersnatch/config/attrs_utils.py

@@ -0,0 +1,72 @@
+from collections.abc import Callable
+from configparser import ConfigParser
+from typing import Any, TypeAlias, TypeVar
+
+import attrs
+from attrs import Attribute
+
+from . import errors
+
+AttrsConverter: TypeAlias = Callable[[Any], Any]
+AttrsFieldTransformer: TypeAlias = Callable[[type, list[Attribute]], list[Attribute]]
+
+_V = TypeVar("_V")
+AttrsValidator: TypeAlias = Callable[[Any, Attribute, _V], _V]
+
+
+def only_if_str(converter_fn: AttrsConverter) -> AttrsConverter:
+    """Wrap an attrs converter function so it is only applied to strings.
+
+    'converter' functions on attrs fields are applied to all values set to the field,
+    *including* the default value. This causes problems if the default value is already
+    the desired type but the converter only handles strings.
+
+    :param AttrsConverter converter_fn: any attrs converter
+    :return AttrsConverter: converter function that uses `converter_fn` if the passed
+        value is a string, and otherwise returns the value unmodified.
+    """
+
+    def _apply_if_str(value: Any) -> Any:
+        if isinstance(value, str):
+            return converter_fn(value)
+        else:
+            return value
+
+    return _apply_if_str
+
+
+def get_name_value_for_option(
+    config: ConfigParser, section_name: str, option: Attribute
+) -> tuple[str, object | None]:
+    option_name = config.optionxform(option.alias or option.name)
+
+    if option.default is attrs.NOTHING and not config.has_option(
+        section_name, option_name
+    ):
+        raise errors.MissingOptionError.for_option(section_name, option_name)
+
+    getter: Callable[..., Any]
+    if option.converter is not None:
+        getter = config.get
+    elif option.type == bool:
+        getter = config.getboolean
+    elif option.type == float:
+        getter = config.getfloat
+    elif option.type == int:
+        getter = config.getint
+    else:
+        getter = config.get
+
+    try:
+        option_value = getter(section_name, option_name, fallback=None)
+    except ValueError as conversion_error:
+        type_name = option.type.__name__ if option.type else "???"
+        message = f"can't convert option name '{option_name}' to expected type '{type_name}': {conversion_error!s}"
+        raise errors.InvalidValueError.for_option(
+            section_name, option_name, message
+        ) from conversion_error
+
+    return option_name, option_value
+
+
+validate_not_empty: AttrsValidator = attrs.validators.min_len(1)

src/bandersnatch/config/comparison_method.py

@@ -0,0 +1,30 @@
+"""Enumeration of supported file comparison strategies"""
+
+import sys
+
+if sys.version_info >= (3, 11):
+    from enum import StrEnum
+else:
+    from bandersnatch.utils import StrEnum
+
+
+class ComparisonMethod(StrEnum):
+    HASH = "hash"
+    STAT = "stat"
+
+
+class InvalidComparisonMethod(ValueError):
+    """We don't have a valid comparison method choice from configuration"""
+
+    pass
+
+
+def get_comparison_value(method: str) -> ComparisonMethod:
+    try:
+        return ComparisonMethod(method)
+    except ValueError:
+        valid_methods = sorted(v.value for v in ComparisonMethod)
+        raise InvalidComparisonMethod(
+            f"{method} is not a valid file comparison method. "
+            + f"Valid options are: {valid_methods}"
+        )

src/bandersnatch/config/core.py

@@ -0,0 +1,63 @@
+import importlib.resources
+import shutil
+import sys
+from collections.abc import Mapping
+from configparser import ConfigParser, ExtendedInterpolation
+from pathlib import Path
+from typing import Protocol, TypeVar, cast
+
+if sys.version_info >= (3, 11):
+    from typing import Self
+else:
+    Self = TypeVar("Self", bound="ConfigModel")
+
+
+class ConfigModel(Protocol):
+
+    @classmethod
+    def from_config_parser(cls: type[Self], source: ConfigParser) -> Self: ...
+
+
+_C = TypeVar("_C", bound=ConfigModel)
+
+
+class BandersnatchConfig(ConfigParser):
+
+    def __init__(self, defaults: Mapping[str, str] | None = None) -> None:
+        super().__init__(
+            defaults=defaults,
+            delimiters=("=",),
+            strict=True,
+            interpolation=ExtendedInterpolation(),
+        )
+
+        self._validate_config_models: dict[str, ConfigModel] = {}
+
+    # This allows writing option names in the config file with either '_' or '-' as word separators
+    def optionxform(self, optionstr: str) -> str:
+        return optionstr.lower().replace("-", "_")
+
+    def read_path(self, file_path: Path) -> None:
+        with file_path.open() as cfg_file:
+            self.read_file(cfg_file)
+
+    def get_validated(self, model: type[_C]) -> _C:
+        name = model.__qualname__
+        if name not in self._validate_config_models:
+            self._validate_config_models[name] = model.from_config_parser(self)
+        return cast(_C, self._validate_config_models[name])
+
+    @classmethod
+    def from_path(
+        cls, source_file: Path, *, defaults: Mapping[str, str] | None = None
+    ) -> "BandersnatchConfig":
+        config = cls(defaults=defaults)
+        config.read_path(source_file)
+        return config
+
+
+def write_default_config_file(dest: Path) -> None:
+    with importlib.resources.path(
+        "bandersnatch", "default.conf"
+    ) as default_config_path:
+        shutil.copy(default_config_path, dest)

src/bandersnatch/config/diff_file_reference.py

@@ -0,0 +1,60 @@
+"""
+Custom configparser section/option reference syntax for the diff-file option.
+
+diff-file supports a "section reference" syntax for it's value:
+
+    [mirror]
+    ...
+    diff-file = /folder{{ <SECTION>_<OPTION> }}/more
+
+<SECTION> matches a configparser section and <OPTION> matches an option in that section.
+The portion of the diff-file value delimited with {{ and }} is replaced with the
+value from the specified option, which should be a string.
+
+The configparser module's ExtendedInterpolation feature is preferred to this custom syntax.
+"""
+
+import re
+from configparser import ConfigParser, NoOptionError, NoSectionError
+
+# Pattern to check if a string contains a custom section reference
+_LEGACY_REF_PAT = r".*\{\{.+_.+\}\}.*"
+
+# Pattern to decompose a custom section reference into parts
+_LEGACY_REF_COMPONENT_PAT = r"""
+    # capture everything from start-of-line to the opening '{{' braces into group 'pre'
+    ^(?P<pre>.*)\{\{
+    # restrict section names to ignore surrounding whitespace (different from
+    # configparser's default SECTRE) and disallow '_' (since that's our separator)
+    \s*
+    (?P<section>[^_\s](?:[^_]*[^_\s]))
+    # allow (but ignore) whitespace around the section-option delimiter
+    \s*_\s*
+    # option names ignore surrounding whitespace and can include any character, but
+    # must start and end with a non-whitespace character
+    (?P<option>\S(?:.*\S)?)
+    \s*
+    # capture from the closing '}}' braces to end-of-line into group 'post'
+    \}\}(?P<post>.*)$
+"""
+
+
+def has_legacy_config_ref(value: str) -> bool:
+    return re.match(_LEGACY_REF_PAT, value) is not None
+
+
+def eval_legacy_config_ref(config: ConfigParser, value: str) -> str:
+    match_result = re.match(_LEGACY_REF_COMPONENT_PAT, value, re.VERBOSE)
+
+    if match_result is None:
+        raise ValueError(f"Unable to parse config option reference from '{value}'")
+
+    pre, sect_name, opt_name, post = match_result.group(
+        "pre", "section", "option", "post"
+    )
+
+    try:
+        ref_value = config.get(sect_name, opt_name)
+        return pre + ref_value + post
+    except (NoSectionError, NoOptionError) as exc:
+        raise ValueError(exc.message)

src/bandersnatch/config/errors.py

@@ -0,0 +1,29 @@
+import sys
+
+if sys.version_info >= (3, 11):
+    from typing import Self
+else:
+    from typing import TypeVar
+
+    Self = TypeVar("Self", bound="ConfigurationError")
+
+
+class ConfigurationError(Exception):
+
+    @classmethod
+    def for_section(cls: type[Self], section: str, message: str) -> Self:
+        return cls(f"Configuration error in [{section}] section: {message}")
+
+
+class MissingOptionError(ConfigurationError):
+
+    @classmethod
+    def for_option(cls: type[Self], section: str, option: str) -> Self:
+        return cls.for_section(section, f"missing required option '{option}'")
+
+
+class InvalidValueError(ConfigurationError):
+
+    @classmethod
+    def for_option(cls: type[Self], section: str, option: str, info: str) -> Self:
+        return cls.for_section(section, f"invalid value for option '{option}': {info}")