add controller tests, move validation testing from e2e

revert unnecessary change

update mocks

add structured-headers to e2e deps

server/src/controllers/asset-upload.controller.spec.ts

@@ -0,0 +1,447 @@
+import { createHash, randomUUID } from 'crypto';
+import { AssetUploadController } from 'src/controllers/asset-upload.controller';
+import { AssetUploadService } from 'src/services/asset-upload.service';
+import { serializeDictionary } from 'structured-headers';
+import request from 'supertest';
+import { factory } from 'test/small.factory';
+import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
+
+const makeAssetData = (overrides?: Partial<any>): string => {
+  return serializeDictionary({
+    filename: 'test-image.jpg',
+    'device-asset-id': 'test-asset-id',
+    'device-id': 'test-device',
+    'file-created-at': new Date('2025-01-02T00:00:00Z').toISOString(),
+    'file-modified-at': new Date('2025-01-01T00:00:00Z').toISOString(),
+    'is-favorite': false,
+    ...overrides,
+  });
+};
+
+describe(AssetUploadController.name, () => {
+  let ctx: ControllerContext;
+  let buffer: Buffer;
+  let checksum: string;
+  const service = mockBaseService(AssetUploadService);
+
+  beforeAll(async () => {
+    ctx = await controllerSetup(AssetUploadController, [{ provide: AssetUploadService, useValue: service }]);
+    return () => ctx.close();
+  });
+
+  beforeEach(() => {
+    service.resetAllMocks();
+    service.startUpload.mockImplementation(async (auth, req, res, dto) => void res.send());
+    service.resumeUpload.mockImplementation(async (auth, req, res, id, dto) => void res.send());
+    service.cancelUpload.mockImplementation(async (auth, id, res) => void res.send());
+    service.getUploadStatus.mockImplementation(async (auth, res, id, dto) => void res.send());
+    ctx.reset();
+
+    buffer = Buffer.from(randomUUID());
+    checksum = `sha=:${createHash('sha1').update(buffer).digest('base64')}:`;
+  });
+
+  describe('POST /upload', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).post('/upload');
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should require Upload-Draft-Interop-Version header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining(['version must be an integer number', 'version must not be less than 3']),
+        }),
+      );
+    });
+
+    it('should require X-Immich-Asset-Data header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(expect.objectContaining({ message: 'x-immich-asset-data header is required' }));
+    });
+
+    it('should require Repr-Digest header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(expect.objectContaining({ message: 'Missing repr-digest header' }));
+    });
+
+    it('should require Upload-Complete header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('uploadComplete')]),
+        }),
+      );
+    });
+
+    it('should require Upload-Length header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([
+            'uploadLength must be an integer number',
+            'uploadLength must not be less than 0',
+          ]),
+        }),
+      );
+    });
+
+    it('should reject invalid Repr-Digest format', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', checksum)
+        .set('Repr-Digest', 'invalid-format')
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(expect.objectContaining({ message: 'Invalid repr-digest header' }));
+    });
+
+    it('should validate device-asset-id is required in asset data', async () => {
+      const assetData = serializeDictionary({
+        filename: 'test.jpg',
+        'device-id': 'test-device',
+        'file-created-at': new Date().toISOString(),
+        'file-modified-at': new Date().toISOString(),
+      });
+
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', assetData)
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('deviceAssetId')]),
+        }),
+      );
+    });
+
+    it('should validate device-id is required in asset data', async () => {
+      const assetData = serializeDictionary({
+        filename: 'test.jpg',
+        'device-asset-id': 'test-asset',
+        'file-created-at': new Date().toISOString(),
+        'file-modified-at': new Date().toISOString(),
+      });
+
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', assetData)
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('deviceId')]),
+        }),
+      );
+    });
+
+    it('should validate filename is required in asset data', async () => {
+      const assetData = serializeDictionary({
+        'device-asset-id': 'test-asset',
+        'device-id': 'test-device',
+        'file-created-at': new Date().toISOString(),
+        'file-modified-at': new Date().toISOString(),
+      });
+
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', assetData)
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('filename')]),
+        }),
+      );
+    });
+
+    it('should accept Upload-Incomplete header for version 3', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '3')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Incomplete', '?0')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).not.toBe(400);
+    });
+
+    it('should validate Upload-Complete is a boolean structured field', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', 'true')
+        .set('Upload-Length', '1024')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('uploadComplete')]),
+        }),
+      );
+    });
+
+    it('should validate Upload-Length is a non-negative integer', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/upload')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('X-Immich-Asset-Data', makeAssetData())
+        .set('Repr-Digest', checksum)
+        .set('Upload-Complete', '?1')
+        .set('Upload-Length', '-100')
+        .send(buffer);
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining(['uploadLength must not be less than 0']),
+        }),
+      );
+    });
+  });
+
+  describe('PATCH /upload/:id', () => {
+    const uploadId = factory.uuid();
+
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).patch(`/upload/${uploadId}`);
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should require Upload-Draft-Interop-Version header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Offset', '0')
+        .set('Upload-Complete', '?1')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining(['version must be an integer number', 'version must not be less than 3']),
+        }),
+      );
+    });
+
+    it('should require Upload-Offset header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('Upload-Complete', '?1')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([
+            'uploadOffset must be an integer number',
+            'uploadOffset must not be less than 0',
+          ]),
+        }),
+      );
+    });
+
+    it('should require Upload-Complete header', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('Upload-Offset', '0')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('uploadComplete')]),
+        }),
+      );
+    });
+
+    it('should validate UUID parameter', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch('/upload/invalid-uuid')
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('Upload-Offset', '0')
+        .set('Upload-Complete', '?0')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(expect.objectContaining({ message: ['id must be a UUID'] }));
+    });
+
+    it('should validate Upload-Offset is a non-negative integer', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '8')
+        .set('Upload-Offset', '-50')
+        .set('Upload-Complete', '?0')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining(['uploadOffset must not be less than 0']),
+        }),
+      );
+    });
+
+    it('should require Content-Type: application/partial-upload for version >= 6', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '6')
+        .set('Upload-Offset', '0')
+        .set('Upload-Complete', '?0')
+        .set('Content-Type', 'application/octet-stream')
+        .send(Buffer.from('test'));
+
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        expect.objectContaining({
+          message: ['contentType must be equal to application/partial-upload'],
+        }),
+      );
+    });
+
+    it('should allow other Content-Type for version < 6', async () => {
+      const { body } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '3')
+        .set('Upload-Offset', '0')
+        .set('Upload-Incomplete', '?1')
+        .set('Content-Type', 'application/octet-stream')
+        .send();
+
+      // Will fail for other reasons, but not content-type validation
+      expect(body).not.toEqual(
+        expect.objectContaining({
+          message: expect.arrayContaining([expect.stringContaining('contentType')]),
+        }),
+      );
+    });
+
+    it('should accept Upload-Incomplete header for version 3', async () => {
+      const { status } = await request(ctx.getHttpServer())
+        .patch(`/upload/${uploadId}`)
+        .set('Upload-Draft-Interop-Version', '3')
+        .set('Upload-Offset', '0')
+        .set('Upload-Incomplete', '?1')
+        .send();
+
+      // Should not fail validation
+      expect(status).not.toBe(400);
+    });
+  });
+
+  describe('DELETE /upload/:id', () => {
+    const uploadId = factory.uuid();
+
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).delete(`/upload/${uploadId}`);
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should validate UUID parameter', async () => {
+      const { status, body } = await request(ctx.getHttpServer()).delete('/upload/invalid-uuid');
+
+      expect(status).toBe(400);
+      expect(body).toEqual(expect.objectContaining({ message: ['id must be a UUID'] }));
+    });
+  });
+
+  describe('HEAD /upload/:id', () => {
+    const uploadId = factory.uuid();
+
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer()).head(`/upload/${uploadId}`);
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+
+    it('should require Upload-Draft-Interop-Version header', async () => {
+      const { status } = await request(ctx.getHttpServer()).head(`/upload/${uploadId}`);
+
+      expect(status).toBe(400);
+    });
+
+    it('should validate UUID parameter', async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .head('/upload/invalid-uuid')
+        .set('Upload-Draft-Interop-Version', '8');
+
+      expect(status).toBe(400);
+    });
+  });
+
+  describe('OPTIONS /upload', () => {
+    it('should return 204 with upload limits', async () => {
+      const { status, headers } = await request(ctx.getHttpServer()).options('/upload');
+
+      expect(status).toBe(204);
+      expect(headers['upload-limit']).toBe('min-size=0');
+    });
+  });
+});

server/src/controllers/asset-upload.controller.ts

@@ -1,5 +1,4 @@
 import {
-  BadRequestException,
   Controller,
   Delete,
   Head,
@@ -14,15 +13,13 @@ import {
   Res,
 } from '@nestjs/common';
 import { ApiHeader, ApiTags } from '@nestjs/swagger';
-import { plainToInstance } from 'class-transformer';
-import { validateSync } from 'class-validator';
 import { Request, Response } from 'express';
-import { IncomingHttpHeaders } from 'node:http';
 import { AuthDto } from 'src/dtos/auth.dto';
 import { GetUploadStatusDto, ResumeUploadDto, StartUploadDto, UploadHeader } from 'src/dtos/upload.dto';
 import { ImmichHeader, Permission } from 'src/enum';
 import { Auth, Authenticated } from 'src/middleware/auth.guard';
 import { AssetUploadService } from 'src/services/asset-upload.service';
+import { validateSyncOrReject } from 'src/utils/request';
 import { UUIDParamDto } from 'src/validation';
 
 const apiInteropVersion = {
@@ -76,8 +73,7 @@ export class AssetUploadController {
   @ApiHeader(apiUploadComplete)
   @ApiHeader(apiContentLength)
   startUpload(@Auth() auth: AuthDto, @Req() req: Request, @Res() res: Response): Promise<void> {
-    const dto = this.getDto(StartUploadDto, req.headers);
-    return this.service.startUpload(auth, req, res, dto);
+    return this.service.startUpload(auth, req, res, validateSyncOrReject(StartUploadDto, req.headers));
   }
 
   @Patch(':id')
@@ -92,8 +88,7 @@ export class AssetUploadController {
   @ApiHeader(apiUploadComplete)
   @ApiHeader(apiContentLength)
   resumeUpload(@Auth() auth: AuthDto, @Req() req: Request, @Res() res: Response, @Param() { id }: UUIDParamDto) {
-    const dto = this.getDto(ResumeUploadDto, req.headers);
-    return this.service.resumeUpload(auth, req, res, id, dto);
+    return this.service.resumeUpload(auth, req, res, id, validateSyncOrReject(ResumeUploadDto, req.headers));
   }
 
   @Delete(':id')
@@ -106,23 +101,11 @@ export class AssetUploadController {
   @Authenticated({ sharedLink: true, permission: Permission.AssetUpload })
   @ApiHeader(apiInteropVersion)
   getUploadStatus(@Auth() auth: AuthDto, @Req() req: Request, @Res() res: Response, @Param() { id }: UUIDParamDto) {
-    const dto = this.getDto(GetUploadStatusDto, req.headers);
-    return this.service.getUploadStatus(auth, res, id, dto);
+    return this.service.getUploadStatus(auth, res, id, validateSyncOrReject(GetUploadStatusDto, req.headers));
   }
 
   @Options()
   @HttpCode(HttpStatus.NO_CONTENT)
   @Header('Upload-Limit', 'min-size=0')
   getUploadOptions() {}
-
-  private getDto<T extends object>(cls: new () => T, headers: IncomingHttpHeaders): T {
-    const dto = plainToInstance(cls, headers, { excludeExtraneousValues: true });
-    const errors = validateSync(dto);
-    if (errors.length > 0) {
-      const constraints = errors.flatMap((e) => (e.constraints ? Object.values(e.constraints) : []));
-      console.warn('Upload DTO validation failed:', JSON.stringify(errors, null, 2));
-      throw new BadRequestException(constraints);
-    }
-    return dto;
-  }
 }