Skip to content

Commit 709b190

Browse files
dante01yoonampagent
dante01yoon
and
ampagent
committed
fix: remove generated assets from load image options
Amp-Thread-ID: https://ampcode.com/threads/T-019fb910-e134-75f8-9e68-ab218bcabca6 Co-authored-by: Amp <amp@ampcode.com>
1 parent 15a8601 commit 709b190

4 files changed

Lines changed: 326 additions & 22 deletions

File tree

‎src/platform/assets/composables/useMediaAssetActions.test.ts‎

Lines changed: 162 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,14 @@ vi.mock('@/stores/dialogStore', () => ({
5656
const mockInvalidateModelsForCategory = vi.hoisted(() => vi.fn())
5757
const mockSetAssetDeleting = vi.hoisted(() => vi.fn())
5858
const mockUpdateHistory = vi.hoisted(() => vi.fn())
59+
const mockUpdateFlatOutputs = vi.hoisted(() => vi.fn())
5960
const mockUpdateInputs = vi.hoisted(() => vi.fn())
6061
const mockHasCategory = vi.hoisted(() => vi.fn())
6162
vi.mock('@/stores/assetsStore', () => ({
6263
useAssetsStore: () => ({
6364
setAssetDeleting: mockSetAssetDeleting,
6465
updateHistory: mockUpdateHistory,
66+
updateFlatOutputs: mockUpdateFlatOutputs,
6567
updateInputs: mockUpdateInputs,
6668
invalidateModelsForCategory: mockInvalidateModelsForCategory,
6769
hasCategory: mockHasCategory
@@ -146,12 +148,14 @@ vi.mock('../utils/outputAssetUtil', async (importOriginal) => {
146148
})
147149

148150
const mockDeleteAsset = vi.hoisted(() => vi.fn())
151+
const mockGetJobAssetIds = vi.hoisted(() => vi.fn())
149152
const mockCreateAssetExport = vi.hoisted(() =>
150153
vi.fn().mockResolvedValue({ task_id: 'test-task-id', status: 'pending' })
151154
)
152155
vi.mock('../services/assetService', () => ({
153156
assetService: {
154157
deleteAsset: mockDeleteAsset,
158+
getJobAssetIds: mockGetJobAssetIds,
155159
createAssetExport: mockCreateAssetExport
156160
}
157161
}))
@@ -165,7 +169,7 @@ vi.mock('@/stores/assetExportStore', () => ({
165169

166170
vi.mock('@/scripts/api', () => ({
167171
api: {
168-
deleteItem: vi.fn(),
172+
fetchApi: vi.fn(),
169173
apiURL: vi.fn((path: string) => `http://localhost:8188/api${path}`),
170174
internalURL: vi.fn((path: string) => `http://localhost:8188${path}`),
171175
addEventListener: vi.fn(),
@@ -301,6 +305,10 @@ describe('useMediaAssetActions', () => {
301305
mockGetAssetType.mockReturnValue('input')
302306
mockResolveOutputAssetItems.mockReset()
303307
mockResolveOutputAssetItems.mockResolvedValue([])
308+
mockGetJobAssetIds.mockResolvedValue([])
309+
vi.mocked(api.fetchApi).mockResolvedValue(
310+
fromAny({ ok: true, status: 200 })
311+
)
304312
})
305313

306314
describe('addWorkflow', () => {
@@ -1245,7 +1253,7 @@ describe('useMediaAssetActions', () => {
12451253
)
12461254
})
12471255

1248-
it('deletes via the history API in OSS instead of failing as an imported file', async () => {
1256+
it('deletes the history job in OSS without requiring the cloud asset API', async () => {
12491257
const actions = useMediaAssetActions()
12501258
const asset = createMockAsset({
12511259
id: 'job-temp',
@@ -1257,13 +1265,162 @@ describe('useMediaAssetActions', () => {
12571265
await actions.deleteAssets(asset)
12581266

12591267
await vi.waitFor(() => {
1260-
expect(vi.mocked(api.deleteItem)).toHaveBeenCalledWith(
1261-
'history',
1262-
'job-temp'
1268+
expect(vi.mocked(api.fetchApi)).toHaveBeenCalledWith(
1269+
'/history',
1270+
expect.objectContaining({
1271+
method: 'POST',
1272+
body: JSON.stringify({ delete: ['job-temp'] })
1273+
})
1274+
)
1275+
})
1276+
expect(mockGetJobAssetIds).not.toHaveBeenCalled()
1277+
expect(mockDeleteAsset).not.toHaveBeenCalled()
1278+
expect(mockUpdateHistory).toHaveBeenCalled()
1279+
expect(mockUpdateFlatOutputs).toHaveBeenCalled()
1280+
})
1281+
})
1282+
1283+
describe('deleteAssets — output asset records', () => {
1284+
beforeEach(() => {
1285+
mockGetAssetType.mockReturnValue('output')
1286+
mockShowDialog.mockImplementation(
1287+
(opts: { props: { onConfirm: () => Promise<void> | void } }) => {
1288+
void opts.props.onConfirm()
1289+
}
1290+
)
1291+
})
1292+
1293+
it.for([false, true])(
1294+
'deletes the history job and exact linked asset records when isCloud=%s',
1295+
async (cloud) => {
1296+
mockIsCloud.value = cloud
1297+
mockGetOutputAssetMetadata.mockReturnValue({ jobId: 'job-1' })
1298+
mockGetJobAssetIds.mockResolvedValue(['output-uuid-1', 'output-uuid-2'])
1299+
mockDeleteAsset.mockResolvedValue(undefined)
1300+
const actions = useMediaAssetActions()
1301+
const asset = createMockAsset({
1302+
id: 'job-1-node-1--generated.png',
1303+
name: 'generated.png',
1304+
hash: 'generated-content-hash',
1305+
tags: ['output'],
1306+
user_metadata: { jobId: 'job-1' }
1307+
})
1308+
1309+
await actions.deleteAssets(asset)
1310+
1311+
expect(vi.mocked(api.fetchApi)).toHaveBeenCalledWith(
1312+
'/history',
1313+
expect.objectContaining({
1314+
body: JSON.stringify({ delete: ['job-1'] })
1315+
})
12631316
)
1317+
expect(mockGetJobAssetIds).toHaveBeenCalledTimes(cloud ? 1 : 0)
1318+
expect(mockDeleteAsset).toHaveBeenCalledTimes(cloud ? 2 : 0)
1319+
if (cloud) {
1320+
expect(mockDeleteAsset).toHaveBeenCalledWith('output-uuid-1')
1321+
expect(mockDeleteAsset).toHaveBeenCalledWith('output-uuid-2')
1322+
expect(mockDeleteAsset.mock.invocationCallOrder[0]).toBeLessThan(
1323+
vi.mocked(api.fetchApi).mock.invocationCallOrder[0]
1324+
)
1325+
}
1326+
expect(mockUpdateFlatOutputs).toHaveBeenCalledOnce()
1327+
}
1328+
)
1329+
1330+
it('falls back to deleting history when linked asset lookup is unavailable', async () => {
1331+
mockIsCloud.value = true
1332+
mockGetJobAssetIds.mockResolvedValue([])
1333+
const actions = useMediaAssetActions()
1334+
const asset = createMockAsset({
1335+
id: 'job-1-node-1--missing.png',
1336+
name: 'missing.png',
1337+
tags: ['output']
12641338
})
1339+
1340+
await actions.deleteAssets(asset)
1341+
12651342
expect(mockDeleteAsset).not.toHaveBeenCalled()
1343+
expect(vi.mocked(api.fetchApi)).toHaveBeenCalledWith(
1344+
'/history',
1345+
expect.objectContaining({
1346+
body: JSON.stringify({ delete: ['job-1-node-1--missing.png'] })
1347+
})
1348+
)
12661349
expect(mockUpdateHistory).toHaveBeenCalled()
1350+
expect(mockUpdateFlatOutputs).toHaveBeenCalled()
1351+
})
1352+
1353+
it('keeps history when linked asset lookup fails', async () => {
1354+
mockIsCloud.value = true
1355+
mockGetJobAssetIds.mockRejectedValue(new Error('lookup failed'))
1356+
const actions = useMediaAssetActions()
1357+
1358+
await actions.deleteAssets(
1359+
createMockAsset({
1360+
id: 'job-1',
1361+
name: 'generated.png',
1362+
tags: ['output']
1363+
})
1364+
)
1365+
1366+
expect(vi.mocked(api.fetchApi)).not.toHaveBeenCalled()
1367+
expect(useToast().add).toHaveBeenCalledWith(
1368+
expect.objectContaining({ severity: 'error' })
1369+
)
1370+
})
1371+
1372+
it('reports asset-record deletion failures and refreshes output stores', async () => {
1373+
mockIsCloud.value = true
1374+
mockGetOutputAssetMetadata.mockReturnValue({ jobId: 'job-1' })
1375+
mockGetJobAssetIds.mockResolvedValue(['output-uuid'])
1376+
mockDeleteAsset.mockRejectedValue(new Error('delete failed'))
1377+
const actions = useMediaAssetActions()
1378+
const asset = createMockAsset({
1379+
id: 'generated-card-id',
1380+
name: 'generated.png',
1381+
tags: ['output']
1382+
})
1383+
1384+
await actions.deleteAssets(asset)
1385+
1386+
expect(vi.mocked(api.fetchApi)).not.toHaveBeenCalled()
1387+
expect(mockUpdateHistory).toHaveBeenCalled()
1388+
expect(mockUpdateFlatOutputs).toHaveBeenCalled()
1389+
expect(useToast().add).toHaveBeenCalledWith(
1390+
expect.objectContaining({
1391+
severity: 'error',
1392+
detail: 'mediaAsset.failedToDeleteAsset'
1393+
})
1394+
)
1395+
})
1396+
1397+
it('reports a history deletion failure after deleting linked assets', async () => {
1398+
mockIsCloud.value = true
1399+
mockGetOutputAssetMetadata.mockReturnValue({ jobId: 'job-1' })
1400+
mockGetJobAssetIds.mockResolvedValue(['output-uuid'])
1401+
mockDeleteAsset.mockResolvedValue(undefined)
1402+
vi.mocked(api.fetchApi).mockResolvedValue(
1403+
fromAny({ ok: false, status: 500 })
1404+
)
1405+
const actions = useMediaAssetActions()
1406+
1407+
await actions.deleteAssets(
1408+
createMockAsset({
1409+
id: 'generated-card-id',
1410+
name: 'generated.png',
1411+
tags: ['output']
1412+
})
1413+
)
1414+
1415+
expect(mockDeleteAsset).toHaveBeenCalledWith('output-uuid')
1416+
expect(mockUpdateHistory).toHaveBeenCalled()
1417+
expect(mockUpdateFlatOutputs).toHaveBeenCalled()
1418+
expect(useToast().add).toHaveBeenCalledWith(
1419+
expect.objectContaining({
1420+
severity: 'error',
1421+
detail: 'mediaAsset.failedToDeleteAsset'
1422+
})
1423+
)
12671424
})
12681425
})
12691426

‎src/platform/assets/composables/useMediaAssetActions.ts‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ import { assetService } from '../services/assetService'
4343

4444
const EXCLUDED_TAGS = new Set(['models', 'input', 'output'])
4545

46+
async function deleteHistoryJob(jobId: string): Promise<void> {
47+
const response = await api.fetchApi('/history', {
48+
method: 'POST',
49+
headers: { 'Content-Type': 'application/json' },
50+
body: JSON.stringify({ delete: [jobId] })
51+
})
52+
if (!response.ok) {
53+
throw new Error(`Unable to delete history job: ${response.status}`)
54+
}
55+
}
56+
4657
function createAssetWidgetPath(asset: AssetItem): string {
4758
const metadata = getOutputAssetMetadata(asset.user_metadata)
4859
const assetType = getAssetType(asset, 'input')
@@ -94,24 +105,24 @@ export function useMediaAssetActions() {
94105
const litegraphService = useLitegraphService()
95106
const nodeDefStore = useNodeDefStore()
96107

97-
/**
98-
* Internal helper to perform the API deletion for a single asset
99-
* Handles both output assets (via history API) and input assets (via asset service)
100-
* @throws Error if deletion fails or is not allowed
101-
*/
102108
const deleteAssetApi = async (
103109
asset: AssetItem,
104110
assetType: string
105111
): Promise<void> => {
106-
// Temp files (e.g. preview-node outputs) are history-backed outputs that
107-
// happen to live in the temp dir, so they delete via the history API too.
108112
if (assetType === 'output' || assetType === 'temp') {
109113
const jobId =
110114
getOutputAssetMetadata(asset.user_metadata)?.jobId || asset.id
111115
if (!jobId) {
112116
throw new Error('Unable to extract job ID from asset')
113117
}
114-
await api.deleteItem('history', jobId)
118+
119+
const assetIds = isCloud ? await assetService.getJobAssetIds(jobId) : []
120+
const results = await Promise.allSettled(
121+
assetIds.map((id) => assetService.deleteAsset(id))
122+
)
123+
const failure = results.find((result) => result.status === 'rejected')
124+
if (failure?.status === 'rejected') throw failure.reason
125+
await deleteHistoryJob(jobId)
115126
} else {
116127
// Input assets can only be deleted in cloud environment
117128
if (!isCloud) {
@@ -706,24 +717,29 @@ export function useMediaAssetActions() {
706717
const failed = results.filter((r) => r.status === 'rejected')
707718

708719
// Log failed deletions for debugging
709-
failed.forEach((result, index) => {
710-
console.warn(
711-
`Failed to delete asset ${assetArray[index].name}:`,
712-
result.reason
713-
)
720+
results.forEach((result, index) => {
721+
if (result.status === 'rejected') {
722+
console.warn(
723+
`Failed to delete asset ${assetArray[index].name}:`,
724+
result.reason
725+
)
726+
}
714727
})
715728

716729
// Update stores after deletions
717-
const hasOutputAssets = assetArray.some((a) => {
718-
const type = getAssetType(a)
730+
const hasOutputAssets = assetArray.some((asset) => {
731+
const type = getAssetType(asset)
719732
return type === 'output' || type === 'temp'
720733
})
721734
const hasInputAssets = assetArray.some(
722-
(a) => getAssetType(a) === 'input'
735+
(asset, index) =>
736+
results[index].status === 'fulfilled' &&
737+
getAssetType(asset) === 'input'
723738
)
724739

725740
if (hasOutputAssets) {
726741
await assetsStore.updateHistory()
742+
await assetsStore.updateFlatOutputs()
727743
}
728744
if (hasInputAssets) {
729745
await assetsStore.updateInputs()

‎src/platform/assets/services/assetService.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,85 @@ describe(assetService.deleteAsset, () => {
434434
expect.objectContaining({ method: 'DELETE' })
435435
)
436436
})
437+
438+
it('treats an already deleted asset as success', async () => {
439+
fetchApiMock.mockResolvedValueOnce(
440+
buildResponse(null, { ok: false, status: 404 })
441+
)
442+
443+
await expect(assetService.deleteAsset('asset-1')).resolves.toBeUndefined()
444+
})
445+
})
446+
447+
describe(assetService.getJobAssetIds, () => {
448+
beforeEach(() => {
449+
vi.clearAllMocks()
450+
})
451+
452+
it('returns every asset ID across offset pages', async () => {
453+
fetchApiMock
454+
.mockResolvedValueOnce(
455+
buildResponse({
456+
assets: [{ id: 'asset-1' }, { id: 'asset-2' }],
457+
pagination: { offset: 0, limit: 500, total: 3, has_more: true }
458+
})
459+
)
460+
.mockResolvedValueOnce(
461+
buildResponse({
462+
assets: [{ id: 'asset-3' }],
463+
pagination: { offset: 2, limit: 500, total: 3, has_more: false }
464+
})
465+
)
466+
467+
await expect(assetService.getJobAssetIds('job/1')).resolves.toEqual([
468+
'asset-1',
469+
'asset-2',
470+
'asset-3'
471+
])
472+
473+
expect(fetchApiMock).toHaveBeenNthCalledWith(
474+
1,
475+
'/jobs/job%2F1/assets?limit=500&offset=0'
476+
)
477+
expect(fetchApiMock).toHaveBeenNthCalledWith(
478+
2,
479+
'/jobs/job%2F1/assets?limit=500&offset=2'
480+
)
481+
})
482+
483+
it('returns no assets when the job assets endpoint is unavailable', async () => {
484+
fetchApiMock.mockResolvedValueOnce(
485+
buildResponse(null, { ok: false, status: 404 })
486+
)
487+
488+
await expect(assetService.getJobAssetIds('job-1')).resolves.toEqual([])
489+
})
490+
491+
it('throws instead of returning an incomplete page without progress', async () => {
492+
fetchApiMock.mockResolvedValueOnce(
493+
buildResponse({
494+
assets: [],
495+
pagination: { offset: 0, limit: 500, total: 1, has_more: true }
496+
})
497+
)
498+
499+
await expect(assetService.getJobAssetIds('job-1')).rejects.toThrow(
500+
'made no progress'
501+
)
502+
})
503+
504+
it('throws when the response offset does not match the requested offset', async () => {
505+
fetchApiMock.mockResolvedValueOnce(
506+
buildResponse({
507+
assets: [{ id: 'asset-1' }],
508+
pagination: { offset: 2, limit: 500, total: 3, has_more: true }
509+
})
510+
)
511+
512+
await expect(assetService.getJobAssetIds('job-1')).rejects.toThrow(
513+
'Invalid job assets pagination offset'
514+
)
515+
})
437516
})
438517

439518
describe(assetService.getAssetModels, () => {

0 commit comments

Comments
 (0)