From ad35f8a81a0d4f1e875c0cc5cf3cc7b58e3af33f Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Mon, 7 Oct 2024 16:44:40 +0200 Subject: [PATCH] Fix file retention after updates on macOS #417 This fixes issue #417 where autoupdate installer files were not deleted on macOS, leading to accumulation of old installers. Key changes: - Store update files in application-specific directory - Clear update files directory on every app launch Other supporting changes: - Refactor file system operations to be more testable and reusable - Improve separation of concerns in directory management - Enhance dependency injection for auto-update logic - Fix async completion to support `await` operations - Add additional logging and revise some log messages during updates --- docs/desktop/desktop-vs-web-features.md | 4 +- .../Directory/ScriptDirectoryProvider.ts | 23 -- .../ScriptFileCreationOrchestrator.ts | 26 +- .../FileSystemExecutablePermissionSetter.ts | 2 +- .../LoggingNodeShellCommandRunner.ts | 2 +- .../System/NodeElectronSystemOperations.ts | 58 +--- .../CodeRunner/System/SystemOperations.ts | 18 +- .../Electron/NodeElectronSaveFileDialog.ts | 4 +- .../Directory/ApplicationDirectoryProvider.ts | 30 ++ ...PersistentApplicationDirectoryProvider.ts} | 58 ++-- .../FileSystem/FileSystemOperations.ts | 20 ++ .../NodeElectronFileSystemOperations.ts | 68 +++++ .../NodeReadbackFileWriter.ts | 22 +- .../ReadbackFileWriter/ReadbackFileWriter.ts | 0 .../ScriptEnvironmentDiagnosticsCollector.ts | 13 +- .../main/Update/AutomaticUpdateCoordinator.ts | 109 ++++--- .../main/Update/ManualUpdater/Downloader.ts | 79 +++-- ...cess.ts => FileSystemAccessorWithRetry.ts} | 14 +- .../InstallationFileCleaner.ts | 107 +++++++ .../InstallationFilepathProvider.ts | 73 +++++ .../main/Update/ManualUpdater/Installer.ts | 2 +- .../main/Update/ManualUpdater/Integrity.ts | 2 +- .../ManualUpdater/ManualUpdateCoordinator.ts | 45 ++- src/presentation/electron/main/index.ts | 2 +- .../CodeRunner/ScriptFileCodeRunner.spec.ts | 8 +- .../ScriptFileCreationOrchestrator.spec.ts | 114 +++---- ...leSystemExecutablePermissionSetter.spec.ts | 14 +- .../NodeElectronSaveFileDialog.spec.ts | 2 +- ...stentApplicationDirectoryProvider.spec.ts} | 151 +++++----- .../NodeReadbackFileWriter.spec.ts | 71 ++--- .../FileReadWriteOperationsStub.ts | 34 --- ...iptEnvironmentDiagnosticsCollector.spec.ts | 12 +- .../InstallationFileCleaner.spec.ts | 284 ++++++++++++++++++ .../InstallationFilepathProvider.spec.ts | 225 ++++++++++++++ tests/unit/shared/ExceptionCollector.ts | 14 +- .../Stubs/ApplicationDirectoryProviderStub.ts | 41 +++ .../Stubs/FileSystemAccessorWithRetryStub.ts | 21 ++ .../shared/Stubs/FileSystemOperationsStub.ts | 161 ++++++++++ tests/unit/shared/Stubs/FileSystemOpsStub.ts | 22 -- tests/unit/shared/Stubs/LocationOpsStub.ts | 49 --- .../shared/Stubs/OperatingSystemOpsStub.ts | 21 -- .../shared/Stubs/ReadbackFileWriterStub.ts | 2 +- .../Stubs/ScriptDirectoryProviderStub.ts | 17 -- .../unit/shared/Stubs/SystemOperationsStub.ts | 26 +- 44 files changed, 1482 insertions(+), 588 deletions(-) delete mode 100644 src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts create mode 100644 src/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider.ts rename src/infrastructure/{CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts => FileSystem/Directory/PersistentApplicationDirectoryProvider.ts} (57%) create mode 100644 src/infrastructure/FileSystem/FileSystemOperations.ts create mode 100644 src/infrastructure/FileSystem/NodeElectronFileSystemOperations.ts rename src/infrastructure/{ => FileSystem}/ReadbackFileWriter/NodeReadbackFileWriter.ts (85%) rename src/infrastructure/{ => FileSystem}/ReadbackFileWriter/ReadbackFileWriter.ts (100%) rename src/presentation/electron/main/Update/ManualUpdater/{RetryFileSystemAccess.ts => FileSystemAccessorWithRetry.ts} (82%) create mode 100644 src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.ts create mode 100644 src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.ts rename tests/unit/infrastructure/{CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts => FileSystem/Directory/PersistentApplicationDirectoryProvider.spec.ts} (51%) rename tests/unit/infrastructure/{ => FileSystem}/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts (81%) delete mode 100644 tests/unit/infrastructure/ReadbackFileWriter/FileReadWriteOperationsStub.ts create mode 100644 tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.spec.ts create mode 100644 tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.spec.ts create mode 100644 tests/unit/shared/Stubs/ApplicationDirectoryProviderStub.ts create mode 100644 tests/unit/shared/Stubs/FileSystemAccessorWithRetryStub.ts create mode 100644 tests/unit/shared/Stubs/FileSystemOperationsStub.ts delete mode 100644 tests/unit/shared/Stubs/FileSystemOpsStub.ts delete mode 100644 tests/unit/shared/Stubs/LocationOpsStub.ts delete mode 100644 tests/unit/shared/Stubs/OperatingSystemOpsStub.ts delete mode 100644 tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts diff --git a/docs/desktop/desktop-vs-web-features.md b/docs/desktop/desktop-vs-web-features.md index 22722491..ec87e389 100644 --- a/docs/desktop/desktop-vs-web-features.md +++ b/docs/desktop/desktop-vs-web-features.md @@ -34,8 +34,10 @@ The desktop version ensures secure delivery through cryptographic signatures and [Security is a top priority](./../../SECURITY.md#update-security-and-integrity) at privacy.sexy. -> **Note for macOS users:** On macOS, the desktop version's auto-update process involves manual steps due to Apple's code signing costs. +> **Note for macOS users:** +> On macOS, the desktop version's auto-update process involves manual steps due to Apple's code signing costs. > Users get notified about updates but might need to complete the installation manually. +> Updater stores update installation files temporarily at `$HOME/Library/Application Support/privacy.sexy/updates`. > Consider [donating](https://github.com/sponsors/undergroundwires) to help improve this process ❤️. ### Logging diff --git a/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts b/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts deleted file mode 100644 index c15980ea..00000000 --- a/src/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider.ts +++ /dev/null @@ -1,23 +0,0 @@ -import type { CodeRunError } from '@/application/CodeRunner/CodeRunner'; - -export interface ScriptDirectoryProvider { - provideScriptDirectory(): Promise; -} - -export type ScriptDirectoryOutcome = SuccessfulDirectoryCreation | FailedDirectoryCreation; - -interface ScriptDirectoryCreationStatus { - readonly success: boolean; - readonly directoryAbsolutePath?: string; - readonly error?: CodeRunError; -} - -interface SuccessfulDirectoryCreation extends ScriptDirectoryCreationStatus { - readonly success: true; - readonly directoryAbsolutePath: string; -} - -interface FailedDirectoryCreation extends ScriptDirectoryCreationStatus { - readonly success: false; - readonly error: CodeRunError; -} diff --git a/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts b/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts index d2bd469c..79fc69d5 100644 --- a/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts +++ b/src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts @@ -1,21 +1,22 @@ import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; import type { Logger } from '@/application/Common/Log/Logger'; import type { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; -import { FileReadbackVerificationErrors, type ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; -import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter'; -import { NodeElectronSystemOperations } from '../System/NodeElectronSystemOperations'; +import { FileReadbackVerificationErrors, type ReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; +import { NodeReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter'; +import { NodeElectronFileSystemOperations } from '@/infrastructure/FileSystem/NodeElectronFileSystemOperations'; +import { PersistentApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; import { TimestampedFilenameGenerator } from './Filename/TimestampedFilenameGenerator'; -import { PersistentDirectoryProvider } from './Directory/PersistentDirectoryProvider'; -import type { SystemOperations } from '../System/SystemOperations'; import type { FilenameGenerator } from './Filename/FilenameGenerator'; import type { ScriptFilenameParts, ScriptFileCreator, ScriptFileCreationOutcome } from './ScriptFileCreator'; -import type { ScriptDirectoryProvider } from './Directory/ScriptDirectoryProvider'; export class ScriptFileCreationOrchestrator implements ScriptFileCreator { constructor( - private readonly system: SystemOperations = new NodeElectronSystemOperations(), + private readonly fileSystem: FileSystemOperations = NodeElectronFileSystemOperations, private readonly filenameGenerator: FilenameGenerator = new TimestampedFilenameGenerator(), - private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(), + private readonly directoryProvider: ApplicationDirectoryProvider + = new PersistentApplicationDirectoryProvider(), private readonly fileWriter: ReadbackFileWriter = new NodeReadbackFileWriter(), private readonly logger: Logger = ElectronLogger, ) { } @@ -26,9 +27,12 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator { ): Promise { const { success: isDirectoryCreated, error: directoryCreationError, directoryAbsolutePath, - } = await this.directoryProvider.provideScriptDirectory(); + } = await this.directoryProvider.provideDirectory('script-runs'); if (!isDirectoryCreated) { - return createFailure(directoryCreationError); + return createFailure({ + type: 'DirectoryCreationError', + message: `[${directoryCreationError.type}] ${directoryCreationError.message}`, + }); } const { success: isFilePathConstructed, error: filePathGenerationError, filePath, @@ -54,7 +58,7 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator { ): FilePathConstructionOutcome { try { const filename = this.filenameGenerator.generateFilename(scriptFilenameParts); - const filePath = this.system.location.combinePaths(directoryPath, filename); + const filePath = this.fileSystem.combinePaths(directoryPath, filename); return { success: true, filePath }; } catch (error) { return { diff --git a/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.ts b/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.ts index 915e9832..c668d120 100644 --- a/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.ts +++ b/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.ts @@ -7,7 +7,7 @@ import type { ExecutablePermissionSetter } from './ExecutablePermissionSetter'; export class FileSystemExecutablePermissionSetter implements ExecutablePermissionSetter { constructor( - private readonly system: SystemOperations = new NodeElectronSystemOperations(), + private readonly system: SystemOperations = NodeElectronSystemOperations, private readonly logger: Logger = ElectronLogger, ) { } diff --git a/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/ShellRunner/LoggingNodeShellCommandRunner.ts b/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/ShellRunner/LoggingNodeShellCommandRunner.ts index 22825590..25db4f96 100644 --- a/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/ShellRunner/LoggingNodeShellCommandRunner.ts +++ b/src/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/ShellRunner/LoggingNodeShellCommandRunner.ts @@ -7,7 +7,7 @@ import type { ShellCommandOutcome, ShellCommandRunner } from './ShellCommandRunn export class LoggingNodeShellCommandRunner implements ShellCommandRunner { constructor( private readonly logger: Logger = ElectronLogger, - private readonly systemOps: SystemOperations = new NodeElectronSystemOperations(), + private readonly systemOps: SystemOperations = NodeElectronSystemOperations, ) { } diff --git a/src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts b/src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts index 87e51530..19429a3a 100644 --- a/src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts +++ b/src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts @@ -1,57 +1,13 @@ -import { join } from 'node:path'; -import { chmod, mkdir } from 'node:fs/promises'; import { exec } from 'node:child_process'; -import { app } from 'electron/main'; -import type { - CommandOps, FileSystemOps, LocationOps, OperatingSystemOps, SystemOperations, -} from './SystemOperations'; +import { NodeElectronFileSystemOperations } from '@/infrastructure/FileSystem/NodeElectronFileSystemOperations'; +import type { SystemOperations } from './SystemOperations'; /** * Thin wrapper for Node and Electron APIs. */ -export class NodeElectronSystemOperations implements SystemOperations { - public readonly operatingSystem: OperatingSystemOps = { - /* - This method returns the directory for storing app's configuration files. - It appends your app's name to the default appData directory. - Conventionally, you should store user data files in this directory. - However, avoid writing large files here as some environments might back up this directory - to cloud storage, potentially causing issues with file size. - - Based on tests it returns: - - - Windows: `%APPDATA%\privacy.sexy` - - Linux: `$HOME/.config/privacy.sexy/runs` - - macOS: `$HOME/Library/Application Support/privacy.sexy/runs` - - For more details, refer to the Electron documentation: https://web.archive.org/web/20240104154857/https://www.electronjs.org/docs/latest/api/app#appgetpathname - */ - getUserDataDirectory: () => { - return app.getPath('userData'); - }, - }; - - public readonly location: LocationOps = { - combinePaths: (...pathSegments) => join(...pathSegments), - }; - - public readonly fileSystem: FileSystemOps = { - setFilePermissions: ( - filePath: string, - mode: string | number, - ) => chmod(filePath, mode), - createDirectory: async ( - directoryPath: string, - isRecursive?: boolean, - ) => { - await mkdir(directoryPath, { recursive: isRecursive }); - // Ignoring the return value from `mkdir`, which is the first directory created - // when `recursive` is true, or empty return value. - // See https://github.com/nodejs/node/pull/31530 - }, - }; - - public readonly command: CommandOps = { +export const NodeElectronSystemOperations: SystemOperations = { + fileSystem: NodeElectronFileSystemOperations, + command: { exec, - }; -} + }, +}; diff --git a/src/infrastructure/CodeRunner/System/SystemOperations.ts b/src/infrastructure/CodeRunner/System/SystemOperations.ts index f94c48da..c40690e9 100644 --- a/src/infrastructure/CodeRunner/System/SystemOperations.ts +++ b/src/infrastructure/CodeRunner/System/SystemOperations.ts @@ -1,25 +1,11 @@ +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; import type { exec } from 'node:child_process'; export interface SystemOperations { - readonly operatingSystem: OperatingSystemOps; - readonly location: LocationOps; - readonly fileSystem: FileSystemOps; + readonly fileSystem: FileSystemOperations; readonly command: CommandOps; } -export interface OperatingSystemOps { - getUserDataDirectory(): string; -} - -export interface LocationOps { - combinePaths(...pathSegments: string[]): string; -} - export interface CommandOps { exec(command: string): ReturnType; } - -export interface FileSystemOps { - setFilePermissions(filePath: string, mode: string | number): Promise; - createDirectory(directoryPath: string, isRecursive?: boolean): Promise; -} diff --git a/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts b/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts index 4970eabb..6eef3ef0 100644 --- a/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts +++ b/src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts @@ -5,8 +5,8 @@ import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; import { FileType, type SaveFileError, type SaveFileErrorType, type SaveFileOutcome, } from '@/presentation/common/Dialog'; -import { FileReadbackVerificationErrors, type ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; -import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter'; +import { FileReadbackVerificationErrors, type ReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; +import { NodeReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter'; import type { ElectronSaveFileDialog } from './ElectronSaveFileDialog'; export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog { diff --git a/src/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider.ts b/src/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider.ts new file mode 100644 index 00000000..a619934c --- /dev/null +++ b/src/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider.ts @@ -0,0 +1,30 @@ +export interface ApplicationDirectoryProvider { + provideDirectory(type: DirectoryType): Promise; +} + +export type DirectoryType = 'update-installation-files' | 'script-runs'; + +export type DirectoryCreationOutcome = SuccessfulDirectoryCreation | FailedDirectoryCreation; + +export type DirectoryCreationErrorType = 'PathConstructionError' | 'DirectoryWriteError' | 'UserDataFolderRetrievalError'; + +export interface DirectoryCreationError { + readonly type: DirectoryCreationErrorType; + readonly message: string; +} + +interface DirectoryCreationStatus { + readonly success: boolean; + readonly directoryAbsolutePath?: string; + readonly error?: DirectoryCreationError; +} + +interface SuccessfulDirectoryCreation extends DirectoryCreationStatus { + readonly success: true; + readonly directoryAbsolutePath: string; +} + +interface FailedDirectoryCreation extends DirectoryCreationStatus { + readonly success: false; + readonly error: DirectoryCreationError; +} diff --git a/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts b/src/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.ts similarity index 57% rename from src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts rename to src/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.ts index 8b2b1e44..44fdd136 100644 --- a/src/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.ts +++ b/src/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.ts @@ -1,32 +1,37 @@ import type { Logger } from '@/application/Common/Log/Logger'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; -import type { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; -import { NodeElectronSystemOperations } from '../../System/NodeElectronSystemOperations'; -import type { SystemOperations } from '../../System/SystemOperations'; -import type { ScriptDirectoryOutcome, ScriptDirectoryProvider } from './ScriptDirectoryProvider'; +import { NodeElectronFileSystemOperations } from '@/infrastructure/FileSystem/NodeElectronFileSystemOperations'; +import type { + DirectoryCreationOutcome, ApplicationDirectoryProvider, DirectoryType, + DirectoryCreationError, DirectoryCreationErrorType, +} from './ApplicationDirectoryProvider'; +import type { FileSystemOperations } from '../FileSystemOperations'; -export const ExecutionSubdirectory = 'runs'; +export const SubdirectoryNames: Record = { + 'script-runs': 'runs', + 'update-installation-files': 'updates', +}; /** - * Provides a dedicated directory for script execution. + * Provides persistent directories. * Benefits of using a persistent directory: * - Antivirus Exclusions: Easier antivirus configuration. * - Auditability: Stores script execution history for troubleshooting. * - Reliability: Avoids issues with directory clean-ups during execution, * seen in Windows Pro Azure VMs when stored on Windows temporary directory. */ -export class PersistentDirectoryProvider implements ScriptDirectoryProvider { +export class PersistentApplicationDirectoryProvider implements ApplicationDirectoryProvider { constructor( - private readonly system: SystemOperations = new NodeElectronSystemOperations(), + private readonly fileSystem: FileSystemOperations = NodeElectronFileSystemOperations, private readonly logger: Logger = ElectronLogger, ) { } - public async provideScriptDirectory(): Promise { + public async provideDirectory(type: DirectoryType): Promise { const { success: isPathConstructed, error: pathConstructionError, directoryPath, - } = this.constructScriptDirectoryPath(); + } = this.constructScriptDirectoryPath(type); if (!isPathConstructed) { return { success: false, @@ -52,7 +57,7 @@ export class PersistentDirectoryProvider implements ScriptDirectoryProvider { private async createDirectory(directoryPath: string): Promise { try { this.logger.info(`Attempting to create script directory at path: ${directoryPath}`); - await this.system.fileSystem.createDirectory(directoryPath, true); + await this.fileSystem.createDirectory(directoryPath, true); this.logger.info(`Script directory successfully created at: ${directoryPath}`); return { success: true, @@ -60,17 +65,26 @@ export class PersistentDirectoryProvider implements ScriptDirectoryProvider { } catch (error) { return { success: false, - error: this.handleException(error, 'DirectoryCreationError'), + error: this.handleError(error, 'DirectoryWriteError'), }; } } - private constructScriptDirectoryPath(): DirectoryPathConstructionOutcome { + private constructScriptDirectoryPath(type: DirectoryType): DirectoryPathConstructionOutcome { + let parentDirectory: string; + try { + parentDirectory = this.fileSystem.getUserDataDirectory(); + } catch (error) { + return { + success: false, + error: this.handleError(error, 'UserDataFolderRetrievalError'), + }; + } try { - const parentDirectory = this.system.operatingSystem.getUserDataDirectory(); - const scriptDirectory = this.system.location.combinePaths( + const subdirectoryName = SubdirectoryNames[type]; + const scriptDirectory = this.fileSystem.combinePaths( parentDirectory, - ExecutionSubdirectory, + subdirectoryName, ); return { success: true, @@ -79,15 +93,15 @@ export class PersistentDirectoryProvider implements ScriptDirectoryProvider { } catch (error) { return { success: false, - error: this.handleException(error, 'DirectoryCreationError'), + error: this.handleError(error, 'PathConstructionError'), }; } } - private handleException( + private handleError( exception: Error, - errorType: CodeRunErrorType, - ): CodeRunError { + errorType: DirectoryCreationErrorType, + ): DirectoryCreationError { const errorMessage = 'Error during script directory creation'; this.logger.error(errorType, errorMessage, exception); return { @@ -99,7 +113,7 @@ export class PersistentDirectoryProvider implements ScriptDirectoryProvider { type DirectoryPathConstructionOutcome = { readonly success: false; - readonly error: CodeRunError; + readonly error: DirectoryCreationError; readonly directoryPath?: undefined; } | { readonly success: true; @@ -109,7 +123,7 @@ type DirectoryPathConstructionOutcome = { type DirectoryPathCreationOutcome = { readonly success: false; - readonly error: CodeRunError; + readonly error: DirectoryCreationError; } | { readonly success: true; readonly error?: undefined; diff --git a/src/infrastructure/FileSystem/FileSystemOperations.ts b/src/infrastructure/FileSystem/FileSystemOperations.ts new file mode 100644 index 00000000..f003ea07 --- /dev/null +++ b/src/infrastructure/FileSystem/FileSystemOperations.ts @@ -0,0 +1,20 @@ +export interface FileSystemOperations { + getUserDataDirectory(): string; + + setFilePermissions(filePath: string, mode: string | number): Promise; + createDirectory(directoryPath: string, isRecursive?: boolean): Promise; + + isFileAvailable(filePath: string): Promise; + isDirectoryAvailable(filePath: string): Promise; + deletePath(filePath: string): Promise; + listDirectoryContents(directoryPath: string): Promise; + + combinePaths(...pathSegments: string[]): string; + + readFile: (filePath: string, encoding: NodeJS.BufferEncoding) => Promise; + writeFile: ( + filePath: string, + fileContents: string, + encoding: NodeJS.BufferEncoding, + ) => Promise; +} diff --git a/src/infrastructure/FileSystem/NodeElectronFileSystemOperations.ts b/src/infrastructure/FileSystem/NodeElectronFileSystemOperations.ts new file mode 100644 index 00000000..966b8b90 --- /dev/null +++ b/src/infrastructure/FileSystem/NodeElectronFileSystemOperations.ts @@ -0,0 +1,68 @@ +import { join } from 'node:path'; +import { + chmod, mkdir, + readdir, rm, stat, + readFile, writeFile, +} from 'node:fs/promises'; +import { app } from 'electron/main'; +import type { FileSystemOperations } from './FileSystemOperations'; +import type { Stats } from 'node:original-fs'; + +/** + * Thin wrapper for Node and Electron APIs. + */ +export const NodeElectronFileSystemOperations: FileSystemOperations = { + combinePaths: (...pathSegments) => join(...pathSegments), + setFilePermissions: ( + filePath: string, + mode: string | number, + ) => chmod(filePath, mode), + createDirectory: async ( + directoryPath: string, + isRecursive?: boolean, + ) => { + await mkdir(directoryPath, { recursive: isRecursive }); + // Ignoring the return value from `mkdir`, which is the first directory created + // when `recursive` is true, or empty return value. + // See https://github.com/nodejs/node/pull/31530 + }, + isFileAvailable: async (path) => isPathAvailable(path, (stats) => stats.isFile()), + isDirectoryAvailable: async (path) => isPathAvailable(path, (stats) => stats.isDirectory()), + deletePath: (path) => rm(path, { recursive: true, force: true }), + listDirectoryContents: (directoryPath) => readdir(directoryPath), + getUserDataDirectory: () => { + /* + This method returns the directory for storing app's configuration files. + It appends your app's name to the default appData directory. + Conventionally, you should store user data files in this directory. + However, avoid writing large files here as some environments might back up this directory + to cloud storage, potentially causing issues with file size. + + Based on tests it returns: + + - Windows: `%APPDATA%\privacy.sexy` + - Linux: `$HOME/.config/privacy.sexy/runs` + - macOS: `$HOME/Library/Application Support/privacy.sexy/runs` + + For more details, refer to the Electron documentation: https://web.archive.org/web/20240104154857/https://www.electronjs.org/docs/latest/api/app#appgetpathname + */ + return app.getPath('userData'); + }, + writeFile, + readFile, +}; + +async function isPathAvailable( + path: string, + condition: (stats: Stats) => boolean, +): Promise { + try { + const stats = await stat(path); + return condition(stats); + } catch (error) { + if (error.code === 'ENOENT') { + return false; // path does not exist + } + throw error; + } +} diff --git a/src/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.ts b/src/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.ts similarity index 85% rename from src/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.ts rename to src/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.ts index 36ca12b6..40d40dfb 100644 --- a/src/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.ts +++ b/src/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.ts @@ -1,22 +1,18 @@ -import { writeFile, access, readFile } from 'node:fs/promises'; -import { constants } from 'node:fs'; import type { Logger } from '@/application/Common/Log/Logger'; -import { ElectronLogger } from '../Log/ElectronLogger'; +import { ElectronLogger } from '../../Log/ElectronLogger'; +import { NodeElectronFileSystemOperations } from '../NodeElectronFileSystemOperations'; import type { FailedFileWrite, ReadbackFileWriter, FileWriteErrorType, FileWriteOutcome, SuccessfulFileWrite, } from './ReadbackFileWriter'; +import type { FileSystemOperations } from '../FileSystemOperations'; const FILE_ENCODING: NodeJS.BufferEncoding = 'utf-8'; export class NodeReadbackFileWriter implements ReadbackFileWriter { constructor( private readonly logger: Logger = ElectronLogger, - private readonly fileSystem: FileReadWriteOperations = { - writeFile, - readFile: (path, encoding) => readFile(path, encoding), - access, - }, + private readonly fileSystem: FileSystemOperations = NodeElectronFileSystemOperations, ) { } public async writeAndVerifyFile( @@ -55,7 +51,9 @@ export class NodeReadbackFileWriter implements ReadbackFileWriter { filePath: string, ): Promise { try { - await this.fileSystem.access(filePath, constants.F_OK); + if (!(await this.fileSystem.isFileAvailable(filePath))) { + return this.reportFailure('FileExistenceVerificationFailed', 'File does not exist.'); + } return this.reportSuccess('Verified file existence without reading.'); } catch (error) { return this.reportFailure('FileExistenceVerificationFailed', error); @@ -107,9 +105,3 @@ export class NodeReadbackFileWriter implements ReadbackFileWriter { }; } } - -export interface FileReadWriteOperations { - readonly writeFile: typeof writeFile; - readonly access: typeof access; - readFile: (filePath: string, encoding: NodeJS.BufferEncoding) => Promise; -} diff --git a/src/infrastructure/ReadbackFileWriter/ReadbackFileWriter.ts b/src/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter.ts similarity index 100% rename from src/infrastructure/ReadbackFileWriter/ReadbackFileWriter.ts rename to src/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter.ts diff --git a/src/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.ts b/src/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.ts index d77cec7b..74ca127a 100644 --- a/src/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.ts +++ b/src/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.ts @@ -1,19 +1,22 @@ import type { ScriptDiagnosticData, ScriptDiagnosticsCollector } from '@/application/ScriptDiagnostics/ScriptDiagnosticsCollector'; import type { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory'; -import { PersistentDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider'; -import type { ScriptDirectoryProvider } from '../CodeRunner/Creation/Directory/ScriptDirectoryProvider'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import { PersistentApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider'; export class ScriptEnvironmentDiagnosticsCollector implements ScriptDiagnosticsCollector { constructor( - private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(), + private readonly directoryProvider: ApplicationDirectoryProvider + = new PersistentApplicationDirectoryProvider(), private readonly environment: RuntimeEnvironment = CurrentEnvironment, ) { } public async collectDiagnosticInformation(): Promise { - const { directoryAbsolutePath } = await this.directoryProvider.provideScriptDirectory(); + const { + directoryAbsolutePath: scriptsDirectory, + } = await this.directoryProvider.provideDirectory('script-runs'); return { - scriptsDirectoryAbsolutePath: directoryAbsolutePath, + scriptsDirectoryAbsolutePath: scriptsDirectory, currentOperatingSystem: this.environment.os, }; } diff --git a/src/presentation/electron/main/Update/AutomaticUpdateCoordinator.ts b/src/presentation/electron/main/Update/AutomaticUpdateCoordinator.ts index aad1ba16..2e06633e 100644 --- a/src/presentation/electron/main/Update/AutomaticUpdateCoordinator.ts +++ b/src/presentation/electron/main/Update/AutomaticUpdateCoordinator.ts @@ -7,63 +7,100 @@ import type { ProgressInfo } from 'electron-builder'; export async function handleAutoUpdate() { const autoUpdater = getAutoUpdater(); - if (await askDownloadAndInstall() === DownloadDialogResult.NotNow) { + if (await askDownloadAndInstall() === UpdateDialogResult.Postpone) { + ElectronLogger.info('User chose to postpone update'); return; } - startHandlingUpdateProgress(autoUpdater); - await autoUpdater.downloadUpdate(); + ElectronLogger.info('User chose to download and install update'); + try { + await startHandlingUpdateProgress(autoUpdater); + } catch (error) { + ElectronLogger.error('Failed to handle auto-update process', { error }); + } } -function startHandlingUpdateProgress(autoUpdater: AppUpdater) { - const progressBar = new UpdateProgressBar(); - progressBar.showIndeterminateState(); - autoUpdater.on('error', (e) => { - progressBar.showError(e); - }); - autoUpdater.on('download-progress', (progress: ProgressInfo) => { - /* - On macOS, download-progress event is not called. - So the indeterminate progress will continue until download is finished. - */ - ElectronLogger.debug('@download-progress@\n', progress); - if (progressBar.isOpen) { // May be closed by the user - progressBar.showProgress(progress); - } - }); - autoUpdater.on('update-downloaded', async (info: UpdateInfo) => { - ElectronLogger.info('@update-downloaded@\n', info); - progressBar.closeIfOpen(); - await handleUpdateDownloaded(autoUpdater); +function startHandlingUpdateProgress(autoUpdater: AppUpdater): Promise { + return new Promise((resolve, reject) => { // Block until update process completes + const progressBar = new UpdateProgressBar(); + progressBar.showIndeterminateState(); + autoUpdater.on('error', (e) => { + progressBar.showError(e); + reject(e); + }); + autoUpdater.on('download-progress', (progress: ProgressInfo) => { + /* + On macOS, download-progress event is not called. + So the indeterminate progress will continue until download is finished. + */ + ElectronLogger.debug('Update download progress', { progress }); + if (progressBar.isOpen) { // May be closed by the user + progressBar.showProgress(progress); + } + }); + autoUpdater.on('update-downloaded', async (info: UpdateInfo) => { + ElectronLogger.info('Update downloaded successfully', { version: info.version }); + progressBar.closeIfOpen(); + try { + await handleUpdateDownloaded(autoUpdater); + } catch (error) { + ElectronLogger.error('Failed to handle downloaded update', { error }); + reject(error); + } + resolve(); + }); + autoUpdater.downloadUpdate(); }); } -async function handleUpdateDownloaded(autoUpdater: AppUpdater) { - if (await askRestartAndInstall() === InstallDialogResult.NotNow) { - return; - } - setTimeout(() => autoUpdater.quitAndInstall(), 1); +async function handleUpdateDownloaded( + autoUpdater: AppUpdater, +): Promise { + return new Promise((resolve, reject) => { // Block until update download process completes + askRestartAndInstall() + .then((result) => { + if (result === InstallDialogResult.InstallAndRestart) { + ElectronLogger.info('User chose to install and restart for update'); + setTimeout(() => { + try { + autoUpdater.quitAndInstall(); + resolve(); + } catch (error) { + ElectronLogger.error('Failed to quit and install update', { error }); + reject(error); + } + }, 1); + } else { + ElectronLogger.info('User chose to postpone update installation'); + resolve(); + } + }) + .catch((error) => { + ElectronLogger.error('Failed to prompt user for restart and install', { error }); + reject(error); + }); + }); } -enum DownloadDialogResult { - Install = 0, - NotNow = 1, +enum UpdateDialogResult { + Update = 0, + Postpone = 1, } -async function askDownloadAndInstall(): Promise { +async function askDownloadAndInstall(): Promise { const updateDialogResult = await dialog.showMessageBox({ type: 'question', buttons: ['Install', 'Not now'], title: 'Confirm Update', message: 'Update available.\n\nWould you like to download and install new version?', detail: 'Application will automatically restart to apply update after download', - defaultId: DownloadDialogResult.Install, - cancelId: DownloadDialogResult.NotNow, + defaultId: UpdateDialogResult.Update, + cancelId: UpdateDialogResult.Postpone, }); return updateDialogResult.response; } enum InstallDialogResult { InstallAndRestart = 0, - NotNow = 1, + Postpone = 1, } async function askRestartAndInstall(): Promise { const installDialogResult = await dialog.showMessageBox({ @@ -72,7 +109,7 @@ async function askRestartAndInstall(): Promise { message: `A new version of ${app.name} has been downloaded.`, detail: 'It will be installed the next time you restart the application.', defaultId: InstallDialogResult.InstallAndRestart, - cancelId: InstallDialogResult.NotNow, + cancelId: InstallDialogResult.Postpone, }); return installDialogResult.response; } diff --git a/src/presentation/electron/main/Update/ManualUpdater/Downloader.ts b/src/presentation/electron/main/Update/ManualUpdater/Downloader.ts index 2f8fd1cf..5fdf47c9 100644 --- a/src/presentation/electron/main/Update/ManualUpdater/Downloader.ts +++ b/src/presentation/electron/main/Update/ManualUpdater/Downloader.ts @@ -1,10 +1,8 @@ -import { existsSync, createWriteStream, type WriteStream } from 'node:fs'; -import { unlink, mkdir } from 'node:fs/promises'; -import path from 'node:path'; -import { app } from 'electron/main'; +import { createWriteStream, type WriteStream } from 'node:fs'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; +import type { Logger } from '@/application/Common/Log/Logger'; import { UpdateProgressBar } from '../ProgressBar/UpdateProgressBar'; -import { retryFileSystemAccess } from './RetryFileSystemAccess'; +import { provideUpdateInstallationFilepath, type InstallationFilepathProvider } from './InstallationFiles/InstallationFilepathProvider'; import type { UpdateInfo } from 'electron-updater'; import type { ReadableStream } from 'node:stream/web'; @@ -18,18 +16,25 @@ export type DownloadUpdateResult = { readonly installerPath: string; }; +interface UpdateDownloadUtilities { + readonly logger: Logger; + readonly provideInstallationFilePath: InstallationFilepathProvider; +} + export async function downloadUpdate( info: UpdateInfo, remoteFileUrl: string, progressBar: UpdateProgressBar, + utilities: UpdateDownloadUtilities = DefaultUtilities, ): Promise { - ElectronLogger.info('Starting manual update download.'); + utilities.logger.info('Starting manual update download.'); progressBar.showIndeterminateState(); try { const { filePath } = await downloadInstallerFile( info.version, remoteFileUrl, (percentage) => { progressBar.showPercentage(percentage); }, + utilities, ); return { success: true, @@ -47,58 +52,40 @@ async function downloadInstallerFile( version: string, remoteFileUrl: string, progressHandler: ProgressCallback, + utilities: UpdateDownloadUtilities, ): Promise<{ readonly filePath: string; }> { - const filePath = `${path.dirname(app.getPath('temp'))}/privacy.sexy/${version}-installer.dmg`; - if (!await ensureFilePathReady(filePath)) { - throw new Error(`Failed to prepare the file path for the installer: ${filePath}`); - } + const filePath = await utilities.provideInstallationFilePath(version); await downloadFileWithProgress( remoteFileUrl, filePath, progressHandler, + utilities, ); return { filePath }; } -async function ensureFilePathReady(filePath: string): Promise { - return retryFileSystemAccess(async () => { - try { - const parentFolder = path.dirname(filePath); - if (existsSync(filePath)) { - ElectronLogger.info(`Existing update file found and will be replaced: ${filePath}`); - await unlink(filePath); - } else { - await mkdir(parentFolder, { recursive: true }); - } - return true; - } catch (error) { - ElectronLogger.error(`Failed to prepare file path for update: ${filePath}`, error); - return false; - } - }); -} - type ProgressCallback = (progress: number) => void; async function downloadFileWithProgress( url: string, filePath: string, progressHandler: ProgressCallback, + utilities: UpdateDownloadUtilities, ) { - // autoUpdater cannot handle DMG files, requiring manual download management for these file types. - ElectronLogger.info(`Retrieving update from ${url}.`); + utilities.logger.info(`Retrieving update from ${url}.`); const response = await fetch(url); if (!response.ok) { throw Error(`Download failed: Server responded with ${response.status} ${response.statusText}.`); } - const contentLength = getContentLengthFromResponse(response); + const contentLength = getContentLengthFromResponse(response, utilities); await withWriteStream(filePath, async (writer) => { - ElectronLogger.info(contentLength.isValid - ? `Saving file to ${filePath} (Size: ${contentLength.totalLength} bytes).` - : `Saving file to ${filePath}.`); + utilities.logger.info(contentLength.isValid + ? `Saving file to '${filePath}' (Size: ${contentLength.totalLength} bytes).` + : `Saving file to '${filePath}'.`); await withReadableStream(response, async (reader) => { - await streamWithProgress(contentLength, reader, writer, progressHandler); + await streamWithProgress(contentLength, reader, writer, progressHandler, utilities); }); + ElectronLogger.info(`Successfully saved the file: '${filePath}'`); }); } @@ -109,16 +96,19 @@ type ResponseContentLength = { readonly isValid: false; }; -function getContentLengthFromResponse(response: Response): ResponseContentLength { +function getContentLengthFromResponse( + response: Response, + utilities: UpdateDownloadUtilities, +): ResponseContentLength { const contentLengthString = response.headers.get('content-length'); const headersInfo = Array.from(response.headers.entries()); if (!contentLengthString) { - ElectronLogger.warn('Missing \'Content-Length\' header in the response.', headersInfo); + utilities.logger.warn('Missing \'Content-Length\' header in the response.', headersInfo); return { isValid: false }; } const contentLength = Number(contentLengthString); if (Number.isNaN(contentLength) || contentLength <= 0) { - ElectronLogger.error('Unable to determine download size from server response.', headersInfo); + utilities.logger.error('Unable to determine download size from server response.', headersInfo); return { isValid: false }; } return { totalLength: contentLength, isValid: true }; @@ -153,6 +143,7 @@ async function streamWithProgress( readStream: ReadableStream, writeStream: WriteStream, progressHandler: ProgressCallback, + utilities: UpdateDownloadUtilities, ): Promise { let receivedLength = 0; let logThreshold = 0; @@ -163,22 +154,23 @@ async function streamWithProgress( writeStream.write(Buffer.from(chunk)); receivedLength += chunk.length; notifyProgress(contentLength, receivedLength, progressHandler); - const progressLog = logProgress(receivedLength, contentLength, logThreshold); + const progressLog = logProgress(receivedLength, contentLength, logThreshold, utilities); logThreshold = progressLog.nextLogThreshold; } - ElectronLogger.info('Update download completed successfully.'); + utilities.logger.info('Update download completed successfully.'); } function logProgress( receivedLength: number, contentLength: ResponseContentLength, logThreshold: number, + utilities: UpdateDownloadUtilities, ): { readonly nextLogThreshold: number; } { const { shouldLog, nextLogThreshold, } = shouldLogProgress(receivedLength, contentLength, logThreshold); if (shouldLog) { - ElectronLogger.debug(`Download progress: ${receivedLength} bytes received.`); + utilities.logger.debug(`Download progress: ${receivedLength} bytes received.`); } return { nextLogThreshold }; } @@ -220,3 +212,8 @@ function createReader(response: Response): ReadableStream { // https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/65542#discussioncomment-6071004 return response.body as ReadableStream; } + +const DefaultUtilities: UpdateDownloadUtilities = { + logger: ElectronLogger, + provideInstallationFilePath: provideUpdateInstallationFilepath, +}; diff --git a/src/presentation/electron/main/Update/ManualUpdater/RetryFileSystemAccess.ts b/src/presentation/electron/main/Update/ManualUpdater/FileSystemAccessorWithRetry.ts similarity index 82% rename from src/presentation/electron/main/Update/ManualUpdater/RetryFileSystemAccess.ts rename to src/presentation/electron/main/Update/ManualUpdater/FileSystemAccessorWithRetry.ts index d3be38df..383e6c72 100644 --- a/src/presentation/electron/main/Update/ManualUpdater/RetryFileSystemAccess.ts +++ b/src/presentation/electron/main/Update/ManualUpdater/FileSystemAccessorWithRetry.ts @@ -1,15 +1,21 @@ import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; import { sleep } from '@/infrastructure/Threading/AsyncSleep'; -export function retryFileSystemAccess( - fileOperation: () => Promise, -): Promise { +export interface FileSystemAccessorWithRetry { + ( + fileOperation: () => Promise, + ): Promise; +} + +export const retryFileSystemAccess: FileSystemAccessorWithRetry = ( + fileOperation, +) => { return retryWithExponentialBackoff( fileOperation, TOTAL_RETRIES, INITIAL_DELAY_MS, ); -} +}; // These values provide a balanced approach for handling transient file system // issues without excessive waiting. diff --git a/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.ts b/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.ts new file mode 100644 index 00000000..55eec706 --- /dev/null +++ b/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.ts @@ -0,0 +1,107 @@ +import type { Logger } from '@/application/Common/Log/Logger'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; +import { PersistentApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider'; +import { NodeElectronFileSystemOperations } from '@/infrastructure/FileSystem/NodeElectronFileSystemOperations'; + +export interface InstallationFileCleaner { + ( + utilities?: UpdateFileUtilities, + ): Promise; +} + +interface UpdateFileUtilities { + readonly logger: Logger; + readonly directoryProvider: ApplicationDirectoryProvider; + readonly fileSystem: FileSystemOperations; +} + +export const clearUpdateInstallationFiles: InstallationFileCleaner = async ( + utilities = DefaultUtilities, +) => { + utilities.logger.info('Starting update installation files cleanup'); + const { success, error, directoryAbsolutePath } = await utilities.directoryProvider.provideDirectory('update-installation-files'); + if (!success) { + utilities.logger.error('Failed to locate installation files directory', { error }); + throw new Error('Cannot locate the installation files directory path'); + } + const installationFileNames = await readDirectoryContents(directoryAbsolutePath, utilities); + if (installationFileNames.length === 0) { + utilities.logger.info('No update installation files found'); + return; + } + utilities.logger.debug(`Found ${installationFileNames.length} installation files to delete`); + utilities.logger.info('Deleting installation files'); + const errors = await executeIndependentTasksAndCollectErrors( + installationFileNames.map(async (fileOrFolderName) => { + await deleteItemFromDirectory(directoryAbsolutePath, fileOrFolderName, utilities); + }), + ); + if (errors.length > 0) { + utilities.logger.error('Failed to delete some installation files', { errors }); + throw new Error(`Failed to delete some items:\n${errors.join('\n')}`); + } + utilities.logger.info('Update installation files cleanup completed successfully'); +}; + +async function deleteItemFromDirectory( + directoryPath: string, + fileOrFolderName: string, + utilities: UpdateFileUtilities, +): Promise { + const itemPath = utilities.fileSystem.combinePaths( + directoryPath, + fileOrFolderName, + ); + try { + utilities.logger.debug(`Deleting installation artifact: ${itemPath}`); + await utilities.fileSystem.deletePath(itemPath); + utilities.logger.debug(`Successfully deleted installation artifact: ${itemPath}`); + } catch (error) { + utilities.logger.error(`Failed to delete installation artifact: ${itemPath}`, { error }); + throw error; + } +} + +async function readDirectoryContents( + directoryPath: string, + utilities: UpdateFileUtilities, +): Promise { + try { + utilities.logger.debug(`Reading directory contents: ${directoryPath}`); + const items = await utilities.fileSystem.listDirectoryContents(directoryPath); + utilities.logger.debug(`Read ${items.length} items from directory: ${directoryPath}`); + return items; + } catch (error) { + utilities.logger.error('Failed to read directory contents', { directoryPath, error }); + throw new Error('Failed to read directory contents', { cause: error }); + } +} + +async function executeIndependentTasksAndCollectErrors( + tasks: (Promise)[], +): Promise { + const results = await Promise.allSettled(tasks); + const errors = results + .filter((result): result is PromiseRejectedResult => result.status === 'rejected') + .map((result) => result.reason); + return errors.map((error) => { + if (!error) { + return 'unknown error'; + } + if (error instanceof Error) { + return error.message; + } + if (typeof error === 'string') { + return error; + } + return String(error); + }); +} + +const DefaultUtilities: UpdateFileUtilities = { + logger: ElectronLogger, + directoryProvider: new PersistentApplicationDirectoryProvider(), + fileSystem: NodeElectronFileSystemOperations, +}; diff --git a/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.ts b/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.ts new file mode 100644 index 00000000..cdc3d625 --- /dev/null +++ b/src/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.ts @@ -0,0 +1,73 @@ +import type { Logger } from '@/application/Common/Log/Logger'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import { PersistentApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import { NodeElectronFileSystemOperations } from '@/infrastructure/FileSystem/NodeElectronFileSystemOperations'; +import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; +import { retryFileSystemAccess, type FileSystemAccessorWithRetry } from '../FileSystemAccessorWithRetry'; + +export interface InstallationFilepathProvider { + ( + version: string, + utilities?: InstallationFilepathProviderUtilities, + ): Promise; +} + +interface InstallationFilepathProviderUtilities { + readonly logger: Logger; + readonly directoryProvider: ApplicationDirectoryProvider; + readonly fileSystem: FileSystemOperations; + readonly accessFileSystemWithRetry: FileSystemAccessorWithRetry; +} + +export const InstallerFileSuffix = '-installer.dmg'; + +export const provideUpdateInstallationFilepath: InstallationFilepathProvider = async ( + version, + utilities = DefaultUtilities, +) => { + const { + success, error, directoryAbsolutePath, + } = await utilities.directoryProvider.provideDirectory('update-installation-files'); + if (!success) { + utilities.logger.error('Error when providing download directory', error); + throw new Error('Failed to provide download directory.'); + } + const filepath = utilities.fileSystem.combinePaths(directoryAbsolutePath, `${version}${InstallerFileSuffix}`); + if (!await makeFilepathAvailable(filepath, utilities)) { + throw new Error(`Failed to prepare the file path for the installer: ${filepath}`); + } + return filepath; +}; + +async function makeFilepathAvailable( + filePath: string, + utilities: InstallationFilepathProviderUtilities, +): Promise { + let isFileAvailable = false; + try { + isFileAvailable = await utilities.fileSystem.isFileAvailable(filePath); + } catch (error) { + throw new Error('File availability check failed'); + } + if (!isFileAvailable) { + return true; + } + return utilities.accessFileSystemWithRetry(async () => { + try { + utilities.logger.info(`Existing update file found and will be replaced: ${filePath}`); + await utilities.fileSystem.deletePath(filePath); + return true; + } catch (error) { + utilities.logger.error(`Failed to prepare file path for update: ${filePath}`, error); + return false; + } + }); +} + +const DefaultUtilities: InstallationFilepathProviderUtilities = { + logger: ElectronLogger, + directoryProvider: new PersistentApplicationDirectoryProvider(), + fileSystem: NodeElectronFileSystemOperations, + accessFileSystemWithRetry: retryFileSystemAccess, +}; diff --git a/src/presentation/electron/main/Update/ManualUpdater/Installer.ts b/src/presentation/electron/main/Update/ManualUpdater/Installer.ts index 46a3e3bf..9e913aad 100644 --- a/src/presentation/electron/main/Update/ManualUpdater/Installer.ts +++ b/src/presentation/electron/main/Update/ManualUpdater/Installer.ts @@ -1,7 +1,7 @@ import { app } from 'electron/main'; import { shell } from 'electron/common'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; -import { retryFileSystemAccess } from './RetryFileSystemAccess'; +import { retryFileSystemAccess } from './FileSystemAccessorWithRetry'; export async function startInstallation(filePath: string): Promise { return retryFileSystemAccess(async () => { diff --git a/src/presentation/electron/main/Update/ManualUpdater/Integrity.ts b/src/presentation/electron/main/Update/ManualUpdater/Integrity.ts index 599eaf01..d87ae6e2 100644 --- a/src/presentation/electron/main/Update/ManualUpdater/Integrity.ts +++ b/src/presentation/electron/main/Update/ManualUpdater/Integrity.ts @@ -1,7 +1,7 @@ import { createHash } from 'node:crypto'; import { createReadStream } from 'node:fs'; import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger'; -import { retryFileSystemAccess } from './RetryFileSystemAccess'; +import { retryFileSystemAccess } from './FileSystemAccessorWithRetry'; export async function checkIntegrity( filePath: string, diff --git a/src/presentation/electron/main/Update/ManualUpdater/ManualUpdateCoordinator.ts b/src/presentation/electron/main/Update/ManualUpdater/ManualUpdateCoordinator.ts index c6f6ce16..a0396d65 100644 --- a/src/presentation/electron/main/Update/ManualUpdater/ManualUpdateCoordinator.ts +++ b/src/presentation/electron/main/Update/ManualUpdater/ManualUpdateCoordinator.ts @@ -14,29 +14,43 @@ import { import { type DownloadUpdateResult, downloadUpdate } from './Downloader'; import { checkIntegrity } from './Integrity'; import { startInstallation } from './Installer'; +import { clearUpdateInstallationFiles } from './InstallationFiles/InstallationFileCleaner'; import type { UpdateInfo } from 'electron-updater'; -export function requiresManualUpdate(): boolean { - return process.platform === 'darwin'; +export function requiresManualUpdate( + nodePlatform: string = process.platform, +): boolean { + // autoUpdater cannot handle DMG files, requiring manual download management for these file types. + return nodePlatform === 'darwin'; } export async function startManualUpdateProcess(info: UpdateInfo) { + try { + await clearUpdateInstallationFiles(); + } catch (error) { + ElectronLogger.warn('Failed to clear previous update installation files', { error }); + } finally { + await executeManualUpdateProcess(info); + } +} + +async function executeManualUpdateProcess(info: UpdateInfo): Promise { try { const updateAction = await promptForManualUpdate(); if (updateAction === ManualUpdateChoice.NoAction) { - ElectronLogger.info('User cancelled the update.'); + ElectronLogger.info('User chose to cancel the update'); return; } const { releaseUrl, downloadUrl } = getRemoteUpdateUrls(info.version); if (updateAction === ManualUpdateChoice.VisitReleasesPage) { - ElectronLogger.info(`Navigating to release page: ${releaseUrl}`); + ElectronLogger.info('User chose to visit release page', { url: releaseUrl }); await shell.openExternal(releaseUrl); } else if (updateAction === ManualUpdateChoice.UpdateNow) { - ElectronLogger.info('Initiating update download and installation.'); + ElectronLogger.info('User chose to download and install update'); await downloadAndInstallUpdate(downloadUrl, info); } } catch (err) { - ElectronLogger.error('Unexpected error during updates', err); + ElectronLogger.error('Failed to execute auto-update process', { error: err }); await handleUnexpectedError(info); } } @@ -56,9 +70,10 @@ async function downloadAndInstallUpdate(fileUrl: string, info: UpdateInfo) { } const userAction = await promptIntegrityCheckFailure(); if (userAction === IntegrityCheckChoice.RetryDownload) { + ElectronLogger.info('User chose to retry download after integrity check failure'); await startManualUpdateProcess(info); } else if (userAction === IntegrityCheckChoice.ContinueAnyway) { - ElectronLogger.warn('Proceeding to install with failed integrity check.'); + ElectronLogger.warn('User chose to proceed with installation despite failed integrity check'); await openInstaller(download.installerPath, info); } } @@ -66,9 +81,9 @@ async function downloadAndInstallUpdate(fileUrl: string, info: UpdateInfo) { async function handleFailedDownload(info: UpdateInfo) { const userAction = await promptDownloadError(); if (userAction === DownloadErrorChoice.Cancel) { - ElectronLogger.info('Update download canceled.'); + ElectronLogger.info('User chose to cancel update download'); } else if (userAction === DownloadErrorChoice.RetryDownload) { - ElectronLogger.info('Retrying update download.'); + ElectronLogger.info('User chose to retry update download'); await startManualUpdateProcess(info); } } @@ -76,9 +91,9 @@ async function handleFailedDownload(info: UpdateInfo) { async function handleUnexpectedError(info: UpdateInfo) { const userAction = await showUnexpectedError(); if (userAction === UnexpectedErrorChoice.Cancel) { - ElectronLogger.info('Unexpected error handling canceled.'); + ElectronLogger.info('User chose to cancel update process after unexpected error'); } else if (userAction === UnexpectedErrorChoice.RetryUpdate) { - ElectronLogger.info('Retrying the update process.'); + ElectronLogger.info('User chose to retry update process after unexpected error'); await startManualUpdateProcess(info); } } @@ -89,8 +104,10 @@ async function openInstaller(installerPath: string, info: UpdateInfo) { } const userAction = await promptInstallerOpenError(); if (userAction === InstallerErrorChoice.RetryDownload) { + ElectronLogger.info('User chose to retry download after installer open error'); await startManualUpdateProcess(info); } else if (userAction === InstallerErrorChoice.RetryOpen) { + ElectronLogger.info('User chose to retry opening installer'); await openInstaller(installerPath, info); } } @@ -119,16 +136,16 @@ async function isIntegrityPreserved( function getRemoteSha512Hash(info: UpdateInfo, fileUrl: string): string | undefined { const fileInfos = info.files.filter((file) => fileUrl.includes(file.url)); if (!fileInfos.length) { - ElectronLogger.error(`Remote hash not found for the URL: ${fileUrl}`, info.files); + ElectronLogger.error('Failed to find remote hash for download URL', { url: fileUrl, files: info.files }); if (info.files.length > 0) { const firstHash = info.files[0].sha512; - ElectronLogger.info(`Selecting the first available hash: ${firstHash}`); + ElectronLogger.info('Using first available hash due to missing match', { hash: firstHash }); return firstHash; } return undefined; } if (fileInfos.length > 1) { - ElectronLogger.error(`Found multiple file entries for the URL: ${fileUrl}`, fileInfos); + ElectronLogger.warn('Multiple file entries found for download URL', { url: fileUrl, entries: fileInfos }); } return fileInfos[0].sha512; } diff --git a/src/presentation/electron/main/index.ts b/src/presentation/electron/main/index.ts index bb6d79d4..aec985d8 100644 --- a/src/presentation/electron/main/index.ts +++ b/src/presentation/electron/main/index.ts @@ -97,7 +97,7 @@ if (isDevelopment) { } } -function loadApplication(window: BrowserWindow) { +function loadApplication(window: BrowserWindow): void { if (RENDERER_URL) { // Populated in a dev server during development loadUrlWithNodeWorkaround(window, RENDERER_URL); } else { diff --git a/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts b/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts index 91f29132..63e1290c 100644 --- a/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts +++ b/tests/integration/infrastructure/CodeRunner/ScriptFileCodeRunner.spec.ts @@ -3,13 +3,13 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { exec } from 'node:child_process'; import { describe, it } from 'vitest'; -import type { ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; import { ScriptFileCreationOrchestrator } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator'; import { ScriptFileCodeRunner } from '@/infrastructure/CodeRunner/ScriptFileCodeRunner'; import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory'; import { OperatingSystem } from '@/domain/OperatingSystem'; import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; import { LinuxTerminalEmulator } from '@/infrastructure/CodeRunner/Execution/CommandDefinition/Commands/LinuxVisibleTerminalCommand'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; describe('ScriptFileCodeRunner', () => { it('executes simple script correctly', async ({ skip }) => { @@ -79,16 +79,16 @@ function isLinuxTerminalEmulatorSupported(): Promise { }); } -function createCodeRunner(directoryProvider: ScriptDirectoryProvider): ScriptFileCodeRunner { +function createCodeRunner(directoryProvider: ApplicationDirectoryProvider): ScriptFileCodeRunner { return new ScriptFileCodeRunner( undefined, new ScriptFileCreationOrchestrator(undefined, undefined, directoryProvider), ); } -function createTemporaryDirectoryProvider(): ScriptDirectoryProvider { +function createTemporaryDirectoryProvider(): ApplicationDirectoryProvider { return { - provideScriptDirectory: async () => { + provideDirectory: async () => { const temporaryDirectoryPathPrefix = join(tmpdir(), 'privacy-sexy-tests-'); const temporaryDirectoryFullPath = await mkdtemp(temporaryDirectoryPathPrefix); return { diff --git a/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts b/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts index ba75f79d..6ba1973f 100644 --- a/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.spec.ts @@ -1,22 +1,20 @@ import { describe, it, expect } from 'vitest'; import { ScriptFileCreationOrchestrator } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator'; import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; -import { FileSystemOpsStub } from '@tests/unit/shared/Stubs/FileSystemOpsStub'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; import type { Logger } from '@/application/Common/Log/Logger'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; -import type { ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; -import { ScriptDirectoryProviderStub } from '@tests/unit/shared/Stubs/ScriptDirectoryProviderStub'; +import { ApplicationDirectoryProviderStub } from '@tests/unit/shared/Stubs/ApplicationDirectoryProviderStub'; import type { FilenameGenerator } from '@/infrastructure/CodeRunner/Creation/Filename/FilenameGenerator'; import { FilenameGeneratorStub } from '@tests/unit/shared/Stubs/FilenameGeneratorStub'; -import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsStub'; -import type { SystemOperations } from '@/infrastructure/CodeRunner/System/SystemOperations'; -import { LocationOpsStub } from '@tests/unit/shared/Stubs/LocationOpsStub'; import type { ScriptFilenameParts } from '@/infrastructure/CodeRunner/Creation/ScriptFileCreator'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; import { expectTrue } from '@tests/shared/Assertions/ExpectTrue'; import type { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; -import { FileReadbackVerificationErrors, FileWriteOperationErrors, type ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; +import { FileReadbackVerificationErrors, FileWriteOperationErrors, type ReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; import { ReadbackFileWriterStub } from '@tests/unit/shared/Stubs/ReadbackFileWriterStub'; +import type { ApplicationDirectoryProvider, DirectoryCreationErrorType } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; describe('ScriptFileCreationOrchestrator', () => { describe('createScriptFile', () => { @@ -25,15 +23,15 @@ describe('ScriptFileCreationOrchestrator', () => { // arrange const pathSegmentSeparator = '/PATH-SEGMENT-SEPARATOR/'; const expectedScriptDirectory = '/expected-script-directory'; - const filesystem = new FileSystemOpsStub(); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator); const context = new ScriptFileCreatorTestSetup() - .withSystem(new SystemOperationsStub() - .withLocation( - new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator), - ) - .withFileSystem(filesystem)) + .withFileSystem(fileSystemStub) .withDirectoryProvider( - new ScriptDirectoryProviderStub().withDirectoryPath(expectedScriptDirectory), + new ApplicationDirectoryProviderStub().withDirectoryPath( + 'script-runs', + expectedScriptDirectory, + ), ); // act @@ -52,13 +50,12 @@ describe('ScriptFileCreationOrchestrator', () => { it('correctly generates filename', async () => { // arrange const pathSegmentSeparator = '/PATH-SEGMENT-SEPARATOR/'; - const filesystem = new FileSystemOpsStub(); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator); const expectedFilename = 'expected-script-file-name'; const context = new ScriptFileCreatorTestSetup() .withFilenameGenerator(new FilenameGeneratorStub().withFilename(expectedFilename)) - .withSystem(new SystemOperationsStub() - .withFileSystem(filesystem) - .withLocation(new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator))); + .withFileSystem(fileSystemStub); // act const { success, scriptFileAbsolutePath } = await context.createScriptFile(); @@ -97,15 +94,13 @@ describe('ScriptFileCreationOrchestrator', () => { const expectedPath = 'expected-script-path'; const filename = 'filename'; const directoryPath = 'directory-path'; - const filesystem = new FileSystemOpsStub(); + const fileSystemStub = new FileSystemOperationsStub() + .withJoinResult(expectedPath, directoryPath, filename); const context = new ScriptFileCreatorTestSetup() .withFilenameGenerator(new FilenameGeneratorStub().withFilename(filename)) - .withDirectoryProvider(new ScriptDirectoryProviderStub().withDirectoryPath(directoryPath)) - .withSystem(new SystemOperationsStub() - .withFileSystem(filesystem) - .withLocation( - new LocationOpsStub().withJoinResult(expectedPath, directoryPath, filename), - )); + .withDirectoryProvider(new ApplicationDirectoryProviderStub() + .withDirectoryPath('script-runs', directoryPath)) + .withFileSystem(fileSystemStub); // act const { success, scriptFileAbsolutePath } = await context.createScriptFile(); @@ -169,11 +164,11 @@ describe('ScriptFileCreationOrchestrator', () => { expectedErrorMessage: 'Error when combining paths', expectLogs: true, buildFaultyContext: (setup, errorMessage) => { - const locationStub = new LocationOpsStub(); - locationStub.combinePaths = () => { + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.combinePaths = () => { throw new Error(errorMessage); }; - return setup.withSystem(new SystemOperationsStub().withLocation(locationStub)); + return setup.withFileSystem(fileSystemStub); }, }, ...FileWriteOperationErrors.map((writeError): FileCreationFailureTestScenario => ({ @@ -214,23 +209,40 @@ describe('ScriptFileCreationOrchestrator', () => { return setup.withFilenameGenerator(filenameGenerator); }, }, - { - description: 'script directory provision failure', - expectedErrorType: 'DirectoryCreationError', - expectedErrorMessage: 'Error when providing directory', - expectLogs: false, - buildFaultyContext: (setup, errorMessage, errorType) => { - const directoryProvider = new ScriptDirectoryProviderStub(); - directoryProvider.provideScriptDirectory = () => Promise.resolve({ - success: false, - error: { - message: errorMessage, - type: errorType, - }, - }); - return setup.withDirectoryProvider(directoryProvider); - }, - }, + ...(() => { + const directoryErrorScenarios: Record = { + DirectoryWriteError: { + directoryErrorMessage: 'Injected error when writing to directory', + }, + PathConstructionError: { + directoryErrorMessage: 'Injected error when constructing path', + }, + UserDataFolderRetrievalError: { + directoryErrorMessage: 'Injected error when locating user data folder', + }, + }; + return Object.entries(directoryErrorScenarios).map(([ + directoryErrorType, { directoryErrorMessage }, + ]): FileCreationFailureTestScenario => ({ + description: `script directory creation failure: ${directoryErrorType}`, + expectedErrorType: 'DirectoryCreationError', + expectedErrorMessage: `[${directoryErrorType}] ${directoryErrorMessage}`, + expectLogs: false, + buildFaultyContext: (setup) => { + const directoryProvider = new ApplicationDirectoryProviderStub(); + directoryProvider.provideDirectory = () => Promise.resolve({ + success: false, + error: { + type: directoryErrorType as DirectoryCreationErrorType, + message: directoryErrorMessage, + }, + }); + return setup.withDirectoryProvider(directoryProvider); + }, + })); + })(), ]; testScenarios.forEach(({ description, expectedErrorType, expectedErrorMessage, buildFaultyContext, expectLogs, @@ -276,11 +288,11 @@ describe('ScriptFileCreationOrchestrator', () => { }); class ScriptFileCreatorTestSetup { - private system: SystemOperations = new SystemOperationsStub(); + private fileSystem: FileSystemOperations = new FileSystemOperationsStub(); private filenameGenerator: FilenameGenerator = new FilenameGeneratorStub(); - private directoryProvider: ScriptDirectoryProvider = new ScriptDirectoryProviderStub(); + private directoryProvider: ApplicationDirectoryProvider = new ApplicationDirectoryProviderStub(); private logger: Logger = new LoggerStub(); @@ -298,7 +310,7 @@ class ScriptFileCreatorTestSetup { return this; } - public withDirectoryProvider(directoryProvider: ScriptDirectoryProvider): this { + public withDirectoryProvider(directoryProvider: ApplicationDirectoryProvider): this { this.directoryProvider = directoryProvider; return this; } @@ -308,8 +320,8 @@ class ScriptFileCreatorTestSetup { return this; } - public withSystem(system: SystemOperations): this { - this.system = system; + public withFileSystem(fileSystem: FileSystemOperations): this { + this.fileSystem = fileSystem; return this; } @@ -330,7 +342,7 @@ class ScriptFileCreatorTestSetup { public createScriptFile(): ReturnType { const creator = new ScriptFileCreationOrchestrator( - this.system, + this.fileSystem, this.filenameGenerator, this.directoryProvider, this.fileWriter, diff --git a/tests/unit/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.spec.ts b/tests/unit/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.spec.ts index 96f6a57b..c017a136 100644 --- a/tests/unit/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.spec.ts +++ b/tests/unit/infrastructure/CodeRunner/Execution/CommandDefinition/Runner/PermissionSetter/FileSystemExecutablePermissionSetter.spec.ts @@ -5,7 +5,7 @@ import { FileSystemExecutablePermissionSetter } from '@/infrastructure/CodeRunne import type { ScriptFileExecutionOutcome } from '@/infrastructure/CodeRunner/Execution/ScriptFileExecutor'; import type { SystemOperations } from '@/infrastructure/CodeRunner/System/SystemOperations'; import { expectTrue } from '@tests/shared/Assertions/ExpectTrue'; -import { FileSystemOpsStub } from '@tests/unit/shared/Stubs/FileSystemOpsStub'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsStub'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; @@ -15,7 +15,7 @@ describe('FileSystemExecutablePermissionSetter', () => { it('sets permissions on the specified file', async () => { // arrange const expectedFilePath = 'expected-file-path'; - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); const context = new TestContext() .withFilePath(expectedFilePath) .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -33,7 +33,7 @@ describe('FileSystemExecutablePermissionSetter', () => { it('applies the correct permissions mode', async () => { // arrange const expectedMode = '755'; - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); const context = new TestContext() .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -49,7 +49,7 @@ describe('FileSystemExecutablePermissionSetter', () => { it('reports success when permissions are set without errors', async () => { // arrange - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); fileSystem.setFilePermissions = () => Promise.resolve(); const context = new TestContext() .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -67,7 +67,7 @@ describe('FileSystemExecutablePermissionSetter', () => { // arrange const thrownErrorMessage = 'File system error'; const expectedErrorMessage = `Error setting script file permission: ${thrownErrorMessage}`; - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); fileSystem.setFilePermissions = () => Promise.reject(new Error(thrownErrorMessage)); const context = new TestContext() .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -84,7 +84,7 @@ describe('FileSystemExecutablePermissionSetter', () => { it('returns expected error type when filesystem throws', async () => { // arrange const expectedErrorType: CodeRunErrorType = 'FilePermissionChangeError'; - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); fileSystem.setFilePermissions = () => Promise.reject(new Error('File system error')); const context = new TestContext() .withSystemOperations(new SystemOperationsStub().withFileSystem(fileSystem)); @@ -103,7 +103,7 @@ describe('FileSystemExecutablePermissionSetter', () => { // arrange const thrownErrorMessage = 'File system error'; const logger = new LoggerStub(); - const fileSystem = new FileSystemOpsStub(); + const fileSystem = new FileSystemOperationsStub(); fileSystem.setFilePermissions = () => Promise.reject(new Error(thrownErrorMessage)); const context = new TestContext() .withLogger(logger) diff --git a/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts b/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts index 6c2a02f1..30184206 100644 --- a/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts +++ b/tests/unit/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.spec.ts @@ -7,7 +7,7 @@ import type { Logger } from '@/application/Common/Log/Logger'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; import { ReadbackFileWriterStub } from '@tests/unit/shared/Stubs/ReadbackFileWriterStub'; -import { FileReadbackVerificationErrors, FileWriteOperationErrors, type ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; +import { FileReadbackVerificationErrors, FileWriteOperationErrors, type ReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; import { ElectronFileDialogOperationsStub } from './ElectronFileDialogOperationsStub'; import { NodePathOperationsStub } from './NodePathOperationsStub'; diff --git a/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts b/tests/unit/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.spec.ts similarity index 51% rename from tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts rename to tests/unit/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.spec.ts index 3b42eb63..93058162 100644 --- a/tests/unit/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider.spec.ts +++ b/tests/unit/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider.spec.ts @@ -1,30 +1,25 @@ import { describe, it, expect } from 'vitest'; import type { Logger } from '@/application/Common/Log/Logger'; -import { ExecutionSubdirectory, PersistentDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider'; -import type { SystemOperations } from '@/infrastructure/CodeRunner/System/SystemOperations'; -import { LocationOpsStub } from '@tests/unit/shared/Stubs/LocationOpsStub'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; -import { OperatingSystemOpsStub } from '@tests/unit/shared/Stubs/OperatingSystemOpsStub'; -import { SystemOperationsStub } from '@tests/unit/shared/Stubs/SystemOperationsStub'; -import { FileSystemOpsStub } from '@tests/unit/shared/Stubs/FileSystemOpsStub'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; -import type { CodeRunErrorType } from '@/application/CodeRunner/CodeRunner'; import { expectTrue } from '@tests/shared/Assertions/ExpectTrue'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; +import type { DirectoryCreationErrorType, DirectoryType } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import { PersistentApplicationDirectoryProvider, SubdirectoryNames } from '@/infrastructure/FileSystem/Directory/PersistentApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; -describe('PersistentDirectoryProvider', () => { +describe('PersistentApplicationDirectoryProvider', () => { describe('createDirectory', () => { describe('path construction', () => { it('bases path on user directory', async () => { // arrange const expectedBaseDirectory = 'base-directory'; const pathSegmentSeparator = '/STUB-SEGMENT-SEPARATOR/'; - const locationOps = new LocationOpsStub() + const fileSystemStub = new FileSystemOperationsStub() + .withUserDirectoryResult(expectedBaseDirectory) .withDefaultSeparator(pathSegmentSeparator); const context = new PersistentDirectoryProviderTestSetup() - .withSystem(new SystemOperationsStub() - .withOperatingSystem(new OperatingSystemOpsStub() - .withUserDirectoryResult(expectedBaseDirectory)) - .withLocation(locationOps)); + .withFileSystem(fileSystemStub); // act const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); @@ -33,51 +28,66 @@ describe('PersistentDirectoryProvider', () => { expectTrue(success); const actualBaseDirectory = directoryAbsolutePath.split(pathSegmentSeparator)[0]; expect(actualBaseDirectory).to.equal(expectedBaseDirectory); - const calls = locationOps.callHistory.filter((call) => call.methodName === 'combinePaths'); + const calls = fileSystemStub.callHistory.filter((call) => call.methodName === 'combinePaths'); expect(calls.length).to.equal(1); const [combinedBaseDirectory] = calls[0].args; expect(combinedBaseDirectory).to.equal(expectedBaseDirectory); }); - it('includes execution subdirectory in path', async () => { - // arrange - const expectedSubdirectory = ExecutionSubdirectory; - const pathSegmentSeparator = '/STUB-SEGMENT-SEPARATOR/'; - const locationOps = new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator); - const context = new PersistentDirectoryProviderTestSetup() - .withSystem(new SystemOperationsStub() - .withLocation(locationOps)); - - // act - const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); - - // assert - expectTrue(success); - const actualSubdirectory = directoryAbsolutePath - .split(pathSegmentSeparator) - .pop(); - expect(actualSubdirectory).to.equal(expectedSubdirectory); - const calls = locationOps.callHistory.filter((call) => call.methodName === 'combinePaths'); - expect(calls.length).to.equal(1); - const [,combinedSubdirectory] = calls[0].args; - expect(combinedSubdirectory).to.equal(expectedSubdirectory); + describe('includes correct execution subdirectory in path', () => { + const testScenarios: readonly { + readonly description: string; + readonly givenDirectoryType: DirectoryType; + readonly expectedSubdirectoryName: string; + }[] = Object.entries(SubdirectoryNames).map(([type, name]) => ({ + description: `returns '${name}' for '${type}'`, + givenDirectoryType: type as DirectoryType, + expectedSubdirectoryName: name, + })); + testScenarios.forEach(({ + description, expectedSubdirectoryName, givenDirectoryType, + }) => { + it(description, async () => { + // arrange + const pathSegmentSeparator = '/STUB-SEGMENT-SEPARATOR/'; + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator); + const context = new PersistentDirectoryProviderTestSetup() + .withFileSystem(fileSystemStub) + .withDirectoryType(givenDirectoryType); + + // act + const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); + + // assert + expectTrue(success); + const actualSubdirectory = directoryAbsolutePath + .split(pathSegmentSeparator) + .pop(); + expect(actualSubdirectory).to.equal(expectedSubdirectoryName); + const calls = fileSystemStub.callHistory.filter((call) => call.methodName === 'combinePaths'); + expect(calls.length).to.equal(1); + const [,combinedSubdirectory] = calls[0].args; + expect(combinedSubdirectory).to.equal(expectedSubdirectoryName); + }); + }); }); it('forms full path correctly', async () => { // arrange + const directoryType: DirectoryType = 'script-runs'; const pathSegmentSeparator = '/'; const baseDirectory = 'base-directory'; - const expectedDirectory = [baseDirectory, ExecutionSubdirectory].join(pathSegmentSeparator); + const expectedDirectory = [baseDirectory, SubdirectoryNames[directoryType]] + .join(pathSegmentSeparator); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator) + .withUserDirectoryResult(baseDirectory); const context = new PersistentDirectoryProviderTestSetup() - .withSystem(new SystemOperationsStub() - .withLocation(new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator)) - .withOperatingSystem( - new OperatingSystemOpsStub().withUserDirectoryResult(baseDirectory), - )); + .withFileSystem(fileSystemStub) + .withDirectoryType(directoryType); // act const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); - - // assert - expectTrue(success); + expect(success).to.equal(true); expect(directoryAbsolutePath).to.equal(expectedDirectory); }); }); @@ -85,16 +95,16 @@ describe('PersistentDirectoryProvider', () => { it('creates directory with recursion', async () => { // arrange const expectedIsRecursive = true; - const filesystem = new FileSystemOpsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new PersistentDirectoryProviderTestSetup() - .withSystem(new SystemOperationsStub().withFileSystem(filesystem)); + .withFileSystem(fileSystemStub); // act const { success, directoryAbsolutePath } = await context.provideScriptDirectory(); // assert expectTrue(success); - const calls = filesystem.callHistory.filter((call) => call.methodName === 'createDirectory'); + const calls = fileSystemStub.callHistory.filter((call) => call.methodName === 'createDirectory'); expect(calls.length).to.equal(1); const [actualPath, actualIsRecursive] = calls[0].args; expect(actualPath).to.equal(directoryAbsolutePath); @@ -104,7 +114,7 @@ describe('PersistentDirectoryProvider', () => { describe('error handling', () => { const testScenarios: ReadonlyArray<{ readonly description: string; - readonly expectedErrorType: CodeRunErrorType; + readonly expectedErrorType: DirectoryCreationErrorType; readonly expectedErrorMessage: string; buildFaultyContext( setup: PersistentDirectoryProviderTestSetup, @@ -113,40 +123,38 @@ describe('PersistentDirectoryProvider', () => { }> = [ { description: 'path combination failure', - expectedErrorType: 'DirectoryCreationError', + expectedErrorType: 'PathConstructionError', expectedErrorMessage: 'Error when combining paths', buildFaultyContext: (setup, errorMessage) => { - const locationStub = new LocationOpsStub(); - locationStub.combinePaths = () => { + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.combinePaths = () => { throw new Error(errorMessage); }; - return setup.withSystem(new SystemOperationsStub().withLocation(locationStub)); + return setup.withFileSystem(fileSystemStub); }, }, { description: 'user data retrieval failure', - expectedErrorType: 'DirectoryCreationError', + expectedErrorType: 'UserDataFolderRetrievalError', expectedErrorMessage: 'Error when locating user data directory', buildFaultyContext: (setup, errorMessage) => { - const operatingSystemStub = new OperatingSystemOpsStub(); - operatingSystemStub.getUserDataDirectory = () => { + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.getUserDataDirectory = () => { throw new Error(errorMessage); }; - return setup.withSystem( - new SystemOperationsStub().withOperatingSystem(operatingSystemStub), - ); + return setup.withFileSystem(fileSystemStub); }, }, { description: 'directory creation failure', - expectedErrorType: 'DirectoryCreationError', + expectedErrorType: 'DirectoryWriteError', expectedErrorMessage: 'Error when creating directory', buildFaultyContext: (setup, errorMessage) => { - const fileSystemStub = new FileSystemOpsStub(); + const fileSystemStub = new FileSystemOperationsStub(); fileSystemStub.createDirectory = () => { throw new Error(errorMessage); }; - return setup.withSystem(new SystemOperationsStub().withFileSystem(fileSystemStub)); + return setup.withFileSystem(fileSystemStub); }, }, ]; @@ -190,12 +198,14 @@ describe('PersistentDirectoryProvider', () => { }); class PersistentDirectoryProviderTestSetup { - private system: SystemOperations = new SystemOperationsStub(); + private fileSystem: FileSystemOperations = new FileSystemOperationsStub(); private logger: Logger = new LoggerStub(); - public withSystem(system: SystemOperations): this { - this.system = system; + private directoryType: DirectoryType = 'script-runs'; + + public withFileSystem(fileSystem: FileSystemOperations): this { + this.fileSystem = fileSystem; return this; } @@ -204,8 +214,13 @@ class PersistentDirectoryProviderTestSetup { return this; } - public provideScriptDirectory(): ReturnType { - const provider = new PersistentDirectoryProvider(this.system, this.logger); - return provider.provideScriptDirectory(); + public withDirectoryType(directoryType: DirectoryType): this { + this.directoryType = directoryType; + return this; + } + + public provideScriptDirectory(): ReturnType { + const provider = new PersistentApplicationDirectoryProvider(this.fileSystem, this.logger); + return provider.provideDirectory(this.directoryType); } } diff --git a/tests/unit/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts b/tests/unit/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts similarity index 81% rename from tests/unit/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts rename to tests/unit/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts index 29d307de..4b376206 100644 --- a/tests/unit/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts +++ b/tests/unit/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter.spec.ts @@ -1,13 +1,13 @@ -import { constants } from 'node:fs'; import { describe, it, expect } from 'vitest'; import type { Logger } from '@/application/Common/Log/Logger'; import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; import type { FunctionKeys } from '@/TypeHelpers'; import { sequenceEqual } from '@/application/Common/Array'; -import type { FileWriteErrorType } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; +import type { FileWriteErrorType } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; import { expectExists } from '@tests/shared/Assertions/ExpectExists'; -import { type FileReadWriteOperations, NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter'; -import { FileReadWriteOperationsStub } from './FileReadWriteOperationsStub'; +import { NodeReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/NodeReadbackFileWriter'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; describe('NodeReadbackFileWriter', () => { describe('writeAndVerifyFile', () => { @@ -26,7 +26,7 @@ describe('NodeReadbackFileWriter', () => { it('writes to specified path', async () => { // arrange const expectedFilePath = 'test.txt'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFilePath(expectedFilePath) .withFileSystem(fileSystemStub); @@ -43,7 +43,7 @@ describe('NodeReadbackFileWriter', () => { it('writes specified contents', async () => { // arrange const expectedFileContents = 'expected file contents'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub) .withFileContents(expectedFileContents); @@ -60,7 +60,7 @@ describe('NodeReadbackFileWriter', () => { it('uses correct encoding', async () => { // arrange const expectedEncoding: NodeJS.BufferEncoding = 'utf-8'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub); @@ -78,7 +78,7 @@ describe('NodeReadbackFileWriter', () => { it('checks correct path', async () => { // arrange const expectedFilePath = 'test-file-path'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub) .withFilePath(expectedFilePath); @@ -87,33 +87,17 @@ describe('NodeReadbackFileWriter', () => { await context.writeAndVerifyFile(); // assert - const accessCalls = fileSystemStub.callHistory.filter((c) => c.methodName === 'access'); + const accessCalls = fileSystemStub.callHistory.filter((c) => c.methodName === 'isFileAvailable'); expect(accessCalls).to.have.lengthOf(1); const [actualFilePath] = accessCalls[0].args; expect(actualFilePath).to.equal(expectedFilePath); }); - it('uses correct mode', async () => { - // arrange - const expectedMode = constants.F_OK; - const fileSystemStub = new FileReadWriteOperationsStub(); - const context = new NodeReadbackFileWriterTestSetup() - .withFileSystem(fileSystemStub); - - // act - await context.writeAndVerifyFile(); - - // assert - const accessCalls = fileSystemStub.callHistory.filter((c) => c.methodName === 'access'); - expect(accessCalls).to.have.lengthOf(1); - const [,actualMode] = accessCalls[0].args; - expect(actualMode).to.equal(expectedMode); - }); }); describe('content verification', () => { it('reads from correct path', async () => { // arrange const expectedFilePath = 'expected-file-path.txt'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub) .withFilePath(expectedFilePath); @@ -130,7 +114,7 @@ describe('NodeReadbackFileWriter', () => { it('uses correct encoding', async () => { // arrange const expectedEncoding: NodeJS.BufferEncoding = 'utf-8'; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub); @@ -146,12 +130,12 @@ describe('NodeReadbackFileWriter', () => { }); it('executes file system operations in correct sequence', async () => { // arrange - const expectedOrder: ReadonlyArray> = [ + const expectedOrder: ReadonlyArray> = [ 'writeFile', - 'access', + 'isFileAvailable', 'readFile', ]; - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); const context = new NodeReadbackFileWriterTestSetup() .withFileSystem(fileSystemStub); @@ -178,19 +162,30 @@ describe('NodeReadbackFileWriter', () => { expectedErrorType: 'WriteOperationFailed', expectedErrorMessage: 'Error when writing file', buildFaultyContext: (setup, errorMessage) => { - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); fileSystemStub.writeFile = () => Promise.reject(errorMessage); return setup .withFileSystem(fileSystemStub); }, }, { - description: 'existence verification error', + description: 'existence verification throws error', expectedErrorType: 'FileExistenceVerificationFailed', expectedErrorMessage: 'Access denied', buildFaultyContext: (setup, errorMessage) => { - const fileSystemStub = new FileReadWriteOperationsStub(); - fileSystemStub.access = () => Promise.reject(errorMessage); + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => Promise.reject(errorMessage); + return setup + .withFileSystem(fileSystemStub); + }, + }, + { + description: 'existence verification returnf alse', + expectedErrorType: 'FileExistenceVerificationFailed', + expectedErrorMessage: 'File does not exist.', + buildFaultyContext: (setup) => { + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => Promise.resolve(false); return setup .withFileSystem(fileSystemStub); }, @@ -200,7 +195,7 @@ describe('NodeReadbackFileWriter', () => { expectedErrorType: 'ReadVerificationFailed', expectedErrorMessage: 'Read error', buildFaultyContext: (setup, errorMessage) => { - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); fileSystemStub.readFile = () => Promise.reject(errorMessage); return setup .withFileSystem(fileSystemStub); @@ -211,7 +206,7 @@ describe('NodeReadbackFileWriter', () => { expectedErrorType: 'ContentVerificationFailed', expectedErrorMessage: 'The contents of the written file do not match the expected contents.', buildFaultyContext: (setup) => { - const fileSystemStub = new FileReadWriteOperationsStub(); + const fileSystemStub = new FileSystemOperationsStub(); fileSystemStub.readFile = () => Promise.resolve('different contents'); return setup .withFileSystem(fileSystemStub); @@ -260,7 +255,7 @@ describe('NodeReadbackFileWriter', () => { class NodeReadbackFileWriterTestSetup { private logger: Logger = new LoggerStub(); - private fileSystem: FileReadWriteOperations = new FileReadWriteOperationsStub(); + private fileSystem: FileSystemOperations = new FileSystemOperationsStub(); private filePath = '/test/file/path.txt'; @@ -271,7 +266,7 @@ class NodeReadbackFileWriterTestSetup { return this; } - public withFileSystem(fileSystem: FileReadWriteOperations): this { + public withFileSystem(fileSystem: FileSystemOperations): this { this.fileSystem = fileSystem; return this; } diff --git a/tests/unit/infrastructure/ReadbackFileWriter/FileReadWriteOperationsStub.ts b/tests/unit/infrastructure/ReadbackFileWriter/FileReadWriteOperationsStub.ts deleted file mode 100644 index 108fc3a8..00000000 --- a/tests/unit/infrastructure/ReadbackFileWriter/FileReadWriteOperationsStub.ts +++ /dev/null @@ -1,34 +0,0 @@ -import type { FileReadWriteOperations } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter'; -import { StubWithObservableMethodCalls } from '@tests/unit/shared/Stubs/StubWithObservableMethodCalls'; - -export class FileReadWriteOperationsStub - extends StubWithObservableMethodCalls - implements FileReadWriteOperations { - private readonly writtenFiles: Record = {}; - - public writeFile = (filePath: string, fileContents: string, encoding: NodeJS.BufferEncoding) => { - this.registerMethodCall({ - methodName: 'writeFile', - args: [filePath, fileContents, encoding], - }); - this.writtenFiles[filePath] = fileContents; - return Promise.resolve(); - }; - - public access = (...args: Parameters) => { - this.registerMethodCall({ - methodName: 'access', - args: [...args], - }); - return Promise.resolve(); - }; - - public readFile = (filePath: string, encoding: NodeJS.BufferEncoding) => { - this.registerMethodCall({ - methodName: 'readFile', - args: [filePath, encoding], - }); - const fileContents = this.writtenFiles[filePath]; - return Promise.resolve(fileContents); - }; -} diff --git a/tests/unit/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.spec.ts b/tests/unit/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.spec.ts index d21a47fb..8cf5f670 100644 --- a/tests/unit/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.spec.ts +++ b/tests/unit/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector.spec.ts @@ -1,10 +1,10 @@ import { describe, it, expect } from 'vitest'; -import type { ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; import type { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment'; import { ScriptEnvironmentDiagnosticsCollector } from '@/infrastructure/ScriptDiagnostics/ScriptEnvironmentDiagnosticsCollector'; import { RuntimeEnvironmentStub } from '@tests/unit/shared/Stubs/RuntimeEnvironmentStub'; -import { ScriptDirectoryProviderStub } from '@tests/unit/shared/Stubs/ScriptDirectoryProviderStub'; +import { ApplicationDirectoryProviderStub } from '@tests/unit/shared/Stubs/ApplicationDirectoryProviderStub'; import { OperatingSystem } from '@/domain/OperatingSystem'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; describe('ScriptEnvironmentDiagnosticsCollector', () => { it('collects operating system path correctly', async () => { @@ -26,8 +26,8 @@ describe('ScriptEnvironmentDiagnosticsCollector', () => { it('collects path correctly', async () => { // arrange const expectedScriptsPath = '/expected/scripts/path'; - const directoryProvider = new ScriptDirectoryProviderStub() - .withDirectoryPath(expectedScriptsPath); + const directoryProvider = new ApplicationDirectoryProviderStub() + .withDirectoryPath('script-runs', expectedScriptsPath); const collector = new CollectorBuilder() .withScriptDirectoryProvider(directoryProvider) .build(); @@ -42,7 +42,7 @@ describe('ScriptEnvironmentDiagnosticsCollector', () => { }); class CollectorBuilder { - private directoryProvider: ScriptDirectoryProvider = new ScriptDirectoryProviderStub(); + private directoryProvider: ApplicationDirectoryProvider = new ApplicationDirectoryProviderStub(); private environment: RuntimeEnvironment = new RuntimeEnvironmentStub(); @@ -51,7 +51,7 @@ class CollectorBuilder { return this; } - public withScriptDirectoryProvider(directoryProvider: ScriptDirectoryProvider): this { + public withScriptDirectoryProvider(directoryProvider: ApplicationDirectoryProvider): this { this.directoryProvider = directoryProvider; return this; } diff --git a/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.spec.ts b/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.spec.ts new file mode 100644 index 00000000..f09846b4 --- /dev/null +++ b/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner.spec.ts @@ -0,0 +1,284 @@ +import { it, describe, expect } from 'vitest'; +import type { Logger } from '@/application/Common/Log/Logger'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import { ApplicationDirectoryProviderStub } from '@tests/unit/shared/Stubs/ApplicationDirectoryProviderStub'; +import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; +import { clearUpdateInstallationFiles } from '@/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFileCleaner'; +import { expectThrowsAsync } from '@tests/shared/Assertions/ExpectThrowsAsync'; +import { collectExceptionAsync } from '@tests/unit/shared/ExceptionCollector'; +import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; +import { expectExists } from '@tests/shared/Assertions/ExpectExists'; +import { indentText } from '@/application/Common/Text/IndentText'; + +describe('InstallationFileCleaner', () => { + describe('clearUpdateInstallationFiles', () => { + describe('deleting files', () => { + it('deletes all update installation files and directories', async () => { + // arrange + const expectedDirectoryEntries = ['file1', 'file2', 'file3', 'directory1', 'directory2']; + const directoryPath = 'directory-name'; + const pathSeparator = 'test-separator'; + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', directoryPath); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSeparator) + .withDirectoryContents(directoryPath, expectedDirectoryEntries); + const context = new TestContext() + .withDirectoryProvider(directoryProviderStub) + .withFileSystem(fileSystemStub); + // act + await context.run(); + // assert + const actualDeletedEntries = fileSystemStub.callHistory + .filter((c) => c.methodName === 'deletePath') + .map((c) => c.args[0]) + .map((path) => path.split(pathSeparator).pop()); + expect(expectedDirectoryEntries.sort()).to.deep.equal(actualDeletedEntries.sort()); + }); + it('deletes files at the correct absolute paths', async () => { + // arrange + const directoryItemName = 'expected-item-name'; + const directoryPath = 'expected-directory'; + const pathSeparator = '[expected-separator]'; + const expectedFullPath = [directoryPath, directoryItemName].join(pathSeparator); + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', directoryPath); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSeparator) + .withDirectoryContents(directoryPath, [directoryItemName]); + const context = new TestContext() + .withDirectoryProvider(directoryProviderStub) + .withFileSystem(fileSystemStub); + // act + await context.run(); + // assert + const actualDeletedEntries = fileSystemStub.callHistory + .filter((c) => c.methodName === 'deletePath') + .map((c) => c.args[0]); + expect(actualDeletedEntries).to.have.lengthOf(1); + const actualFullPath = actualDeletedEntries[0]; + expect(actualFullPath).to.equal(expectedFullPath); + }); + it('continues deleting other items if one cannot be deleted', async () => { + // arrange + const expectedDeletedItems = ['success-1', 'success-2', 'success-3']; + const expectedDirectoryEntries = ['fail-1', ...expectedDeletedItems, 'fail-2']; + const directoryPath = 'directory-name'; + const pathSeparator = 'test-separator'; + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', directoryPath); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSeparator) + .withDirectoryContents(directoryPath, expectedDirectoryEntries); + fileSystemStub.deletePath = async (path) => { + await FileSystemOperationsStub.prototype + .deletePath.call(fileSystemStub, path); // register call history + if (expectedDeletedItems.some((item) => path.endsWith(item))) { + return; + } + throw new Error(`Path is not configured to succeed, so it fails: ${path}`); + }; + const context = new TestContext() + .withDirectoryProvider(directoryProviderStub) + .withFileSystem(fileSystemStub); + // act + try { + await context.run(); + } catch { /* Swallow */ } + // assert + const actualDeletedEntries = fileSystemStub.callHistory + .filter((c) => c.methodName === 'deletePath') + .map((c) => c.args[0]) + .map((path) => path.split(pathSeparator).pop()); + expect(expectedDirectoryEntries.sort()).to.deep.equal(actualDeletedEntries.sort()); + }); + }); + it('does nothing if directory is empty', async () => { + // arrange + const directoryPath = 'directory-path'; + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', directoryPath); + const fileSystemStub = new FileSystemOperationsStub() + .withDirectoryContents(directoryPath, []); + const context = new TestContext() + .withDirectoryProvider(directoryProviderStub) + .withFileSystem(fileSystemStub); + // act + await context.run(); + // assert + const actualDeletedEntries = fileSystemStub.callHistory + .filter((c) => c.methodName === 'deletePath'); + expect(actualDeletedEntries).to.have.lengthOf(0); + }); + describe('error handling', () => { + it('throws if installation directory cannot be provided', async () => { + // arrange + const expectedError = 'Cannot locate the installation files directory path'; + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withFailure(); + const context = new TestContext() + .withDirectoryProvider(directoryProviderStub); + // act + const act = () => context.run(); + // assert + await expectThrowsAsync(act, expectedError); + }); + it('throws if directory contents cannot be listed', async () => { + // arrange + const expectedError = 'Failed to to read directory contents.'; + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.listDirectoryContents = () => Promise.reject(new Error(expectedError)); + const context = new TestContext() + .withFileSystem(fileSystemStub); + // act + const act = () => context.run(); + // assert + await expectThrowsAsync(act, expectedError); + }); + it('throws if all items cannot be deleted', async () => { + // arrange + const itemsWithErrors: Map = new Map([ + ['item-1', new Error('Access Denied: item-1')], + ['item-2', new Error('Disk I/O Error: item-2')], + ]); + const expectedErrorParts = [ + 'Failed to delete some items', + ...[...itemsWithErrors.values()].map((item: Error) => item.message), + ]; + const loggerStub = new LoggerStub(); + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.listDirectoryContents = async () => { + await FileSystemOperationsStub.prototype + .listDirectoryContents.call(fileSystemStub); // register call history + return [...itemsWithErrors.keys()]; + }; + fileSystemStub.deletePath = (path) => { + const name = [...itemsWithErrors.keys()] + .find((fileName) => path.endsWith(fileName)); + if (!name) { + return Promise.resolve(); + } + const error = itemsWithErrors.get(name)!; + return Promise.reject(error); + }; + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withLogger(loggerStub); + // act + const act = () => context.run(); + // assert + const error = await collectExceptionAsync(act); + expectExists(error, formatAssertionMessage([ + `FileSystem calls: ${JSON.stringify(fileSystemStub.callHistory)}`, + `Log calls: ${JSON.stringify(loggerStub.callHistory)}`, + ])); + const errorMessage = error.message; + const notExistingErrorMessageParts = expectedErrorParts.filter( + (e) => !errorMessage.includes(e), + ); + expect(notExistingErrorMessageParts).to.have.lengthOf(0, formatAssertionMessage([ + 'Actual error message:', + indentText(errorMessage), + 'Expected parts:', + indentText(expectedErrorParts.map((part) => `- ${part}`).join('\n')), + ])); + }); + it('throws if some items cannot be deleted', async () => { + // arrange + const itemsWithErrors: Map = new Map([ + ['item-1', new Error('Access Denied: item-1')], + ['item-2', new Error('Disk I/O Error: item-2')], + ]); + const expectedErrorParts = [ + 'Failed to delete some items', + ...[...itemsWithErrors.values()].map((item: Error) => item.message), + ]; + const itemsWithSuccess = ['successful-item-1', 'successful-item-2']; + const allItems = [ + itemsWithSuccess[0], + ...[...itemsWithErrors.keys()], + itemsWithSuccess[1], + ]; + const fileSystemStub = new FileSystemOperationsStub(); + const loggerStub = new LoggerStub(); + fileSystemStub.listDirectoryContents = async () => { + await FileSystemOperationsStub.prototype + .listDirectoryContents.call(fileSystemStub); // register call history + return allItems; + }; + fileSystemStub.deletePath = async (path) => { + await FileSystemOperationsStub.prototype + .deletePath.call(fileSystemStub, path); // register call history + const name = [...itemsWithErrors.keys()].find((n) => path.endsWith(n)); + if (!name) { + return; + } + const error = itemsWithErrors.get(name)!; + throw error; + }; + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withLogger(loggerStub); + // act + const act = () => context.run(); + // assert + const error = await collectExceptionAsync(act); + expectExists(error, formatAssertionMessage([ + `Calls: ${JSON.stringify(fileSystemStub.callHistory)}`, + `Logs: ${JSON.stringify(loggerStub.callHistory)}`, + ])); + const errorMessage = error.message; + const notExistingErrorMessageParts = expectedErrorParts.filter( + (e) => !error.message.includes(e), + ); + expect(notExistingErrorMessageParts) + .to.have.lengthOf(0, formatAssertionMessage([ + 'Actual error message:', + indentText(errorMessage), + 'Expected parts:', + indentText(expectedErrorParts.map((part) => `- ${part}`).join('\n')), + ])); + expect(itemsWithSuccess.some((item) => errorMessage.includes(item))) + .to.equal(false, formatAssertionMessage([ + 'Actual error message:', + indentText(errorMessage), + 'Unexpected parts:', + indentText(itemsWithSuccess.map((part) => `- ${part}`).join('\n')), + ])); + }); + }); + }); +}); + +class TestContext { + private logger: Logger = new LoggerStub(); + + private directoryProvider: ApplicationDirectoryProvider = new ApplicationDirectoryProviderStub(); + + private fileSystem: FileSystemOperations = new FileSystemOperationsStub(); + + public withDirectoryProvider(directoryProvider: ApplicationDirectoryProvider): this { + this.directoryProvider = directoryProvider; + return this; + } + + public withFileSystem(fileSystem: FileSystemOperations): this { + this.fileSystem = fileSystem; + return this; + } + + public withLogger(logger: Logger): this { + this.logger = logger; + return this; + } + + public run(): ReturnType { + return clearUpdateInstallationFiles({ + logger: this.logger, + directoryProvider: this.directoryProvider, + fileSystem: this.fileSystem, + }); + } +} diff --git a/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.spec.ts b/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.spec.ts new file mode 100644 index 00000000..ec7be248 --- /dev/null +++ b/tests/unit/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider.spec.ts @@ -0,0 +1,225 @@ +import { it, describe, expect } from 'vitest'; +import type { Logger } from '@/application/Common/Log/Logger'; +import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub'; +import { ApplicationDirectoryProviderStub } from '@tests/unit/shared/Stubs/ApplicationDirectoryProviderStub'; +import type { ApplicationDirectoryProvider } from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import { FileSystemOperationsStub } from '@tests/unit/shared/Stubs/FileSystemOperationsStub'; +import type { FileSystemAccessorWithRetry } from '@/presentation/electron/main/Update/ManualUpdater/FileSystemAccessorWithRetry'; +import { FileSystemAccessorWithRetryStub } from '@tests/unit/shared/Stubs/FileSystemAccessorWithRetryStub'; +import { InstallerFileSuffix, provideUpdateInstallationFilepath } from '@/presentation/electron/main/Update/ManualUpdater/InstallationFiles/InstallationFilepathProvider'; +import { expectThrowsAsync } from '@tests/shared/Assertions/ExpectThrowsAsync'; +import { collectExceptionAsync } from '@tests/unit/shared/ExceptionCollector'; +import { expectExists } from '@tests/shared/Assertions/ExpectExists'; +import { formatAssertionMessage } from '@tests/shared/FormatAssertionMessage'; + +describe('InstallationFilePathProvider', () => { + describe('provideUpdateInstallationFilePath', () => { + it('returns correct filepath', async () => { + // arrange + const version = '1.2.3'; + const baseDirectoryPath = '/updates'; + const pathSegmentSeparator = '/separator/'; + const expectedPath = [ + baseDirectoryPath, pathSegmentSeparator, version, InstallerFileSuffix, + ].join(''); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator); + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', baseDirectoryPath); + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withDirectoryProvider(directoryProviderStub) + .withVersion(version); + // act + const actualPath = await context.run(); + // assert + expect(actualPath).to.equal(expectedPath); + }); + it('checks if file exists at correct path', async () => { + // arrange + const version = '1.2.3'; + const baseDirectoryPath = '/updates'; + const pathSegmentSeparator = '/separator/'; + const expectedPath = [ + baseDirectoryPath, pathSegmentSeparator, version, InstallerFileSuffix, + ].join(''); + const fileSystemStub = new FileSystemOperationsStub() + .withDefaultSeparator(pathSegmentSeparator); + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', baseDirectoryPath); + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withDirectoryProvider(directoryProviderStub) + .withVersion(version); + // act + await context.run(); + // assert + const calls = fileSystemStub.callHistory.filter((c) => c.methodName === 'isFileAvailable'); + expect(calls).to.have.lengthOf(1); + const [actualFilePath] = calls[0].args; + expect(actualFilePath).to.equal(expectedPath); + }); + it('deletes file at correct path', async () => { + // arrange + const version = '1.2.3'; + const baseDirectoryPath = '/updates'; + const pathSegmentSeparator = '/separator/'; + const expectedPath = [ + baseDirectoryPath, pathSegmentSeparator, version, InstallerFileSuffix, + ].join(''); + const isFileAvailable = true; + const fileSystemStub = new FileSystemOperationsStub() + .withFileAvailability(expectedPath, isFileAvailable) + .withDefaultSeparator(pathSegmentSeparator); + const directoryProviderStub = new ApplicationDirectoryProviderStub() + .withDirectoryPath('update-installation-files', baseDirectoryPath); + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withDirectoryProvider(directoryProviderStub) + .withVersion(version); + // act + await context.run(); + // assert + const calls = fileSystemStub.callHistory.filter((c) => c.methodName === 'deletePath'); + expect(calls).to.have.lengthOf(1); + const [deletedFilePath] = calls[0].args; + expect(deletedFilePath).to.equal(expectedPath); + }); + it('deletes existing file', async () => { + // arrange + const isFileAvailable = true; + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => Promise.resolve(isFileAvailable); + const context = new TestContext() + .withFileSystem(fileSystemStub); + // act + await context.run(); + // assert + const calls = fileSystemStub.callHistory.filter((c) => c.methodName === 'deletePath'); + expect(calls).to.have.lengthOf(1); + }); + it('does not attempt to delete non-existent file', async () => { + // arrange + const isFileAvailable = false; + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => Promise.resolve(isFileAvailable); + const context = new TestContext() + .withFileSystem(fileSystemStub); + // act + await context.run(); + // assert + const calls = fileSystemStub.callHistory.filter((c) => c.methodName === 'deletePath'); + expect(calls).to.have.lengthOf(0); + }); + describe('file system error handling', () => { + it('retries on file deletion failure', async () => { + // arrange + const forcedRetries = 2; + const expectedTotalCalls = forcedRetries + 1; + const isFileAvailable = true; + const accessorStub = new FileSystemAccessorWithRetryStub() + .withAlwaysRetry(forcedRetries); + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => Promise.resolve(isFileAvailable); + const context = new TestContext() + .withFileSystem(fileSystemStub) + .withAccessor(accessorStub.get()); + // act + await context.run(); + // assert + const calls = fileSystemStub.callHistory + .filter((c) => c.methodName === 'deletePath'); + expect(calls).to.have.lengthOf(expectedTotalCalls); + }); + }); + describe('error handling', () => { + it('throws when directory provision fails', async () => { + // arrange + const expectedErrorMessage = 'Failed to provide download directory.'; + const directoryProvider = new ApplicationDirectoryProviderStub() + .withFailure(); + const context = new TestContext() + .withDirectoryProvider(directoryProvider); + // act + const act = () => context.run(); + // assert + await expectThrowsAsync(act, expectedErrorMessage); + }); + it('throws on file availability check failure', async () => { + // arrange + const expectedErrorMessage = 'File availability check failed'; + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.isFileAvailable = () => { + return Promise.reject(new Error(expectedErrorMessage)); + }; + const context = new TestContext() + .withFileSystem(fileSystemStub); + // act + const act = () => context.run(); + // assert + await expectThrowsAsync(act, expectedErrorMessage); + }); + it('throws on existing file deletion failure', async () => { + // arrange + const expectedErrorMessagePart = 'Failed to prepare the file path for the installer'; + const fileSystemStub = new FileSystemOperationsStub(); + fileSystemStub.deletePath = () => { + return Promise.reject(new Error('Internal error')); + }; + const context = new TestContext() + .withFileSystem(fileSystemStub); + // act + const act = () => context.run(); + // assert + const error = await collectExceptionAsync(act); + expectExists(error, formatAssertionMessage([ + `File system calls: ${fileSystemStub.methodCalls}`, + ])); + expect(error.message).to.include(expectedErrorMessagePart); + }); + }); + }); +}); + +class TestContext { + private version = '3.5.5'; + + private logger: Logger = new LoggerStub(); + + private directoryProvider + : ApplicationDirectoryProvider = new ApplicationDirectoryProviderStub(); + + private fileSystem: FileSystemOperations = new FileSystemOperationsStub(); + + private accessor: FileSystemAccessorWithRetry = new FileSystemAccessorWithRetryStub().get(); + + public withVersion(version: string): this { + this.version = version; + return this; + } + + public withAccessor(accessor: FileSystemAccessorWithRetry): this { + this.accessor = accessor; + return this; + } + + public withDirectoryProvider(directoryProvider: ApplicationDirectoryProvider): this { + this.directoryProvider = directoryProvider; + return this; + } + + public withFileSystem(fileSystem: FileSystemOperations): this { + this.fileSystem = fileSystem; + return this; + } + + public run() { + return provideUpdateInstallationFilepath(this.version, { + logger: this.logger, + directoryProvider: this.directoryProvider, + fileSystem: this.fileSystem, + accessFileSystemWithRetry: this.accessor, + }); + } +} diff --git a/tests/unit/shared/ExceptionCollector.ts b/tests/unit/shared/ExceptionCollector.ts index f25feba0..fd9d1c66 100644 --- a/tests/unit/shared/ExceptionCollector.ts +++ b/tests/unit/shared/ExceptionCollector.ts @@ -12,7 +12,19 @@ function collectException( error = err; } if (!error) { - throw new Error('action did not throw'); + throw new Error('Action did not throw'); + } + return error; +} + +export async function collectExceptionAsync( + action: () => Promise, +): Promise { + let error: Error | undefined; + try { + await action(); + } catch (err) { + error = err; } return error; } diff --git a/tests/unit/shared/Stubs/ApplicationDirectoryProviderStub.ts b/tests/unit/shared/Stubs/ApplicationDirectoryProviderStub.ts new file mode 100644 index 00000000..cb9e2471 --- /dev/null +++ b/tests/unit/shared/Stubs/ApplicationDirectoryProviderStub.ts @@ -0,0 +1,41 @@ +import type { + DirectoryCreationOutcome, + ApplicationDirectoryProvider, + DirectoryType, + DirectoryCreationError, +} from '@/infrastructure/FileSystem/Directory/ApplicationDirectoryProvider'; + +export class ApplicationDirectoryProviderStub implements ApplicationDirectoryProvider { + private directoryPaths: Record = { + 'update-installation-files': `[${ApplicationDirectoryProviderStub.name}]update installation files directory`, + 'script-runs': `[${ApplicationDirectoryProviderStub.name}]scripts directory`, + }; + + private failure: DirectoryCreationError | undefined = undefined; + + public withDirectoryPath(type: DirectoryType, directoryPath: string): this { + this.directoryPaths[type] = directoryPath; + return this; + } + + public provideDirectory(type: DirectoryType): Promise { + if (this.failure) { + return Promise.resolve({ + success: false, + error: this.failure, + }); + } + return Promise.resolve({ + success: true, + directoryAbsolutePath: this.directoryPaths[type], + }); + } + + public withFailure(error?: DirectoryCreationError): this { + this.failure = error ?? { + type: 'DirectoryWriteError', + message: `[${ApplicationDirectoryProviderStub.name}]injected failure`, + }; + return this; + } +} diff --git a/tests/unit/shared/Stubs/FileSystemAccessorWithRetryStub.ts b/tests/unit/shared/Stubs/FileSystemAccessorWithRetryStub.ts new file mode 100644 index 00000000..52fddba8 --- /dev/null +++ b/tests/unit/shared/Stubs/FileSystemAccessorWithRetryStub.ts @@ -0,0 +1,21 @@ +import type { FileSystemAccessorWithRetry } from '@/presentation/electron/main/Update/ManualUpdater/FileSystemAccessorWithRetry'; + +export class FileSystemAccessorWithRetryStub { + private retryAmount = 0; + + public withAlwaysRetry(retryAmount: number): this { + this.retryAmount = retryAmount; + return this; + } + + public get(): FileSystemAccessorWithRetry { + return async (fileOperation) => { + const result = await fileOperation(); + for (let i = 0; i < this.retryAmount; i++) { + // eslint-disable-next-line no-await-in-loop + await fileOperation(); + } + return result; + }; + } +} diff --git a/tests/unit/shared/Stubs/FileSystemOperationsStub.ts b/tests/unit/shared/Stubs/FileSystemOperationsStub.ts new file mode 100644 index 00000000..37740ea6 --- /dev/null +++ b/tests/unit/shared/Stubs/FileSystemOperationsStub.ts @@ -0,0 +1,161 @@ +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; +import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; + +export class FileSystemOperationsStub + extends StubWithObservableMethodCalls + implements FileSystemOperations { + private readonly writtenFiles: Map = new Map(); + + private readonly fileAvailability: Map = new Map(); + + private directoryContents: Map = new Map(); + + private userDataDirectory = `/${FileSystemOperationsStub.name}-user-data-dir/`; + + private combinePathSequence = new Array(); + + private combinePathScenarios = new Map(); + + private combinePathDefaultSeparator = `/[${FileSystemOperationsStub.name}]PATH-SEGMENT-SEPARATOR/`; + + public setFilePermissions(filePath: string, mode: string | number): Promise { + this.registerMethodCall({ + methodName: 'setFilePermissions', + args: [filePath, mode], + }); + return Promise.resolve(); + } + + public writeFile = (filePath: string, fileContents: string, encoding: NodeJS.BufferEncoding) => { + this.registerMethodCall({ + methodName: 'writeFile', + args: [filePath, fileContents, encoding], + }); + this.writtenFiles.set(filePath, fileContents); + return Promise.resolve(); + }; + + public readFile = (filePath: string, encoding: NodeJS.BufferEncoding) => { + this.registerMethodCall({ + methodName: 'readFile', + args: [filePath, encoding], + }); + const fileContents = this.writtenFiles.get(filePath); + return Promise.resolve(fileContents ?? `[${FileSystemOperationsStub.name}] file-contents`); + }; + + public createDirectory(directoryPath: string, isRecursive?: boolean): Promise { + this.registerMethodCall({ + methodName: 'createDirectory', + args: [directoryPath, isRecursive], + }); + return Promise.resolve(); + } + + public isFileAvailable(filePath: string): Promise { + this.registerMethodCall({ + methodName: 'isFileAvailable', + args: [filePath], + }); + const availability = this.fileAvailability.get(filePath); + if (availability !== undefined) { + return Promise.resolve(availability); + } + const fileContents = this.writtenFiles.get(filePath); + if (fileContents !== undefined) { + return Promise.resolve(true); + } + return Promise.resolve(true); + } + + public isDirectoryAvailable(directoryPath: string): Promise { + this.registerMethodCall({ + methodName: 'isDirectoryAvailable', + args: [directoryPath], + }); + return Promise.resolve(true); + } + + public deletePath(filePath: string): Promise { + this.registerMethodCall({ + methodName: 'deletePath', + args: [filePath], + }); + return Promise.resolve(); + } + + public withUserDirectoryResult(directory: string): this { + this.userDataDirectory = directory; + return this; + } + + public getUserDataDirectory(): string { + this.registerMethodCall({ + methodName: 'getUserDataDirectory', + args: [], + }); + return this.userDataDirectory; + } + + public listDirectoryContents(directoryPath: string): Promise { + this.registerMethodCall({ + methodName: 'listDirectoryContents', + args: [directoryPath], + }); + const contents = this.directoryContents.get(directoryPath); + return Promise.resolve(contents ?? []); + } + + public withDirectoryContents( + directoryPath: string, + fileOrFolderNames: readonly string[], + ): this { + this.directoryContents.set(directoryPath, [...fileOrFolderNames]); + return this; + } + + public withFileAvailability( + filePath: string, + isAvailable: boolean, + ): this { + this.fileAvailability.set(filePath, isAvailable); + return this; + } + + public withJoinResult(returnValue: string, ...paths: string[]): this { + this.combinePathScenarios.set(getCombinePathsScenarioKey(paths), returnValue); + return this; + } + + public withJoinResultSequence(...valuesToReturn: string[]): this { + this.combinePathSequence.push(...valuesToReturn); + this.combinePathSequence.reverse(); + return this; + } + + public withDefaultSeparator(defaultSeparator: string): this { + this.combinePathDefaultSeparator = defaultSeparator; + return this; + } + + public combinePaths(...pathSegments: string[]): string { + this.registerMethodCall({ + methodName: 'combinePaths', + args: pathSegments, + }); + const nextInSequence = this.combinePathSequence.pop(); + if (nextInSequence) { + return nextInSequence; + } + const key = getCombinePathsScenarioKey(pathSegments); + const foundScenario = this.combinePathScenarios.get(key); + if (foundScenario) { + return foundScenario; + } + return pathSegments.join(this.combinePathDefaultSeparator); + } +} + +function getCombinePathsScenarioKey(paths: string[]): string { + return paths.join('|'); +} diff --git a/tests/unit/shared/Stubs/FileSystemOpsStub.ts b/tests/unit/shared/Stubs/FileSystemOpsStub.ts deleted file mode 100644 index 464b310b..00000000 --- a/tests/unit/shared/Stubs/FileSystemOpsStub.ts +++ /dev/null @@ -1,22 +0,0 @@ -import type { FileSystemOps } from '@/infrastructure/CodeRunner/System/SystemOperations'; -import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; - -export class FileSystemOpsStub - extends StubWithObservableMethodCalls - implements FileSystemOps { - public setFilePermissions(filePath: string, mode: string | number): Promise { - this.registerMethodCall({ - methodName: 'setFilePermissions', - args: [filePath, mode], - }); - return Promise.resolve(); - } - - public createDirectory(directoryPath: string, isRecursive?: boolean): Promise { - this.registerMethodCall({ - methodName: 'createDirectory', - args: [directoryPath, isRecursive], - }); - return Promise.resolve(); - } -} diff --git a/tests/unit/shared/Stubs/LocationOpsStub.ts b/tests/unit/shared/Stubs/LocationOpsStub.ts deleted file mode 100644 index 7edd8e5a..00000000 --- a/tests/unit/shared/Stubs/LocationOpsStub.ts +++ /dev/null @@ -1,49 +0,0 @@ -import type { LocationOps } from '@/infrastructure/CodeRunner/System/SystemOperations'; -import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; - -export class LocationOpsStub - extends StubWithObservableMethodCalls - implements LocationOps { - private sequence = new Array(); - - private scenarios = new Map(); - - private defaultSeparator = `/[${LocationOpsStub.name}]PATH-SEGMENT-SEPARATOR/`; - - public withJoinResult(returnValue: string, ...paths: string[]): this { - this.scenarios.set(LocationOpsStub.getScenarioKey(paths), returnValue); - return this; - } - - public withJoinResultSequence(...valuesToReturn: string[]): this { - this.sequence.push(...valuesToReturn); - this.sequence.reverse(); - return this; - } - - public withDefaultSeparator(defaultSeparator: string): this { - this.defaultSeparator = defaultSeparator; - return this; - } - - public combinePaths(...pathSegments: string[]): string { - this.registerMethodCall({ - methodName: 'combinePaths', - args: pathSegments, - }); - const nextInSequence = this.sequence.pop(); - if (nextInSequence) { - return nextInSequence; - } - const key = LocationOpsStub.getScenarioKey(pathSegments); - const foundScenario = this.scenarios.get(key); - if (foundScenario) { - return foundScenario; - } - return pathSegments.join(this.defaultSeparator); - } - - private static getScenarioKey(paths: string[]): string { - return paths.join('|'); - } -} diff --git a/tests/unit/shared/Stubs/OperatingSystemOpsStub.ts b/tests/unit/shared/Stubs/OperatingSystemOpsStub.ts deleted file mode 100644 index f3664f6d..00000000 --- a/tests/unit/shared/Stubs/OperatingSystemOpsStub.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { OperatingSystemOps } from '@/infrastructure/CodeRunner/System/SystemOperations'; -import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; - -export class OperatingSystemOpsStub - extends StubWithObservableMethodCalls - implements OperatingSystemOps { - private userDataDirectory = `/${OperatingSystemOpsStub.name}-user-data-dir/`; - - public withUserDirectoryResult(directory: string): this { - this.userDataDirectory = directory; - return this; - } - - public getUserDataDirectory(): string { - this.registerMethodCall({ - methodName: 'getUserDataDirectory', - args: [], - }); - return this.userDataDirectory; - } -} diff --git a/tests/unit/shared/Stubs/ReadbackFileWriterStub.ts b/tests/unit/shared/Stubs/ReadbackFileWriterStub.ts index 89cd2e38..ed7bb202 100644 --- a/tests/unit/shared/Stubs/ReadbackFileWriterStub.ts +++ b/tests/unit/shared/Stubs/ReadbackFileWriterStub.ts @@ -1,4 +1,4 @@ -import type { FileWriteErrorType, FileWriteOutcome, ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter'; +import type { FileWriteErrorType, FileWriteOutcome, ReadbackFileWriter } from '@/infrastructure/FileSystem/ReadbackFileWriter/ReadbackFileWriter'; import { StubWithObservableMethodCalls } from './StubWithObservableMethodCalls'; export class ReadbackFileWriterStub diff --git a/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts b/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts deleted file mode 100644 index f63704c0..00000000 --- a/tests/unit/shared/Stubs/ScriptDirectoryProviderStub.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { ScriptDirectoryOutcome, ScriptDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/ScriptDirectoryProvider'; - -export class ScriptDirectoryProviderStub implements ScriptDirectoryProvider { - private directoryPath = `[${ScriptDirectoryProviderStub.name}]scriptDirectory`; - - public withDirectoryPath(directoryPath: string): this { - this.directoryPath = directoryPath; - return this; - } - - public provideScriptDirectory(): Promise { - return Promise.resolve({ - success: true, - directoryAbsolutePath: this.directoryPath, - }); - } -} diff --git a/tests/unit/shared/Stubs/SystemOperationsStub.ts b/tests/unit/shared/Stubs/SystemOperationsStub.ts index 446f0733..5084c4e1 100644 --- a/tests/unit/shared/Stubs/SystemOperationsStub.ts +++ b/tests/unit/shared/Stubs/SystemOperationsStub.ts @@ -1,35 +1,17 @@ import type { CommandOps, - FileSystemOps, - OperatingSystemOps, - LocationOps, SystemOperations, } from '@/infrastructure/CodeRunner/System/SystemOperations'; +import type { FileSystemOperations } from '@/infrastructure/FileSystem/FileSystemOperations'; import { CommandOpsStub } from './CommandOpsStub'; -import { FileSystemOpsStub } from './FileSystemOpsStub'; -import { LocationOpsStub } from './LocationOpsStub'; -import { OperatingSystemOpsStub } from './OperatingSystemOpsStub'; +import { FileSystemOperationsStub } from './FileSystemOperationsStub'; export class SystemOperationsStub implements SystemOperations { - public operatingSystem: OperatingSystemOps = new OperatingSystemOpsStub(); - - public location: LocationOps = new LocationOpsStub(); - - public fileSystem: FileSystemOps = new FileSystemOpsStub(); + public fileSystem: FileSystemOperations = new FileSystemOperationsStub(); public command: CommandOps = new CommandOpsStub(); - public withOperatingSystem(operatingSystemOps: OperatingSystemOps): this { - this.operatingSystem = operatingSystemOps; - return this; - } - - public withLocation(location: LocationOps): this { - this.location = location; - return this; - } - - public withFileSystem(fileSystem: FileSystemOps): this { + public withFileSystem(fileSystem: FileSystemOperations): this { this.fileSystem = fileSystem; return this; }