diff --git a/e2e/src/specs/server/api/library.e2e-spec.ts b/e2e/src/specs/server/api/library.e2e-spec.ts index 4d67a8464781d..1cd46169563a9 100644 --- a/e2e/src/specs/server/api/library.e2e-spec.ts +++ b/e2e/src/specs/server/api/library.e2e-spec.ts @@ -859,6 +859,48 @@ describe('/libraries', () => { ]); }); + it('should not delete excluded external files from disk when trash is emptied', async () => { + const relativePath = 'temp/offline-empty-trash/offline.png'; + const filePath = `${testAssetDir}/${relativePath}`; + const internalFilePath = `${testAssetDirInternal}/${relativePath}`; + + utils.createImageFile(filePath); + + const library = await utils.createLibrary(admin.accessToken, { + ownerId: admin.userId, + importPaths: [`${testAssetDirInternal}/temp/offline-empty-trash`], + }); + + await utils.scan(admin.accessToken, library.id); + + const { assets } = await utils.searchAssets(admin.accessToken, { + libraryId: library.id, + originalPath: internalFilePath, + }); + expect(assets.count).toBe(1); + + await utils.updateLibrary(admin.accessToken, library.id, { + exclusionPatterns: ['**/offline-empty-trash/**'], + }); + + await utils.scan(admin.accessToken, library.id); + + const trashedAsset = await utils.getAssetInfo(admin.accessToken, assets.items[0].id); + expect(trashedAsset.isTrashed).toBe(true); + expect(trashedAsset.isOffline).toBe(true); + + const { status } = await request(app).post('/trash/empty').set('Authorization', `Bearer ${admin.accessToken}`); + expect(status).toBe(200); + + await utils.waitForQueueFinish(admin.accessToken, 'backgroundTask'); + + expect(existsSync(filePath)).toBe(true); + + if (existsSync(filePath)) { + utils.removeImageFile(filePath); + } + }); + it('should not set an asset offline if its file exists, is in an import path, and not covered by an exclusion pattern', async () => { const library = await utils.createLibrary(admin.accessToken, { ownerId: admin.userId, diff --git a/server/src/repositories/trash.repository.ts b/server/src/repositories/trash.repository.ts index f6f13188d4bc8..a71fd9382fd54 100644 --- a/server/src/repositories/trash.repository.ts +++ b/server/src/repositories/trash.repository.ts @@ -7,8 +7,8 @@ import { DB } from 'src/schema'; export class TrashRepository { constructor(@InjectKysely() private db: Kysely) {} - getDeletedIds(): AsyncIterableIterator<{ id: string }> { - return this.db.selectFrom('asset').select(['id']).where('status', '=', AssetStatus.Deleted).stream(); + getDeletedIds(): AsyncIterableIterator<{ id: string; isOffline: boolean }> { + return this.db.selectFrom('asset').select(['id', 'isOffline']).where('status', '=', AssetStatus.Deleted).stream(); } @GenerateSql({ params: [DummyValue.UUID] }) diff --git a/server/src/services/trash.service.spec.ts b/server/src/services/trash.service.spec.ts index e43c49e5432d5..24c1bc1eedf07 100644 --- a/server/src/services/trash.service.spec.ts +++ b/server/src/services/trash.service.spec.ts @@ -4,10 +4,19 @@ import { TrashService } from 'src/services/trash.service'; import { authStub } from 'test/fixtures/auth.stub'; import { newTestService, ServiceMocks } from 'test/utils'; -async function* makeAssetIdStream(count: number): AsyncIterableIterator<{ id: string }> { +async function* makeAssetIdStream(count: number): AsyncIterableIterator<{ id: string; isOffline: boolean }> { for (let i = 0; i < count; i++) { await Promise.resolve(); - yield { id: `asset-${i + 1}` }; + yield { id: `asset-${i + 1}`, isOffline: false }; + } +} + +async function* makeDeletedAssetStream( + assets: Array<{ id: string; isOffline: boolean }>, +): AsyncIterableIterator<{ id: string; isOffline: boolean }> { + for (const asset of assets) { + await Promise.resolve(); + yield asset; } } @@ -99,5 +108,27 @@ describe(TrashService.name, () => { }, ]); }); + + it('should not delete offline assets on disk', async () => { + mocks.trash.getDeletedIds.mockReturnValue( + makeDeletedAssetStream([ + { id: 'asset-1', isOffline: false }, + { id: 'asset-2', isOffline: true }, + ]), + ); + + await expect(sut.handleEmptyTrash()).resolves.toEqual(JobStatus.Success); + + expect(mocks.job.queueAll).toHaveBeenCalledWith([ + { + name: JobName.AssetDelete, + data: { id: 'asset-1', deleteOnDisk: true }, + }, + { + name: JobName.AssetDelete, + data: { id: 'asset-2', deleteOnDisk: false }, + }, + ]); + }); }); }); diff --git a/server/src/services/trash.service.ts b/server/src/services/trash.service.ts index f1d368111ef4c..e41be871ef3b6 100644 --- a/server/src/services/trash.service.ts +++ b/server/src/services/trash.service.ts @@ -50,9 +50,9 @@ export class TrashService extends BaseService { const assets = this.trashRepository.getDeletedIds(); let count = 0; - const batch: string[] = []; - for await (const { id } of assets) { - batch.push(id); + const batch: Array<{ id: string; isOffline: boolean }> = []; + for await (const asset of assets) { + batch.push(asset); if (batch.length === JOBS_ASSET_PAGINATION_SIZE) { await this.handleBatch(batch); @@ -70,14 +70,14 @@ export class TrashService extends BaseService { return JobStatus.Success; } - private async handleBatch(ids: string[]) { - this.logger.debug(`Queueing ${ids.length} asset(s) for deletion from the trash`); + private async handleBatch(assets: Array<{ id: string; isOffline: boolean }>) { + this.logger.debug(`Queueing ${assets.length} asset(s) for deletion from the trash`); await this.jobRepository.queueAll( - ids.map((assetId) => ({ + assets.map(({ id, isOffline }) => ({ name: JobName.AssetDelete, data: { - id: assetId, - deleteOnDisk: true, + id, + deleteOnDisk: !isOffline, }, })), );