Skip to content

Commit

Permalink
Fix file retention after updates on macOS $417
Browse files Browse the repository at this point in the history
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
  • Loading branch information
undergroundwires committed Oct 3, 2024
1 parent 4e06d54 commit 564627e
Show file tree
Hide file tree
Showing 41 changed files with 1,383 additions and 537 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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,
) { }
Expand All @@ -26,9 +27,12 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
): Promise<ScriptFileCreationOutcome> {
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,
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
};
}
},
};
18 changes: 2 additions & 16 deletions src/infrastructure/CodeRunner/System/SystemOperations.ts
Original file line number Diff line number Diff line change
@@ -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<typeof exec>;
}

export interface FileSystemOps {
setFilePermissions(filePath: string, mode: string | number): Promise<void>;
createDirectory(directoryPath: string, isRecursive?: boolean): Promise<void>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
export interface ApplicationDirectoryProvider {
provideDirectory(type: DirectoryType): Promise<DirectoryCreationOutcome>;
}

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;
}
Original file line number Diff line number Diff line change
@@ -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<DirectoryType, string> = {
'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<ScriptDirectoryOutcome> {
public async provideDirectory(type: DirectoryType): Promise<DirectoryCreationOutcome> {
const {
success: isPathConstructed,
error: pathConstructionError,
directoryPath,
} = this.constructScriptDirectoryPath();
} = this.constructScriptDirectoryPath(type);
if (!isPathConstructed) {
return {
success: false,
Expand All @@ -52,25 +57,34 @@ export class PersistentDirectoryProvider implements ScriptDirectoryProvider {
private async createDirectory(directoryPath: string): Promise<DirectoryPathCreationOutcome> {
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,
};
} 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,
Expand All @@ -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 {
Expand All @@ -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;
Expand All @@ -109,7 +123,7 @@ type DirectoryPathConstructionOutcome = {

type DirectoryPathCreationOutcome = {
readonly success: false;
readonly error: CodeRunError;
readonly error: DirectoryCreationError;
} | {
readonly success: true;
readonly error?: undefined;
Expand Down
20 changes: 20 additions & 0 deletions src/infrastructure/FileSystem/FileSystemOperations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export interface FileSystemOperations {
getUserDataDirectory(): string;

setFilePermissions(filePath: string, mode: string | number): Promise<void>;
createDirectory(directoryPath: string, isRecursive?: boolean): Promise<void>;

isFileAvailable(filePath: string): Promise<boolean>;
isDirectoryAvailable(filePath: string): Promise<boolean>;
deletePath(filePath: string): Promise<void>;
listDirectoryContents(directoryPath: string): Promise<string[]>;

combinePaths(...pathSegments: string[]): string;

readFile: (filePath: string, encoding: NodeJS.BufferEncoding) => Promise<string>;
writeFile: (
filePath: string,
fileContents: string,
encoding: NodeJS.BufferEncoding,
) => Promise<void>;
}
Loading

0 comments on commit 564627e

Please sign in to comment.