feat(server,web): make user deletion delay configurable (#7663)

* feat(server,web): make user deletion delay configurable

* alphabetical order

* add min for user.deleteDelay in SettingInputField

* make config.user.deleteDelay SettingInputField min consistent format

* fix e2e test

* update description on user delete delay
This commit is contained in:
Sam Holton
2024-03-06 00:45:40 -05:00
committed by GitHub
parent 52dfe5fc92
commit 9125999d1a
33 changed files with 366 additions and 16 deletions

View File

@@ -88,6 +88,8 @@ export class ServerConfigDto {
loginPageMessage!: string;
@ApiProperty({ type: 'integer' })
trashDays!: number;
@ApiProperty({ type: 'integer' })
userDeleteDelay!: number;
isInitialized!: boolean;
isOnboarded!: boolean;
externalDomain!: string;

View File

@@ -196,6 +196,7 @@ describe(ServerInfoService.name, () => {
loginPageMessage: '',
oauthButtonText: 'Login with OAuth',
trashDays: 30,
userDeleteDelay: 7,
isInitialized: undefined,
isOnboarded: false,
externalDomain: '',

View File

@@ -96,6 +96,7 @@ export class ServerInfoService {
return {
loginPageMessage: config.server.loginPageMessage,
trashDays: config.trash.days,
userDeleteDelay: config.user.deleteDelay,
oauthButtonText: config.oauth.buttonText,
isInitialized,
isOnboarded: onboarding?.isOnboarded || false,

View File

@@ -0,0 +1,11 @@
import { ApiProperty } from '@nestjs/swagger';
import { Type } from 'class-transformer';
import { IsInt, Min } from 'class-validator';
export class SystemConfigUserDto {
@IsInt()
@Min(1)
@Type(() => Number)
@ApiProperty({ type: 'integer' })
deleteDelay!: number;
}

View File

@@ -16,6 +16,7 @@ import { SystemConfigStorageTemplateDto } from './system-config-storage-template
import { SystemConfigThemeDto } from './system-config-theme.dto';
import { SystemConfigThumbnailDto } from './system-config-thumbnail.dto';
import { SystemConfigTrashDto } from './system-config-trash.dto';
import { SystemConfigUserDto } from './system-config-user.dto';
export class SystemConfigDto implements SystemConfig {
@Type(() => SystemConfigFFmpegDto)
@@ -92,6 +93,11 @@ export class SystemConfigDto implements SystemConfig {
@ValidateNested()
@IsObject()
server!: SystemConfigServerDto;
@Type(() => SystemConfigUserDto)
@ValidateNested()
@IsObject()
user!: SystemConfigUserDto;
}
export function mapConfig(config: SystemConfig): SystemConfigDto {

View File

@@ -140,6 +140,9 @@ export const defaults = Object.freeze<SystemConfig>({
externalDomain: '',
loginPageMessage: '',
},
user: {
deleteDelay: 7,
},
});
export enum FeatureFlag {

View File

@@ -23,6 +23,7 @@ const updates: SystemConfigEntity[] = [
{ key: SystemConfigKey.FFMPEG_CRF, value: 30 },
{ key: SystemConfigKey.OAUTH_AUTO_LAUNCH, value: true },
{ key: SystemConfigKey.TRASH_DAYS, value: 10 },
{ key: SystemConfigKey.USER_DELETE_DELAY, value: 15 },
];
const updatedConfig = Object.freeze<SystemConfig>({
@@ -140,6 +141,9 @@ const updatedConfig = Object.freeze<SystemConfig>({
enabled: false,
},
},
user: {
deleteDelay: 15,
},
});
describe(SystemConfigService.name, () => {
@@ -199,6 +203,7 @@ describe(SystemConfigService.name, () => {
{ key: SystemConfigKey.FFMPEG_CRF, value: 30 },
{ key: SystemConfigKey.OAUTH_AUTO_LAUNCH, value: true },
{ key: SystemConfigKey.TRASH_DAYS, value: 10 },
{ key: SystemConfigKey.USER_DELETE_DELAY, value: 15 },
]);
await expect(sut.getConfig()).resolves.toEqual(updatedConfig);
@@ -206,7 +211,12 @@ describe(SystemConfigService.name, () => {
it('should load the config from a file', async () => {
process.env.IMMICH_CONFIG_FILE = 'immich-config.json';
const partialConfig = { ffmpeg: { crf: 30 }, oauth: { autoLaunch: true }, trash: { days: 10 } };
const partialConfig = {
ffmpeg: { crf: 30 },
oauth: { autoLaunch: true },
trash: { days: 10 },
user: { deleteDelay: 15 },
};
configMock.readFile.mockResolvedValue(JSON.stringify(partialConfig));
await expect(sut.getConfig()).resolves.toEqual(updatedConfig);

View File

@@ -13,7 +13,9 @@ import {
newJobRepositoryMock,
newLibraryRepositoryMock,
newStorageRepositoryMock,
newSystemConfigRepositoryMock,
newUserRepositoryMock,
systemConfigStub,
userStub,
} from '@test';
import { when } from 'jest-when';
@@ -26,6 +28,7 @@ import {
IJobRepository,
ILibraryRepository,
IStorageRepository,
ISystemConfigRepository,
IUserRepository,
} from '../repositories';
import { UpdateUserDto } from './dto/update-user.dto';
@@ -48,17 +51,28 @@ describe(UserService.name, () => {
let jobMock: jest.Mocked<IJobRepository>;
let libraryMock: jest.Mocked<ILibraryRepository>;
let storageMock: jest.Mocked<IStorageRepository>;
let configMock: jest.Mocked<ISystemConfigRepository>;
beforeEach(() => {
albumMock = newAlbumRepositoryMock();
assetMock = newAssetRepositoryMock();
configMock = newSystemConfigRepositoryMock();
cryptoRepositoryMock = newCryptoRepositoryMock();
jobMock = newJobRepositoryMock();
libraryMock = newLibraryRepositoryMock();
storageMock = newStorageRepositoryMock();
userMock = newUserRepositoryMock();
sut = new UserService(albumMock, assetMock, cryptoRepositoryMock, jobMock, libraryMock, storageMock, userMock);
sut = new UserService(
albumMock,
assetMock,
cryptoRepositoryMock,
jobMock,
libraryMock,
storageMock,
configMock,
userMock,
);
when(userMock.get).calledWith(authStub.admin.user.id, {}).mockResolvedValue(userStub.admin);
when(userMock.get).calledWith(authStub.admin.user.id, { withDeleted: true }).mockResolvedValue(userStub.admin);
@@ -461,6 +475,22 @@ describe(UserService.name, () => {
expect(jobMock.queueAll).toHaveBeenCalledWith([]);
});
it('should skip users not ready for deletion - deleteDelay30', async () => {
configMock.load.mockResolvedValue(systemConfigStub.deleteDelay30);
userMock.getDeletedUsers.mockResolvedValue([
{},
{ deletedAt: undefined },
{ deletedAt: null },
{ deletedAt: makeDeletedAt(15) },
] as UserEntity[]);
await sut.handleUserDeleteCheck();
expect(userMock.getDeletedUsers).toHaveBeenCalled();
expect(jobMock.queue).not.toHaveBeenCalled();
expect(jobMock.queueAll).toHaveBeenCalledWith([]);
});
it('should queue user ready for deletion', async () => {
const user = { id: 'deleted-user', deletedAt: makeDeletedAt(10) };
userMock.getDeletedUsers.mockResolvedValue([user] as UserEntity[]);
@@ -470,6 +500,16 @@ describe(UserService.name, () => {
expect(userMock.getDeletedUsers).toHaveBeenCalled();
expect(jobMock.queueAll).toHaveBeenCalledWith([{ name: JobName.USER_DELETION, data: { id: user.id } }]);
});
it('should queue user ready for deletion - deleteDelay30', async () => {
const user = { id: 'deleted-user', deletedAt: makeDeletedAt(31) };
userMock.getDeletedUsers.mockResolvedValue([user] as UserEntity[]);
await sut.handleUserDeleteCheck();
expect(userMock.getDeletedUsers).toHaveBeenCalled();
expect(jobMock.queueAll).toHaveBeenCalledWith([{ name: JobName.USER_DELETION, data: { id: user.id } }]);
});
});
describe('handleUserDelete', () => {

View File

@@ -13,16 +13,19 @@ import {
IJobRepository,
ILibraryRepository,
IStorageRepository,
ISystemConfigRepository,
IUserRepository,
UserFindOptions,
} from '../repositories';
import { StorageCore, StorageFolder } from '../storage';
import { SystemConfigCore } from '../system-config/system-config.core';
import { CreateUserDto, UpdateUserDto } from './dto';
import { CreateProfileImageResponseDto, UserResponseDto, mapCreateProfileImageResponse, mapUser } from './response-dto';
import { UserCore } from './user.core';
@Injectable()
export class UserService {
private configCore: SystemConfigCore;
private logger = new ImmichLogger(UserService.name);
private userCore: UserCore;
@@ -33,9 +36,11 @@ export class UserService {
@Inject(IJobRepository) private jobRepository: IJobRepository,
@Inject(ILibraryRepository) libraryRepository: ILibraryRepository,
@Inject(IStorageRepository) private storageRepository: IStorageRepository,
@Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository,
@Inject(IUserRepository) private userRepository: IUserRepository,
) {
this.userCore = UserCore.create(cryptoRepository, libraryRepository, userRepository);
this.configCore = SystemConfigCore.create(configRepository);
}
async getAll(auth: AuthDto, isAll: boolean): Promise<UserResponseDto[]> {
@@ -140,22 +145,26 @@ export class UserService {
async handleUserDeleteCheck() {
const users = await this.userRepository.getDeletedUsers();
const config = await this.configCore.getConfig();
await this.jobRepository.queueAll(
users.flatMap((user) =>
this.isReadyForDeletion(user) ? [{ name: JobName.USER_DELETION, data: { id: user.id } }] : [],
this.isReadyForDeletion(user, config.user.deleteDelay)
? [{ name: JobName.USER_DELETION, data: { id: user.id } }]
: [],
),
);
return true;
}
async handleUserDelete({ id }: IEntityJob) {
const config = await this.configCore.getConfig();
const user = await this.userRepository.get(id, { withDeleted: true });
if (!user) {
return false;
}
// just for extra protection here
if (!this.isReadyForDeletion(user)) {
if (!this.isReadyForDeletion(user, config.user.deleteDelay)) {
this.logger.warn(`Skipped user that was not ready for deletion: id=${id}`);
return false;
}
@@ -184,12 +193,12 @@ export class UserService {
return true;
}
private isReadyForDeletion(user: UserEntity): boolean {
private isReadyForDeletion(user: UserEntity, deleteDelay: number): boolean {
if (!user.deletedAt) {
return false;
}
return DateTime.now().minus({ days: 7 }) > DateTime.fromJSDate(user.deletedAt);
return DateTime.now().minus({ days: deleteDelay }) > DateTime.fromJSDate(user.deletedAt);
}
private async findOrFail(id: string, options: UserFindOptions) {

View File

@@ -108,6 +108,8 @@ export enum SystemConfigKey {
TRASH_DAYS = 'trash.days',
THEME_CUSTOM_CSS = 'theme.customCss',
USER_DELETE_DELAY = 'user.deleteDelay',
}
export enum TranscodePolicy {
@@ -276,4 +278,7 @@ export interface SystemConfig {
externalDomain: string;
loginPageMessage: string;
};
user: {
deleteDelay: number;
};
}