mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-08 10:32:13 -03:00
feat(sidebar): scan a single folder from the folder context menu (#1108)
The Refresh menu can scope a scan to one model root, but the folder a user is
looking at lives in the sidebar's unified tree, which merges every root into one
relative-path namespace. "Scan this folder" therefore addresses the folder, not a
root: the backend walks that relative path under every root that holds it, which
is also what makes the action safe while another drive is switched off.
Backend:
* GET /scan accepts `folder=<rel>` (alone or with `roots=`), rejected together
with full_rebuild=true like the roots parameter. Validation reuses
normalize_relative_folder(), extracted to module scope from ModelMoveService so
the folder operations and the scan endpoint reject the same input (absolute
paths, drive letters, `..` climbing) instead of each carrying its own copy.
* The reconcile summary carries `scope_label` (the folder) for a folder scope, so
the result toast names the folder the user clicked instead of the roots it
happens to live under; the completed WS payload carries it too.
* `folder` is a scope prefix exactly like a root: only that subtree is re-read or
pruned, and an unreachable root keeps the entries that fall inside it.
Frontend:
* The sidebar folder context menu gains "Scan this folder" above "Check for
updates in this folder" (they share the refresh divider); the entry is gated by
the same supportsFolderManagement flag as the other folder operations.
* SidebarManager.scanFolder() resolves the node through the existing
_resolveFolderCandidates() before doing anything: a folder no root holds any
more explains itself ("no longer exists on disk") instead of scanning nothing,
and an unresolvable multi-root node is refused rather than guessed.
* PageControls.refreshModels() and BaseModelApi.refreshModels() forward the folder
scope, and _showRefreshSummary() prefers scope_label over the walked roots.
Verified live on the three-root sandbox with one drive switched off:
GET /scan?folder=pack000 walks drive-G and drive-Y, reports scope_label=pack000,
keeps drive-Z's 6 entries under that folder (kept_unreachable=6) and leaves all
420 models cached. 3704 passed, 7 skipped; frontend 1495 passed (150 files); vue
widgets 96 passed. The 2 new sidebar keys are [TODO: Translate] placeholders
pending the feature owner's go-ahead.
This commit is contained in:
@@ -436,6 +436,43 @@ describe('BaseModelApiClient.refreshModels scan progress', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('requests a folder scan without a root', async () => {
|
||||
global.fetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ status: 'success' }),
|
||||
});
|
||||
|
||||
const client = await createClient();
|
||||
await client.refreshModels(false, { folder: 'pack/sub' });
|
||||
|
||||
const [url] = global.fetch.mock.calls[0];
|
||||
// The backend walks this relative path under every root that holds it.
|
||||
expect(url.searchParams.get('folder')).toBe('pack/sub');
|
||||
expect(url.searchParams.getAll('roots')).toEqual([]);
|
||||
});
|
||||
|
||||
it('names a folder scan by the folder, not by the roots it walked', async () => {
|
||||
const fetchControl = mockFetchPending();
|
||||
const client = await createClient();
|
||||
const { promise } = await startRefresh(client, false, { folder: 'pack' });
|
||||
|
||||
fetchControl.resolveOk({
|
||||
status: 'success',
|
||||
scope_label: 'pack',
|
||||
scanned_roots: ['a/loras', 'usb/loras'],
|
||||
added: 3,
|
||||
removed: 0,
|
||||
kept_unreachable: 0,
|
||||
});
|
||||
await promise;
|
||||
|
||||
expect(showToastMock).toHaveBeenCalledWith(
|
||||
'toast.api.refreshCompleteScoped',
|
||||
{ scope: 'pack', added: 3, removed: 0 },
|
||||
'success'
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the generic completion toast for a full-library scan', async () => {
|
||||
const fetchControl = mockFetchPending();
|
||||
const client = await createClient();
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
import { describe, it, beforeEach, expect, vi } from 'vitest';
|
||||
|
||||
const {
|
||||
SIDEBAR_MANAGER_MODULE,
|
||||
STORAGE_HELPERS_MODULE,
|
||||
MODEL_API_FACTORY_MODULE,
|
||||
I18N_MODULE,
|
||||
BULK_MANAGER_MODULE,
|
||||
UI_HELPERS_MODULE,
|
||||
UPDATE_CHECK_MODULE,
|
||||
STATE_MODULE,
|
||||
MODAL_MANAGER_MODULE,
|
||||
} = vi.hoisted(() => ({
|
||||
SIDEBAR_MANAGER_MODULE: new URL('../../../static/js/components/SidebarManager.js', import.meta.url).pathname,
|
||||
STORAGE_HELPERS_MODULE: new URL('../../../static/js/utils/storageHelpers.js', import.meta.url).pathname,
|
||||
MODEL_API_FACTORY_MODULE: new URL('../../../static/js/api/modelApiFactory.js', import.meta.url).pathname,
|
||||
I18N_MODULE: new URL('../../../static/js/utils/i18nHelpers.js', import.meta.url).pathname,
|
||||
BULK_MANAGER_MODULE: new URL('../../../static/js/managers/BulkManager.js', import.meta.url).pathname,
|
||||
UI_HELPERS_MODULE: new URL('../../../static/js/utils/uiHelpers.js', import.meta.url).pathname,
|
||||
UPDATE_CHECK_MODULE: new URL('../../../static/js/utils/updateCheckHelpers.js', import.meta.url).pathname,
|
||||
STATE_MODULE: new URL('../../../static/js/state/index.js', import.meta.url).pathname,
|
||||
MODAL_MANAGER_MODULE: new URL('../../../static/js/managers/ModalManager.js', import.meta.url).pathname,
|
||||
}));
|
||||
|
||||
vi.mock(MODEL_API_FACTORY_MODULE, () => ({ getModelApiClient: vi.fn() }));
|
||||
vi.mock(I18N_MODULE, () => ({ translate: (key, _args, fallback) => fallback || key }));
|
||||
vi.mock(BULK_MANAGER_MODULE, () => ({ bulkManager: {} }));
|
||||
vi.mock(UI_HELPERS_MODULE, () => ({ showToast: vi.fn(), showActionToast: vi.fn() }));
|
||||
vi.mock(UPDATE_CHECK_MODULE, () => ({ performFolderUpdateCheck: vi.fn() }));
|
||||
vi.mock(MODAL_MANAGER_MODULE, () => ({
|
||||
modalManager: { showModal: vi.fn(), closeModal: vi.fn() },
|
||||
}));
|
||||
|
||||
const { SidebarManager } = await import(SIDEBAR_MANAGER_MODULE);
|
||||
const { showToast } = await import(UI_HELPERS_MODULE);
|
||||
|
||||
function createApiClient(overrides = {}) {
|
||||
return {
|
||||
apiConfig: {
|
||||
config: {
|
||||
displayName: 'LoRA',
|
||||
singularName: 'lora',
|
||||
supportsMove: true,
|
||||
supportsFolderManagement: true,
|
||||
},
|
||||
},
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({ roots: ['/models/loras', '/mnt/usb/loras'] }),
|
||||
resolveFolder: vi.fn().mockResolvedValue({
|
||||
success: true,
|
||||
folder: 'pack',
|
||||
candidates: [
|
||||
{ folder_path: '/models/loras/pack', root: '/models/loras', is_symlink: false },
|
||||
{ folder_path: '/mnt/usb/loras/pack', root: '/mnt/usb/loras', is_symlink: false },
|
||||
],
|
||||
}),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function createManager(apiClient) {
|
||||
const manager = new SidebarManager();
|
||||
manager.pageType = 'loras';
|
||||
manager.apiClient = apiClient;
|
||||
manager.renderFolderDisplay = vi.fn();
|
||||
manager.renderEmptyState = vi.fn();
|
||||
return manager;
|
||||
}
|
||||
|
||||
describe('SidebarManager folder scan', () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
document.body.innerHTML = '';
|
||||
showToast.mockClear();
|
||||
});
|
||||
|
||||
it('scans the relative folder instead of guessing a root', async () => {
|
||||
const apiClient = createApiClient();
|
||||
const manager = createManager(apiClient);
|
||||
const refreshModels = vi.fn().mockResolvedValue(undefined);
|
||||
manager.pageControls = { refreshModels };
|
||||
|
||||
await manager.scanFolder('pack');
|
||||
|
||||
// The backend walks this relative path under every root that holds it, so
|
||||
// the request carries no root at all.
|
||||
expect(refreshModels).toHaveBeenCalledWith(false, { folder: 'pack' });
|
||||
expect(showToast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the registered page controls after a re-init', async () => {
|
||||
const apiClient = createApiClient();
|
||||
const manager = createManager(apiClient);
|
||||
manager.pageControls = null;
|
||||
manager.lastPageControls = { refreshModels: vi.fn().mockResolvedValue(undefined) };
|
||||
|
||||
await manager.scanFolder('pack');
|
||||
|
||||
expect(manager.lastPageControls.refreshModels).toHaveBeenCalledWith(false, { folder: 'pack' });
|
||||
});
|
||||
|
||||
it('explains a folder that no root holds any more', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockResolvedValue({ success: true, folder: 'gone', candidates: [] }),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
const refreshModels = vi.fn();
|
||||
manager.pageControls = { refreshModels };
|
||||
|
||||
await manager.scanFolder('gone');
|
||||
|
||||
expect(refreshModels).not.toHaveBeenCalled();
|
||||
expect(showToast).toHaveBeenCalledWith('sidebar.scanFolderResult.missing', {}, 'warning');
|
||||
});
|
||||
|
||||
it('reports a failed resolution without scanning', async () => {
|
||||
const apiClient = createApiClient({
|
||||
resolveFolder: vi.fn().mockRejectedValue(new Error('offline')),
|
||||
fetchModelRoots: vi.fn().mockResolvedValue({ roots: ['/models/loras', '/mnt/usb/loras'] }),
|
||||
});
|
||||
const manager = createManager(apiClient);
|
||||
const refreshModels = vi.fn();
|
||||
manager.pageControls = { refreshModels };
|
||||
|
||||
await manager.scanFolder('pack');
|
||||
|
||||
// Multi-root + no resolver: guessing a root is exactly what the folder
|
||||
// operations refuse to do, and a scan must not guess either.
|
||||
expect(refreshModels).not.toHaveBeenCalled();
|
||||
expect(showToast).toHaveBeenCalledWith('sidebar.scanFolderResult.missing', {}, 'warning');
|
||||
});
|
||||
|
||||
it('does nothing when no page controls are registered', async () => {
|
||||
const apiClient = createApiClient();
|
||||
const manager = createManager(apiClient);
|
||||
manager.pageControls = null;
|
||||
manager.lastPageControls = null;
|
||||
|
||||
await manager.scanFolder('pack');
|
||||
|
||||
expect(showToast).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -2,11 +2,12 @@ import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'fs';
|
||||
import path from 'path';
|
||||
|
||||
// Regression guard for the sidebar folder context-menu layout: the update check
|
||||
// sits on top, the folder operations form a single group, and the destructive
|
||||
// entry stays last behind its own divider. SidebarManager gates those groups
|
||||
// per page and collapses the dividers when a group is hidden, so a reorder here
|
||||
// also changes what the recipes page shows.
|
||||
// Regression guard for the sidebar folder context-menu layout: the two refresh
|
||||
// actions (scan this folder, check for updates) sit on top, the folder
|
||||
// operations form a single group, and the destructive entry stays last behind
|
||||
// its own divider. SidebarManager gates those groups per page and collapses the
|
||||
// dividers when a group is hidden, so a reorder here also changes what the
|
||||
// recipes page shows.
|
||||
describe('Sidebar folder context menu layout', () => {
|
||||
const repoRoot = path.resolve(__dirname, '../../..');
|
||||
const html = readFileSync(
|
||||
@@ -24,8 +25,9 @@ describe('Sidebar folder context menu layout', () => {
|
||||
return /data-action="([^"]+)"/.exec(rest)?.[1] || null;
|
||||
});
|
||||
|
||||
it('keeps the update check first and the folder operations grouped', () => {
|
||||
it('keeps the refresh actions first and the folder operations grouped', () => {
|
||||
expect(sequence).toEqual([
|
||||
'scan-folder',
|
||||
'check-folder-updates',
|
||||
'separator',
|
||||
'create-subfolder',
|
||||
|
||||
@@ -307,6 +307,65 @@ async def test_scan_models_rejects_roots_with_full_rebuild():
|
||||
assert service.received_scope == "not-called"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scan_models_accepts_folder_param():
|
||||
service = ScopedScanService(roots=["/mnt/a", "/mnt/b"], summary=SUMMARY)
|
||||
handler = ModelQueryHandler(service=service, logger=logging.getLogger(__name__))
|
||||
|
||||
response = await handler.scan_models(
|
||||
SimpleNamespace(query=QueryParams({"folder": "pack\\sub"}))
|
||||
)
|
||||
|
||||
assert response.status == 200
|
||||
assert service.received_scope is not None
|
||||
# Backslashes are normalized and no root is pinned: the backend walks this
|
||||
# relative folder under every root that holds it.
|
||||
assert service.received_scope.folder == "pack/sub"
|
||||
assert service.received_scope.roots is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scan_models_rejects_absolute_folder():
|
||||
service = ScopedScanService(roots=["/mnt/a"], summary=SUMMARY)
|
||||
handler = ModelQueryHandler(service=service, logger=logging.getLogger(__name__))
|
||||
|
||||
for folder in ("/mnt/a/pack", "../pack", "C:/models/pack"):
|
||||
response = await handler.scan_models(
|
||||
SimpleNamespace(query=QueryParams({"folder": folder}))
|
||||
)
|
||||
assert response.status == 400, folder
|
||||
assert "error" in json.loads(response.text)
|
||||
|
||||
assert service.received_scope == "not-called"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scan_models_rejects_folder_with_full_rebuild():
|
||||
service = ScopedScanService(roots=["/mnt/a"], summary=SUMMARY)
|
||||
handler = ModelQueryHandler(service=service, logger=logging.getLogger(__name__))
|
||||
|
||||
response = await handler.scan_models(
|
||||
SimpleNamespace(query=QueryParams({"folder": "pack", "full_rebuild": "true"}))
|
||||
)
|
||||
|
||||
assert response.status == 400
|
||||
assert service.received_scope == "not-called"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scan_models_combines_folder_and_roots():
|
||||
service = ScopedScanService(roots=["/mnt/a", "/mnt/b"], summary=SUMMARY)
|
||||
handler = ModelQueryHandler(service=service, logger=logging.getLogger(__name__))
|
||||
|
||||
response = await handler.scan_models(
|
||||
SimpleNamespace(query=QueryParams({"folder": "pack", "roots": ["/mnt/b"]}))
|
||||
)
|
||||
|
||||
assert response.status == 200
|
||||
assert service.received_scope.folder == "pack"
|
||||
assert service.received_scope.roots == ("/mnt/b",)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_model_roots_reports_details():
|
||||
service = ScopedScanService(roots=["/mnt/a", "/mnt/b"])
|
||||
|
||||
@@ -2609,6 +2609,46 @@ async def test_reconcile_folder_scope_keeps_folder_tree_outside_scope(tmp_path:
|
||||
assert {item["file_path"] for item in scanner._cache.raw_data} == set()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_folder_scope_covers_every_root_holding_it(tmp_path: Path):
|
||||
"""The sidebar's unified tree has no root identity: a folder scope means
|
||||
"this relative folder under every root that holds it"."""
|
||||
primary = tmp_path / "primary"
|
||||
usb = tmp_path / "usb"
|
||||
for root in (primary, usb):
|
||||
(root / "pack").mkdir(parents=True)
|
||||
(root / "other").mkdir()
|
||||
(root / "other" / "keep.txt").write_text("k", encoding="utf-8")
|
||||
(primary / "pack" / "a.txt").write_text("a", encoding="utf-8")
|
||||
(usb / "pack" / "b.txt").write_text("b", encoding="utf-8")
|
||||
# Only the primary root holds this folder; the usb copy is gone.
|
||||
(primary / "only-primary").mkdir()
|
||||
(primary / "only-primary" / "c.txt").write_text("c", encoding="utf-8")
|
||||
(usb / "only-primary").mkdir()
|
||||
|
||||
scanner = MultiRootDummyScanner([primary, usb])
|
||||
await scanner._initialize_cache()
|
||||
|
||||
(usb / "pack" / "b.txt").unlink()
|
||||
(primary / "only-primary" / "c.txt").unlink()
|
||||
(primary / "pack" / "new.txt").write_text("n", encoding="utf-8")
|
||||
|
||||
summary = await scanner._reconcile_cache(scope=ReconcileScope(folder="pack"))
|
||||
|
||||
assert summary is not None
|
||||
assert summary["scope_label"] == "pack"
|
||||
assert summary["added"] == 1
|
||||
assert summary["removed"] == 1
|
||||
paths = {item["file_path"] for item in scanner._cache.raw_data}
|
||||
# Both copies of the scoped folder were walked...
|
||||
assert _normalize_path(primary / "pack" / "new.txt") in paths
|
||||
assert _normalize_path(usb / "pack" / "b.txt") not in paths
|
||||
# ...and nothing outside it was touched, including the folder that only one
|
||||
# root holds.
|
||||
assert _normalize_path(usb / "other" / "keep.txt") in paths
|
||||
assert _normalize_path(primary / "only-primary" / "c.txt") in paths
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_keeps_entries_under_unreadable_dir(tmp_path: Path, monkeypatch):
|
||||
"""A directory os.walk cannot enter (permissions, I/O error, offline
|
||||
|
||||
Reference in New Issue
Block a user