Skip to content

Commit

Permalink
Merge pull request #442 from internxt/fix/lopp-deleting-files
Browse files Browse the repository at this point in the history
[PB-1513]: Fix/loop-when-delete-files
  • Loading branch information
miguelsw authored Feb 9, 2024
2 parents 9ac6b6a + f0678a1 commit 09c59e8
Show file tree
Hide file tree
Showing 15 changed files with 214 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export function buildControllers(container: DependencyContainer) {

const deleteController = new DeleteController(
container.fileDeleter,
container.folderDeleter
container.retryFolderDeleter,
container.fileFolderContainerDetector,
container.folderContainerDetector
);

const renameOrMoveController = new RenameOrMoveController(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import { FileDeleter } from '../../../../context/virtual-drive/files/application/FileDeleter';
import { FolderDeleter } from '../../../../context/virtual-drive/folders/application/FolderDeleter';
import { RetryFolderDeleter } from '../../../../context/virtual-drive/folders/application/RetryFolderDeleter';
import { DelayQueue } from '../../../../context/virtual-drive/shared/domain/DelayQueue';
import { CallbackController } from './CallbackController';
import Logger from 'electron-log';
import { FileFolderContainerDetector } from '../../../../context/virtual-drive/files/application/FileFolderContainerDetector';
import { Folder } from '../../../../context/virtual-drive/folders/domain/Folder';
import { FolderContainerDetector } from '../../../../context/virtual-drive/folders/application/FolderContainerDetector';

export class DeleteController extends CallbackController {
private readonly filesQueue: DelayQueue;
private readonly foldersQueue: DelayQueue;

constructor(
private readonly fileDeleter: FileDeleter,
private readonly folderDeleter: FolderDeleter
private readonly retryFolderDeleter: RetryFolderDeleter,
private readonly fileFolderContainerDetector: FileFolderContainerDetector,
private readonly folderContainerDetector: FolderContainerDetector
) {
super();

Expand All @@ -19,7 +24,12 @@ export class DeleteController extends CallbackController {
};

const deleteFolder = async (folder: string) => {
await this.folderDeleter.run(folder);
try {
await this.retryFolderDeleter.run(folder);
} catch (error) {
Logger.error('Error deleting folder: ', error);
// TODO: create tree of placeholders that are not deleted
}
};

const canDeleteFolders = () => {
Expand All @@ -34,8 +44,7 @@ export class DeleteController extends CallbackController {
);

const canDeleteFiles = () => {
// Files cannot be deleted if there are folders on the queue
return this.foldersQueue.size === 0;
return this.foldersQueue.isEmpty;
};

this.filesQueue = new DelayQueue('files', deleteFile, canDeleteFiles);
Expand All @@ -54,10 +63,41 @@ export class DeleteController extends CallbackController {
if (this.isFolderPlaceholder(trimmedId)) {
const [_, folderUuid] = trimmedId.split(':');
Logger.debug(`Adding folder: ${folderUuid} to the trash queue`);
this.CleanQueuesByFolder(folderUuid);
this.foldersQueue.push(folderUuid);
return;
}

throw new Error(`Placeholder Id not identified: ${trimmedId}`);
}

private CleanQueuesByFolder(folderUuid: Folder['uuid']) {
// always remove files from the filesQueue if a folder is added
this.CleanQueueFile(folderUuid);
// remove child folders from the queue if a parent folder exists
this.CleanQueueFolder(folderUuid);
}

private CleanQueueFile(folderUuid: Folder['uuid']) {
const files = this.filesQueue.values;
const filesToDelete = files.filter((file) =>
this.fileFolderContainerDetector.run(file, folderUuid)
);
filesToDelete.forEach((file) => {
this.filesQueue.removeOne(file);
});
}

private CleanQueueFolder(folderUuid: Folder['uuid']) {
const reversedFolders = this.foldersQueue.reversedValues;
reversedFolders.forEach((folder) => {
const isParentFolder = this.folderContainerDetector.run(
folder,
folderUuid
);
if (isParentFolder) {
this.foldersQueue.removeOne(folder);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import { FilesPlaceholderCreator } from '../../../../context/virtual-drive/files
import { RepositoryPopulator } from '../../../../context/virtual-drive/files/application/RepositoryPopulator';
import { RetrieveAllFiles } from '../../../../context/virtual-drive/files/application/RetrieveAllFiles';
import { SameFileWasMoved } from '../../../../context/virtual-drive/files/application/SameFileWasMoved';
import { FileFolderContainerDetector } from '../../../../context/virtual-drive/files/application/FileFolderContainerDetector';
import { FileSyncronizer } from '../../../../context/virtual-drive/files/application/FileSyncronizer';
import { FilePlaceholderConverter } from '../../../../context/virtual-drive/files/application/FIlePlaceholderConverter';
import { FileSyncStatusUpdater } from '../../../../context/virtual-drive/files/application/FileSyncStatusUpdater';

export interface FilesContainer {
fileFinderByContentsId: FileFinderByContentsId;
fileDeleter: FileDeleter;
fileFolderContainerDetector: FileFolderContainerDetector;
filePathUpdater: FilePathUpdater;
fileCreator: FileCreator;
fileSyncronizer: FileSyncronizer;
Expand Down
7 changes: 7 additions & 0 deletions src/apps/sync-engine/dependency-injection/files/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { SDKRemoteFileSystem } from '../../../../context/virtual-drive/files/inf
import { NodeWinLocalFileSystem } from '../../../../context/virtual-drive/files/infrastructure/NodeWinLocalFileSystem';
import { LocalFileIdProvider } from '../../../../context/virtual-drive/shared/application/LocalFileIdProvider';
import { DependencyInjectionHttpClientsProvider } from '../common/clients';
import { FileFolderContainerDetector } from '../../../../context/virtual-drive/files/application/FileFolderContainerDetector';
import { FileSyncronizer } from '../../../../context/virtual-drive/files/application/FileSyncronizer';
import { FilePlaceholderConverter } from '../../../../context/virtual-drive/files/application/FIlePlaceholderConverter';
import { FileSyncStatusUpdater } from '../../../../context/virtual-drive/files/application/FileSyncStatusUpdater';
Expand Down Expand Up @@ -67,6 +68,11 @@ export async function buildFilesContainer(
ipcRendererSyncEngine
);

const fileFolderContainerDetector = new FileFolderContainerDetector(
repository,
folderContainer.folderFinder
);

const sameFileWasMoved = new SameFileWasMoved(
repository,
localFileSystem,
Expand Down Expand Up @@ -147,6 +153,7 @@ export async function buildFilesContainer(
fileDeleter,
filePathUpdater,
fileCreator,
fileFolderContainerDetector,
fileSyncronizer,
filePlaceholderCreatorFromContentsId: filePlaceholderCreatorFromContentsId,
createFilePlaceholderOnDeletionFailed:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { AllParentFoldersStatusIsExists } from '../../../../context/virtual-drive/folders/application/AllParentFoldersStatusIsExists';
import { FolderByPartialSearcher } from '../../../../context/virtual-drive/folders/application/FolderByPartialSearcher';
import { FolderContainerDetector } from '../../../../context/virtual-drive/folders/application/FolderContainerDetector';
import { FolderCreator } from '../../../../context/virtual-drive/folders/application/FolderCreator';
import { FolderDeleter } from '../../../../context/virtual-drive/folders/application/FolderDeleter';
import { FolderFinder } from '../../../../context/virtual-drive/folders/application/FolderFinder';
Expand All @@ -9,6 +10,7 @@ import { FoldersPlaceholderCreator } from '../../../../context/virtual-drive/fol
import { OfflineFolderCreator } from '../../../../context/virtual-drive/folders/application/Offline/OfflineFolderCreator';
import { OfflineFolderPathUpdater } from '../../../../context/virtual-drive/folders/application/Offline/OfflineFolderPathUpdater';
import { RetrieveAllFolders } from '../../../../context/virtual-drive/folders/application/RetrieveAllFolders';
import { RetryFolderDeleter } from '../../../../context/virtual-drive/folders/application/RetryFolderDeleter';
import { SynchronizeOfflineModifications } from '../../../../context/virtual-drive/folders/application/SynchronizeOfflineModifications';
import { SynchronizeOfflineModificationsOnFolderCreated } from '../../../../context/virtual-drive/folders/application/SynchronizeOfflineModificationsOnFolderCreated';
import { FolderPlaceholderUpdater } from '../../../../context/virtual-drive/folders/application/UpdatePlaceholderFolder';
Expand All @@ -19,7 +21,9 @@ import { FoldersFatherSyncStatusUpdater } from '../../../../context/virtual-driv
export interface FoldersContainer {
folderCreator: FolderCreator;
folderFinder: FolderFinder;
folderContainerDetector: FolderContainerDetector;
folderDeleter: FolderDeleter;
retryFolderDeleter: RetryFolderDeleter;
allParentFoldersStatusIsExists: AllParentFoldersStatusIsExists;
folderPathUpdater: FolderPathUpdater;
folderByPartialSearcher: FolderByPartialSearcher;
Expand Down
7 changes: 7 additions & 0 deletions src/apps/sync-engine/dependency-injection/folders/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import { DependencyInjectionEventRepository } from '../common/eventRepository';
import { DependencyInjectionVirtualDrive } from '../common/virtualDrive';
import { SharedContainer } from '../shared/SharedContainer';
import { FoldersContainer } from './FoldersContainer';
import { RetryFolderDeleter } from '../../../../context/virtual-drive/folders/application/RetryFolderDeleter';
import { FolderContainerDetector } from '../../../../context/virtual-drive/folders/application/FolderContainerDetector';
import { FolderPlaceholderConverter } from '../../../../context/virtual-drive/folders/application/FolderPlaceholderConverter';
import { FolderSyncStatusUpdater } from '../../../../context/virtual-drive/folders/application/FolderSyncStatusUpdater';
import { FoldersFatherSyncStatusUpdater } from '../../../../context/virtual-drive/folders/application/FoldersFatherSyncStatusUpdater';
Expand Down Expand Up @@ -69,6 +71,8 @@ export async function buildFoldersContainer(
allParentFoldersStatusIsExists
);

const retryFolderDeleter = new RetryFolderDeleter(folderDeleter);

const folderCreator = new FolderCreator(
repository,
remoteFileSystem,
Expand Down Expand Up @@ -137,6 +141,7 @@ export async function buildFoldersContainer(
shredContainer.relativePathToAbsoluteConverter
);

const folderContainerDetector = new FolderContainerDetector(repository);
const foldersFatherSyncStatusUpdater = new FoldersFatherSyncStatusUpdater(
localFileSystem,
repository
Expand All @@ -146,8 +151,10 @@ export async function buildFoldersContainer(
folderCreator,
folderFinder,
folderDeleter,
retryFolderDeleter,
allParentFoldersStatusIsExists: allParentFoldersStatusIsExists,
folderPathUpdater,
folderContainerDetector,
folderByPartialSearcher,
synchronizeOfflineModificationsOnFolderCreated,
offline: {
Expand Down
3 changes: 3 additions & 0 deletions src/apps/sync-engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,8 @@ setUp()
})
.catch((error) => {
Logger.error('[SYNC ENGINE] Error setting up', error);
if (error.toString().includes('Error: ConnectSyncRoot failed')) {
Logger.info('[SYNC ENGINE] We neeed to restart the app virtual drive');
}
ipcRenderer.send('SYNC_ENGINE_PROCESS_SETUP_FAILED');
});
4 changes: 1 addition & 3 deletions src/context/virtual-drive/files/application/FileDeleter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class FileDeleter {
);

const message = error instanceof Error ? error.message : 'Unknown error';

this.local.createPlaceHolder(file);
this.ipc.send('FILE_DELETION_ERROR', {
name: file.name,
extension: file.type,
Expand All @@ -80,8 +80,6 @@ export class FileDeleter {
errorName: 'BAD_RESPONSE',
process: 'SYNC',
});

this.local.createPlaceHolder(file);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { FolderFinder } from '../../folders/application/FolderFinder';
import { File } from '../../files/domain/File';
import { Folder } from '../../folders/domain/Folder';
import { FileRepository } from '../domain/FileRepository';
import { FileNotFoundError } from '../domain/errors/FileNotFoundError';

export class FileFolderContainerDetector {
constructor(
private readonly repository: FileRepository,
private readonly folderFinder: FolderFinder
) {}

run(contentId: File['contentsId'], folderContentId: Folder['uuid']): boolean {
const file = this.repository.searchByPartial({
contentsId: contentId,
});
if (!file) {
throw new FileNotFoundError(contentId);
}
const folder = this.folderFinder.findFromId(file.folderId);
return folder.uuid === folderContentId;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Folder } from '../domain/Folder';
import { FolderRepository } from '../domain/FolderRepository';

export class FolderContainerDetector {
constructor(private readonly repository: FolderRepository) {}

run(
fodlerContentId: Folder['uuid'],
parentFolderContentId: Folder['uuid']
): boolean {
const folder = this.repository.searchByPartial({ uuid: fodlerContentId });

if (!folder) {
throw new Error('Folder not found');
}

const parent = this.repository.searchByPartial({
id: folder.parentId as number,
});

if (!parent) {
throw new Error('Parent folder not found');
}

return parent.uuid === parentFolderContentId;
}
}
8 changes: 8 additions & 0 deletions src/context/virtual-drive/folders/application/FolderFinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,12 @@ export class FolderFinder {

return folder;
}

findFromId(id: Folder['id']): Folder {
const folder = this.repository.searchByPartial({ id });
if (!folder) {
throw new Error('Folder not found');
}
return folder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OfflineFolder } from '../../domain/OfflineFolder';
import { OfflineFolderRepository } from '../../domain/OfflineFolderRepository';
import { FolderFinder } from '../FolderFinder';
import { FolderRepository } from '../../domain/FolderRepository';
import { FolderStatuses } from '../../domain/FolderStatus';

export class OfflineFolderCreator {
constructor(
Expand All @@ -16,6 +17,7 @@ export class OfflineFolderCreator {

const onlineFolder = this.repository.searchByPartial({
path: folderPath.value,
status: FolderStatuses.EXISTS,
});

if (onlineFolder) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import Logger from 'electron-log';
import { FolderDeleter } from './FolderDeleter';
import { MaxRetriesDeletingFolderError } from '../domain/errors/MaxRetriesDeletingFolderError';

export class RetryFolderDeleter {
private static NUMBER_OF_RETRIES = 2;
private static MILLISECOND_BETWEEN_TRIES = 1_000;
private static INITIAL_DELAY = 100;
constructor(private readonly deleter: FolderDeleter) {}
async retryDeleter(asyncFunction: () => Promise<any>) {
let retryCount = 0;

while (retryCount <= RetryFolderDeleter.NUMBER_OF_RETRIES) {
try {
const result = await asyncFunction();
return result;
} catch (error: unknown) {
if (error instanceof Error) {
Logger.warn(
`Folder deleter attempt ${retryCount + 1} failed: ${error.message}`
);
} else {
Logger.warn(
`Folder deleter attempt ${
retryCount + 1
} failed with an unknown error.`
);
}

await new Promise((resolve) => {
setTimeout(resolve, RetryFolderDeleter.MILLISECOND_BETWEEN_TRIES);
});

retryCount++;
}
}
throw new MaxRetriesDeletingFolderError(
RetryFolderDeleter.NUMBER_OF_RETRIES
);
}

async run(folder: string): Promise<any> {
await new Promise((resolve) => {
setTimeout(resolve, RetryFolderDeleter.INITIAL_DELAY);
});

const deleter = () => this.deleter.run(folder);
return this.retryDeleter(deleter);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class MaxRetriesDeletingFolderError extends Error {
constructor(retriesNumber: number) {
super(`Max retries (${retriesNumber}) reached. Deleter still failed.`);
}
}
Loading

0 comments on commit 09c59e8

Please sign in to comment.