From 879588e252db9d0c899429fe63a7880e53aa6f13 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Fri, 20 Feb 2026 12:14:50 +0800 Subject: [PATCH] refactor(settings): invert sync logic from whitelist to blacklist Replace _SYNC_KEYS (37 keys) with _NO_SYNC_KEYS (5 keys) in SettingsHandler. New settings automatically sync to frontend unless explicitly excluded. Changes: - SettingsHandler now syncs all settings except those in _NO_SYNC_KEYS - Added keys() method to SettingsManager for iteration - Updated tests to use new behavior Benefits: - No more missing keys when adding new settings - Reduced maintenance burden - Explicit exclusions for sensitive/internal settings only Fixes: #86 --- py/routes/handlers/misc_handlers.py | 60 ++++++------------- py/services/settings_manager.py | 4 ++ .../__snapshots__/test_api_snapshots.ambr | 1 + tests/routes/test_api_snapshots.py | 3 + tests/routes/test_misc_routes.py | 20 ++++++- 5 files changed, 42 insertions(+), 46 deletions(-) diff --git a/py/routes/handlers/misc_handlers.py b/py/routes/handlers/misc_handlers.py index 46b71e85..94e48c2e 100644 --- a/py/routes/handlers/misc_handlers.py +++ b/py/routes/handlers/misc_handlers.py @@ -220,45 +220,17 @@ class HealthCheckHandler: class SettingsHandler: """Sync settings between backend and frontend.""" - _SYNC_KEYS = ( - "civitai_api_key", - "default_lora_root", - "default_checkpoint_root", - "default_unet_root", - "default_embedding_root", - "base_model_path_mappings", - "download_path_templates", - "enable_metadata_archive_db", - "language", - "use_portable_settings", - "onboarding_completed", - "dismissed_banners", - "proxy_enabled", - "proxy_type", - "proxy_host", - "proxy_port", - "proxy_username", - "proxy_password", - "example_images_path", - "optimize_example_images", - "auto_download_example_images", - "blur_mature_content", - "autoplay_on_hover", - "display_density", - "card_info_display", - "show_folder_sidebar", - "include_trigger_words", - "show_only_sfw", - "compact_mode", - "priority_tags", - "model_card_footer_action", - "model_name_display", - "update_flag_strategy", - "auto_organize_exclusions", - "metadata_refresh_skip_paths", - "filter_presets", - "hide_early_access_updates", - ) + # Settings keys that should NOT be synced to frontend. + # All other settings are synced by default. + _NO_SYNC_KEYS = frozenset({ + # Internal/performance settings (not used by frontend) + "hash_chunk_size_mb", + "download_stall_timeout_seconds", + # Complex internal structures retrieved via separate endpoints + "folder_paths", + "libraries", + "active_library", + }) _PROXY_KEYS = { "proxy_enabled", @@ -305,10 +277,12 @@ class SettingsHandler: async def get_settings(self, request: web.Request) -> web.Response: try: response_data = {} - for key in self._SYNC_KEYS: - value = self._settings.get(key) - if value is not None: - response_data[key] = value + # Sync all settings except those in _NO_SYNC_KEYS + for key in self._settings.keys(): + if key not in self._NO_SYNC_KEYS: + value = self._settings.get(key) + if value is not None: + response_data[key] = value settings_file = getattr(self._settings, "settings_file", None) if settings_file: response_data["settings_file"] = settings_file diff --git a/py/services/settings_manager.py b/py/services/settings_manager.py index 85497e4f..a02293a2 100644 --- a/py/services/settings_manager.py +++ b/py/services/settings_manager.py @@ -994,6 +994,10 @@ class SettingsManager: self._save_settings() logger.info(f"Deleted setting: {key}") + def keys(self) -> Iterable[str]: + """Return all setting keys.""" + return self.settings.keys() + def _prepare_portable_switch(self, use_portable: bool) -> None: """Prepare switching the settings storage location.""" diff --git a/tests/routes/__snapshots__/test_api_snapshots.ambr b/tests/routes/__snapshots__/test_api_snapshots.ambr index 36c4814a..83a386bf 100644 --- a/tests/routes/__snapshots__/test_api_snapshots.ambr +++ b/tests/routes/__snapshots__/test_api_snapshots.ambr @@ -26,6 +26,7 @@ 'settings': dict({ 'civitai_api_key': 'test-key', 'language': 'en', + 'theme': 'dark', }), 'success': True, }) diff --git a/tests/routes/test_api_snapshots.py b/tests/routes/test_api_snapshots.py index 029d65be..90239b85 100644 --- a/tests/routes/test_api_snapshots.py +++ b/tests/routes/test_api_snapshots.py @@ -45,6 +45,9 @@ class DummySettings: def set(self, key, value): self.data[key] = value + def keys(self): + return self.data.keys() + async def noop_async(*_args, **_kwargs): """No-op async function.""" diff --git a/tests/routes/test_misc_routes.py b/tests/routes/test_misc_routes.py index bdd3448e..1cf4dad1 100644 --- a/tests/routes/test_misc_routes.py +++ b/tests/routes/test_misc_routes.py @@ -44,6 +44,9 @@ class DummySettings: def delete(self, key): self.data.pop(key, None) + def keys(self): + return self.data.keys() + class DummyDownloader: def __init__(self): @@ -62,8 +65,14 @@ async def dummy_downloader_factory(): @pytest.mark.asyncio -async def test_get_settings_filters_sync_keys(): - settings_service = DummySettings({"civitai_api_key": "abc", "extraneous": "value"}) +async def test_get_settings_excludes_no_sync_keys(): + """Verify that settings in _NO_SYNC_KEYS are not synced, but others are.""" + settings_service = DummySettings({ + "civitai_api_key": "abc", + "hash_chunk_size_mb": 10, + "folder_paths": {"/some/path"}, + "regular_setting": "value", + }) handler = SettingsHandler( settings_service=settings_service, metadata_provider_updater=noop_async, @@ -74,7 +83,12 @@ async def test_get_settings_filters_sync_keys(): payload = json.loads(response.text) assert payload["success"] is True - assert payload["settings"] == {"civitai_api_key": "abc"} + # Regular settings should be synced + assert payload["settings"]["civitai_api_key"] == "abc" + assert payload["settings"]["regular_setting"] == "value" + # _NO_SYNC_KEYS should not be synced + assert "hash_chunk_size_mb" not in payload["settings"] + assert "folder_paths" not in payload["settings"] @pytest.mark.asyncio