Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PB-1513]: Fix/loop-when-delete-files #442

Merged
merged 3 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -66,6 +67,11 @@ export async function buildFilesContainer(
ipcRendererSyncEngine
);

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

const sameFileWasMoved = new SameFileWasMoved(
repository,
localFileSystem,
Expand Down Expand Up @@ -140,6 +146,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
Loading