fix: add limit 1 to backup queries instead of fetching all items in a folder (#1039)

Author: apsantiso

src/modules/backups/backup.usecase.spec.ts

@@ -6,7 +6,6 @@ import { SequelizeBackupRepository } from './backup.repository';
 import { BridgeService } from '../../externals/bridge/bridge.service';
 import { CryptoService } from '../../externals/crypto/crypto.service';
 import { FolderUseCases } from '../folder/folder.usecase';
-import { FileUseCases } from '../file/file.usecase';
 import {
   NotFoundException,
   ConflictException,
@@ -15,16 +14,17 @@ import {
 import { type Folder } from '../folder/folder.domain';
 import { DevicePlatform } from './device.domain';
 import { SequelizeFolderRepository } from '../folder/folder.repository';
+import { SequelizeFileRepository } from '../file/file.repository';
 
 describe('BackupUseCase', () => {
   let backupUseCase: BackupUseCase;
   let backupRepository: SequelizeBackupRepository;
   let folderRepository: SequelizeFolderRepository;
+  let fileRepository: SequelizeFileRepository;
 
   let bridgeService: BridgeService;
   let cryptoService: CryptoService;
   let folderUseCases: FolderUseCases;
-  let fileUseCases: FileUseCases;
 
   const userMocked = newUser({
     attributes: {
@@ -55,10 +55,12 @@ describe('BackupUseCase', () => {
     bridgeService = module.get<BridgeService>(BridgeService);
     cryptoService = module.get<CryptoService>(CryptoService);
     folderUseCases = module.get<FolderUseCases>(FolderUseCases);
-    fileUseCases = module.get<FileUseCases>(FileUseCases);
     folderRepository = module.get<SequelizeFolderRepository>(
       SequelizeFolderRepository,
     );
+    fileRepository = module.get<SequelizeFileRepository>(
+      SequelizeFileRepository,
+    );
   });
 
   describe('activate', () => {
@@ -81,9 +83,8 @@ describe('BackupUseCase', () => {
 
   describe('createDeviceAsFolder', () => {
     it('When a folder with the same name exists, then it should throw a ConflictException', async () => {
-      jest
-        .spyOn(folderUseCases, 'getFolders')
-        .mockResolvedValue([{ id: 1, name: 'Device Folder' }] as any);
+      const existentFolder = newFolder();
+      jest.spyOn(folderRepository, 'findOne').mockResolvedValue(existentFolder);
 
       await expect(
         backupUseCase.createDeviceAsFolder(userMocked, 'Device Folder'),
@@ -92,10 +93,11 @@ describe('BackupUseCase', () => {
 
     it('When no folder with the same name exists, then it should create the folder', async () => {
       const mockFolder = newFolder();
-      jest.spyOn(folderUseCases, 'getFolders').mockResolvedValue([]);
+      jest.spyOn(folderRepository, 'findOne').mockResolvedValue(null);
       jest
-        .spyOn(folderUseCases, 'createFolderDevice')
+        .spyOn(folderRepository, 'createFolder')
         .mockResolvedValue(mockFolder);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       const result = await backupUseCase.createDeviceAsFolder(
         userMocked,
@@ -123,6 +125,7 @@ describe('BackupUseCase', () => {
       jest
         .spyOn(folderUseCases, 'getFoldersByUserId')
         .mockResolvedValue([mockFolder]);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       const result = await backupUseCase.getDevicesAsFolder(userMocked);
 
@@ -189,10 +192,9 @@ describe('BackupUseCase', () => {
   describe('isFolderEmpty', () => {
     it('When the folder has no subfolders or files, then it should return true', async () => {
       const parentFolder = newFolder();
-      jest
-        .spyOn(folderUseCases, 'getFoldersByParentUuid')
-        .mockResolvedValue([]);
-      jest.spyOn(fileUseCases, 'getByFolderAndUser').mockResolvedValue([]);
+
+      jest.spyOn(folderRepository, 'findOne').mockResolvedValue(null);
+      jest.spyOn(fileRepository, 'findOneBy').mockResolvedValue(null);
 
       const result = await backupUseCase.isFolderEmpty(
         userMocked,
@@ -203,12 +205,9 @@ describe('BackupUseCase', () => {
 
     it('When the folder has subfolders or files, then it should return false', async () => {
       const parentFolder = newFolder();
-      const mockFolder = newFolder();
-
-      jest
-        .spyOn(folderUseCases, 'getFoldersByParentUuid')
-        .mockResolvedValue([mockFolder]);
-      jest.spyOn(fileUseCases, 'getByFolderAndUser').mockResolvedValue([]);
+      const folderInBackup = newFolder();
+      jest.spyOn(folderRepository, 'findOne').mockResolvedValue(folderInBackup);
+      jest.spyOn(fileRepository, 'findOneBy').mockResolvedValue(null);
 
       const result = await backupUseCase.isFolderEmpty(
         userMocked,
@@ -354,6 +353,7 @@ describe('BackupUseCase', () => {
       jest
         .spyOn(folderUseCases, 'updateByFolderIdAndForceUpdatedAt')
         .mockResolvedValue(updatedFolder);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       const result = await backupUseCase.updateDeviceAsFolder(
         userMocked,
@@ -570,6 +570,7 @@ describe('BackupUseCase', () => {
       jest
         .spyOn(folderRepository, 'findByUuids')
         .mockResolvedValue([mockFolder]);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       const result = await backupUseCase.getUserDevices(userMocked, {}, 10, 0);
 
@@ -837,6 +838,7 @@ describe('BackupUseCase', () => {
       jest
         .spyOn(backupRepository, 'createDevice')
         .mockResolvedValue(mockDevice);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       const result = await backupUseCase.createDeviceForExistingFolder(
         userMocked,
@@ -1078,6 +1080,7 @@ describe('BackupUseCase', () => {
       jest
         .spyOn(folderRepository, 'updateOneAndReturn')
         .mockResolvedValue(mockFolder);
+      jest.spyOn(backupUseCase, 'isFolderEmpty').mockResolvedValue(true);
 
       jest
         .spyOn(backupRepository, 'updateDeviceName')

src/modules/backups/backup.usecase.ts

@@ -11,7 +11,6 @@ import { type User } from '../user/user.domain';
 import { BridgeService } from './../../externals/bridge/bridge.service';
 import { CryptoService } from './../../externals/crypto/crypto.service';
 import { FolderUseCases } from '../folder/folder.usecase';
-import { FileUseCases } from '../file/file.usecase';
 import { Folder, type FolderAttributes } from '../folder/folder.domain';
 import { SequelizeUserRepository } from '../user/user.repository';
 import { type BackupModel } from './models/backup.model';
@@ -21,6 +20,8 @@ import { type CreateDeviceAndAttachFolderDto } from './dto/create-device-and-att
 import { type DevicePlatform } from './device.domain';
 import { type UpdateDeviceAndFolderDto } from './dto/update-device-and-folder.dto';
 import { SequelizeFolderRepository } from '../folder/folder.repository';
+import { SequelizeFileRepository } from '../file/file.repository';
+import { FileStatus } from '../file/file.domain';
 
 @Injectable()
 export class BackupUseCase {
@@ -32,8 +33,7 @@ export class BackupUseCase {
     @Inject(forwardRef(() => FolderUseCases))
     private readonly folderUsecases: FolderUseCases,
     private readonly folderRepository: SequelizeFolderRepository,
-    @Inject(forwardRef(() => FileUseCases))
-    private readonly fileUsecases: FileUseCases,
+    private readonly fileRepository: SequelizeFileRepository,
   ) {}
 
   async deleteUserBackups(userId: number) {
@@ -49,6 +49,7 @@ export class BackupUseCase {
     return this.backupRepository.findAllLegacyDevices(user);
   }
 
+  // @deprecated
   async deleteDevice(user: User, deviceId: number) {
     const device = await this.backupRepository.findDeviceByUserAndId(
       user,
@@ -97,18 +98,20 @@ export class BackupUseCase {
       bucket = backupsBucket;
     }
 
-    const folders = await this.folderUsecases.getFolders(user.id, {
+    // We do not have an index to cover this query, but it is not a frequent operation
+    const folder = await this.folderRepository.findOne({
       bucket,
       plainName: deviceName,
       deleted: false,
       removed: false,
+      userId: user.id,
     });
 
-    if (folders.length > 0) {
+    if (folder) {
       throw new ConflictException('Folder with the same name already exists');
     }
 
-    const createdFolder = await this.folderUsecases.createFolderDevice(user, {
+    const createdFolder = await this.folderRepository.createFolder(user.id, {
       plainName: deviceName,
       bucket,
     });
@@ -483,16 +486,19 @@ export class BackupUseCase {
   }
 
   async isFolderEmpty(user: User, folder: Folder) {
-    const folders = await this.folderUsecases.getFoldersByParentUuid(
-      folder.uuid,
-      user.id,
-    );
-    const files = await this.fileUsecases.getByFolderAndUser(
-      folder.id,
-      user.id,
-      { deleted: false },
-    );
-    return folders.length === 0 && files.length === 0;
+    const folderInFolder = await this.folderRepository.findOne({
+      parentUuid: folder.uuid,
+      userId: user.id,
+      removed: false,
+      deleted: false,
+    });
+    const fileInFolder = await this.fileRepository.findOneBy({
+      folderUuid: folder.uuid,
+      userId: user.id,
+      status: FileStatus.EXISTS,
+    });
+
+    return !folderInFolder && !fileInFolder;
   }
 
   async sumExistentBackupSizes(userId: number) {

src/modules/file/file.usecase.spec.ts

@@ -3313,25 +3313,6 @@ describe('FileUseCases', () => {
     });
   });
 
-  describe('getFilesByFolderUuid', () => {
-    it('When called with folder uuid and status, then it should return files', async () => {
-      const folderUuid = v4();
-      const status = FileStatus.EXISTS;
-      const mockFiles = [newFile()];
-      jest
-        .spyOn(fileRepository, 'getFilesByFolderUuid')
-        .mockResolvedValue(mockFiles);
-
-      const result = await service.getFilesByFolderUuid(folderUuid, status);
-
-      expect(result).toEqual(mockFiles);
-      expect(fileRepository.getFilesByFolderUuid).toHaveBeenCalledWith(
-        folderUuid,
-        status,
-      );
-    });
-  });
-
   describe('hasUploadedFiles', () => {
     it('When user has uploaded files, then it should return true', async () => {
       const mockFile = newFile();

src/modules/file/file.usecase.ts

@@ -490,13 +490,6 @@ export class FileUseCases {
     };
   }
 
-  async getFilesByFolderUuid(
-    folderUuid: FileAttributes['folderUuid'],
-    status: FileStatus,
-  ) {
-    return this.fileRepository.getFilesByFolderUuid(folderUuid, status);
-  }
-
   getFilesByIds(user: User, fileIds: File['id'][]): Promise<File[]> {
     return this.fileRepository.findByIds(user.id, fileIds);
   }