mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-06-05 20:54:56 +00:00
fix(assets): invalidate caches and exclude missing assets on per-output delete
Codex adversarial review on PR #12503 surfaced two real follow-up gaps: 1. `findOutputAssetIdByHash` queried without `exclude_tags=missing`, so a soft-deleted asset that still carries the same hash could shadow the live output we want to delete. Mirror the exclusion every other asset query uses. 2. After a successful per-output cloud delete, only `assetsStore.updateHistory()` refreshes. The `jobOutputCache` (job-detail + task LRU) and the `useOutputStacks` per-job children cache both still served the deleted child, leaving the row visible and selectable in the sidebar until the user collapsed the stack. Expose `invalidateJobOutputs(jobId)` from the cache module, call it from `deleteAssets` for every fulfilled output delete, and have `useOutputStacks` cross-check cached children against the cover's refreshed `allOutputs` so stale children disappear from the rendered list immediately. Adds two regression tests: cache invalidation is asserted on the cloud delete path, and `useOutputStacks` prunes a child whose filename drops off the cover's allOutputs.
This commit is contained in:
@@ -174,6 +174,11 @@ vi.mock('@/scripts/api', () => ({
|
||||
}
|
||||
}))
|
||||
|
||||
const mockInvalidateJobOutputs = vi.hoisted(() => vi.fn())
|
||||
vi.mock('@/services/jobOutputCache', () => ({
|
||||
invalidateJobOutputs: mockInvalidateJobOutputs
|
||||
}))
|
||||
|
||||
const mockAppGraph = vi.hoisted(() => ({ value: { _nodes: [] as unknown[] } }))
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
@@ -1084,6 +1089,7 @@ describe('useMediaAssetActions', () => {
|
||||
|
||||
it('routes cloud output deletion through assetService.deleteAsset using the resolved UUID', async () => {
|
||||
mockIsCloud.value = true
|
||||
mockGetOutputAssetMetadata.mockReturnValue({ jobId: 'job-abc' })
|
||||
mockFindOutputAssetIdByHash.mockResolvedValue(
|
||||
'real-output-asset-uuid-1234'
|
||||
)
|
||||
@@ -1111,6 +1117,10 @@ describe('useMediaAssetActions', () => {
|
||||
|
||||
const { api } = await import('@/scripts/api')
|
||||
expect(api.deleteItem).not.toHaveBeenCalled()
|
||||
|
||||
// Stack/folder/job-detail caches must be invalidated so the deleted
|
||||
// child does not linger in the rendered list after success.
|
||||
expect(mockInvalidateJobOutputs).toHaveBeenCalledWith('job-abc')
|
||||
})
|
||||
|
||||
it('falls back to history delete in OSS mode (no per-asset endpoint available)', async () => {
|
||||
|
||||
@@ -14,6 +14,7 @@ import { app } from '@/scripts/app'
|
||||
import { useLitegraphService } from '@/services/litegraphService'
|
||||
import { useNodeDefStore } from '@/stores/nodeDefStore'
|
||||
import { getOutputAssetMetadata } from '../schemas/assetMetadataSchema'
|
||||
import { invalidateJobOutputs } from '@/services/jobOutputCache'
|
||||
import { useAssetsStore } from '@/stores/assetsStore'
|
||||
import { useDialogStore } from '@/stores/dialogStore'
|
||||
import { useNodeOutputStore } from '@/stores/nodeOutputStore'
|
||||
@@ -684,6 +685,16 @@ export function useMediaAssetActions() {
|
||||
)
|
||||
|
||||
if (hasOutputAssets) {
|
||||
// Drop cached job-detail/task entries so the next stack
|
||||
// expand or folder open sees the post-delete output list.
|
||||
assetArray.forEach((asset, index) => {
|
||||
if (results[index].status !== 'fulfilled') return
|
||||
if (getAssetType(asset) !== 'output') return
|
||||
const jobId =
|
||||
getOutputAssetMetadata(asset.user_metadata)?.jobId ??
|
||||
asset.id
|
||||
if (jobId) invalidateJobOutputs(jobId)
|
||||
})
|
||||
await assetsStore.updateHistory()
|
||||
}
|
||||
if (hasInputAssets) {
|
||||
|
||||
@@ -202,4 +202,65 @@ describe('useOutputStacks', () => {
|
||||
child.id
|
||||
])
|
||||
})
|
||||
|
||||
it('drops cached children whose filename is no longer in the cover.allOutputs (FE-814)', async () => {
|
||||
const childKept = createAsset({
|
||||
id: 'child-keep',
|
||||
name: 'keep.png',
|
||||
user_metadata: undefined
|
||||
})
|
||||
const childDeleted = createAsset({
|
||||
id: 'child-gone',
|
||||
name: 'gone.png',
|
||||
user_metadata: undefined
|
||||
})
|
||||
|
||||
vi.mocked(mocks.resolveOutputAssetItems).mockResolvedValue([
|
||||
childKept,
|
||||
childDeleted
|
||||
])
|
||||
|
||||
const initialCover = createAsset({
|
||||
id: 'parent',
|
||||
name: 'parent.png',
|
||||
user_metadata: {
|
||||
jobId: 'job-1',
|
||||
nodeId: 'node-1',
|
||||
subfolder: 'outputs',
|
||||
allOutputs: [
|
||||
{ filename: 'parent.png' },
|
||||
{ filename: 'keep.png' },
|
||||
{ filename: 'gone.png' }
|
||||
]
|
||||
}
|
||||
})
|
||||
const assets = ref<AssetItem[]>([initialCover])
|
||||
|
||||
const { assetItems, toggleStack } = useOutputStacks({ assets })
|
||||
await toggleStack(initialCover)
|
||||
|
||||
expect(assetItems.value.map((i) => i.asset.id)).toEqual([
|
||||
initialCover.id,
|
||||
childKept.id,
|
||||
childDeleted.id
|
||||
])
|
||||
|
||||
// Simulate a successful per-output delete: updateHistory() refreshes the
|
||||
// cover with a shorter allOutputs. The previously cached child whose
|
||||
// filename is no longer present must vanish from the rendered list.
|
||||
assets.value = [
|
||||
createAsset({
|
||||
...initialCover,
|
||||
user_metadata: {
|
||||
...initialCover.user_metadata!,
|
||||
allOutputs: [{ filename: 'parent.png' }, { filename: 'keep.png' }]
|
||||
}
|
||||
})
|
||||
]
|
||||
|
||||
expect(assetItems.value.map((i) => i.asset.id)).toEqual([
|
||||
'parent',
|
||||
childKept.id
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
getOutputKey,
|
||||
resolveOutputAssetItems
|
||||
} from '@/platform/assets/utils/outputAssetUtil'
|
||||
import type { ResultItemImpl } from '@/stores/queueStore'
|
||||
|
||||
export type OutputStackListItem = {
|
||||
key: string
|
||||
@@ -38,7 +39,8 @@ export function useOutputStacks({ assets }: UseOutputStacksOptions) {
|
||||
}
|
||||
|
||||
const children = stackChildrenByJobId.value[jobId] ?? []
|
||||
for (const child of children) {
|
||||
const filteredChildren = filterChildrenAgainstCover(asset, children)
|
||||
for (const child of filteredChildren) {
|
||||
items.push({
|
||||
key: `asset-${child.id}`,
|
||||
asset: child,
|
||||
@@ -59,6 +61,25 @@ export function useOutputStacks({ assets }: UseOutputStacksOptions) {
|
||||
return metadata?.jobId ?? null
|
||||
}
|
||||
|
||||
// Drops cached children whose underlying output is no longer present on the
|
||||
// cover's refreshed `allOutputs` list. Lets per-output deletes disappear
|
||||
// immediately after `updateHistory()` re-fetches the cover, without waiting
|
||||
// for the user to collapse/expand the stack.
|
||||
function filterChildrenAgainstCover(
|
||||
cover: AssetItem,
|
||||
children: AssetItem[]
|
||||
): AssetItem[] {
|
||||
const metadata = getOutputAssetMetadata(cover.user_metadata)
|
||||
const allOutputs = metadata?.allOutputs as
|
||||
| readonly ResultItemImpl[]
|
||||
| undefined
|
||||
if (!allOutputs?.length) return children
|
||||
const validFilenames = new Set(
|
||||
allOutputs.map((output) => output.filename).filter(Boolean)
|
||||
)
|
||||
return children.filter((child) => validFilenames.has(child.name))
|
||||
}
|
||||
|
||||
function isStackExpanded(asset: AssetItem): boolean {
|
||||
const jobId = getStackJobId(asset)
|
||||
if (!jobId) return false
|
||||
|
||||
@@ -601,6 +601,7 @@ function createAssetService() {
|
||||
const queryParams = new URLSearchParams({
|
||||
asset_hash: hash,
|
||||
include_tags: OUTPUT_TAG,
|
||||
exclude_tags: MISSING_TAG,
|
||||
limit: '1',
|
||||
include_public: 'false'
|
||||
})
|
||||
|
||||
@@ -114,3 +114,12 @@ export async function getJobWorkflow(
|
||||
const detail = await getJobDetail(jobId)
|
||||
return await extractWorkflow(detail)
|
||||
}
|
||||
|
||||
/**
|
||||
* Drops cached task and detail entries for a job so subsequent reads pick up
|
||||
* server-side changes (e.g. an output deleted via DELETE /assets/{id}).
|
||||
*/
|
||||
export function invalidateJobOutputs(jobId: string): void {
|
||||
taskCache.delete(jobId)
|
||||
jobDetailCache.delete(jobId)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user