Skip to content

Commit 13ab2a9

Browse files
committed
fix: rework file handling so we always explicitly create, overwrite or create/overwrite
1 parent 94fc1f2 commit 13ab2a9

File tree

7 files changed

+42
-15
lines changed

7 files changed

+42
-15
lines changed

Diff for: server/src/interfaces/storage.interface.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ export interface IStorageRepository {
3535
createZipStream(): ImmichZipStream;
3636
createReadStream(filepath: string, mimeType?: string | null): Promise<ImmichReadStream>;
3737
readFile(filepath: string, options?: FileReadOptions<Buffer>): Promise<Buffer>;
38-
writeFile(filepath: string, buffer: Buffer): Promise<void>;
38+
createFile(filepath: string, buffer: Buffer): Promise<void>;
39+
createOrOverwriteFile(filepath: string, buffer: Buffer): Promise<void>;
40+
overwriteFile(filepath: string, buffer: Buffer): Promise<void>;
3941
realpath(filepath: string): Promise<string>;
4042
unlink(filepath: string): Promise<void>;
4143
unlinkDir(folder: string, options?: { recursive?: boolean; force?: boolean }): Promise<void>;

Diff for: server/src/repositories/storage.repository.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,16 @@ export class StorageRepository implements IStorageRepository {
4040
return fs.stat(filepath);
4141
}
4242

43-
writeFile(filepath: string, buffer: Buffer) {
44-
return fs.writeFile(filepath, buffer);
43+
createFile(filepath: string, buffer: Buffer) {
44+
return fs.writeFile(filepath, buffer, { flag: 'wx' });
45+
}
46+
47+
createOrOverwriteFile(filepath: string, buffer: Buffer) {
48+
return fs.writeFile(filepath, buffer, { flag: 'w' });
49+
}
50+
51+
overwriteFile(filepath: string, buffer: Buffer) {
52+
return fs.writeFile(filepath, buffer, { flag: 'r+' });
4553
}
4654

4755
rename(source: string, target: string) {

Diff for: server/src/services/metadata.service.spec.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ describe(MetadataService.name, () => {
511511

512512
await sut.handleMetadataExtraction({ id: assetStub.livePhotoMotionAsset.id });
513513
expect(assetMock.getByIds).toHaveBeenCalledWith([assetStub.livePhotoMotionAsset.id]);
514-
expect(storageMock.writeFile).not.toHaveBeenCalled();
514+
expect(storageMock.createOrOverwriteFile).not.toHaveBeenCalled();
515515
expect(jobMock.queue).not.toHaveBeenCalled();
516516
expect(jobMock.queueAll).not.toHaveBeenCalled();
517517
expect(assetMock.update).not.toHaveBeenCalledWith(
@@ -581,7 +581,7 @@ describe(MetadataService.name, () => {
581581
type: AssetType.VIDEO,
582582
});
583583
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
584-
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
584+
expect(storageMock.createFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
585585
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
586586
id: assetStub.livePhotoWithOriginalFileName.id,
587587
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
@@ -624,7 +624,7 @@ describe(MetadataService.name, () => {
624624
type: AssetType.VIDEO,
625625
});
626626
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
627-
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
627+
expect(storageMock.createFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
628628
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
629629
id: assetStub.livePhotoWithOriginalFileName.id,
630630
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
@@ -668,7 +668,7 @@ describe(MetadataService.name, () => {
668668
type: AssetType.VIDEO,
669669
});
670670
expect(userMock.updateUsage).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.ownerId, 512);
671-
expect(storageMock.writeFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
671+
expect(storageMock.createFile).toHaveBeenCalledWith(assetStub.livePhotoMotionAsset.originalPath, video);
672672
expect(assetMock.update).toHaveBeenNthCalledWith(1, {
673673
id: assetStub.livePhotoWithOriginalFileName.id,
674674
livePhotoVideoId: fileStub.livePhotoMotion.uuid,
@@ -716,7 +716,7 @@ describe(MetadataService.name, () => {
716716

717717
await sut.handleMetadataExtraction({ id: assetStub.livePhotoStillAsset.id });
718718
expect(assetMock.create).toHaveBeenCalledTimes(0);
719-
expect(storageMock.writeFile).toHaveBeenCalledTimes(0);
719+
expect(storageMock.createOrOverwriteFile).toHaveBeenCalledTimes(0);
720720
// The still asset gets saved by handleMetadataExtraction, but not the video
721721
expect(assetMock.update).toHaveBeenCalledTimes(1);
722722
expect(jobMock.queue).toHaveBeenCalledTimes(0);

Diff for: server/src/services/metadata.service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ export class MetadataService {
529529
const existsOnDisk = await this.storageRepository.checkFileExists(motionAsset.originalPath);
530530
if (!existsOnDisk) {
531531
this.storageCore.ensureFolders(motionAsset.originalPath);
532-
await this.storageRepository.writeFile(motionAsset.originalPath, video);
532+
await this.storageRepository.createFile(motionAsset.originalPath, video);
533533
this.logger.log(`Wrote motion photo video to ${motionAsset.originalPath}`);
534534
await this.jobRepository.queue({ name: JobName.METADATA_EXTRACTION, data: { id: motionAsset.id } });
535535
}

Diff for: server/src/services/storage.service.spec.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,15 @@ describe(StorageService.name, () => {
4949

5050
await expect(sut.onBootstrap()).rejects.toThrow('Failed to validate folder mount');
5151

52-
expect(storageMock.writeFile).not.toHaveBeenCalled();
52+
expect(storageMock.createOrOverwriteFile).not.toHaveBeenCalled();
5353
expect(systemMock.set).not.toHaveBeenCalled();
5454
});
5555

5656
it('should throw an error if .immich is present but read-only', async () => {
5757
systemMock.get.mockResolvedValue({ mountFiles: true });
58-
storageMock.writeFile.mockRejectedValue(new Error("ENOENT: no such file or directory, open '/app/.immich'"));
58+
storageMock.createOrOverwriteFile.mockRejectedValue(
59+
new Error("ENOENT: no such file or directory, open '/app/.immich'"),
60+
);
5961

6062
await expect(sut.onBootstrap()).rejects.toThrow('Failed to validate folder mount');
6163

Diff for: server/src/services/storage.service.ts

+16-3
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class StorageService {
3232
for (const folder of Object.values(StorageFolder)) {
3333
if (!flags.mountFiles) {
3434
this.logger.log(`Writing initial mount file for the ${folder} folder`);
35-
await this.verifyWriteAccess(folder);
35+
await this.createMountFile(folder);
3636
}
3737

3838
await this.verifyReadAccess(folder);
@@ -81,11 +81,24 @@ export class StorageService {
8181
}
8282
}
8383

84-
private async verifyWriteAccess(folder: StorageFolder) {
84+
private async createMountFile(folder: StorageFolder) {
8585
const { folderPath, filePath } = this.getMountFilePaths(folder);
8686
try {
8787
this.storageRepository.mkdirSync(folderPath);
88-
await this.storageRepository.writeFile(filePath, Buffer.from(`${Date.now()}`));
88+
await this.storageRepository.createFile(filePath, Buffer.from(`${Date.now()}`));
89+
} catch (error) {
90+
this.logger.error(`Failed to create ${filePath}: ${error}`);
91+
this.logger.error(
92+
`The "${folder}" folder cannot be written to, please make sure the volume is mounted with the correct permissions`,
93+
);
94+
throw new ImmichStartupError(`Failed to validate folder mount (write to "<MEDIA_LOCATION>/${folder}")`);
95+
}
96+
}
97+
98+
private async verifyWriteAccess(folder: StorageFolder) {
99+
const { filePath } = this.getMountFilePaths(folder);
100+
try {
101+
await this.storageRepository.overwriteFile(filePath, Buffer.from(`${Date.now()}`));
89102
} catch (error) {
90103
this.logger.error(`Failed to write ${filePath}: ${error}`);
91104
this.logger.error(

Diff for: server/test/repositories/storage.repository.mock.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ export const newStorageRepositoryMock = (reset = true): Mocked<IStorageRepositor
4848
createZipStream: vitest.fn(),
4949
createReadStream: vitest.fn(),
5050
readFile: vitest.fn(),
51-
writeFile: vitest.fn(),
51+
createFile: vitest.fn(),
52+
createOrOverwriteFile: vitest.fn(),
53+
overwriteFile: vitest.fn(),
5254
unlink: vitest.fn(),
5355
unlinkDir: vitest.fn().mockResolvedValue(true),
5456
removeEmptyDirs: vitest.fn(),

0 commit comments

Comments
 (0)