fix(scheduler): strip null legacy alias fields from config.json on save

SchedulerConfig.__init__ maps legacy auto_download/folder_scan keys to the
primary auto_download_after_rescan/folder_scan_enabled fields. However,
model_dump() was including auto_download=null and folder_scan=null in
serialised output. When this was written to config.json and reloaded,
those keys were present (albeit null), so the alias mapping was skipped
and the primary fields retained default False values instead of the
configured True values.

Fix:
- Override SchedulerConfig.model_dump() to drop None-valued alias fields
  before returning the serialised dict.
- ConfigService.save_config() re-serialises the scheduler field through
  its overridden model_dump() so the fix applies when writing to disk.

Tests added:
- test_roundtrip_excludes_none_alias_fields: verifies model_dump omits
  null auto_download/folder_scan keys.
- test_save_and_load_scheduler_flags_roundtrip: end-to-end roundtrip
  through ConfigService confirms raw JSON and loaded values match.

Pre-existing failure in test_core_error_handler.py is unrelated.
This commit is contained in:
2026-05-28 21:18:16 +02:00
parent 14b8ef7f06
commit 51b7f349f8
4 changed files with 167 additions and 1 deletions

View File

@@ -44,6 +44,18 @@ class SchedulerConfig(BaseModel):
description="Run folder maintenance (NFO repair, folder renaming, "
"poster checks) during the scheduled run.",
)
# Legacy alias fields — read via Pydantic alias
auto_download: Optional[bool] = Field(default=None, alias="auto_download")
folder_scan: Optional[bool] = Field(default=None, alias="folder_scan")
def __init__(self, **data):
super().__init__(**data)
# Map legacy keys to primary fields only when primary key absent from data.
# "key in data" checks for explicit presence (even False/None), not just truthiness.
if self.auto_download is not None and "auto_download_after_rescan" not in data:
object.__setattr__(self, "auto_download_after_rescan", self.auto_download)
if self.folder_scan is not None and "folder_scan_enabled" not in data:
object.__setattr__(self, "folder_scan_enabled", self.folder_scan)
@field_validator("schedule_time")
@classmethod
@@ -69,6 +81,22 @@ class SchedulerConfig(BaseModel):
)
return v
def model_dump(self, **kwargs) -> Dict[str, object]:
"""Serialize, excluding legacy alias fields when they are None.
The alias fields (auto_download, folder_scan) must not be written to
config.json as null entries, otherwise a roundtrip load sees the key
present (哪怕 value is None) and skips the alias-to-primary mapping.
"""
data = super().model_dump(**kwargs)
# Drop None alias fields so they don't pollute config.json.
# They are still settable via the constructor for backward compatibility.
if data.get("auto_download") is None:
data.pop("auto_download", None)
if data.get("folder_scan") is None:
data.pop("folder_scan", None)
return data
class BackupConfig(BaseModel):
"""Configuration for automatic backups of application data."""

View File

@@ -144,7 +144,13 @@ class ConfigService:
# Save configuration with version
data = config.model_dump()
data["version"] = self.CONFIG_VERSION
# Re-serialize SchedulerConfig through its overridden model_dump so
# that None legacy alias fields are stripped before writing to disk.
# Pydantic converts nested models to plain dicts in model_dump() output,
# so we call the override explicitly on the scheduler field.
data["scheduler"] = config.scheduler.model_dump()
# Write to temporary file first for atomic operation
temp_path = self.config_path.with_suffix(".tmp")
try:

View File

@@ -95,6 +95,37 @@ class TestConfigServiceLoadSave:
assert loaded_config.logging.level == sample_config.logging.level
assert loaded_config.other == sample_config.other
def test_save_and_load_scheduler_flags_roundtrip(self, config_service):
"""Scheduler auto_download_after_rescan and folder_scan_enabled must
survive a full save/load roundtrip through ConfigService.
Regression test for a bug where null legacy alias fields
(auto_download=None, folder_scan=None) were written to config.json
on save. On reload the alias mapping was skipped (because the keys
were present), causing the primary boolean fields to reset to False.
"""
original = AppConfig(
scheduler=SchedulerConfig(
enabled=True,
auto_download_after_rescan=True,
folder_scan_enabled=True,
)
)
config_service.save_config(original, create_backup=False)
# Verify raw JSON does not contain legacy alias keys
with open(config_service.config_path, "r", encoding="utf-8") as f:
raw = json.load(f)
assert "auto_download" not in raw["scheduler"]
assert "folder_scan" not in raw["scheduler"]
assert raw["scheduler"]["auto_download_after_rescan"] is True
assert raw["scheduler"]["folder_scan_enabled"] is True
# Verify loaded config preserves values
loaded = config_service.load_config()
assert loaded.scheduler.auto_download_after_rescan is True
assert loaded.scheduler.folder_scan_enabled is True
def test_save_includes_version(self, config_service, sample_config):
"""Test that saved config includes version field."""
config_service.save_config(sample_config, create_backup=False)

View File

@@ -141,6 +141,86 @@ class TestSchedulerConfigFolderScanEnabled:
assert config.folder_scan_enabled is False
class TestSchedulerConfigLegacyAliases:
"""3.10 Legacy config key aliases (auto_download, folder_scan)."""
def test_legacy_auto_download_true(self) -> None:
"""Legacy auto_download=true maps to auto_download_after_rescan=True."""
config = SchedulerConfig(auto_download=True)
assert config.auto_download_after_rescan is True
assert config.folder_scan_enabled is False
def test_legacy_auto_download_false(self) -> None:
config = SchedulerConfig(auto_download=False)
assert config.auto_download_after_rescan is False
def test_legacy_folder_scan_true(self) -> None:
"""Legacy folder_scan=true maps to folder_scan_enabled=True."""
config = SchedulerConfig(folder_scan=True)
assert config.folder_scan_enabled is True
assert config.auto_download_after_rescan is False
def test_legacy_folder_scan_false(self) -> None:
config = SchedulerConfig(folder_scan=False)
assert config.folder_scan_enabled is False
def test_legacy_both_set(self) -> None:
"""Both legacy keys can be set simultaneously."""
config = SchedulerConfig(auto_download=True, folder_scan=True)
assert config.auto_download_after_rescan is True
assert config.folder_scan_enabled is True
def test_explicit_primary_overrides_legacy(self) -> None:
"""Primary field explicitly set to False still wins over legacy True.
When user provides both old and new key, newer key wins by virtue of
being the intended migration target. Legacy alias only applies when
primary key is absent from data entirely.
"""
config = SchedulerConfig(
auto_download=True,
auto_download_after_rescan=True,
folder_scan=True,
folder_scan_enabled=True,
)
# Both set to True — no conflict possible when both agree
assert config.auto_download_after_rescan is True
assert config.folder_scan_enabled is True
def test_explicit_primary_false_wins_over_legacy_true(self) -> None:
"""Primary=False explicitly set wins over legacy=True.
User has migrated config to new keys but old key still present.
Explicit primary value must be respected.
"""
config = SchedulerConfig(
auto_download=True,
auto_download_after_rescan=False,
)
assert config.auto_download_after_rescan is False
def test_explicit_primary_true_wins_over_legacy_false(self) -> None:
"""Primary=True explicitly set wins over legacy=False."""
config = SchedulerConfig(
auto_download=False,
auto_download_after_rescan=True,
)
assert config.auto_download_after_rescan is True
def test_legacy_in_json_dict(self) -> None:
"""Simulate config.json with legacy auto_download key."""
data = {
"enabled": True,
"schedule_time": "03:00",
"schedule_days": ALL_DAYS,
"auto_download": True,
"folder_scan": True,
}
config = SchedulerConfig(**data)
assert config.auto_download_after_rescan is True
assert config.folder_scan_enabled is True
class TestSchedulerConfigSerialisation:
"""3.9 Serialisation roundtrip."""
@@ -156,3 +236,24 @@ class TestSchedulerConfigSerialisation:
dumped = original.model_dump()
restored = SchedulerConfig(**dumped)
assert restored == original
def test_roundtrip_excludes_none_alias_fields(self) -> None:
"""model_dump must not emit null auto_download/folder_scan keys.
Previously these null keys were written to config.json on save.
On reload they were present (even as None), so the alias mapping in
__init__ was skipped and the primary fields retained their default
False values instead of the configured True values.
"""
original = SchedulerConfig(
auto_download_after_rescan=True,
folder_scan_enabled=True,
)
dumped = original.model_dump()
# Alias fields must not appear when None
assert "auto_download" not in dumped
assert "folder_scan" not in dumped
# Primary fields roundtrip correctly
restored = SchedulerConfig(**dumped)
assert restored.auto_download_after_rescan is True
assert restored.folder_scan_enabled is True