fix: library deletion

immich-app · Oct 10, 2024 · feb7c39 · feb7c39
1 parent 94048de
commit feb7c39
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 28 deletions.
diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts
@@ -139,7 +139,7 @@ describe(LibraryService.name, () => {
     });
   });
 
-  describe('handleQueueAssetRefresh', () => {
+  describe('handleQueueSyncFiles', () => {
     it('should queue refresh of a new asset', async () => {
       assetMock.getWith.mockResolvedValue({ items: [], hasNextPage: false });
 
@@ -563,8 +563,8 @@ describe(LibraryService.name, () => {
       expect(jobMock.queueAll).not.toHaveBeenCalled();
     });
 
-    it('should throw BadRequestException when asset does not exist', async () => {
-      storageMock.stat.mockRejectedValue(new Error("ENOENT, no such file or directory '/data/user1/photo.jpg'"));
+    it('should fail when the file could not be read', async () => {
+      storageMock.stat.mockRejectedValue(new Error('Could not read file'));
 
       const mockLibraryJob: ILibraryFileJob = {
         id: libraryStub.externalLibrary1.id,
@@ -576,6 +576,27 @@ describe(LibraryService.name, () => {
       assetMock.create.mockResolvedValue(assetStub.image);
 
       await expect(sut.handleSyncFile(mockLibraryJob)).resolves.toBe(JobStatus.FAILED);
+      expect(libraryMock.get).not.toHaveBeenCalled();
+      expect(assetMock.create).not.toHaveBeenCalled();
+    });
+
+    it('should skip if the file could not be found', async () => {
+      const error = new Error('File not found') as any;
+      error.code = 'ENOENT';
+      storageMock.stat.mockRejectedValue(error);
+
+      const mockLibraryJob: ILibraryFileJob = {
+        id: libraryStub.externalLibrary1.id,
+        ownerId: userStub.admin.id,
+        assetPath: '/data/user1/photo.jpg',
+      };
+
+      assetMock.getByLibraryIdAndOriginalPath.mockResolvedValue(null);
+      assetMock.create.mockResolvedValue(assetStub.image);
+
+      await expect(sut.handleSyncFile(mockLibraryJob)).resolves.toBe(JobStatus.SKIPPED);
+      expect(libraryMock.get).not.toHaveBeenCalled();
+      expect(assetMock.create).not.toHaveBeenCalled();
     });
   });
 
@@ -658,6 +679,10 @@ describe(LibraryService.name, () => {
 
       expect(libraryMock.getStatistics).toHaveBeenCalledWith(libraryStub.externalLibrary1.id);
     });
+
+    it('should throw an error if the library could not be found', async () => {
+      await expect(sut.getStatistics('foo')).rejects.toBeInstanceOf(BadRequestException);
+    });
   });
 
   describe('create', () => {
@@ -787,6 +812,13 @@ describe(LibraryService.name, () => {
     });
   });
 
+  describe('getAll', () => {
+    it('should get all libraries', async () => {
+      libraryMock.getAll.mockResolvedValue([libraryStub.externalLibrary1]);
+      await expect(sut.getAll()).resolves.toEqual([expect.objectContaining({ id: libraryStub.externalLibrary1.id })]);
+    });
+  });
+
   describe('handleQueueCleanup', () => {
     it('should queue cleanup jobs', async () => {
       libraryMock.getAllDeleted.mockResolvedValue([libraryStub.externalLibrary1, libraryStub.externalLibrary2]);
@@ -815,7 +847,17 @@ describe(LibraryService.name, () => {
     });
   });
 
+  describe('onShutdown', () => {
+    it('should do nothing if instance does not have the watch lock', async () => {
+      await sut.onShutdown();
+    });
+  });
+
   describe('watchAll', () => {
+    it('should return false if instance does not have the watch lock', async () => {
+      await expect(sut.watchAll()).resolves.toBe(false);
+    });
+
     describe('watching disabled', () => {
       beforeEach(async () => {
         systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchDisabled);
@@ -990,15 +1032,14 @@ describe(LibraryService.name, () => {
     it('should delete an empty library', async () => {
       libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
       assetMock.getAll.mockResolvedValue({ items: [], hasNextPage: false });
-      libraryMock.delete.mockImplementation(async () => {});
 
       await expect(sut.handleDeleteLibrary({ id: libraryStub.externalLibrary1.id })).resolves.toBe(JobStatus.SUCCESS);
+      expect(libraryMock.delete).toHaveBeenCalled();
     });
 
-    it('should delete a library with assets', async () => {
+    it('should delete all assets in a library', async () => {
       libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
       assetMock.getAll.mockResolvedValue({ items: [assetStub.image1], hasNextPage: false });
-      libraryMock.delete.mockImplementation(async () => {});
 
       assetMock.getById.mockResolvedValue(assetStub.image1);
 

diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts
@@ -341,7 +341,10 @@ export class LibraryService extends BaseService {
 
     this.logger.debug(`Will delete all assets in library ${libraryId}`);
     for await (const assets of assetPagination) {
-      assetsFound = true;
+      if (assets.length > 0) {
+        assetsFound = true;
+      }
+
       this.logger.debug(`Queueing deletion of ${assets.length} asset(s) in library ${libraryId}`);
       await this.jobRepository.queueAll(
         assets.map((asset) => ({
@@ -545,30 +548,30 @@ export class LibraryService extends BaseService {
       }
     }
 
-    if (validImportPaths) {
-      const assetsOnDisk = this.storageRepository.walk({
-        pathsToCrawl: validImportPaths,
-        includeHidden: false,
-        exclusionPatterns: library.exclusionPatterns,
-        take: JOBS_LIBRARY_PAGINATION_SIZE,
-      });
+    if (validImportPaths.length === 0) {
+      this.logger.warn(`No valid import paths found for library ${library.id}`);
+    }
+
+    const assetsOnDisk = this.storageRepository.walk({
+      pathsToCrawl: validImportPaths,
+      includeHidden: false,
+      exclusionPatterns: library.exclusionPatterns,
+      take: JOBS_LIBRARY_PAGINATION_SIZE,
+    });
 
-      let count = 0;
+    let count = 0;
 
-      for await (const assetBatch of assetsOnDisk) {
-        count += assetBatch.length;
-        this.logger.debug(`Discovered ${count} asset(s) on disk for library ${library.id}...`);
-        await this.syncFiles(library, assetBatch);
-        this.logger.verbose(`Queued scan of ${assetBatch.length} crawled asset(s) in library ${library.id}...`);
-      }
+    for await (const assetBatch of assetsOnDisk) {
+      count += assetBatch.length;
+      this.logger.debug(`Discovered ${count} asset(s) on disk for library ${library.id}...`);
+      await this.syncFiles(library, assetBatch);
+      this.logger.verbose(`Queued scan of ${assetBatch.length} crawled asset(s) in library ${library.id}...`);
+    }
 
-      if (count > 0) {
-        this.logger.debug(`Finished queueing scan of ${count} assets on disk for library ${library.id}`);
-      } else {
-        this.logger.debug(`No non-excluded assets found in any import path for library ${library.id}`);
-      }
-    } else {
-      this.logger.warn(`No valid import paths found for library ${library.id}`);
+    if (count > 0) {
+      this.logger.debug(`Finished queueing scan of ${count} assets on disk for library ${library.id}`);
+    } else if (validImportPaths.length > 0) {
+      this.logger.debug(`No non-excluded assets found in any import path for library ${library.id}`);
     }
 
     await this.libraryRepository.update({ id: job.id, refreshedAt: new Date() });