fix(server): never unlink an untracked-file path that an asset now references (#31194)

fix(server): never unlink an untracked-file path that an asset now references (#31074)

(cherry picked from commit c1f2756f62)

Co-authored-by: Aditya Raj Singh <aditya@bncw.in>

server/src/queries/integrity.repository.sql

@@ -65,6 +65,28 @@ from
 where
   "person"."thumbnailPath" in $1
 
+-- IntegrityRepository.getTrackedPaths
+select
+  "asset"."originalPath" as "path"
+from
+  "asset"
+where
+  "asset"."originalPath" in $1
+union
+select
+  "asset_file"."path" as "path"
+from
+  "asset_file"
+where
+  "asset_file"."path" in $2
+union
+select
+  "person"."thumbnailPath" as "path"
+from
+  "person"
+where
+  "person"."thumbnailPath" in $3
+
 -- IntegrityRepository.getAssetCount
 select
   count(*) as "count"

server/src/repositories/integrity.repository.ts

@@ -94,6 +94,24 @@ export class IntegrityRepository {
       .execute();
   }
 
+  @GenerateSql({ params: [DummyValue.STRING] })
+  getTrackedPaths(paths: string[]) {
+    return this.db
+      .selectFrom('asset')
+      .select('asset.originalPath as path')
+      .where('asset.originalPath', 'in', paths)
+      .union((eb) =>
+        eb.selectFrom('asset_file').select('asset_file.path as path').where('asset_file.path', 'in', paths),
+      )
+      .union((eb) =>
+        eb
+          .selectFrom('person')
+          .select((eb) => eb.ref('person.thumbnailPath').$castTo<string>().as('path'))
+          .where('person.thumbnailPath', 'in', paths),
+      )
+      .execute();
+  }
+
   @GenerateSql({ params: [] })
   getAssetCount() {
     return this.db

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

@@ -13,6 +13,69 @@ describe(IntegrityService.name, () => {
     expect(sut).toBeDefined();
   });
 
+  describe('handleUntrackedRefresh', () => {
+    beforeEach(() => {
+      mocks.integrityReport.getTrackedPaths.mockResolvedValue([]);
+    });
+
+    it('should delete a report whose path is now referenced by an asset', async () => {
+      const path = '/data/upload/admin/ab/asset.mov';
+      mocks.integrityReport.getTrackedPaths.mockResolvedValue([{ path }] as never);
+
+      await sut.handleUntrackedRefresh({ items: [{ reportId: 'report-id', path }] });
+
+      expect(mocks.integrityReport.deleteByIds).toHaveBeenCalledWith(['report-id']);
+      expect(mocks.storage.stat).not.toHaveBeenCalled();
+    });
+
+    it('should keep a report whose path is still untracked and present on disk', async () => {
+      mocks.storage.stat.mockResolvedValue({} as never);
+
+      await sut.handleUntrackedRefresh({ items: [{ reportId: 'report-id', path: '/data/upload/orphan.mov' }] });
+
+      expect(mocks.integrityReport.deleteByIds).not.toHaveBeenCalled();
+    });
+
+    it('should not query for references when the batch is empty', async () => {
+      await sut.handleUntrackedRefresh({ items: [] });
+
+      expect(mocks.integrityReport.getTrackedPaths).not.toHaveBeenCalled();
+      expect(mocks.integrityReport.deleteByIds).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('deleteIntegrityReport', () => {
+    it('should not unlink a path that is now tracked', async () => {
+      const path = '/data/upload/admin/ab/asset.mov';
+      mocks.integrityReport.getById.mockResolvedValue({ path } as never);
+      mocks.integrityReport.getTrackedPaths.mockResolvedValue([{ path }] as never);
+
+      await sut.deleteIntegrityReport('user-id', 'report-id');
+
+      expect(mocks.storage.unlink).not.toHaveBeenCalled();
+      expect(mocks.integrityReport.deleteById).toHaveBeenCalledWith('report-id');
+    });
+  });
+
+  describe('handleDeleteIntegrityReports', () => {
+    it('should unlink only paths that are still untracked', async () => {
+      const tracked = '/data/upload/admin/ab/asset.mov';
+      const untracked = '/data/upload/orphan.mov';
+      mocks.integrityReport.getTrackedPaths.mockResolvedValue([{ path: tracked }] as never);
+      mocks.storage.unlink.mockResolvedValue();
+
+      await sut.handleDeleteIntegrityReports({
+        reports: [
+          { id: 'tracked-report', path: tracked },
+          { id: 'untracked-report', path: untracked },
+        ] as never,
+      });
+
+      expect(mocks.storage.unlink).toHaveBeenCalledExactlyOnceWith(untracked);
+      expect(mocks.integrityReport.deleteByIds).toHaveBeenCalledWith(['tracked-report', 'untracked-report']);
+    });
+  });
+
   describe('handleDeleteAllIntegrityReports', () => {
     beforeEach(() => {
       mocks.integrityReport.streamIntegrityReportsByProperty.mockReturnValue((function* () {})() as never);

server/src/services/integrity.service.ts

@@ -190,7 +190,10 @@ export class IntegrityService extends BaseService {
     } else if (fileAssetId) {
       await this.assetRepository.deleteFiles([{ id: fileAssetId }]);
     } else {
-      await this.storageRepository.unlink(path);
+      const trackedPaths = await this.integrityRepository.getTrackedPaths([path]);
+      if (trackedPaths.length === 0) {
+        await this.storageRepository.unlink(path);
+      }
       await this.integrityRepository.deleteById(id);
     }
   }
@@ -311,8 +314,17 @@ export class IntegrityService extends BaseService {
   async handleUntrackedRefresh({ items }: IIntegrityPathWithReportJob): Promise<JobStatus> {
     this.logger.log(`Processing batch of ${items.length} reports to check if they are out of date.`);
 
+    const tracked =
+      items.length > 0 ? await this.integrityRepository.getTrackedPaths(items.map(({ path }) => path)) : [];
+    const trackedPaths = new Set(tracked.map(({ path }) => path));
+
     const results = await Promise.all(
       items.map(async ({ reportId, path }) => {
+        // The path was untracked when the report was written; an asset may reference it now.
+        if (trackedPaths.has(path)) {
+          return reportId;
+        }
+
         try {
           await this.storageRepository.stat(path);
           return;
@@ -697,7 +709,13 @@ export class IntegrityService extends BaseService {
     }
 
     if (byPath.length > 0) {
-      await Promise.all(byPath.map(({ path }) => this.storageRepository.unlink(path).catch(() => void 0)));
+      const tracked = await this.integrityRepository.getTrackedPaths(byPath.map(({ path }) => path));
+      const trackedPaths = new Set(tracked.map(({ path }) => path));
+      await Promise.all(
+        byPath
+          .filter(({ path }) => !trackedPaths.has(path))
+          .map(({ path }) => this.storageRepository.unlink(path).catch(() => void 0)),
+      );
       await this.integrityRepository.deleteByIds(byPath.map(({ id }) => id));
     }