mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-21 03:01:27 -03:00
feat(backend): gate Other Models behind opt-in management toggles
Other Models management is now opt-in: enable_other_models (default false) plus the enabled_other_sub_types allow-list replace the unreleased additive enabled_other_folders key. - config._get_enabled_other_folder_keys() is the single scan gate; a new refresh_other_roots() rebuilds roots and preview roots on toggle. - ModelScanner gains a _should_keep_cached_entry() hydration hook and on_library_changed(reconcile=...) so switching a sub_type off drops its entries (and hash/autov3 rows) at load time and switching it on rescans. - OtherScanner filters location-derived entries accordingly. - Other routes reject every other type while off (or a disabled sub_type) and expose an "other_disabled" page flag; download routing returns a disabled marker instead of guessing; the download manager refuses other-type downloads and default-path routing for switched-off sub_types. - Doctor / init-status / refresh-all skip the other scanner while off; the scanner stays registered so staged pending-deletes still merge. - Tests updated with explicit opt-in fixtures plus new gating coverage.
This commit is contained in:
@@ -26,6 +26,14 @@ def _make_config(**overrides) -> config_module.Config:
|
||||
return config
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def enable_other_models():
|
||||
"""Other Models is opt-in; enable it for the enabled-state tests."""
|
||||
manager = get_settings_manager()
|
||||
manager.set("enable_other_models", True)
|
||||
yield
|
||||
|
||||
|
||||
class TestPrepareOtherPaths:
|
||||
"""Unit tests for Config._prepare_other_paths."""
|
||||
|
||||
@@ -196,7 +204,7 @@ class TestInitOtherPaths:
|
||||
controlnet_dir.mkdir()
|
||||
|
||||
self._stub_folder_paths(monkeypatch, {"controlnet": str(controlnet_dir)})
|
||||
get_settings_manager().set("enabled_other_folders", ["controlnet"])
|
||||
get_settings_manager().set("enabled_other_sub_types", ["controlnet"])
|
||||
|
||||
config = _make_config()
|
||||
roots = config._init_other_paths()
|
||||
@@ -207,12 +215,45 @@ class TestInitOtherPaths:
|
||||
== "controlnet"
|
||||
)
|
||||
|
||||
def test_disabled_sub_type_is_not_scanned(self, monkeypatch, tmp_path):
|
||||
vae_dir = tmp_path / "vae"
|
||||
upscaler_dir = tmp_path / "upscale_models"
|
||||
vae_dir.mkdir()
|
||||
upscaler_dir.mkdir()
|
||||
|
||||
self._stub_folder_paths(
|
||||
monkeypatch, {"vae": str(vae_dir), "upscale_models": str(upscaler_dir)}
|
||||
)
|
||||
get_settings_manager().set("enabled_other_sub_types", ["vae"])
|
||||
|
||||
config = _make_config()
|
||||
roots = config._init_other_paths()
|
||||
|
||||
assert roots == [_normalize(str(vae_dir))]
|
||||
assert _normalize(str(upscaler_dir)) not in config.other_root_subtypes
|
||||
|
||||
def test_feature_disabled_scans_nothing(self, monkeypatch, tmp_path):
|
||||
vae_dir = tmp_path / "vae"
|
||||
vae_dir.mkdir()
|
||||
|
||||
self._stub_folder_paths(monkeypatch, {"vae": str(vae_dir)})
|
||||
get_settings_manager().set("enable_other_models", False)
|
||||
|
||||
config = _make_config()
|
||||
roots = config._init_other_paths()
|
||||
|
||||
assert roots == []
|
||||
assert config.other_root_subtypes == {}
|
||||
assert config.other_folder_roots == {}
|
||||
|
||||
def test_unknown_opt_in_keys_are_ignored(self, monkeypatch, tmp_path):
|
||||
vae_dir = tmp_path / "vae"
|
||||
vae_dir.mkdir()
|
||||
|
||||
self._stub_folder_paths(monkeypatch, {"vae": str(vae_dir)})
|
||||
get_settings_manager().set("enabled_other_folders", ["not_a_real_key", 42])
|
||||
get_settings_manager().set(
|
||||
"enabled_other_sub_types", ["vae", "not_a_real_key", 42]
|
||||
)
|
||||
|
||||
config = _make_config()
|
||||
roots = config._init_other_paths()
|
||||
|
||||
@@ -5,6 +5,22 @@ import json
|
||||
import pytest
|
||||
|
||||
from py.routes.handlers.download_routing_handlers import DownloadRoutingHandler
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def enable_other_models():
|
||||
"""Other Models is opt-in; enable every sub_type for the routing tests."""
|
||||
manager = get_settings_manager()
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.settings["enabled_other_sub_types"] = [
|
||||
"vae",
|
||||
"upscaler",
|
||||
"text_encoder",
|
||||
"clip_vision",
|
||||
"controlnet",
|
||||
]
|
||||
yield
|
||||
|
||||
|
||||
class FakeRequest:
|
||||
@@ -149,3 +165,32 @@ async def test_other_invalid_selected_file_type_rejected():
|
||||
FakeRequest({"model_type": "VAE", "selected_file_type": 123})
|
||||
)
|
||||
assert response.status == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_other_routing_disabled_when_feature_off():
|
||||
get_settings_manager().settings["enable_other_models"] = False
|
||||
|
||||
handler = DownloadRoutingHandler()
|
||||
response = await handler.get_download_routing(
|
||||
FakeRequest({"model_type": "VAE", "file_types": ["Model"]})
|
||||
)
|
||||
payload = json.loads(response.text)
|
||||
assert payload["sub_type"] is None
|
||||
assert payload["disabled"] is True
|
||||
assert payload["reason"] == "other_models_disabled"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_other_routing_disabled_for_switched_off_sub_type():
|
||||
get_settings_manager().settings["enabled_other_sub_types"] = ["vae"]
|
||||
|
||||
handler = DownloadRoutingHandler()
|
||||
response = await handler.get_download_routing(
|
||||
FakeRequest({"model_type": "Upscaler", "file_types": ["Model"]})
|
||||
)
|
||||
payload = json.loads(response.text)
|
||||
assert payload["sub_type"] is None
|
||||
assert payload["disabled"] is True
|
||||
assert payload["reason"] == "other_sub_type_disabled"
|
||||
assert payload["requested_sub_type"] == "upscaler"
|
||||
|
||||
@@ -61,6 +61,12 @@ class DummySettings:
|
||||
def get(self, key, default=None):
|
||||
return self.data.get(key, default)
|
||||
|
||||
def is_other_models_enabled(self):
|
||||
return bool(self.data.get("enable_other_models", False))
|
||||
|
||||
def get_enabled_other_sub_types(self):
|
||||
return list(self.data.get("enabled_other_sub_types") or [])
|
||||
|
||||
def set(self, key, value):
|
||||
self.data[key] = value
|
||||
|
||||
|
||||
@@ -30,6 +30,20 @@ def routes():
|
||||
return handler
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def enable_other_models():
|
||||
"""Other Models is opt-in; these tests exercise the enabled state."""
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
manager = get_settings_manager()
|
||||
manager.set("enable_other_models", True)
|
||||
manager.set(
|
||||
"enabled_other_sub_types",
|
||||
["vae", "upscaler", "text_encoder", "clip_vision", "controlnet"],
|
||||
)
|
||||
yield
|
||||
|
||||
|
||||
def test_common_and_specific_routes_registered():
|
||||
"""Registration smoke test: /api/lm/other/* surface plus the /other page."""
|
||||
app = web.Application()
|
||||
@@ -64,6 +78,39 @@ def test_validate_civitai_model_type_rejects_foreign_types(model_type):
|
||||
assert OtherRoutes()._validate_civitai_model_type(model_type) is False
|
||||
|
||||
|
||||
def test_validate_rejects_everything_when_feature_disabled():
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
get_settings_manager().set("enable_other_models", False)
|
||||
|
||||
handler = OtherRoutes()
|
||||
for model_type in ("VAE", "Upscaler", "TextEncoder", "CLIPVision", "Other"):
|
||||
assert handler._validate_civitai_model_type(model_type) is False
|
||||
|
||||
|
||||
def test_validate_rejects_switched_off_sub_type():
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
get_settings_manager().set("enabled_other_sub_types", ["vae"])
|
||||
|
||||
handler = OtherRoutes()
|
||||
assert handler._validate_civitai_model_type("VAE") is True
|
||||
assert handler._validate_civitai_model_type("Upscaler") is False
|
||||
|
||||
|
||||
def test_page_context_reports_feature_state():
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
manager = get_settings_manager()
|
||||
handler = OtherRoutes()
|
||||
provider = handler._get_page_context_provider()
|
||||
|
||||
assert provider(None) == {"other_disabled": False}
|
||||
|
||||
manager.set("enable_other_models", False)
|
||||
assert provider(None) == {"other_disabled": True}
|
||||
|
||||
|
||||
def test_get_expected_model_types_mentions_supported_types():
|
||||
expected = OtherRoutes()._get_expected_model_types()
|
||||
for name in ("VAE", "Upscaler", "TextEncoder", "CLIPVision", "Controlnet"):
|
||||
|
||||
@@ -43,6 +43,14 @@ def isolate_settings(monkeypatch, tmp_path):
|
||||
"text_encoder": str(tmp_path / "text_encoders"),
|
||||
"clip_vision": str(tmp_path / "clip_vision"),
|
||||
},
|
||||
"enable_other_models": True,
|
||||
"enabled_other_sub_types": [
|
||||
"vae",
|
||||
"upscaler",
|
||||
"text_encoder",
|
||||
"clip_vision",
|
||||
"controlnet",
|
||||
],
|
||||
"download_path_templates": {
|
||||
"lora": "{base_model}/{first_tag}",
|
||||
"checkpoint": "{base_model}/{first_tag}",
|
||||
@@ -231,6 +239,40 @@ async def test_download_rejects_unknown_model_type(
|
||||
assert result["error"].startswith("Model type")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_rejects_other_when_feature_disabled(
|
||||
monkeypatch, scanners, metadata_provider, tmp_path
|
||||
):
|
||||
"""The opt-in feature is off: no other-type download is accepted."""
|
||||
metadata_provider.payload = _other_payload("VAE")
|
||||
get_settings_manager().settings["enable_other_models"] = False
|
||||
|
||||
manager = DownloadManager()
|
||||
result = await manager.download_from_civitai(
|
||||
model_version_id=99, save_dir=str(tmp_path)
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert "disabled" in result["error"].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_paths_reject_switched_off_sub_type(
|
||||
monkeypatch, scanners, metadata_provider, tmp_path
|
||||
):
|
||||
"""A disabled sub_type refuses default-path routing (manual pick still works)."""
|
||||
metadata_provider.payload = _other_payload("VAE")
|
||||
get_settings_manager().settings["enabled_other_sub_types"] = ["upscaler"]
|
||||
|
||||
manager = DownloadManager()
|
||||
result = await manager.download_from_civitai(
|
||||
model_version_id=99, use_default_paths=True
|
||||
)
|
||||
|
||||
assert result["success"] is False
|
||||
assert "disabled" in result["error"].lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_early_gate_checks_other_scanner(
|
||||
monkeypatch, scanners, metadata_provider, tmp_path
|
||||
|
||||
@@ -173,6 +173,51 @@ class TestOtherScannerRoots:
|
||||
assert result["sub_type"] == "upscaler"
|
||||
|
||||
|
||||
class TestOtherScannerHydrationFilter:
|
||||
"""Persisted entries for roots that are no longer managed are dropped."""
|
||||
|
||||
def test_keeps_entries_under_enabled_roots(self, other_config):
|
||||
scanner = _make_scanner()
|
||||
assert (
|
||||
scanner._should_keep_cached_entry(
|
||||
{"file_path": f"{other_config['vae']}/model.safetensors"}
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_drops_entries_under_disabled_root(self, other_config, monkeypatch):
|
||||
scanner = _make_scanner()
|
||||
# Only vae stays managed; the upscaler root disappeared from the map.
|
||||
monkeypatch.setattr(
|
||||
config_module.config,
|
||||
"other_root_subtypes",
|
||||
{other_config["vae"]: "vae"},
|
||||
)
|
||||
|
||||
assert (
|
||||
scanner._should_keep_cached_entry(
|
||||
{"file_path": f"{other_config['upscaler']}/model.safetensors"}
|
||||
)
|
||||
is False
|
||||
)
|
||||
assert (
|
||||
scanner._should_keep_cached_entry(
|
||||
{"file_path": f"{other_config['vae']}/model.safetensors"}
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_drops_everything_when_feature_off(self, monkeypatch):
|
||||
monkeypatch.setattr(config_module.config, "other_root_subtypes", {})
|
||||
scanner = _make_scanner()
|
||||
assert (
|
||||
scanner._should_keep_cached_entry(
|
||||
{"file_path": "/models/vae/model.safetensors"}
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
class TestOtherScannerLazyHash:
|
||||
"""Lazy hashing: pending by default, singleflight on-demand calculation."""
|
||||
|
||||
|
||||
@@ -2513,7 +2513,7 @@ async def test_on_library_changed_bumps_cache_version(tmp_path: Path, monkeypatc
|
||||
scanner = DummyScanner(str(tmp_path))
|
||||
assert scanner.cache_version == 0
|
||||
|
||||
async def _noop_initialize() -> None:
|
||||
async def _noop_initialize(reconcile: bool = False) -> None:
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(scanner, "initialize_in_background", _noop_initialize)
|
||||
|
||||
@@ -1222,7 +1222,33 @@ def test_default_other_roots_stay_empty_without_other_folders(manager):
|
||||
assert manager.get("default_other_roots") == {}
|
||||
|
||||
|
||||
def test_other_models_disabled_by_default(manager):
|
||||
assert manager.is_other_models_enabled() is False
|
||||
assert manager.get_enabled_other_sub_types() == []
|
||||
assert manager.is_other_sub_type_enabled("vae") is False
|
||||
|
||||
|
||||
def test_auto_set_default_other_roots_skipped_when_feature_off(manager):
|
||||
manager.settings["enable_other_models"] = False
|
||||
manager.settings["default_other_roots"] = {}
|
||||
manager.settings["folder_paths"] = {"vae": ["/vae"]}
|
||||
|
||||
manager._auto_set_default_roots()
|
||||
|
||||
assert manager.get("default_other_roots") == {}
|
||||
|
||||
|
||||
def test_set_enabled_other_sub_types_normalizes(manager):
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.set("enabled_other_sub_types", ["controlnet", "vae", "nope", "vae", 42])
|
||||
|
||||
assert manager.get("enabled_other_sub_types") == ["vae", "controlnet"]
|
||||
assert manager.is_other_sub_type_enabled("vae") is True
|
||||
assert manager.is_other_sub_type_enabled("upscaler") is False
|
||||
|
||||
|
||||
def test_auto_set_default_other_roots(manager):
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.settings["default_other_roots"] = {}
|
||||
manager.settings["folder_paths"] = {
|
||||
"vae": ["/vae"],
|
||||
@@ -1243,6 +1269,7 @@ def test_auto_set_default_other_roots(manager):
|
||||
|
||||
def test_auto_set_default_other_roots_text_encoder_dual_key_union(manager):
|
||||
"""text_encoder candidates merge text_encoders and the legacy clip key."""
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.settings["default_other_roots"] = {}
|
||||
manager.settings["folder_paths"] = {
|
||||
"clip": ["/legacy-clip"],
|
||||
@@ -1261,6 +1288,7 @@ def test_auto_set_default_other_roots_text_encoder_dual_key_union(manager):
|
||||
|
||||
|
||||
def test_auto_set_default_other_roots_repairs_stale(manager):
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.settings["default_other_roots"] = {"vae": "/stale-vae"}
|
||||
manager.settings["folder_paths"] = {"vae": ["/vae"]}
|
||||
|
||||
@@ -1270,6 +1298,7 @@ def test_auto_set_default_other_roots_repairs_stale(manager):
|
||||
|
||||
|
||||
def test_auto_set_default_other_roots_uses_extra_folder_paths(manager):
|
||||
manager.settings["enable_other_models"] = True
|
||||
manager.settings["default_other_roots"] = {}
|
||||
manager.settings["folder_paths"] = {"vae": []}
|
||||
manager.settings["extra_folder_paths"] = {"vae": ["/extra-vae"]}
|
||||
|
||||
Reference in New Issue
Block a user