From 28af1ec2d83c71bb7edf03f466c073d3935c12e8 Mon Sep 17 00:00:00 2001 From: Pymetheus Date: Sun, 13 Sep 2026 21:47:24 +0200 Subject: [PATCH] refactor: refactor settings loading and logging This change moves settings to nested API/database sections, adds TOML-backed configuration with env overrides, and uses nested-delimited env names like DATABASE__HOST. It also tightens validation for ports and secrets, updates logger wiring and masking, and refreshes tests to cover the new behavior. --- .config/.env.example | 4 +- .config/config.dev.toml | 2 +- src/package_name/core/config.py | 76 +++++++++++--------- src/package_name/core/logger.py | 1 - src/package_name/main.py | 2 +- tests/conftest.py | 16 +---- tests/test_config.py | 68 ++++-------------- tests/test_logger.py | 121 ++++++++++++++++++++++++-------- 8 files changed, 154 insertions(+), 136 deletions(-) diff --git a/.config/.env.example b/.config/.env.example index 532537c..ce8d1c2 100644 --- a/.config/.env.example +++ b/.config/.env.example @@ -1,2 +1,2 @@ -API_KEY=secret_example_123 -DATABASE_URL=sqlite:///./example.db +API__KEY=secret_example_123 +DATABASE__URL=sqlite:///./example.db diff --git a/.config/config.dev.toml b/.config/config.dev.toml index 77ad6c2..8e54137 100644 --- a/.config/config.dev.toml +++ b/.config/config.dev.toml @@ -4,5 +4,5 @@ app_name = "[[REPO_NAME]]" host = "db.develop.com" port = 5678 -[http] +[api] base_url = "https://api.develop.com" diff --git a/src/package_name/core/config.py b/src/package_name/core/config.py index 5fa1e2e..7d427d1 100644 --- a/src/package_name/core/config.py +++ b/src/package_name/core/config.py @@ -1,14 +1,14 @@ import os -import tomllib from pathlib import Path import structlog -from pydantic import BaseModel, Field, SecretStr -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic import BaseModel, Field, SecretStr, ValidationError +from pydantic_settings import BaseSettings, PydanticBaseSettingsSource, SettingsConfigDict, TomlConfigSettingsSource -logger = structlog.getLogger(__name__) +logger = structlog.get_logger(__name__) -CONFIG_DIR = Path(__file__).resolve().parents[3] / ".config" +PROJECT_ROOT = Path(__file__).resolve().parents[3] +CONFIG_DIR = PROJECT_ROOT / ".config" ENV_MODE: str = os.getenv("APP_ENV", "dev").lower() @@ -18,16 +18,20 @@ class DatabaseSettings(BaseModel): """ host: str = Field(default="localhost") - port: int = Field(default=1111) + port: int = Field(default=5432, ge=1, le=65535) + url: SecretStr = Field(default=SecretStr("sqlite:///./default.db")) -class HTTPSettings(BaseModel): + +class APISettings(BaseModel): """ Configuration schema for HTTP client settings. """ base_url: str = Field(default="https://api.default.com") + key: SecretStr = Field(default=SecretStr("default_secret")) + class Settings(BaseSettings): """ @@ -35,45 +39,53 @@ class Settings(BaseSettings): """ model_config = SettingsConfigDict( - env_file=str(CONFIG_DIR / f".env.{ENV_MODE}"), env_file_encoding="utf-8", extra="ignore" + env_file=str(CONFIG_DIR / f".env.{ENV_MODE}"), + env_file_encoding="utf-8", + env_nested_delimiter="__", + toml_file=CONFIG_DIR / f"config.{ENV_MODE}.toml", + extra="ignore", + frozen=True, ) - # SECRETS - api_key: SecretStr = Field(default=SecretStr("default_secret")) - database_url: SecretStr = Field(default=SecretStr("sqlite:///./default.db")) - # APP Settings app_name: str = Field(default="package_name") # Nested APP Settings database: DatabaseSettings = Field(default_factory=DatabaseSettings) - http: HTTPSettings = Field(default_factory=HTTPSettings) + api: APISettings = Field(default_factory=APISettings) + + @classmethod + def settings_customise_sources( + cls, + settings_cls: type[BaseSettings], + init_settings: PydanticBaseSettingsSource, + env_settings: PydanticBaseSettingsSource, + dotenv_settings: PydanticBaseSettingsSource, + file_secret_settings: PydanticBaseSettingsSource, + ) -> tuple[PydanticBaseSettingsSource, ...]: + return ( + init_settings, + env_settings, + dotenv_settings, + TomlConfigSettingsSource(settings_cls), + file_secret_settings, + ) @classmethod def load(cls) -> "Settings": """ - Load configuration from TOML files and merge with environment variables. + Load configuration from TOML files and environment variables. Returns: - Settings: An initialized configuration instance. - """ - - config_path = CONFIG_DIR / f"config.{ENV_MODE}.toml" - toml_data = {} + Settings: Validated application settings. - if config_path.exists(): - try: - with open(config_path, "rb") as f: - toml_data = tomllib.load(f) - except (tomllib.TOMLDecodeError, PermissionError) as e: - logger.warning("Failed to load TOML data, fallback to defaults", exc_info=e) - except Exception as e: - logger.warning("Unexpected error reading TOML, fallback to defaults", exc_info=e) - else: - logger.warning("Missing config file, fallback to defaults", path=config_path) + Raises: + ValidationError: If configuration values are invalid. + """ try: - return cls(**toml_data) - except Exception as e: - logger.warning("Failed to validate TOML data, fallback to defaults", error=str(e)) return cls() + + except ValidationError: + logger.exception("Invalid application configuration") + raise diff --git a/src/package_name/core/logger.py b/src/package_name/core/logger.py index d7c5904..bb0d09d 100644 --- a/src/package_name/core/logger.py +++ b/src/package_name/core/logger.py @@ -7,7 +7,6 @@ import structlog -EventDict = dict[str, Any] ProcessorType = Any diff --git a/src/package_name/main.py b/src/package_name/main.py index 11a30c4..49e547e 100644 --- a/src/package_name/main.py +++ b/src/package_name/main.py @@ -32,7 +32,7 @@ def main() -> None: # Load settings settings = Settings.load() - logger.info("Application started.", host=settings.database.host, api_key=settings.api_key) + logger.info("Application started.", host=settings.database.host, api_key=settings.api.key) logger.info("Reached end of application.") diff --git a/tests/conftest.py b/tests/conftest.py index cc8b5bb..8dbe9b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,15 +1,3 @@ -from unittest.mock import patch +import os -import pytest - - -@pytest.fixture -def clean_env(): - """ - Ensure no environment variables or real .env files interfere. - """ - with ( - patch.dict("os.environ", clear=True), - patch("pydantic_settings.sources.providers.dotenv.DotEnvSettingsSource._read_env_files", return_value={}), - ): - yield +os.environ.setdefault("APP_ENV", "dev") diff --git a/tests/test_config.py b/tests/test_config.py index 852ed42..86383ff 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,66 +1,26 @@ -import tomllib -from unittest.mock import MagicMock, patch +import pytest +from pydantic import ValidationError from src.package_name.core.config import Settings -def test_settings_load_from_toml(clean_env): - """Test that settings correctly load and merge TOML data.""" - mock_toml_dict = {"app_name": "test-app", "database": {"port": 9999}} +def test_load_uses_the_active_environment_configuration(): + settings = Settings.load() - with ( - patch("pathlib.Path.exists", return_value=True), - patch("builtins.open", MagicMock()), - patch("tomllib.load", return_value=mock_toml_dict), - ): - settings = Settings.load() - assert settings.app_name == "test-app" - assert settings.database.port == 9999 - assert settings.database.host == "localhost" + assert settings.app_name == "[[REPO_NAME]]" + assert settings.database.host == "db.develop.com" -def test_settings_invalid_toml_fallback(clean_env): - """Test fallback to defaults if TOML is corrupted.""" +def test_environment_overrides_toml_configuration(monkeypatch): + monkeypatch.setenv("DATABASE__HOST", "override-host") - with ( - patch("pathlib.Path.exists", return_value=True), - patch("builtins.open", MagicMock()), - patch("tomllib.load", side_effect=tomllib.TOMLDecodeError("Bad TOML")), - ): - settings = Settings.load() - assert settings.app_name == "package_name" + settings = Settings.load() + assert settings.database.host == "override-host" -def test_settings_unexpected_toml_fallback(clean_env): - """Test fallback to defaults if unexpected error occurs.""" - with ( - patch("pathlib.Path.exists", return_value=True), - patch("builtins.open", MagicMock()), - patch("tomllib.load", side_effect=Exception), - ): - settings = Settings.load() - assert settings.app_name == "package_name" +def test_invalid_environment_configuration_is_rejected(monkeypatch): + monkeypatch.setenv("DATABASE__PORT", "0") - -def test_settings_default_fallback(clean_env): - """Test that settings return defaults when no file exists.""" - with patch("pathlib.Path.exists", return_value=False): - settings = Settings.load() - assert settings.app_name == "package_name" - - -def test_settings_pydantic_validation_error_fallback(clean_env): - """Test fallback to defaults if TOML data fails Pydantic validation.""" - - invalid_data = {"database": {"port": ["not", "an", "int"]}} - - with ( - patch("pathlib.Path.exists", return_value=True), - patch("builtins.open", MagicMock()), - patch("tomllib.load", return_value=invalid_data), - ): - settings = Settings.load() - - assert settings.database.port == 1111 - assert settings.app_name == "package_name" + with pytest.raises(ValidationError): + Settings.load() diff --git a/tests/test_logger.py b/tests/test_logger.py index 3725283..e03f4a6 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -1,54 +1,113 @@ -import json import logging +from logging.handlers import RotatingFileHandler +from pathlib import Path import structlog -from src.package_name.core.logger import setup_logging +from src.package_name.core.logger import ( + add_environment_info, + mask_sensitive_data, + setup_logging, +) -def test_logging_masking_and_env(capsys): - """Test that sensitive data is masked and env info is added.""" +def test_add_environment_info_uses_app_environment(monkeypatch): + monkeypatch.setenv("APP_ENV", "TEST") - setup_logging(pretty_print=False, level=logging.INFO) - log = structlog.get_logger() + event = add_environment_info(None, "", {}) - log.info("test_event", password="secret_password", user="jdoe") # pragma: allowlist secret + assert event["_env"] == "test" - out, _ = capsys.readouterr() - data = json.loads(out) - assert data["password"] == "********" - assert data["user"] == "jdoe" - assert data["_env"] == "dev" - assert data["event"] == "test_event" +def test_add_environment_info_defaults_to_dev(monkeypatch): + monkeypatch.delenv("APP_ENV", raising=False) + event = add_environment_info(None, "", {}) -def test_foreign_logger_integration(capsys): - """Test that standard logging library calls are also processed/masked.""" + assert event["_env"] == "dev" + + +def test_mask_sensitive_data_masks_sensitive_values(): + event = { + "username": "user", + "password": "secret", # pragma: allowlist secret + "access_token": "token-value", + "api_key": "key-value", # pragma: allowlist secret + "auth_header": "bearer-value", + "message": "hello", + } + + result = mask_sensitive_data(None, "", event) + + assert result == { + "username": "user", + "password": "********", + "access_token": "********", + "api_key": "********", + "auth_header": "********", + "message": "hello", + } + + +def test_mask_sensitive_data_is_case_insensitive(): + event = { + "Password": "secret", # pragma: allowlist secret + "API_TOKEN": "token-value", + "client_SECRET": "secret-value", # pragma: allowlist secret + } + + result = mask_sensitive_data(None, "", event) + + assert all(value == "********" for value in result.values()) - setup_logging(pretty_print=False) - std_log = logging.getLogger("external_lib") - std_log.warning("external_leak", extra={"token": "12345"}) +def test_setup_logging_configures_pretty_console_logging(): + setup_logging(pretty_print=True) - out, _ = capsys.readouterr() - data = json.loads(out) + root_logger = logging.getLogger() - assert data["logger"] == "external_lib" - assert data["token"] == "********" + assert root_logger.level == logging.INFO + assert len(root_logger.handlers) == 1 + assert isinstance(root_logger.handlers[0], logging.StreamHandler) + assert not isinstance(root_logger.handlers[0], logging.FileHandler) -def test_file_logging(tmp_path): - """Test that log files are created when write_to_disk is True.""" +def test_setup_logging_configures_json_console_logging(): + setup_logging(pretty_print=False) + + root_logger = logging.getLogger() + + assert root_logger.level == logging.INFO + assert len(root_logger.handlers) == 1 + assert isinstance(root_logger.handlers[0], logging.StreamHandler) + assert not isinstance(root_logger.handlers[0], logging.FileHandler) - log_dir = tmp_path / ".log" - setup_logging(write_to_disk=True, log_dir=log_dir) - log = structlog.get_logger() - log.info("file_test") +def test_setup_logging_creates_rotating_log_file(tmp_path: Path): + setup_logging( + write_to_disk=True, + log_dir=tmp_path, + pretty_print=False, + ) + + log_file = tmp_path / "logs.json" - log_file = log_dir / "logs.json" assert log_file.exists() + assert log_file.is_file() + + root_logger = logging.getLogger() + + assert len(root_logger.handlers) == 2 + assert any(isinstance(handler, RotatingFileHandler) for handler in root_logger.handlers) + assert any( + isinstance(handler, logging.StreamHandler) and not isinstance(handler, RotatingFileHandler) + for handler in root_logger.handlers + ) + + +def test_setup_logging_configures_structlog(): + setup_logging() + + logger = structlog.get_logger("test") - content = log_file.read_text() - assert "file_test" in content + assert logger is not None