fix(server): asset delete logic (#10077)

* fix(server): asset delete logic

* test: e2e

e2e/src/api/specs/library.e2e-spec.ts

@@ -1,4 +1,11 @@
-import { LibraryResponseDto, LoginResponseDto, ScanLibraryDto, getAllLibraries, scanLibrary } from '@immich/sdk';
+import {
+  LibraryResponseDto,
+  LoginResponseDto,
+  ScanLibraryDto,
+  getAllLibraries,
+  removeOfflineFiles,
+  scanLibrary,
+} from '@immich/sdk';
 import { cpSync, existsSync } from 'node:fs';
 import { Socket } from 'socket.io-client';
 import { userDto, uuidDto } from 'src/fixtures';
@@ -384,6 +391,51 @@ describe('/libraries', () => {
       );
     });
 
+    it('should not try to delete offline files', async () => {
+      utils.createImageFile(`${testAssetDir}/temp/offline1/assetA.png`);
+
+      const library = await utils.createLibrary(admin.accessToken, {
+        ownerId: admin.userId,
+        importPaths: [`${testAssetDirInternal}/temp/offline1`],
+      });
+
+      await scan(admin.accessToken, library.id);
+      await utils.waitForQueueFinish(admin.accessToken, 'library');
+
+      const { assets: initialAssets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
+      expect(initialAssets).toEqual({
+        count: 1,
+        total: 1,
+        facets: [],
+        items: [expect.objectContaining({ originalFileName: 'assetA.png' })],
+        nextPage: null,
+      });
+
+      utils.removeImageFile(`${testAssetDir}/temp/offline1/assetA.png`);
+
+      await scan(admin.accessToken, library.id);
+      await utils.waitForQueueFinish(admin.accessToken, 'library');
+
+      const { assets: offlineAssets } = await utils.metadataSearch(admin.accessToken, {
+        libraryId: library.id,
+        isOffline: true,
+      });
+      expect(offlineAssets).toEqual({
+        count: 1,
+        total: 1,
+        facets: [],
+        items: [expect.objectContaining({ originalFileName: 'assetA.png' })],
+        nextPage: null,
+      });
+
+      utils.createImageFile(`${testAssetDir}/temp/offline1/assetA.png`);
+      await removeOfflineFiles({ id: library.id }, { headers: asBearerAuth(admin.accessToken) });
+      await utils.waitForQueueFinish(admin.accessToken, 'library');
+      await utils.waitForWebsocketEvent({ event: 'assetDelete', total: 1 });
+
+      expect(existsSync(`${testAssetDir}/temp/offline1/assetA.png`)).toBe(true);
+    });
+
     it('should scan new files', async () => {
       const library = await utils.createLibrary(admin.accessToken, {
         ownerId: admin.userId,
@@ -507,10 +559,10 @@ describe('/libraries', () => {
     it('should remove offline files', async () => {
       const library = await utils.createLibrary(admin.accessToken, {
         ownerId: admin.userId,
-        importPaths: [`${testAssetDirInternal}/temp`],
+        importPaths: [`${testAssetDirInternal}/temp/offline2`],
       });
 
-      utils.createImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
+      utils.createImageFile(`${testAssetDir}/temp/offline2/assetA.png`);
 
       await scan(admin.accessToken, library.id);
       await utils.waitForQueueFinish(admin.accessToken, 'library');
@@ -518,9 +570,9 @@ describe('/libraries', () => {
       const { assets: initialAssets } = await utils.metadataSearch(admin.accessToken, {
         libraryId: library.id,
       });
-      expect(initialAssets.count).toBe(3);
+      expect(initialAssets.count).toBe(1);
 
-      utils.removeImageFile(`${testAssetDir}/temp/directoryA/assetB.png`);
+      utils.removeImageFile(`${testAssetDir}/temp/offline2/assetA.png`);
 
       await scan(admin.accessToken, library.id);
       await utils.waitForQueueFinish(admin.accessToken, 'library');
@@ -541,7 +593,7 @@ describe('/libraries', () => {
 
       const { assets } = await utils.metadataSearch(admin.accessToken, { libraryId: library.id });
 
-      expect(assets.count).toBe(2);
+      expect(assets.count).toBe(0);
     });
 
     it('should not remove online files', async () => {

server/src/interfaces/job.interface.ts

@@ -120,6 +120,10 @@ export interface IEntityJob extends IBaseJob {
   source?: 'upload' | 'sidecar-write' | 'copy';
 }
 
+export interface IAssetDeleteJob extends IEntityJob {
+  deleteOnDisk: boolean;
+}
+
 export interface ILibraryFileJob extends IEntityJob {
   ownerId: string;
   assetPath: string;
@@ -246,7 +250,7 @@ export type JobItem =
 
   // Asset Deletion
   | { name: JobName.PERSON_CLEANUP; data?: IBaseJob }
-  | { name: JobName.ASSET_DELETION; data: IEntityJob }
+  | { name: JobName.ASSET_DELETION; data: IAssetDeleteJob }
   | { name: JobName.ASSET_DELETION_CHECK; data?: IBaseJob }
 
   // Library Management

server/src/services/asset.service.spec.ts

@@ -389,8 +389,8 @@ describe(AssetService.name, () => {
       await sut.deleteAll(authStub.user1, { ids: ['asset1', 'asset2'], force: true });
 
       expect(jobMock.queueAll).toHaveBeenCalledWith([
-        { name: JobName.ASSET_DELETION, data: { id: 'asset1' } },
+        { name: JobName.ASSET_DELETION, data: { id: 'asset1', deleteOnDisk: true } },
-        { name: JobName.ASSET_DELETION, data: { id: 'asset2' } },
+        { name: JobName.ASSET_DELETION, data: { id: 'asset2', deleteOnDisk: true } },
       ]);
     });
 
@@ -410,7 +410,7 @@ describe(AssetService.name, () => {
 
       assetMock.getById.mockResolvedValue(assetWithFace);
 
-      await sut.handleAssetDeletion({ id: assetWithFace.id });
+      await sut.handleAssetDeletion({ id: assetWithFace.id, deleteOnDisk: true });
 
       expect(jobMock.queue.mock.calls).toEqual([
         [
@@ -435,7 +435,7 @@ describe(AssetService.name, () => {
     it('should update stack primary asset if deleted asset was primary asset in a stack', async () => {
       assetMock.getById.mockResolvedValue(assetStub.primaryImage as AssetEntity);
 
-      await sut.handleAssetDeletion({ id: assetStub.primaryImage.id });
+      await sut.handleAssetDeletion({ id: assetStub.primaryImage.id, deleteOnDisk: true });
 
       expect(assetStackMock.update).toHaveBeenCalledWith({
         id: 'stack-1',
@@ -446,10 +446,21 @@ describe(AssetService.name, () => {
     it('should delete a live photo', async () => {
       assetMock.getById.mockResolvedValue(assetStub.livePhotoStillAsset);
 
-      await sut.handleAssetDeletion({ id: assetStub.livePhotoStillAsset.id });
+      await sut.handleAssetDeletion({
+        id: assetStub.livePhotoStillAsset.id,
+        deleteOnDisk: true,
+      });
 
       expect(jobMock.queue.mock.calls).toEqual([
-        [{ name: JobName.ASSET_DELETION, data: { id: assetStub.livePhotoMotionAsset.id } }],
+        [
+          {
+            name: JobName.ASSET_DELETION,
+            data: {
+              id: assetStub.livePhotoMotionAsset.id,
+              deleteOnDisk: true,
+            },
+          },
+        ],
         [
           {
             name: JobName.DELETE_FILES,
@@ -463,7 +474,7 @@ describe(AssetService.name, () => {
 
     it('should update usage', async () => {
       assetMock.getById.mockResolvedValue(assetStub.image);
-      await sut.handleAssetDeletion({ id: assetStub.image.id });
+      await sut.handleAssetDeletion({ id: assetStub.image.id, deleteOnDisk: true });
       expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.image.ownerId, -5000);
     });
   });

server/src/services/asset.service.ts

@@ -27,7 +27,7 @@ import { IAssetStackRepository } from 'src/interfaces/asset-stack.interface';
 import { IAssetRepository } from 'src/interfaces/asset.interface';
 import { ClientEvent, IEventRepository } from 'src/interfaces/event.interface';
 import {
-  IEntityJob,
+  IAssetDeleteJob,
   IJobRepository,
   ISidecarWriteJob,
   JOBS_ASSET_PAGINATION_SIZE,
@@ -256,15 +256,21 @@ export class AssetService {
 
     for await (const assets of assetPagination) {
       await this.jobRepository.queueAll(
-        assets.map((asset) => ({ name: JobName.ASSET_DELETION, data: { id: asset.id } })),
+        assets.map((asset) => ({
+          name: JobName.ASSET_DELETION,
+          data: {
+            id: asset.id,
+            deleteOnDisk: true,
+          },
+        })),
       );
     }
 
     return JobStatus.SUCCESS;
   }
 
-  async handleAssetDeletion(job: IEntityJob): Promise<JobStatus> {
+  async handleAssetDeletion(job: IAssetDeleteJob): Promise<JobStatus> {
-    const { id } = job;
+    const { id, deleteOnDisk } = job;
 
     const asset = await this.assetRepository.getById(id, {
       faces: {
@@ -301,12 +307,14 @@ export class AssetService {
 
     // TODO refactor this to use cascades
     if (asset.livePhotoVideoId) {
-      await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } });
+      await this.jobRepository.queue({
+        name: JobName.ASSET_DELETION,
+        data: { id: asset.livePhotoVideoId, deleteOnDisk },
+      });
     }
 
     const files = [asset.thumbnailPath, asset.previewPath, asset.encodedVideoPath];
-    // skip originals if the user deleted the whole library
+    if (deleteOnDisk) {
-    if (!asset.library?.deletedAt) {
       files.push(asset.sidecarPath, asset.originalPath);
     }
 
@@ -321,7 +329,12 @@ export class AssetService {
     await this.access.requirePermission(auth, Permission.ASSET_DELETE, ids);
 
     if (force) {
-      await this.jobRepository.queueAll(ids.map((id) => ({ name: JobName.ASSET_DELETION, data: { id } })));
+      await this.jobRepository.queueAll(
+        ids.map((id) => ({
+          name: JobName.ASSET_DELETION,
+          data: { id, deleteOnDisk: true },
+        })),
+      );
     } else {
       await this.assetRepository.softDeleteAll(ids);
       this.eventRepository.clientSend(ClientEvent.ASSET_TRASH, auth.user.id, ids);

server/src/services/library.service.spec.ts

@@ -1276,7 +1276,7 @@ describe(LibraryService.name, () => {
       await expect(sut.handleOfflineRemoval({ id: libraryStub.externalLibrary1.id })).resolves.toBe(JobStatus.SUCCESS);
 
       expect(jobMock.queueAll).toHaveBeenCalledWith([
-        { name: JobName.ASSET_DELETION, data: { id: assetStub.image1.id } },
+        { name: JobName.ASSET_DELETION, data: { id: assetStub.image1.id, deleteOnDisk: false } },
       ]);
     });
   });

server/src/services/library.service.ts

@@ -355,7 +355,13 @@ export class LibraryService {
     const assetIds = await this.repository.getAssetIds(job.id, true);
     this.logger.debug(`Will delete ${assetIds.length} asset(s) in library ${job.id}`);
     await this.jobRepository.queueAll(
-      assetIds.map((assetId) => ({ name: JobName.ASSET_DELETION, data: { id: assetId } })),
+      assetIds.map((assetId) => ({
+        name: JobName.ASSET_DELETION,
+        data: {
+          id: assetId,
+          deleteOnDisk: false,
+        },
+      })),
     );
 
     if (assetIds.length === 0) {
@@ -544,7 +550,13 @@ export class LibraryService {
     for await (const assets of assetPagination) {
       this.logger.debug(`Removing ${assets.length} offline assets`);
       await this.jobRepository.queueAll(
-        assets.map((asset) => ({ name: JobName.ASSET_DELETION, data: { id: asset.id } })),
+        assets.map((asset) => ({
+          name: JobName.ASSET_DELETION,
+          data: {
+            id: asset.id,
+            deleteOnDisk: false,
+          },
+        })),
       );
     }
 

server/src/services/metadata.service.spec.ts

@@ -510,7 +510,7 @@ describe(MetadataService.name, () => {
       await sut.handleMetadataExtraction({ id: assetStub.livePhotoWithOriginalFileName.id });
       expect(jobMock.queue).toHaveBeenNthCalledWith(1, {
         name: JobName.ASSET_DELETION,
-        data: { id: assetStub.livePhotoWithOriginalFileName.livePhotoVideoId },
+        data: { id: assetStub.livePhotoWithOriginalFileName.livePhotoVideoId, deleteOnDisk: true },
       });
       expect(jobMock.queue).toHaveBeenNthCalledWith(2, {
         name: JobName.METADATA_EXTRACTION,

server/src/services/metadata.service.ts

@@ -460,7 +460,10 @@ export class MetadataService {
         // (if it did, getByChecksum() would've returned a motionAsset with the same ID as livePhotoVideoId)
         // note asset.livePhotoVideoId is not motionAsset.id yet
         if (asset.livePhotoVideoId) {
-          await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } });
+          await this.jobRepository.queue({
+            name: JobName.ASSET_DELETION,
+            data: { id: asset.livePhotoVideoId, deleteOnDisk: true },
+          });
           this.logger.log(`Removed old motion photo video asset (${asset.livePhotoVideoId})`);
         }
       }

server/src/services/trash.service.spec.ts

@@ -79,7 +79,7 @@ describe(TrashService.name, () => {
       assetMock.getByUserId.mockResolvedValue({ items: [assetStub.image], hasNextPage: false });
       await expect(sut.empty(authStub.user1)).resolves.toBeUndefined();
       expect(jobMock.queueAll).toHaveBeenCalledWith([
-        { name: JobName.ASSET_DELETION, data: { id: assetStub.image.id } },
+        { name: JobName.ASSET_DELETION, data: { id: assetStub.image.id, deleteOnDisk: true } },
       ]);
     });
   });

server/src/services/trash.service.ts

@@ -49,7 +49,13 @@ export class TrashService {
 
     for await (const assets of assetPagination) {
       await this.jobRepository.queueAll(
-        assets.map((asset) => ({ name: JobName.ASSET_DELETION, data: { id: asset.id } })),
+        assets.map((asset) => ({
+          name: JobName.ASSET_DELETION,
+          data: {
+            id: asset.id,
+            deleteOnDisk: true,
+          },
+        })),
       );
     }
   }