From 1d258ebaeec66118901bf41f7b25ae2ff70480e3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 16:37:53 -0700 Subject: [PATCH] perf(jira): preserve replacement attachment download singleflight --- .../attachment-image-cache-generation.test.ts | 57 +++++++++++++++++++ src/main/jira/attachment-image-cache.ts | 5 +- 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/main/jira/attachment-image-cache-generation.test.ts diff --git a/src/main/jira/attachment-image-cache-generation.test.ts b/src/main/jira/attachment-image-cache-generation.test.ts new file mode 100644 index 00000000000..e92ca5c5a53 --- /dev/null +++ b/src/main/jira/attachment-image-cache-generation.test.ts @@ -0,0 +1,57 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + _resetAttachmentImageCache, + clearAttachmentImagesForSite, + getCachedAttachmentDataUrl, + loadAttachmentDataUrlWithCache +} from './attachment-image-cache' + +type Image = { dataUrl: string; byteSize: number } | null +function deferredImage() { + let resolve!: (image: Image) => void + let reject!: (error: Error) => void + const promise = new Promise((done, fail) => { + resolve = done + reject = fail + }) + return { promise, resolve, reject } +} + +beforeEach(_resetAttachmentImageCache) + +describe.each(['site', 'all'] as const)('attachment download after clearing %s', (scope) => { + it.each(['success', 'empty', 'failure'] as const)( + 'keeps the replacement singleflight when the old download completes with %s', + async (outcome) => { + const old = deferredImage() + const replacement = deferredImage() + let downloads = 0 + const load = () => { + downloads += 1 + return downloads === 1 ? old.promise : replacement.promise + } + const args = { siteId: 'site-a', attachmentId: 'image-1', load } + const first = loadAttachmentDataUrlWithCache(args).catch(() => 'old failure') + clearAttachmentImagesForSite(scope === 'site' ? 'site-a' : undefined) + const second = loadAttachmentDataUrlWithCache(args) + expect(downloads).toBe(2) + + if (outcome === 'failure') { + old.reject(new Error('old failure')) + } else { + old.resolve(outcome === 'empty' ? null : { dataUrl: 'old image', byteSize: 3 }) + } + expect(await first).toBe( + outcome === 'failure' ? 'old failure' : outcome === 'empty' ? null : 'old image' + ) + expect(getCachedAttachmentDataUrl('site-a', 'image-1')).toBeNull() + + const third = loadAttachmentDataUrlWithCache(args) + expect(downloads).toBe(2) + replacement.resolve({ dataUrl: 'new image', byteSize: 3 }) + expect(await second).toBe('new image') + expect(await third).toBe('new image') + expect(getCachedAttachmentDataUrl('site-a', 'image-1')).toBe('new image') + } + ) +}) diff --git a/src/main/jira/attachment-image-cache.ts b/src/main/jira/attachment-image-cache.ts index f9fdbfe3f7d..9d118ffdc9c 100644 --- a/src/main/jira/attachment-image-cache.ts +++ b/src/main/jira/attachment-image-cache.ts @@ -133,7 +133,10 @@ export async function loadAttachmentDataUrlWithCache(args: { } return loaded.dataUrl } finally { - inFlight.delete(key) + // A cleared generation no longer owns the current download's singleflight slot. + if (currentEpoch(args.siteId) === epochAtStart) { + inFlight.delete(key) + } } })()