cjones/immich
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: migrate user repository to kysely (#15296)

Browse Source 
* refactor: migrate user repository to kysely

* refactor: migrate user repository to kysely

* refactor: migrate user repository to kysely

* refactor: migrate user repository to kysely

* fix: test

* clean up

* fix: metadata retrieval bug

* use correct typeing for upsert metadata

* pr feedback

* pr feedback

* fix: add deletedAt check

* fix: get non deleted user by default

* remove console.log

* fix: stop kysely after command finishes

* final clean up

---------

Co-authored-by: Jason Rasmussen <jason@rasm.me>
This commit is contained in:
Alex
2025-01-13 19:30:34 -06:00
committed by GitHub
parent a6c8eb57f1
commit 3da750117f
18 changed files with 447 additions and 312 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/services/album.service.spec.ts
+2 -2
View File
@@ -153,7 +153,7 @@ describe(AlbumService.name, () => {
});
it('should require valid userIds', async () => {
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(
sut.create(authStub.admin, {
albumName: 'Empty album',
@@ -299,7 +299,7 @@ describe(AlbumService.name, () => {
it('should throw an error if the userId does not exist', async () => {
accessMock.album.checkOwnerAccess.mockResolvedValue(new Set([albumStub.sharedWithAdmin.id]));
albumMock.getById.mockResolvedValue(albumStub.sharedWithAdmin);
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(
sut.addUsers(authStub.user1, albumStub.sharedWithAdmin.id, { albumUsers: [{ userId: 'user-3' }] }),
).rejects.toBeInstanceOf(BadRequestException);
server/src/services/auth.service.spec.ts
+12 -12
View File
@@ -96,7 +96,7 @@ describe('AuthService', () => {
});
it('should check the user exists', async () => {
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
await expect(sut.login(fixtures.login, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException);
expect(userMock.getByEmail).toHaveBeenCalledTimes(1);
});
@@ -144,7 +144,7 @@ describe('AuthService', () => {
const auth = { user: { email: 'test@imimch.com' } } as AuthDto;
const dto = { password: 'old-password', newPassword: 'new-password' };
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
await expect(sut.changePassword(auth, dto)).rejects.toBeInstanceOf(UnauthorizedException);
});
@@ -227,7 +227,7 @@ describe('AuthService', () => {
});
it('should sign up the admin', async () => {
userMock.getAdmin.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(void 0);
userMock.create.mockResolvedValue({
...dto,
id: 'admin',
@@ -309,7 +309,7 @@ describe('AuthService', () => {
it('should not accept a key without a user', async () => {
sharedLinkMock.getByKey.mockResolvedValue(sharedLinkStub.expired);
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(
sut.authenticate({
headers: { 'x-immich-share-key': 'key' },
@@ -473,7 +473,7 @@ describe('AuthService', () => {
it('should not allow auto registering', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthEnabled);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).rejects.toBeInstanceOf(
BadRequestException,
);
@@ -510,7 +510,7 @@ describe('AuthService', () => {
it('should allow auto registering by default', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.enabled);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
@@ -525,7 +525,7 @@ describe('AuthService', () => {
it('should throw an error if user should be auto registered but the email claim does not exist', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.enabled);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
sessionMock.create.mockResolvedValue(sessionStub.valid);
@@ -559,7 +559,7 @@ describe('AuthService', () => {
it('should use the default quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
@@ -572,7 +572,7 @@ describe('AuthService', () => {
it('should ignore an invalid storage quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 'abc' });
@@ -586,7 +586,7 @@ describe('AuthService', () => {
it('should ignore a negative quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: -5 });
@@ -600,7 +600,7 @@ describe('AuthService', () => {
it('should not set quota for 0 quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 0 });
@@ -620,7 +620,7 @@ describe('AuthService', () => {
it('should use a valid storage quota', async () => {
systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
userMock.getByEmail.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getAdmin.mockResolvedValue(userStub.user1);
userMock.create.mockResolvedValue(userStub.user1);
oauthMock.getProfile.mockResolvedValue({ sub, email, immich_quota: 5 });
server/src/services/auth.service.ts
+1 -1
View File
@@ -65,7 +65,7 @@ export class AuthService extends BaseService {
if (user) {
const isAuthenticated = this.validatePassword(dto.password, user);
if (!isAuthenticated) {
user = null;
user = undefined;
}
}
server/src/services/base.service.ts
+4 -2
View File
@@ -1,8 +1,10 @@
import { BadRequestException, Inject } from '@nestjs/common';
import { Insertable } from 'kysely';
import sanitize from 'sanitize-filename';
import { SystemConfig } from 'src/config';
import { SALT_ROUNDS } from 'src/constants';
import { StorageCore } from 'src/cores/storage.core';
import { Users } from 'src/db';
import { UserEntity } from 'src/entities/user.entity';
import { IAccessRepository } from 'src/interfaces/access.interface';
import { IActivityRepository } from 'src/interfaces/activity.interface';
@@ -131,7 +133,7 @@ export class BaseService {
return checkAccess(this.accessRepository, request);
}
async createUser(dto: Partial<UserEntity> & { email: string }): Promise<UserEntity> {
async createUser(dto: Insertable<Users> & { email: string }): Promise<UserEntity> {
const user = await this.userRepository.getByEmail(dto.email);
if (user) {
throw new BadRequestException('User exists');
@@ -144,7 +146,7 @@ export class BaseService {
}
}
const payload: Partial<UserEntity> = { ...dto };
const payload: Insertable<Users> = { ...dto };
if (payload.password) {
payload.password = await this.cryptoRepository.hashBcrypt(payload.password, SALT_ROUNDS);
}
server/src/services/cli.service.spec.ts
+1 -1
View File
@@ -25,7 +25,7 @@ describe(CliService.name, () => {
describe('resetAdminPassword', () => {
it('should only work when there is an admin account', async () => {
userMock.getAdmin.mockResolvedValue(null);
userMock.getAdmin.mockResolvedValue(void 0);
const ask = vitest.fn().mockResolvedValue('new-password');
await expect(sut.resetAdminPassword(ask)).rejects.toThrowError('Admin account does not exist');
server/src/services/cli.service.ts
+4
View File
@@ -48,4 +48,8 @@ export class CliService extends BaseService {
config.oauth.enabled = true;
await this.updateConfig(config);
}
cleanup() {
return this.databaseRepository.shutdown();
}
}
server/src/services/user-admin.service.spec.ts
+7 -7
View File
@@ -19,13 +19,13 @@ describe(UserAdminService.name, () => {
({ sut, jobMock, userMock } = newTestService(UserAdminService));
userMock.get.mockImplementation((userId) =>
Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? null),
Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? undefined),
);
});
describe('create', () => {
it('should not create a user if there is no local admin account', async () => {
userMock.getAdmin.mockResolvedValueOnce(null);
userMock.getAdmin.mockResolvedValueOnce(void 0);
await expect(
sut.create({
@@ -66,8 +66,8 @@ describe(UserAdminService.name, () => {
email: 'immich@test.com',
storageLabel: 'storage_label',
};
userMock.getByEmail.mockResolvedValue(null);
userMock.getByStorageLabel.mockResolvedValue(null);
userMock.getByEmail.mockResolvedValue(void 0);
userMock.getByStorageLabel.mockResolvedValue(void 0);
userMock.update.mockResolvedValue(userStub.user1);
await sut.update(authStub.user1, userStub.user1.id, update);
@@ -108,7 +108,7 @@ describe(UserAdminService.name, () => {
});
it('update user information should throw error if user not found', async () => {
userMock.get.mockResolvedValueOnce(null);
userMock.get.mockResolvedValueOnce(void 0);
await expect(
sut.update(authStub.admin, userStub.user1.id, { shouldChangePassword: true }),
@@ -118,7 +118,7 @@ describe(UserAdminService.name, () => {
describe('delete', () => {
it('should throw error if user could not be found', async () => {
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(sut.delete(authStub.admin, userStub.admin.id, {})).rejects.toThrowError(BadRequestException);
expect(userMock.delete).not.toHaveBeenCalled();
@@ -166,7 +166,7 @@ describe(UserAdminService.name, () => {
describe('restore', () => {
it('should throw error if user could not be found', async () => {
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(sut.restore(authStub.admin, userStub.admin.id)).rejects.toThrowError(BadRequestException);
expect(userMock.update).not.toHaveBeenCalled();
});
server/src/services/user.service.spec.ts
+4 -4
View File
@@ -33,7 +33,7 @@ describe(UserService.name, () => {
({ sut, albumMock, jobMock, storageMock, systemMock, userMock } = newTestService(UserService));
userMock.get.mockImplementation((userId) =>
Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? null),
Promise.resolve([userStub.admin, userStub.user1].find((user) => user.id === userId) ?? undefined),
);
});
@@ -81,7 +81,7 @@ describe(UserService.name, () => {
});
it('should throw an error if a user is not found', async () => {
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(sut.get(authStub.admin.user.id)).rejects.toBeInstanceOf(BadRequestException);
expect(userMock.get).toHaveBeenCalledWith(authStub.admin.user.id, { withDeleted: false });
});
@@ -100,7 +100,7 @@ describe(UserService.name, () => {
describe('createProfileImage', () => {
it('should throw an error if the user does not exist', async () => {
const file = { path: '/profile/path' } as Express.Multer.File;
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
userMock.update.mockResolvedValue({ ...userStub.admin, profileImagePath: file.path });
await expect(sut.createProfileImage(authStub.admin, file)).rejects.toThrowError(BadRequestException);
@@ -155,7 +155,7 @@ describe(UserService.name, () => {
describe('getUserProfileImage', () => {
it('should throw an error if the user does not exist', async () => {
userMock.get.mockResolvedValue(null);
userMock.get.mockResolvedValue(void 0);
await expect(sut.getProfileImage(userStub.admin.id)).rejects.toBeInstanceOf(BadRequestException);