From 048d437b0b7f6b21312f0e5c20e99a2c9dde50fa Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Wed, 20 Mar 2024 20:40:41 +0100 Subject: [PATCH] fix(web): prevent duplicate time bucket loads (#8091) --- web/src/lib/stores/asset.store.spec.ts | 41 ++++++++++++++++++++++---- web/src/lib/stores/assets.store.ts | 30 ++++++++++--------- web/src/lib/utils.ts | 2 +- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/web/src/lib/stores/asset.store.spec.ts b/web/src/lib/stores/asset.store.spec.ts index d97692ef67..55f3fe1ff5 100644 --- a/web/src/lib/stores/asset.store.spec.ts +++ b/web/src/lib/stores/asset.store.spec.ts @@ -1,4 +1,5 @@ import { sdkMock } from '$lib/__mocks__/sdk.mock'; +import { AbortError } from '$lib/utils'; import { TimeBucketSize, type AssetResponseDto } from '@immich/sdk'; import { assetFactory } from '@test-data/factories/asset-factory'; import { AssetStore, BucketPosition } from './assets.store'; @@ -62,7 +63,15 @@ describe('AssetStore', () => { { count: 1, timeBucket: '2024-01-03T00:00:00.000Z' }, { count: 3, timeBucket: '2024-01-01T00:00:00.000Z' }, ]); - sdkMock.getTimeBucket.mockImplementation(({ timeBucket }) => Promise.resolve(bucketAssets[timeBucket])); + sdkMock.getTimeBucket.mockImplementation(async ({ timeBucket }, { signal } = {}) => { + // Allow request to be aborted + await new Promise((resolve) => setTimeout(resolve, 0)); + if (signal?.aborted) { + throw new AbortError(); + } + + return bucketAssets[timeBucket]; + }); await assetStore.init({ width: 0, height: 0 }); }); @@ -87,17 +96,39 @@ describe('AssetStore', () => { }); it('cancels bucket loading', async () => { - const abortSpy = vi.spyOn(AbortController.prototype, 'abort'); - const loadPromise = assetStore.loadBucket('2024-01-01T00:00:00.000Z', BucketPosition.Unknown); - const bucket = assetStore.getBucketByDate('2024-01-01T00:00:00.000Z'); - expect(bucket).not.toBeNull(); + const loadPromise = assetStore.loadBucket(bucket!.bucketDate, BucketPosition.Unknown); + const abortSpy = vi.spyOn(bucket!.cancelToken!, 'abort'); assetStore.cancelBucket(bucket!); expect(abortSpy).toBeCalledTimes(1); + await loadPromise; expect(assetStore.getBucketByDate('2024-01-01T00:00:00.000Z')?.assets.length).toEqual(0); }); + + it('prevents loading buckets multiple times', async () => { + await Promise.all([ + assetStore.loadBucket('2024-01-01T00:00:00.000Z', BucketPosition.Unknown), + assetStore.loadBucket('2024-01-01T00:00:00.000Z', BucketPosition.Unknown), + ]); + expect(sdkMock.getTimeBucket).toBeCalledTimes(1); + + await assetStore.loadBucket('2024-01-01T00:00:00.000Z', BucketPosition.Unknown); + expect(sdkMock.getTimeBucket).toBeCalledTimes(1); + }); + + it('allows loading a canceled bucket', async () => { + const bucket = assetStore.getBucketByDate('2024-01-01T00:00:00.000Z'); + const loadPromise = assetStore.loadBucket(bucket!.bucketDate, BucketPosition.Unknown); + + assetStore.cancelBucket(bucket!); + await loadPromise; + expect(bucket?.assets.length).toEqual(0); + + await assetStore.loadBucket(bucket!.bucketDate, BucketPosition.Unknown); + expect(bucket!.assets.length).toEqual(3); + }); }); describe('addAssets', () => { diff --git a/web/src/lib/stores/assets.store.ts b/web/src/lib/stores/assets.store.ts index 30fc06085d..45b26c1a17 100644 --- a/web/src/lib/stores/assets.store.ts +++ b/web/src/lib/stores/assets.store.ts @@ -229,21 +229,21 @@ export class AssetStore { } async loadBucket(bucketDate: string, position: BucketPosition): Promise { + const bucket = this.getBucketByDate(bucketDate); + if (!bucket) { + return; + } + + bucket.position = position; + + if (bucket.cancelToken || bucket.assets.length > 0) { + this.emit(false); + return; + } + + bucket.cancelToken = new AbortController(); + try { - const bucket = this.getBucketByDate(bucketDate); - if (!bucket) { - return; - } - - bucket.position = position; - - if (bucket.assets.length > 0) { - this.emit(false); - return; - } - - bucket.cancelToken = new AbortController(); - const assets = await getTimeBucket( { ...this.options, @@ -278,6 +278,8 @@ export class AssetStore { this.emit(true); } catch (error) { handleError(error, 'Failed to load assets'); + } finally { + bucket.cancelToken = null; } } diff --git a/web/src/lib/utils.ts b/web/src/lib/utils.ts index 4b1892ae86..2cebba6301 100644 --- a/web/src/lib/utils.ts +++ b/web/src/lib/utils.ts @@ -27,7 +27,7 @@ interface UploadRequestOptions { onUploadProgress?: (event: ProgressEvent) => void; } -class AbortError extends Error { +export class AbortError extends Error { name = 'AbortError'; }