refactor: controller tests (#18100)

e2e/src/api/specs/asset.e2e-spec.ts

@@ -432,20 +432,6 @@ describe('/asset', () => {
   });
 
   describe('PUT /assets/:id', () => {
-    it('should require authentication', async () => {
-      const { status, body } = await request(app).put(`/assets/:${uuidDto.notFound}`);
-      expect(status).toBe(401);
-      expect(body).toEqual(errorDto.unauthorized);
-    });
-
-    it('should require a valid id', async () => {
-      const { status, body } = await request(app)
-        .put(`/assets/${uuidDto.invalid}`)
-        .set('Authorization', `Bearer ${user1.accessToken}`);
-      expect(status).toBe(400);
-      expect(body).toEqual(errorDto.badRequest(['id must be a UUID']));
-    });
-
     it('should require access', async () => {
       const { status, body } = await request(app)
         .put(`/assets/${user2Assets[0].id}`)

e2e/src/api/specs/auth.e2e-spec.ts

@@ -19,17 +19,6 @@ describe(`/auth/admin-sign-up`, () => {
       expect(body).toEqual(signupResponseDto.admin);
     });
 
-    it('should sign up the admin with a local domain', async () => {
-      const { status, body } = await request(app)
-        .post('/auth/admin-sign-up')
-        .send({ ...signupDto.admin, email: 'admin@local' });
-      expect(status).toEqual(201);
-      expect(body).toEqual({
-        ...signupResponseDto.admin,
-        email: 'admin@local',
-      });
-    });
-
     it('should not allow a second admin to sign up', async () => {
       await signUpAdmin({ signUpDto: signupDto.admin });
 
@@ -57,22 +46,6 @@ describe('/auth/*', () => {
       expect(body).toEqual(errorDto.incorrectLogin);
     });
 
-    for (const key of Object.keys(loginDto.admin)) {
-      it(`should not allow null ${key}`, async () => {
-        const { status, body } = await request(app)
-          .post('/auth/login')
-          .send({ ...loginDto.admin, [key]: null });
-        expect(status).toBe(400);
-        expect(body).toEqual(errorDto.badRequest());
-      });
-
-      it('should reject an invalid email', async () => {
-        const { status, body } = await request(app).post('/auth/login').send({ email: [], password });
-        expect(status).toBe(400);
-        expect(body).toEqual(errorDto.invalidEmail);
-      });
-    }
-
     it('should accept a correct password', async () => {
       const { status, body, headers } = await request(app).post('/auth/login').send({ email, password });
       expect(status).toBe(201);
@@ -127,14 +100,6 @@ describe('/auth/*', () => {
   });
 
   describe('POST /auth/change-password', () => {
-    it('should require authentication', async () => {
-      const { status, body } = await request(app)
-        .post(`/auth/change-password`)
-        .send({ password, newPassword: 'Password1234' });
-      expect(status).toBe(401);
-      expect(body).toEqual(errorDto.unauthorized);
-    });
-
     it('should require the current password', async () => {
       const { status, body } = await request(app)
         .post(`/auth/change-password`)

e2e/src/api/specs/download.e2e-spec.ts

@@ -1,6 +1,5 @@
 import { AssetMediaResponseDto, LoginResponseDto } from '@immich/sdk';
 import { readFile, writeFile } from 'node:fs/promises';
-import { errorDto } from 'src/responses';
 import { app, tempDir, utils } from 'src/utils';
 import request from 'supertest';
 import { beforeAll, describe, expect, it } from 'vitest';
@@ -17,15 +16,6 @@ describe('/download', () => {
   });
 
   describe('POST /download/info', () => {
-    it('should require authentication', async () => {
-      const { status, body } = await request(app)
-        .post(`/download/info`)
-        .send({ assetIds: [asset1.id] });
-
-      expect(status).toBe(401);
-      expect(body).toEqual(errorDto.unauthorized);
-    });
-
     it('should download info', async () => {
       const { status, body } = await request(app)
         .post('/download/info')
@@ -42,15 +32,6 @@ describe('/download', () => {
   });
 
   describe('POST /download/archive', () => {
-    it('should require authentication', async () => {
-      const { status, body } = await request(app)
-        .post(`/download/archive`)
-        .send({ assetIds: [asset1.id, asset2.id] });
-
-      expect(status).toBe(401);
-      expect(body).toEqual(errorDto.unauthorized);
-    });
-
     it('should download an archive', async () => {
       const { status, body } = await request(app)
         .post('/download/archive')

server/src/controllers/activity.controller.spec.ts

@@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(ActivityController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(ActivityService);
 
   beforeAll(async () => {
-    ctx = await controllerSetup(ActivityController, [
-      { provide: ActivityService, useValue: mockBaseService(ActivityService) },
-    ]);
+    ctx = await controllerSetup(ActivityController, [{ provide: ActivityService, useValue: service }]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/album.controller.spec.ts

@@ -6,13 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(AlbumController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(AlbumService);
 
   beforeAll(async () => {
-    ctx = await controllerSetup(AlbumController, [{ provide: AlbumService, useValue: mockBaseService(AlbumService) }]);
+    ctx = await controllerSetup(AlbumController, [{ provide: AlbumService, useValue: service }]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/api-key.controller.spec.ts

@@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(APIKeyController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(ApiKeyService);
 
   beforeAll(async () => {
-    ctx = await controllerSetup(APIKeyController, [
-      { provide: ApiKeyService, useValue: mockBaseService(ApiKeyService) },
-    ]);
+    ctx = await controllerSetup(APIKeyController, [{ provide: ApiKeyService, useValue: service }]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/auth.controller.spec.ts

@@ -1,4 +1,5 @@
 import { AuthController } from 'src/controllers/auth.controller';
+import { LoginResponseDto } from 'src/dtos/auth.dto';
 import { AuthService } from 'src/services/auth.service';
 import request from 'supertest';
 import { errorDto } from 'test/medium/responses';
@@ -14,6 +15,7 @@ describe(AuthController.name, () => {
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 
@@ -56,5 +58,88 @@ describe(AuthController.name, () => {
       expect(status).toEqual(201);
       expect(service.adminSignUp).toHaveBeenCalledWith(expect.objectContaining({ email: 'admin@immich.cloud' }));
     });
+
+    it('should accept an email with a local domain', async () => {
+      const { status } = await request(ctx.getHttpServer())
+        .post('/auth/admin-sign-up')
+        .send({ name: 'admin', password: 'password', email: 'admin@local' });
+      expect(status).toEqual(201);
+    });
+  });
+
+  describe('POST /auth/login', () => {
+    it(`should require an email and password`, async () => {
+      const { status, body } = await request(ctx.getHttpServer()).post('/auth/login').send({ name: 'admin' });
+      expect(status).toBe(400);
+      expect(body).toEqual(
+        errorDto.badRequest([
+          'email should not be empty',
+          'email must be an email',
+          'password should not be empty',
+          'password must be a string',
+        ]),
+      );
+    });
+
+    it(`should not allow null email`, async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/auth/login')
+        .send({ name: 'admin', email: null, password: 'password' });
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['email should not be empty', 'email must be an email']));
+    });
+
+    it(`should not allow null password`, async () => {
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/auth/login')
+        .send({ name: 'admin', email: 'admin@immich.cloud', password: null });
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['password should not be empty', 'password must be a string']));
+    });
+
+    it('should reject an invalid email', async () => {
+      service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto);
+
+      const { status, body } = await request(ctx.getHttpServer())
+        .post('/auth/login')
+        .send({ name: 'admin', email: [], password: 'password' });
+
+      expect(status).toBe(400);
+      expect(body).toEqual(errorDto.badRequest(['email must be an email']));
+    });
+
+    it('should transform the email to all lowercase', async () => {
+      service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto);
+
+      const { status } = await request(ctx.getHttpServer())
+        .post('/auth/login')
+        .send({ name: 'admin', email: 'aDmIn@iMmIcH.ApP', password: 'password' });
+
+      expect(status).toBe(201);
+      expect(service.login).toHaveBeenCalledWith(
+        expect.objectContaining({ email: 'admin@immich.app' }),
+        expect.anything(),
+      );
+    });
+
+    it('should accept an email with a local domain', async () => {
+      service.login.mockResolvedValue({ accessToken: 'access-token' } as LoginResponseDto);
+
+      const { status } = await request(ctx.getHttpServer())
+        .post('/auth/login')
+        .send({ name: 'admin', email: 'admin@local', password: 'password' });
+
+      expect(status).toEqual(201);
+      expect(service.login).toHaveBeenCalledWith(expect.objectContaining({ email: 'admin@local' }), expect.anything());
+    });
+  });
+
+  describe('POST /auth/change-password', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer())
+        .post('/auth/change-password')
+        .send({ password: 'password', newPassword: 'Password1234' });
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
   });
 });

server/src/controllers/auth.controller.ts

@@ -23,8 +23,8 @@ export class AuthController {
 
   @Post('login')
   async login(
-    @Body() loginCredential: LoginCredentialDto,
     @Res({ passthrough: true }) res: Response,
+    @Body() loginCredential: LoginCredentialDto,
     @GetLoginDetails() loginDetails: LoginDetails,
   ): Promise<LoginResponseDto> {
     const body = await this.service.login(loginCredential, loginDetails);

server/src/controllers/download.controller.spec.ts

@@ -0,0 +1,46 @@
+import { DownloadController } from 'src/controllers/download.controller';
+import { DownloadService } from 'src/services/download.service';
+import request from 'supertest';
+import { factory } from 'test/small.factory';
+import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
+import { Readable } from 'typeorm/platform/PlatformTools.js';
+
+describe(DownloadController.name, () => {
+  let ctx: ControllerContext;
+  const service = mockBaseService(DownloadService);
+
+  beforeAll(async () => {
+    ctx = await controllerSetup(DownloadController, [{ provide: DownloadService, useValue: service }]);
+    return () => ctx.close();
+  });
+
+  beforeEach(() => {
+    service.resetAllMocks();
+    ctx.reset();
+  });
+
+  describe('POST /download/info', () => {
+    it('should be an authenticated route', async () => {
+      await request(ctx.getHttpServer())
+        .post('/download/info')
+        .send({ assetIds: [factory.uuid()] });
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+  });
+
+  describe('POST /download/archive', () => {
+    it('should be an authenticated route', async () => {
+      const stream = new Readable({
+        read() {
+          this.push('test');
+          this.push(null);
+        },
+      });
+      service.downloadArchive.mockResolvedValue({ stream });
+      await request(ctx.getHttpServer())
+        .post('/download/archive')
+        .send({ assetIds: [factory.uuid()] });
+      expect(ctx.authenticate).toHaveBeenCalled();
+    });
+  });
+});

server/src/controllers/notification.controller.spec.ts

@@ -7,15 +7,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(NotificationController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(NotificationService);
 
   beforeAll(async () => {
-    ctx = await controllerSetup(NotificationController, [
-      { provide: NotificationService, useValue: mockBaseService(NotificationService) },
-    ]);
+    ctx = await controllerSetup(NotificationController, [{ provide: NotificationService, useValue: service }]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/search.controller.spec.ts

@@ -6,15 +6,15 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(SearchController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(SearchService);
 
   beforeAll(async () => {
-    ctx = await controllerSetup(SearchController, [
-      { provide: SearchService, useValue: mockBaseService(SearchService) },
-    ]);
+    ctx = await controllerSetup(SearchController, [{ provide: SearchService, useValue: service }]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/server.controller.spec.ts

@@ -6,16 +6,20 @@ import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils'
 
 describe(ServerController.name, () => {
   let ctx: ControllerContext;
+  const serverService = mockBaseService(ServerService);
+  const versionService = mockBaseService(VersionService);
 
   beforeAll(async () => {
     ctx = await controllerSetup(ServerController, [
-      { provide: ServerService, useValue: mockBaseService(ServerService) },
-      { provide: VersionService, useValue: mockBaseService(VersionService) },
+      { provide: ServerService, useValue: serverService },
+      { provide: VersionService, useValue: versionService },
     ]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    serverService.resetAllMocks();
+    versionService.resetAllMocks();
     ctx.reset();
   });
 

server/src/controllers/user.controller.spec.ts

@@ -8,16 +8,18 @@ import { automock, ControllerContext, controllerSetup, mockBaseService } from 't
 
 describe(UserController.name, () => {
   let ctx: ControllerContext;
+  const service = mockBaseService(UserService);
 
   beforeAll(async () => {
     ctx = await controllerSetup(UserController, [
       { provide: LoggingRepository, useValue: automock(LoggingRepository, { strict: false }) },
-      { provide: UserService, useValue: mockBaseService(UserService) },
+      { provide: UserService, useValue: service },
     ]);
     return () => ctx.close();
   });
 
   beforeEach(() => {
+    service.resetAllMocks();
     ctx.reset();
   });
 

server/test/medium/responses.ts

@@ -66,12 +66,6 @@ export const errorDto = {
     message: 'The server already has an admin',
     correlationId: expect.any(String),
   },
-  invalidEmail: {
-    error: 'Bad Request',
-    statusCode: 400,
-    message: ['email must be an email'],
-    correlationId: expect.any(String),
-  },
 };
 
 export const signupResponseDto = {

server/test/utils.ts

@@ -82,14 +82,13 @@ export type ControllerContext = {
 
 export const controllerSetup = async (controller: ClassConstructor<unknown>, providers: Provider[]) => {
   const noopInterceptor = { intercept: (ctx: never, next: CallHandler<unknown>) => next.handle() };
-  const authenticate = vi.fn();
   const moduleRef = await Test.createTestingModule({
     controllers: [controller],
     providers: [
       { provide: APP_PIPE, useValue: new ValidationPipe({ transform: true, whitelist: true }) },
       { provide: APP_GUARD, useClass: AuthGuard },
       { provide: LoggingRepository, useValue: LoggingRepository.create() },
-      { provide: AuthService, useValue: { authenticate } },
+      { provide: AuthService, useValue: { authenticate: vi.fn() } },
       ...providers,
     ],
   })
@@ -101,6 +100,9 @@ export const controllerSetup = async (controller: ClassConstructor<unknown>, pro
   const app = moduleRef.createNestApplication();
   await app.init();
 
+  // allow the AuthController to override the AuthService itself
+  const authenticate = app.get<Mocked<AuthService>>(AuthService).authenticate as Mock;
+
   return {
     authenticate,
     getHttpServer: () => app.getHttpServer(),
@@ -113,13 +115,17 @@ export const controllerSetup = async (controller: ClassConstructor<unknown>, pro
   };
 };
 
+export type AutoMocked<T> = Mocked<T> & { resetAllMocks: () => void };
+
 const mockFn = (label: string, { strict }: { strict: boolean }) => {
   const message = `Called a mock function without a mock implementation (${label})`;
-  return vitest.fn().mockImplementation(() => {
-    if (strict) {
-      assert.fail(message);
-    } else {
-      // console.warn(message);
+  return vitest.fn(() => {
+    {
+      if (strict) {
+        assert.fail(message);
+      } else {
+        // console.warn(message);
+      }
     }
   });
 };
@@ -134,11 +140,13 @@ export const automock = <T>(
     args?: ConstructorParameters<ClassConstructor<T>>;
     strict?: boolean;
   },
-): Mocked<T> => {
+): AutoMocked<T> => {
   const mock: Record<string, unknown> = {};
   const strict = options?.strict ?? true;
   const args = options?.args ?? [];
 
+  const mocks: Mock[] = [];
+
   const instance = new Dependency(...args);
   for (const property of Object.getOwnPropertyNames(Dependency.prototype)) {
     if (property === 'constructor') {
@@ -151,7 +159,9 @@ export const automock = <T>(
 
       const target = instance[property as keyof T];
       if (typeof target === 'function') {
-        mock[property] = mockFn(label, { strict });
+        const mockImplementation = mockFn(label, { strict });
+        mock[property] = mockImplementation;
+        mocks.push(mockImplementation);
         continue;
       }
     } catch {
@@ -159,7 +169,14 @@ export const automock = <T>(
     }
   }
 
-  return mock as Mocked<T>;
+  const result = mock as AutoMocked<T>;
+  result.resetAllMocks = () => {
+    for (const mock of mocks) {
+      mock.mockReset();
+    }
+  };
+
+  return result;
 };
 
 export type ServiceOverrides = {