Merge ec5eee4b9e into 6d499c782a

server/src/middleware/file-upload.interceptor.ts

@@ -3,13 +3,14 @@ import { PATH_METADATA } from '@nestjs/common/constants';
 import { Reflector } from '@nestjs/core';
 import { transformException } from '@nestjs/platform-express/multer/multer/multer.utils';
 import { NextFunction, RequestHandler } from 'express';
-import multer, { StorageEngine, diskStorage } from 'multer';
-import { createHash, randomUUID } from 'node:crypto';
+import multer, { StorageEngine } from 'multer';
+import { randomUUID } from 'node:crypto';
 import { Observable } from 'rxjs';
 import { UploadFieldName } from 'src/dtos/asset-media.dto';
 import { RouteKey } from 'src/enum';
 import { AuthRequest } from 'src/middleware/auth.guard';
 import { LoggingRepository } from 'src/repositories/logging.repository';
+import { StorageRepository } from 'src/repositories/storage.repository';
 import { AssetMediaService } from 'src/services/asset-media.service';
 import { ImmichFile, UploadFile, UploadFiles } from 'src/types';
 import { asUploadRequest, mapToUploadFile } from 'src/utils/asset.util';
@@ -26,8 +27,6 @@ export function getFiles(files: UploadFiles) {
   };
 }
 
-type DiskStorageCallback = (error: Error | null, result: string) => void;
-
 type ImmichMulterFile = Express.Multer.File & { uuid: string };
 
 interface Callback<T> {
@@ -49,26 +48,25 @@ export class FileUploadInterceptor implements NestInterceptor {
     userProfile: RequestHandler;
     assetUpload: RequestHandler;
   };
-  private defaultStorage: StorageEngine;
+  private multerStorage: StorageEngine;
 
   constructor(
     private reflect: Reflector,
     private assetService: AssetMediaService,
+    private storageRepository: StorageRepository,
     private logger: LoggingRepository,
   ) {
     this.logger.setContext(FileUploadInterceptor.name);
 
-    this.defaultStorage = diskStorage({
-      filename: this.filename.bind(this),
-      destination: this.destination.bind(this),
-    });
+    // Create custom storage engine that delegates to StorageRepository
+    this.multerStorage = {
+      _handleFile: this.handleFile.bind(this),
+      _removeFile: this.removeFile.bind(this),
+    };
 
     const instance = multer({
       fileFilter: this.fileFilter.bind(this),
-      storage: {
-        _handleFile: this.handleFile.bind(this),
-        _removeFile: this.removeFile.bind(this),
-      },
+      storage: this.multerStorage,
     });
 
     this.handlers = {
@@ -102,20 +100,6 @@ export class FileUploadInterceptor implements NestInterceptor {
     return callbackify(() => this.assetService.canUploadFile(asUploadRequest(request, file)), callback);
   }
 
-  private filename(request: AuthRequest, file: Express.Multer.File, callback: DiskStorageCallback) {
-    return callbackify(
-      () => this.assetService.getUploadFilename(asUploadRequest(request, file)),
-      callback as Callback<string>,
-    );
-  }
-
-  private destination(request: AuthRequest, file: Express.Multer.File, callback: DiskStorageCallback) {
-    return callbackify(
-      () => this.assetService.getUploadFolder(asUploadRequest(request, file)),
-      callback as Callback<string>,
-    );
-  }
-
   private handleFile(request: AuthRequest, file: Express.Multer.File, callback: Callback<Partial<ImmichFile>>) {
     (file as ImmichMulterFile).uuid = randomUUID();
 
@@ -124,25 +108,44 @@ export class FileUploadInterceptor implements NestInterceptor {
       this.assetService.onUploadError(request, file).catch(this.logger.error);
     });
 
-    if (!this.isAssetUploadFile(file)) {
-      this.defaultStorage._handleFile(request, file, callback);
-      return;
-    }
-
-    const hash = createHash('sha1');
-    file.stream.on('data', (chunk) => hash.update(chunk));
-    this.defaultStorage._handleFile(request, file, (error, info) => {
-      if (error) {
-        hash.destroy();
+    // Get destination folder and filename from AssetMediaService
+    const uploadRequest = asUploadRequest(request, file);
+    const folder = this.assetService.getUploadFolder(uploadRequest);
+    const filename = this.assetService.getUploadFilename(uploadRequest);
+    const destination = `${folder}/${filename}`;
+
+    // Determine if we should compute checksum (only for asset files, not profile images)
+    const shouldComputeChecksum = this.isAssetUploadFile(file);
+
+    // Upload using StorageRepository
+    this.storageRepository
+      .uploadFromStream(file.stream, destination, { computeChecksum: shouldComputeChecksum })
+      .then((result) => {
+        callback(null, {
+          path: result.path,
+          size: result.size,
+          checksum: result.checksum,
+        });
+      })
+      .catch((error) => {
+        this.logger.error(`Error uploading file: ${error.message}`, error.stack);
         callback(error);
-      } else {
-        callback(null, { ...info, checksum: hash.digest() });
-      }
-    });
+      });
   }
 
-  private removeFile(request: AuthRequest, file: Express.Multer.File, callback: (error: Error | null) => void) {
-    this.defaultStorage._removeFile(request, file, callback);
+  private removeFile(_request: AuthRequest, file: Express.Multer.File, callback: (error: Error | null) => void) {
+    // If the file was uploaded, remove it
+    if (file.path) {
+      this.storageRepository
+        .unlink(file.path)
+        .then(() => callback(null))
+        .catch((error) => {
+          this.logger.error(`Error removing file: ${error.message}`, error.stack);
+          callback(error);
+        });
+    } else {
+      callback(null);
+    }
   }
 
   private isAssetUploadFile(file: Express.Multer.File) {

server/src/repositories/storage.repository.ts

@@ -2,10 +2,12 @@ import { Injectable } from '@nestjs/common';
 import archiver from 'archiver';
 import chokidar, { ChokidarOptions } from 'chokidar';
 import { escapePath, glob, globStream } from 'fast-glob';
+import { createHash } from 'node:crypto';
 import { constants, createReadStream, createWriteStream, existsSync, mkdirSync, ReadOptionsWithBuffer } from 'node:fs';
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import { Readable, Writable } from 'node:stream';
+import { pipeline } from 'node:stream/promises';
 import { CrawlOptionsDto, WalkOptionsDto } from 'src/dtos/library.dto';
 import { LoggingRepository } from 'src/repositories/logging.repository';
 import { mimeTypes } from 'src/utils/mime-types';
@@ -35,6 +37,16 @@ export interface DiskUsage {
   total: number;
 }
 
+export interface UploadResult {
+  path: string;
+  size: number;
+  checksum?: Buffer;
+}
+
+export interface UploadOptions {
+  computeChecksum?: boolean;
+}
+
 @Injectable()
 export class StorageRepository {
   constructor(private logger: LoggingRepository) {
@@ -65,6 +77,59 @@ export class StorageRepository {
     return createWriteStream(filepath, { flags: 'w' });
   }
 
+  /**
+   * Upload a file from a readable stream to the specified destination.
+   * Optionally computes a SHA1 checksum while streaming.
+   *
+   * @param stream - The readable stream to upload from
+   * @param destination - The full path where the file should be written
+   * @param options - Upload options (e.g., computeChecksum)
+   * @returns Upload result containing path, size, and optional checksum
+   */
+  async uploadFromStream(stream: Readable, destination: string, options: UploadOptions = {}): Promise<UploadResult> {
+    // Ensure the directory exists
+    const directory = path.dirname(destination);
+    this.mkdirSync(directory);
+
+    let checksum: Buffer | undefined;
+    let size = 0;
+
+    // Create write stream
+    const writeStream = this.createWriteStream(destination);
+
+    // If checksum computation is requested, set up hash stream
+    if (options.computeChecksum) {
+      const hash = createHash('sha1');
+
+      stream.on('data', (chunk: Buffer) => {
+        hash.update(chunk);
+        size += chunk.length;
+      });
+
+      stream.on('end', () => {
+        checksum = hash.digest();
+      });
+
+      stream.on('error', () => {
+        hash.destroy();
+      });
+    } else {
+      // Track size even without checksum
+      stream.on('data', (chunk: Buffer) => {
+        size += chunk.length;
+      });
+    }
+
+    // Pipe the stream to the destination file
+    await pipeline(stream, writeStream);
+
+    return {
+      path: destination,
+      size,
+      checksum,
+    };
+  }
+
   createOrOverwriteFile(filepath: string, buffer: Buffer) {
     return fs.writeFile(filepath, buffer, { flag: 'w' });
   }

server/src/services/asset-media.service.spec.ts

@@ -306,14 +306,12 @@ describe(AssetMediaService.name, () => {
       expect(sut.getUploadFolder(uploadFile.filename(UploadFieldName.PROFILE_DATA, 'image.jpg'))).toEqual(
         expect.stringContaining('/data/profile/admin_id'),
       );
-      expect(mocks.storage.mkdirSync).toHaveBeenCalledWith(expect.stringContaining('/data/profile/admin_id'));
     });
 
     it('should return upload for everything else', () => {
       expect(sut.getUploadFolder(uploadFile.filename(UploadFieldName.ASSET_DATA, 'image.jpg'))).toEqual(
         expect.stringContaining('/data/upload/admin_id/ra/nd'),
       );
-      expect(mocks.storage.mkdirSync).toHaveBeenCalledWith(expect.stringContaining('/data/upload/admin_id/ra/nd'));
     });
   });
 

server/src/services/asset-media.service.ts

@@ -114,7 +114,7 @@ export class AssetMediaService extends BaseService {
       folder = StorageCore.getFolderLocation(StorageFolder.Profile, auth.user.id);
     }
 
-    this.storageRepository.mkdirSync(folder);
+    // Note: Directory creation is now handled by StorageRepository.uploadFromStream
 
     return folder;
   }

server/test/repositories/storage.repository.mock.ts

@@ -53,6 +53,7 @@ export const newStorageRepositoryMock = (): Mocked<RepositoryInterface<StorageRe
     readTextFile: vitest.fn(),
     createFile: vitest.fn(),
     createWriteStream: vitest.fn(),
+    uploadFromStream: vitest.fn(),
     createOrOverwriteFile: vitest.fn(),
     existsSync: vitest.fn(),
     overwriteFile: vitest.fn(),