diff --git a/src/server/models/config.py b/src/server/models/config.py index 0f14fbf..610c463 100644 --- a/src/server/models/config.py +++ b/src/server/models/config.py @@ -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.""" diff --git a/src/server/services/config_service.py b/src/server/services/config_service.py index 405ec44..cda39fc 100644 --- a/src/server/services/config_service.py +++ b/src/server/services/config_service.py @@ -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: diff --git a/tests/unit/test_config_service.py b/tests/unit/test_config_service.py index c24b80c..58ba037 100644 --- a/tests/unit/test_config_service.py +++ b/tests/unit/test_config_service.py @@ -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) diff --git a/tests/unit/test_scheduler_config_model.py b/tests/unit/test_scheduler_config_model.py index 0437ba8..bd16a03 100644 --- a/tests/unit/test_scheduler_config_model.py +++ b/tests/unit/test_scheduler_config_model.py @@ -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