refactor: centralize filename extraction callsites
This commit is contained in:
@@ -28,6 +28,7 @@ class DownloadService:
|
||||
"get_default_root_for_category"
|
||||
)
|
||||
self.get_download_directory = context.require("get_download_directory")
|
||||
self.get_filename_from_path = context.require("get_filename_from_path")
|
||||
self.get_override_settings_from_request = context.require(
|
||||
"get_override_settings_from_request"
|
||||
)
|
||||
@@ -103,7 +104,7 @@ class DownloadService:
|
||||
download_host = urlparse(url).hostname
|
||||
|
||||
if not filename:
|
||||
filename = unquote(urlparse(url).path.split("/")[-1])
|
||||
filename = self.get_filename_from_path(unquote(urlparse(url).path))
|
||||
filename = self.sanitize_download_filename(filename)
|
||||
|
||||
if not filename:
|
||||
|
||||
@@ -6,6 +6,7 @@ from unittest.mock import AsyncMock, MagicMock
|
||||
import pytest
|
||||
from aiohttp import web
|
||||
|
||||
from core.path_utils import get_filename_from_path
|
||||
from core.routes.context import RouteContext
|
||||
from core.routes.downloads import register_download_routes
|
||||
from core.type_utils import to_bool, to_int
|
||||
@@ -68,6 +69,7 @@ def _build_download_routes(overrides=None):
|
||||
),
|
||||
"get_default_root_for_category": MagicMock(return_value=r"C:\base"),
|
||||
"get_download_directory": MagicMock(return_value=r"C:\models"),
|
||||
"get_filename_from_path": get_filename_from_path,
|
||||
"get_override_settings_from_request": AsyncMock(
|
||||
return_value={"aria2c_path": "aria2c"}
|
||||
),
|
||||
@@ -228,6 +230,27 @@ async def test_download_route_supports_huggingface_headers_and_optional_inputs()
|
||||
values["get_default_root_for_category"].assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_route_derives_encoded_filename_from_url_path():
|
||||
handlers, values = _build_download_routes()
|
||||
handler = handlers[("POST", "/model_resolver/download")]
|
||||
|
||||
response = await handler(
|
||||
_request(
|
||||
{
|
||||
"url": "https://example.com/download/model%20name.safetensors",
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
body = json.loads(response.text)
|
||||
assert response.status == 200
|
||||
assert body["filename"] == "model name.safetensors"
|
||||
assert values["start_background_download"].call_args.kwargs["filename"] == (
|
||||
"model name.safetensors"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_route_handles_existing_civitai_token_and_path_errors():
|
||||
handlers, values = _build_download_routes()
|
||||
|
||||
@@ -5236,6 +5236,9 @@ test('downloads tab renders workflow label for active downloads', () => {
|
||||
getDownloadFolderContext() {
|
||||
return null;
|
||||
},
|
||||
getFilenameFromPath(value = '') {
|
||||
return String(value).split(/[\\/]/).at(-1) || '';
|
||||
},
|
||||
getDownloadDisplayProgress(progress) {
|
||||
return {
|
||||
percent: progress.progress || 0,
|
||||
@@ -5266,6 +5269,68 @@ test('downloads tab renders workflow label for active downloads', () => {
|
||||
|
||||
assert.match(html, /<span>Workflow<\/span>/);
|
||||
assert.match(html, /Workflow B/);
|
||||
|
||||
const fallbackHtml = renderQueueDownloadsHtml.call(dialog, [{
|
||||
downloadId: 'download-2',
|
||||
info: {
|
||||
missing: {
|
||||
original_path: 'models\\fallback.safetensors',
|
||||
node_id: 9,
|
||||
widget_index: 1,
|
||||
node_type: 'CheckpointLoaderSimple',
|
||||
category: 'checkpoints',
|
||||
},
|
||||
lastProgress: { status: 'downloading', progress: 10 },
|
||||
},
|
||||
}]);
|
||||
|
||||
assert.match(fallbackHtml, /fallback\.safetensors/);
|
||||
});
|
||||
|
||||
test('download queue context derives fallback names from Windows paths', () => {
|
||||
const getDownloadQueueContext = eval(`(${extractMethod(queueMethodsSource, 'getDownloadQueueContext')})`);
|
||||
const dialog = {
|
||||
getDownloadFolderContext() {
|
||||
return null;
|
||||
},
|
||||
getFilenameFromPath(value = '') {
|
||||
return String(value).split(/[\\/]/).at(-1) || '';
|
||||
},
|
||||
getDownloadWorkflowLabel() {
|
||||
return '';
|
||||
},
|
||||
getWorkflowContextId() {
|
||||
return 'workflow-1';
|
||||
},
|
||||
getWorkflowContextRouteKey() {
|
||||
return 'workflow-route-1';
|
||||
},
|
||||
canSwitchToDownloadWorkflow() {
|
||||
return true;
|
||||
},
|
||||
};
|
||||
|
||||
const context = getDownloadQueueContext.call(dialog, {}, {
|
||||
missing: { original_path: 'models\\fallback.safetensors' },
|
||||
}, '', 'download-1');
|
||||
|
||||
assert.equal(context.name, 'fallback.safetensors');
|
||||
});
|
||||
|
||||
test('context menu model data derives fallback names from Windows paths', () => {
|
||||
const buildContextMenuModelData = eval(`(${extractMethod(searchPanelMethodsSource, 'buildContextMenuModelData')})`);
|
||||
const dialog = {
|
||||
getFilenameFromPath(value = '') {
|
||||
return String(value).split(/[\\/]/).at(-1) || '';
|
||||
},
|
||||
};
|
||||
|
||||
const model = buildContextMenuModelData.call(dialog, {}, '', {
|
||||
resolved_path: 'models\\fallback.safetensors',
|
||||
});
|
||||
|
||||
assert.equal(model.name, 'fallback.safetensors');
|
||||
assert.equal(model.original_path, 'fallback.safetensors');
|
||||
});
|
||||
|
||||
test('active download refresh preserves existing cards for hover and tooltip state', () => {
|
||||
|
||||
@@ -239,15 +239,18 @@ test('queue storage remembers completed downloads using progress and missing-mod
|
||||
...queueStorageMethods,
|
||||
downloadHistoryStorageKey: 'completed-history',
|
||||
updateQueuePanel() {},
|
||||
getFilenameFromPath(value = '') {
|
||||
return String(value).split(/[\\/]/).at(-1) || '';
|
||||
},
|
||||
getDownloadWorkflowLabel: () => 'Workflow A',
|
||||
getCategoryDisplayName: category => category.toUpperCase(),
|
||||
};
|
||||
|
||||
const entry = context.rememberCompletedDownloadHistory(
|
||||
'download-1',
|
||||
{
|
||||
missing: {
|
||||
original_path: 'loras/fallback.safetensors',
|
||||
{
|
||||
missing: {
|
||||
original_path: 'loras\\fallback.safetensors',
|
||||
category: 'loras',
|
||||
node_id: 7,
|
||||
widget_index: 2,
|
||||
|
||||
@@ -857,7 +857,7 @@ export const queueMethods = {
|
||||
context_scope: 'download_queue',
|
||||
download_id: downloadId,
|
||||
open_folder_label: folderContext ? 'Open Download Folder' : '',
|
||||
name: folderContext?.name || progress?.filename || info?.filename || missing?.original_path?.split(/[/\\]/).pop() || 'Download',
|
||||
name: folderContext?.name || progress?.filename || info?.filename || this.getFilenameFromPath(missing?.original_path) || 'Download',
|
||||
workflow_id: info.workflowId || info.workflow_id || this.getWorkflowContextId(info) || '',
|
||||
workflow_label: label,
|
||||
workflow_route_key: info.workflowRouteKey || this.getWorkflowContextRouteKey(info) || '',
|
||||
@@ -1202,7 +1202,7 @@ export const queueMethods = {
|
||||
const filename = progress.filename
|
||||
|| info.filename
|
||||
|| info.missing?.download_source?.filename
|
||||
|| info.missing?.original_path?.split(/[/\\]/).pop()
|
||||
|| this.getFilenameFromPath(info.missing?.original_path)
|
||||
|| 'model';
|
||||
const category = this.getCategoryDisplayName?.(info.category || info.missing?.category || '') || info.category || '';
|
||||
const nodeLabel = info.missing?.subgraph_name || info.missing?.node_type || (info.missing?.subgraph_id ? 'Subgraph' : 'Node');
|
||||
|
||||
@@ -190,7 +190,7 @@ export const queueStorageMethods = {
|
||||
const filename = progress.filename
|
||||
|| info.filename
|
||||
|| missing.download_source?.filename
|
||||
|| missing.original_path?.split(/[/\\]/).pop()
|
||||
|| this.getFilenameFromPath(missing.original_path)
|
||||
|| '';
|
||||
if (!filename) return null;
|
||||
|
||||
|
||||
@@ -2104,7 +2104,7 @@ export const searchPanelMethods = {
|
||||
...(extra && typeof extra === 'object' ? extra : {})
|
||||
};
|
||||
const resolvedPath = merged.path || merged.resolved_path || '';
|
||||
const filename = merged.filename || fallbackName || resolvedPath.split(/[/\\]/).pop() || '';
|
||||
const filename = merged.filename || fallbackName || this.getFilenameFromPath(resolvedPath) || '';
|
||||
return {
|
||||
...merged,
|
||||
name: merged.name || filename,
|
||||
|
||||
Reference in New Issue
Block a user