merge: Optimizations to InternalStorageService (resolves #753) (!694)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/694

Closes #753

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

2 files changed, 33 insertions, 24 deletions

diff --git a/packages/backend/src/core/DriveService.ts b/packages/backend/src/core/DriveService.ts
index 520c339d7d..086f2f94d5 100644
--- a/packages/backend/src/core/DriveService.ts
+++ b/packages/backend/src/core/DriveService.ts
@@ -227,25 +227,33 @@ 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<string>, ...(Promise<string> | undefined)[]] = [
+				this.internalStorageService.saveFromPath(accessKey, path),
+			];
 
 			if (alts.thumbnail) {
-				thumbnailUrl = this.internalStorageService.saveFromBuffer(thumbnailAccessKey, alts.thumbnail.data);
-				this.registerLogger.info(`thumbnail stored: ${thumbnailAccessKey}`);
+				promises.push(this.internalStorageService.saveFromBuffer(thumbnailAccessKey, alts.thumbnail.data));
 			}
 
 			if (alts.webpublic) {
-				webpublicUrl = this.internalStorageService.saveFromBuffer(webpublicAccessKey, alts.webpublic.data);
+				promises.push(this.internalStorageService.saveFromBuffer(webpublicAccessKey, alts.webpublic.data));
+			}
+
+			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;
-			file.webpublicUrl = webpublicUrl;
+			file.thumbnailUrl = thumbnailUrl ?? null;
+			file.webpublicUrl = webpublicUrl ?? null;
 			file.accessKey = accessKey;
 			file.thumbnailAccessKey = thumbnailAccessKey;
 			file.webpublicAccessKey = webpublicAccessKey;
@@ -720,19 +728,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 +750,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..f7371f8e79 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';
@@ -23,6 +24,8 @@ export class InternalStorageService {
 		@Inject(DI.config)
 		private config: Config,
 	) {
+		// No one should erase the working directory *while the server is running*.
+		fs.mkdirSync(path, { recursive: true });
 	}
 
 	@bindThis
@@ -36,21 +39,19 @@ 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<string> {
+		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<string> {
+		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<void> {
+		await unlink(this.resolvePath(key));
 	}
 }