From 2deb64486bbd44e3ba5a3bca2cd4ac867374aee9 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 16 Oct 2024 11:52:22 -0400 Subject: [PATCH 1/4] use async IO for InternalStorageService --- packages/backend/src/core/DriveService.ts | 32 ++++++++++--------- .../src/core/InternalStorageService.ts | 17 +++++----- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts index 744bc65be7..781b435c9a 100644 --- a/packages/backend/src/core/DriveService.ts +++ b/packages/backend/src/core/DriveService.ts @@ -227,25 +227,27 @@ export class DriveService { const thumbnailAccessKey = 'thumbnail-' + randomUUID(); const webpublicAccessKey = 'webpublic-' + randomUUID(); - const url = this.internalStorageService.saveFromPath(accessKey, path); - - let thumbnailUrl: string | null = null; - let webpublicUrl: string | null = null; + // Ugly type is just to help TS figure out that 2nd / 3rd promises are optional. + const promises: [Promise, ...(Promise | undefined)[]] = [ + this.internalStorageService.saveFromPath(accessKey, path), + ]; if (alts.thumbnail) { - thumbnailUrl = this.internalStorageService.saveFromBuffer(thumbnailAccessKey, alts.thumbnail.data); + promises.push(this.internalStorageService.saveFromBuffer(thumbnailAccessKey, alts.thumbnail.data)); this.registerLogger.info(`thumbnail stored: ${thumbnailAccessKey}`); } if (alts.webpublic) { - webpublicUrl = this.internalStorageService.saveFromBuffer(webpublicAccessKey, alts.webpublic.data); + promises.push(this.internalStorageService.saveFromBuffer(webpublicAccessKey, alts.webpublic.data)); this.registerLogger.info(`web stored: ${webpublicAccessKey}`); } + const [url, thumbnailUrl, webpublicUrl] = await Promise.all(promises); + file.storedInternal = true; file.url = url; - file.thumbnailUrl = thumbnailUrl; - file.webpublicUrl = webpublicUrl; + file.thumbnailUrl = thumbnailUrl ?? null; + file.webpublicUrl = webpublicUrl ?? null; file.accessKey = accessKey; file.thumbnailAccessKey = thumbnailAccessKey; file.webpublicAccessKey = webpublicAccessKey; @@ -720,19 +722,19 @@ export class DriveService { @bindThis public async deleteFileSync(file: MiDriveFile, isExpired = false, deleter?: MiUser) { + const promises = []; + if (file.storedInternal) { - this.internalStorageService.del(file.accessKey!); + promises.push(this.internalStorageService.del(file.accessKey!)); if (file.thumbnailUrl) { - this.internalStorageService.del(file.thumbnailAccessKey!); + promises.push(this.internalStorageService.del(file.thumbnailAccessKey!)); } if (file.webpublicUrl) { - this.internalStorageService.del(file.webpublicAccessKey!); + promises.push(this.internalStorageService.del(file.webpublicAccessKey!)); } } else if (!file.isLink) { - const promises = []; - promises.push(this.deleteObjectStorageFile(file.accessKey!)); if (file.thumbnailUrl) { @@ -742,10 +744,10 @@ export class DriveService { if (file.webpublicUrl) { promises.push(this.deleteObjectStorageFile(file.webpublicAccessKey!)); } - - await Promise.all(promises); } + await Promise.all(promises); + this.deletePostProcess(file, isExpired, deleter); } diff --git a/packages/backend/src/core/InternalStorageService.ts b/packages/backend/src/core/InternalStorageService.ts index 4fb8a93e49..02a0a745d6 100644 --- a/packages/backend/src/core/InternalStorageService.ts +++ b/packages/backend/src/core/InternalStorageService.ts @@ -4,6 +4,7 @@ */ import * as fs from 'node:fs'; +import { copyFile, mkdir, unlink, writeFile } from 'node:fs/promises'; import * as Path from 'node:path'; import { fileURLToPath } from 'node:url'; import { dirname } from 'node:path'; @@ -36,21 +37,21 @@ export class InternalStorageService { } @bindThis - public saveFromPath(key: string, srcPath: string) { - fs.mkdirSync(path, { recursive: true }); - fs.copyFileSync(srcPath, this.resolvePath(key)); + public async saveFromPath(key: string, srcPath: string): Promise { + await mkdir(path, { recursive: true }); + await copyFile(srcPath, this.resolvePath(key)); return `${this.config.url}/files/${key}`; } @bindThis - public saveFromBuffer(key: string, data: Buffer) { - fs.mkdirSync(path, { recursive: true }); - fs.writeFileSync(this.resolvePath(key), data); + public async saveFromBuffer(key: string, data: Buffer): Promise { + await mkdir(path, { recursive: true }); + await writeFile(this.resolvePath(key), data); return `${this.config.url}/files/${key}`; } @bindThis - public del(key: string) { - fs.unlink(this.resolvePath(key), () => {}); + public async del(key: string): Promise { + await unlink(this.resolvePath(key)); } } From b1d9314d6ef7ff58698a1589d5c6e09b08b89801 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Wed, 16 Oct 2024 11:54:06 -0400 Subject: [PATCH 2/4] pre-create the `files` directory to reduce IO operations --- packages/backend/src/core/InternalStorageService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/InternalStorageService.ts b/packages/backend/src/core/InternalStorageService.ts index 02a0a745d6..7d2ea6ba20 100644 --- a/packages/backend/src/core/InternalStorageService.ts +++ b/packages/backend/src/core/InternalStorageService.ts @@ -24,6 +24,8 @@ export class InternalStorageService { @Inject(DI.config) private config: Config, ) { + // No one should erase the working directly *while the server is running*. + fs.mkdirSync(path, { recursive: true }); } @bindThis @@ -38,14 +40,12 @@ export class InternalStorageService { @bindThis public async saveFromPath(key: string, srcPath: string): Promise { - await mkdir(path, { recursive: true }); await copyFile(srcPath, this.resolvePath(key)); return `${this.config.url}/files/${key}`; } @bindThis public async saveFromBuffer(key: string, data: Buffer): Promise { - await mkdir(path, { recursive: true }); await writeFile(this.resolvePath(key), data); return `${this.config.url}/files/${key}`; } From 7aee3c161716fccbcc3a712ef50a0bc3b707d808 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 20 Oct 2024 00:11:14 -0400 Subject: [PATCH 3/4] fix comment typo in InternalStorageService.ts --- packages/backend/src/core/InternalStorageService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/InternalStorageService.ts b/packages/backend/src/core/InternalStorageService.ts index 7d2ea6ba20..f7371f8e79 100644 --- a/packages/backend/src/core/InternalStorageService.ts +++ b/packages/backend/src/core/InternalStorageService.ts @@ -24,7 +24,7 @@ export class InternalStorageService { @Inject(DI.config) private config: Config, ) { - // No one should erase the working directly *while the server is running*. + // No one should erase the working directory *while the server is running*. fs.mkdirSync(path, { recursive: true }); } From fcd2c93a19b6bad823d8543ab4ebc4da613b063e Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 20 Oct 2024 00:13:07 -0400 Subject: [PATCH 4/4] ensure that "thumbnail stored" / "web stored" messages only appear after success --- packages/backend/src/core/DriveService.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts index 781b435c9a..e75928d83e 100644 --- a/packages/backend/src/core/DriveService.ts +++ b/packages/backend/src/core/DriveService.ts @@ -234,16 +234,22 @@ export class DriveService { if (alts.thumbnail) { promises.push(this.internalStorageService.saveFromBuffer(thumbnailAccessKey, alts.thumbnail.data)); - this.registerLogger.info(`thumbnail stored: ${thumbnailAccessKey}`); } if (alts.webpublic) { promises.push(this.internalStorageService.saveFromBuffer(webpublicAccessKey, alts.webpublic.data)); - this.registerLogger.info(`web stored: ${webpublicAccessKey}`); } const [url, thumbnailUrl, webpublicUrl] = await Promise.all(promises); + if (thumbnailUrl) { + this.registerLogger.info(`thumbnail stored: ${thumbnailAccessKey}`); + } + + if (webpublicUrl) { + this.registerLogger.info(`web stored: ${webpublicAccessKey}`); + } + file.storedInternal = true; file.url = url; file.thumbnailUrl = thumbnailUrl ?? null;