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
This commit is contained in:
undergroundwires
2024-10-07 17:33:47 +02:00
parent 4e06d543b3
commit 2f31bc7b06
44 changed files with 1484 additions and 590 deletions

View File

@@ -1,211 +0,0 @@
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';
describe('PersistentDirectoryProvider', () => {
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()
.withDefaultSeparator(pathSegmentSeparator);
const context = new PersistentDirectoryProviderTestSetup()
.withSystem(new SystemOperationsStub()
.withOperatingSystem(new OperatingSystemOpsStub()
.withUserDirectoryResult(expectedBaseDirectory))
.withLocation(locationOps));
// act
const { success, directoryAbsolutePath } = await context.provideScriptDirectory();
// assert
expectTrue(success);
const actualBaseDirectory = directoryAbsolutePath.split(pathSegmentSeparator)[0];
expect(actualBaseDirectory).to.equal(expectedBaseDirectory);
const calls = locationOps.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);
});
it('forms full path correctly', async () => {
// arrange
const pathSegmentSeparator = '/';
const baseDirectory = 'base-directory';
const expectedDirectory = [baseDirectory, ExecutionSubdirectory].join(pathSegmentSeparator);
const context = new PersistentDirectoryProviderTestSetup()
.withSystem(new SystemOperationsStub()
.withLocation(new LocationOpsStub().withDefaultSeparator(pathSegmentSeparator))
.withOperatingSystem(
new OperatingSystemOpsStub().withUserDirectoryResult(baseDirectory),
));
// act
const { success, directoryAbsolutePath } = await context.provideScriptDirectory();
// assert
expectTrue(success);
expect(directoryAbsolutePath).to.equal(expectedDirectory);
});
});
describe('directory creation', () => {
it('creates directory with recursion', async () => {
// arrange
const expectedIsRecursive = true;
const filesystem = new FileSystemOpsStub();
const context = new PersistentDirectoryProviderTestSetup()
.withSystem(new SystemOperationsStub().withFileSystem(filesystem));
// act
const { success, directoryAbsolutePath } = await context.provideScriptDirectory();
// assert
expectTrue(success);
const calls = filesystem.callHistory.filter((call) => call.methodName === 'createDirectory');
expect(calls.length).to.equal(1);
const [actualPath, actualIsRecursive] = calls[0].args;
expect(actualPath).to.equal(directoryAbsolutePath);
expect(actualIsRecursive).to.equal(expectedIsRecursive);
});
});
describe('error handling', () => {
const testScenarios: ReadonlyArray<{
readonly description: string;
readonly expectedErrorType: CodeRunErrorType;
readonly expectedErrorMessage: string;
buildFaultyContext(
setup: PersistentDirectoryProviderTestSetup,
errorMessage: string,
): PersistentDirectoryProviderTestSetup;
}> = [
{
description: 'path combination failure',
expectedErrorType: 'DirectoryCreationError',
expectedErrorMessage: 'Error when combining paths',
buildFaultyContext: (setup, errorMessage) => {
const locationStub = new LocationOpsStub();
locationStub.combinePaths = () => {
throw new Error(errorMessage);
};
return setup.withSystem(new SystemOperationsStub().withLocation(locationStub));
},
},
{
description: 'user data retrieval failure',
expectedErrorType: 'DirectoryCreationError',
expectedErrorMessage: 'Error when locating user data directory',
buildFaultyContext: (setup, errorMessage) => {
const operatingSystemStub = new OperatingSystemOpsStub();
operatingSystemStub.getUserDataDirectory = () => {
throw new Error(errorMessage);
};
return setup.withSystem(
new SystemOperationsStub().withOperatingSystem(operatingSystemStub),
);
},
},
{
description: 'directory creation failure',
expectedErrorType: 'DirectoryCreationError',
expectedErrorMessage: 'Error when creating directory',
buildFaultyContext: (setup, errorMessage) => {
const fileSystemStub = new FileSystemOpsStub();
fileSystemStub.createDirectory = () => {
throw new Error(errorMessage);
};
return setup.withSystem(new SystemOperationsStub().withFileSystem(fileSystemStub));
},
},
];
testScenarios.forEach(({
description, expectedErrorType, expectedErrorMessage, buildFaultyContext,
}) => {
it(`handles error - ${description}`, async () => {
// arrange
const context = buildFaultyContext(
new PersistentDirectoryProviderTestSetup(),
expectedErrorMessage,
);
// act
const { success, error } = await context.provideScriptDirectory();
// assert
expect(success).to.equal(false);
expectExists(error);
expect(error.message).to.include(expectedErrorMessage);
expect(error.type).to.equal(expectedErrorType);
});
it(`logs error - ${description}`, async () => {
// arrange
const loggerStub = new LoggerStub();
const context = buildFaultyContext(
new PersistentDirectoryProviderTestSetup()
.withLogger(loggerStub),
expectedErrorMessage,
);
// act
await context.provideScriptDirectory();
// assert
loggerStub.assertLogsContainMessagePart('error', expectedErrorMessage);
});
});
});
});
});
class PersistentDirectoryProviderTestSetup {
private system: SystemOperations = new SystemOperationsStub();
private logger: Logger = new LoggerStub();
public withSystem(system: SystemOperations): this {
this.system = system;
return this;
}
public withLogger(logger: Logger): this {
this.logger = logger;
return this;
}
public provideScriptDirectory(): ReturnType<PersistentDirectoryProvider['provideScriptDirectory']> {
const provider = new PersistentDirectoryProvider(this.system, this.logger);
return provider.provideScriptDirectory();
}
}

View File

@@ -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<DirectoryCreationErrorType, {
readonly directoryErrorMessage: string;
}> = {
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<ScriptFileCreationOrchestrator['createScriptFile']> {
const creator = new ScriptFileCreationOrchestrator(
this.system,
this.fileSystem,
this.filenameGenerator,
this.directoryProvider,
this.fileWriter,

View File

@@ -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)