diff --git a/src/stores/assetsStore.test.ts b/src/stores/assetsStore.test.ts index ecb449dd681..f3e47cb1843 100644 --- a/src/stores/assetsStore.test.ts +++ b/src/stores/assetsStore.test.ts @@ -2358,4 +2358,204 @@ describe('assetsStore - Flat Output Assets (cloud-only)', () => { expect(store.flatOutputAssets.map((x) => x.id)).toEqual(['shared-1']) }) + + describe('in-flight tracking: refresh vs loadMore', () => { + const deferredPage = () => { + let resolve!: (page: AssetResponse) => void + let reject!: (err: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } + } + + /** + * Loads a full first page so `hasMore` is true and a loadMore is legal, + * then clears the mock so each test's assertions see only its own calls. + */ + const setupStoreWithFirstPage = async () => { + const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) => + makeAsset(`a${i}`, `f${i}.png`) + ) + vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce( + makePage(firstPage, { hasMore: true }) + ) + const store = useAssetsStore() + await store.updateFlatOutputs() + vi.mocked(assetService.getAssetsPageByTag).mockClear() + return { store, firstPageIds: firstPage.map((a) => a.id) } + } + + it('refresh during an in-flight loadMore runs its reset path and is not dropped', async () => { + const { store } = await setupStoreWithFirstPage() + const loadMorePage = deferredPage() + const refreshPage = deferredPage() + + vi.mocked(assetService.getAssetsPageByTag) + .mockReturnValueOnce(loadMorePage.promise) + .mockReturnValueOnce(refreshPage.promise) + + const loadMoreResult = store.loadMoreFlatOutputs() + const refreshResult = store.updateFlatOutputs() + + expect(vi.mocked(assetService.getAssetsPageByTag)).toHaveBeenCalledTimes( + 2 + ) + + refreshPage.resolve(makePage([makeAsset('fresh-1', 'fresh.png')])) + loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')])) + await Promise.all([loadMoreResult, refreshResult]) + + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh-1']) + }) + + it('does not double-advance the offset with a discarded loadMore page', async () => { + const { store } = await setupStoreWithFirstPage() + const loadMorePage = deferredPage() + const refreshPage = deferredPage() + + vi.mocked(assetService.getAssetsPageByTag) + .mockReturnValueOnce(loadMorePage.promise) + .mockReturnValueOnce(refreshPage.promise) + + const loadMoreResult = store.loadMoreFlatOutputs() + const refreshResult = store.updateFlatOutputs() + + refreshPage.resolve( + makePage([makeAsset('fresh-1', 'fresh.png')], { hasMore: true }) + ) + loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')])) + await Promise.all([loadMoreResult, refreshResult]) + + // The refreshed list holds one asset, so the next page starts at 1. A + // discarded page that still advanced the offset would skip a row here. + vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce( + makePage([makeAsset('extra-1', 'extra.png')]) + ) + await store.loadMoreFlatOutputs() + + expect(assetService.getAssetsPageByTag).toHaveBeenLastCalledWith( + 'output', + true, + { limit: FLAT_OUTPUT_PAGE_SIZE, offset: 1 } + ) + expect(store.flatOutputAssets.map((a) => a.id)).toEqual([ + 'fresh-1', + 'extra-1' + ]) + }) + + it('discards a stale loadMore even when the refresh reports no further pages', async () => { + const { store } = await setupStoreWithFirstPage() + const loadMorePage = deferredPage() + const refreshPage = deferredPage() + + vi.mocked(assetService.getAssetsPageByTag) + .mockReturnValueOnce(loadMorePage.promise) + .mockReturnValueOnce(refreshPage.promise) + + const loadMoreResult = store.loadMoreFlatOutputs() + const refreshResult = store.updateFlatOutputs() + + refreshPage.resolve( + makePage([makeAsset('fresh-1', 'fresh.png')], { hasMore: false }) + ) + loadMorePage.resolve( + makePage([makeAsset('extra-1', 'extra.png')], { hasMore: true }) + ) + await Promise.all([loadMoreResult, refreshResult]) + + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh-1']) + expect(store.flatOutputHasMore).toBe(false) + }) + + it('keeps the list and the loaded-id check in step when a refresh fails mid-loadMore', async () => { + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + const { store, firstPageIds } = await setupStoreWithFirstPage() + const loadMorePage = deferredPage() + const refreshPage = deferredPage() + + vi.mocked(assetService.getAssetsPageByTag) + .mockReturnValueOnce(loadMorePage.promise) + .mockReturnValueOnce(refreshPage.promise) + + const loadMoreResult = store.loadMoreFlatOutputs() + const refreshResult = store.updateFlatOutputs() + + const err = new Error('network down') + refreshPage.reject(err) + loadMorePage.resolve(makePage([makeAsset('extra-1', 'extra.png')])) + await Promise.all([loadMoreResult, refreshResult]) + + expect(store.flatOutputError).toBe(err) + expect(store.flatOutputLoading).toBe(false) + expect(store.flatOutputIsLoadingMore).toBe(false) + // The failed refresh dropped nothing; the stale loadMore was discarded. + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(firstPageIds) + + // A retry restarts from the head. The re-served first page is already + // in seenIds, so it must not be appended a second time. + vi.mocked(assetService.getAssetsPageByTag).mockResolvedValueOnce( + makePage( + firstPageIds.map((id, i) => makeAsset(id, `f${i}.png`)), + { hasMore: true } + ) + ) + await store.loadMoreFlatOutputs() + + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(firstPageIds) + } finally { + consoleSpy.mockRestore() + } + }) + + it('defers a loadMore started during a refresh instead of stranding hasMore', async () => { + const { store } = await setupStoreWithFirstPage() + const refreshPage = deferredPage() + + vi.mocked(assetService.getAssetsPageByTag).mockReturnValueOnce( + refreshPage.promise + ) + + const refreshResult = store.updateFlatOutputs() + const loadMoreResult = store.loadMoreFlatOutputs() + + // The refresh already rewound the offset to the head, so the loadMore has + // no next page to ask for and must not issue a second request. + expect(vi.mocked(assetService.getAssetsPageByTag)).toHaveBeenCalledTimes( + 1 + ) + + const head = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) => + makeAsset(`b${i}`, `g${i}.png`) + ) + refreshPage.resolve(makePage(head, { hasMore: true })) + await Promise.all([refreshResult, loadMoreResult]) + + expect(store.flatOutputHasMore).toBe(true) + expect(store.flatOutputAssets).toHaveLength(FLAT_OUTPUT_PAGE_SIZE) + }) + + it('concurrent refreshes deduplicate the network call and produce consistent state', async () => { + const page = deferredPage() + vi.mocked(assetService.getAssetsPageByTag).mockReturnValueOnce( + page.promise + ) + + const store = useAssetsStore() + const r1 = store.updateFlatOutputs() + const r2 = store.updateFlatOutputs() + + expect(vi.mocked(assetService.getAssetsPageByTag)).toHaveBeenCalledTimes( + 1 + ) + + page.resolve(makePage([makeAsset('only-1', 'only.png')])) + await Promise.all([r1, r2]) + + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['only-1']) + }) + }) }) diff --git a/src/stores/assetsStore.ts b/src/stores/assetsStore.ts index 3ad1d55c3d2..c56b08e20ea 100644 --- a/src/stores/assetsStore.ts +++ b/src/stores/assetsStore.ts @@ -1,7 +1,7 @@ import { useAsyncState, whenever } from '@vueuse/core' import { delay, difference } from 'es-toolkit' import { defineStore } from 'pinia' -import { computed, reactive, ref, shallowReactive } from 'vue' +import { computed, reactive, ref, shallowReactive, shallowRef } from 'vue' import { mapInputFileToAssetItem, mapTaskOutputToAssetItem @@ -265,32 +265,85 @@ export const useAssetsStore = defineStore('assets', () => { } const flatOutputAssets = ref([]) - const flatOutputLoading = ref(false) const flatOutputError = ref(null) const flatOutputOffset = ref(0) const flatOutputHasMore = ref(true) - const flatOutputIsLoadingMore = ref(false) const flatOutputSeenIds = new Set() let flatOutputNextCursor: string | undefined - let flatOutputInFlight: Promise | null = null + const flatOutputRefreshInFlight = shallowRef | null>( + null + ) + const flatOutputLoadMoreInFlight = shallowRef | null>(null) + const flatOutputLoading = computed( + () => flatOutputRefreshInFlight.value !== null + ) + const flatOutputIsLoadingMore = computed( + () => flatOutputLoadMoreInFlight.value !== null + ) + // Incremented on each refresh; loadMore results captured from a prior epoch + // are discarded so a stale page can't append onto a freshly-refreshed list. + let flatOutputRefreshEpoch = 0 - async function fetchFlatOutputs(loadMore: boolean): Promise { - if (flatOutputInFlight) return flatOutputInFlight + async function updateFlatOutputs(): Promise { + if (flatOutputRefreshInFlight.value) return flatOutputRefreshInFlight.value - if (loadMore) { - if (!flatOutputHasMore.value) return flatOutputAssets.value - flatOutputIsLoadingMore.value = true - } else { - flatOutputLoading.value = true - flatOutputOffset.value = 0 - flatOutputNextCursor = undefined - flatOutputHasMore.value = true - flatOutputSeenIds.clear() + flatOutputRefreshEpoch++ + flatOutputOffset.value = 0 + flatOutputNextCursor = undefined + flatOutputHasMore.value = true + flatOutputError.value = null + + const inFlight = (async () => { + try { + const page = await assetService.getAssetsPageByTag(OUTPUT_TAG, true, { + limit: FLAT_OUTPUT_PAGE_SIZE, + offset: 0 + }) + // Swapped only on success, so a failed refresh leaves the loaded-id + // check consistent with the list still on screen. + flatOutputSeenIds.clear() + for (const asset of page.assets) flatOutputSeenIds.add(asset.id) + flatOutputAssets.value = page.assets + flatOutputOffset.value = page.assets.length + flatOutputNextCursor = page.next_cursor + flatOutputHasMore.value = page.assets.length > 0 && page.has_more + return flatOutputAssets.value + } catch (err) { + flatOutputError.value = err + console.error('Failed to fetch output assets:', err) + return [] + } finally { + flatOutputRefreshInFlight.value = null + } + })() + + // Assigned after the IIFE starts but before it can settle, so the `finally` + // above never clears a slot that has not been filled yet. + flatOutputRefreshInFlight.value = inFlight + return inFlight + } + + async function loadMoreFlatOutputs(): Promise { + if (!flatOutputHasMore.value) return + if (flatOutputLoadMoreInFlight.value) { + await flatOutputLoadMoreInFlight.value + return } + // A refresh has already reset the offset and cursor to the head, so there + // is no next page to ask for yet; the refresh itself delivers that page. + // Issuing one anyway re-fetches the head, which dedupes to nothing and + // would strand `flatOutputHasMore` at false. + if (flatOutputRefreshInFlight.value) { + await flatOutputRefreshInFlight.value + return + } + flatOutputError.value = null - flatOutputInFlight = (async () => { - const requestedAfter = loadMore ? flatOutputNextCursor : undefined + const capturedRefreshEpoch = flatOutputRefreshEpoch + const requestedAfter = flatOutputNextCursor + + const inFlight = (async () => { try { const page = await assetService.getAssetsPageByTag(OUTPUT_TAG, true, { limit: FLAT_OUTPUT_PAGE_SIZE, @@ -298,14 +351,12 @@ export const useAssetsStore = defineStore('assets', () => { ? { after: requestedAfter } : { offset: flatOutputOffset.value }) }) + if (capturedRefreshEpoch !== flatOutputRefreshEpoch) return + const batch = page.assets - const fresh = loadMore - ? batch.filter((asset) => !flatOutputSeenIds.has(asset.id)) - : batch + const fresh = batch.filter((asset) => !flatOutputSeenIds.has(asset.id)) for (const asset of fresh) flatOutputSeenIds.add(asset.id) - flatOutputAssets.value = loadMore - ? [...flatOutputAssets.value, ...fresh] - : batch + flatOutputAssets.value = [...flatOutputAssets.value, ...fresh] flatOutputOffset.value += batch.length const nextCursor = page.next_cursor const cursorStuck = @@ -313,25 +364,16 @@ export const useAssetsStore = defineStore('assets', () => { flatOutputNextCursor = cursorStuck ? undefined : nextCursor flatOutputHasMore.value = fresh.length > 0 && page.has_more && !cursorStuck - return flatOutputAssets.value } catch (err) { flatOutputError.value = err console.error('Failed to fetch output assets:', err) - return loadMore ? flatOutputAssets.value : [] } finally { - if (loadMore) flatOutputIsLoadingMore.value = false - else flatOutputLoading.value = false - flatOutputInFlight = null + flatOutputLoadMoreInFlight.value = null } })() - return flatOutputInFlight - } - - const updateFlatOutputs = () => fetchFlatOutputs(false) - const loadMoreFlatOutputs = async () => { - if (flatOutputIsLoadingMore.value) return - await fetchFlatOutputs(true) + flatOutputLoadMoreInFlight.value = inFlight + await inFlight } /**