fix: null validation (#25891)

This commit is contained in:
Jason Rasmussen
2026-02-04 12:27:52 -05:00
committed by GitHub
parent 440b3b4c6f
commit 9dddccd831
18 changed files with 357 additions and 47 deletions

View File

@@ -53,7 +53,7 @@ class AssetBulkUpdateDto {
///
String? description;
/// Duplicate asset ID
/// Duplicate ID
String? duplicateId;
/// Asset IDs to update

View File

@@ -19,6 +19,7 @@ class UserAdminCreateDto {
required this.name,
this.notify,
required this.password,
this.pinCode,
this.quotaSizeInBytes,
this.shouldChangePassword,
this.storageLabel,
@@ -54,6 +55,9 @@ class UserAdminCreateDto {
/// User password
String password;
/// PIN code
String? pinCode;
/// Storage quota in bytes
///
/// Minimum value: 0
@@ -79,6 +83,7 @@ class UserAdminCreateDto {
other.name == name &&
other.notify == notify &&
other.password == password &&
other.pinCode == pinCode &&
other.quotaSizeInBytes == quotaSizeInBytes &&
other.shouldChangePassword == shouldChangePassword &&
other.storageLabel == storageLabel;
@@ -92,12 +97,13 @@ class UserAdminCreateDto {
(name.hashCode) +
(notify == null ? 0 : notify!.hashCode) +
(password.hashCode) +
(pinCode == null ? 0 : pinCode!.hashCode) +
(quotaSizeInBytes == null ? 0 : quotaSizeInBytes!.hashCode) +
(shouldChangePassword == null ? 0 : shouldChangePassword!.hashCode) +
(storageLabel == null ? 0 : storageLabel!.hashCode);
@override
String toString() => 'UserAdminCreateDto[avatarColor=$avatarColor, email=$email, isAdmin=$isAdmin, name=$name, notify=$notify, password=$password, quotaSizeInBytes=$quotaSizeInBytes, shouldChangePassword=$shouldChangePassword, storageLabel=$storageLabel]';
String toString() => 'UserAdminCreateDto[avatarColor=$avatarColor, email=$email, isAdmin=$isAdmin, name=$name, notify=$notify, password=$password, pinCode=$pinCode, quotaSizeInBytes=$quotaSizeInBytes, shouldChangePassword=$shouldChangePassword, storageLabel=$storageLabel]';
Map<String, dynamic> toJson() {
final json = <String, dynamic>{};
@@ -119,6 +125,11 @@ class UserAdminCreateDto {
// json[r'notify'] = null;
}
json[r'password'] = this.password;
if (this.pinCode != null) {
json[r'pinCode'] = this.pinCode;
} else {
// json[r'pinCode'] = null;
}
if (this.quotaSizeInBytes != null) {
json[r'quotaSizeInBytes'] = this.quotaSizeInBytes;
} else {
@@ -152,6 +163,7 @@ class UserAdminCreateDto {
name: mapValueOfType<String>(json, r'name')!,
notify: mapValueOfType<bool>(json, r'notify'),
password: mapValueOfType<String>(json, r'password')!,
pinCode: mapValueOfType<String>(json, r'pinCode'),
quotaSizeInBytes: mapValueOfType<int>(json, r'quotaSizeInBytes'),
shouldChangePassword: mapValueOfType<bool>(json, r'shouldChangePassword'),
storageLabel: mapValueOfType<String>(json, r'storageLabel'),

View File

@@ -15760,7 +15760,7 @@
"type": "string"
},
"duplicateId": {
"description": "Duplicate asset ID",
"description": "Duplicate ID",
"nullable": true,
"type": "string"
},
@@ -19038,6 +19038,7 @@
"format": "uuid",
"type": "string"
},
"minItems": 1,
"type": "array"
}
},
@@ -19128,6 +19129,7 @@
"format": "uuid",
"type": "string"
},
"minItems": 1,
"type": "array"
},
"readAt": {
@@ -25069,6 +25071,12 @@
"description": "User password",
"type": "string"
},
"pinCode": {
"description": "PIN code",
"example": "123456",
"nullable": true,
"type": "string"
},
"quotaSizeInBytes": {
"description": "Storage quota in bytes",
"format": "int64",

View File

@@ -233,6 +233,8 @@ export type UserAdminCreateDto = {
notify?: boolean;
/** User password */
password: string;
/** PIN code */
pinCode?: string | null;
/** Storage quota in bytes */
quotaSizeInBytes?: number | null;
/** Require password change on next login */
@@ -822,7 +824,7 @@ export type AssetBulkUpdateDto = {
dateTimeRelative?: number;
/** Asset description */
description?: string;
/** Duplicate asset ID */
/** Duplicate ID */
duplicateId?: string | null;
/** Asset IDs to update */
ids: string[];

View File

@@ -24,6 +24,34 @@ describe(AssetController.name, () => {
await request(ctx.getHttpServer()).put(`/assets`);
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should require a valid uuid', async () => {
const { status, body } = await request(ctx.getHttpServer())
.put(`/assets`)
.send({ ids: ['123'] });
expect(status).toBe(400);
expect(body).toEqual(factory.responses.badRequest(['each value in ids must be a UUID']));
});
it('should require duplicateId to be a string', async () => {
const id = factory.uuid();
const { status, body } = await request(ctx.getHttpServer())
.put(`/assets`)
.send({ ids: [id], duplicateId: true });
expect(status).toBe(400);
expect(body).toEqual(factory.responses.badRequest(['duplicateId must be a string']));
});
it('should accept a null duplicateId', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer())
.put(`/assets`)
.send({ ids: [id], duplicateId: null });
expect(service.updateAll).toHaveBeenCalledWith(undefined, expect.objectContaining({ duplicateId: null }));
});
});
describe('DELETE /assets', () => {

View File

@@ -0,0 +1,36 @@
import { NotificationAdminController } from 'src/controllers/notification-admin.controller';
import { NotificationAdminService } from 'src/services/notification-admin.service';
import request from 'supertest';
import { factory } from 'test/small.factory';
import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
describe(NotificationAdminController.name, () => {
let ctx: ControllerContext;
const service = mockBaseService(NotificationAdminService);
beforeAll(async () => {
ctx = await controllerSetup(NotificationAdminController, [
{ provide: NotificationAdminService, useValue: service },
]);
return () => ctx.close();
});
beforeEach(() => {
service.resetAllMocks();
ctx.reset();
});
describe('POST /admin/notifications', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).post('/admin/notifications');
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should accept a null readAt', async () => {
await request(ctx.getHttpServer())
.post(`/admin/notifications`)
.send({ title: 'Test', userId: factory.uuid(), readAt: null });
expect(service.create).toHaveBeenCalledWith(undefined, expect.objectContaining({ readAt: null }));
});
});
});

View File

@@ -37,9 +37,33 @@ describe(NotificationController.name, () => {
describe('PUT /notifications', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).get('/notifications');
await request(ctx.getHttpServer()).put('/notifications');
expect(ctx.authenticate).toHaveBeenCalled();
});
describe('ids', () => {
it('should require a list', async () => {
const { status, body } = await request(ctx.getHttpServer()).put(`/notifications`).send({ ids: true });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(expect.arrayContaining(['ids must be an array'])));
});
it('should require uuids', async () => {
const { status, body } = await request(ctx.getHttpServer())
.put(`/notifications`)
.send({ ids: [true] });
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(['each value in ids must be a UUID']));
});
it('should accept valid uuids', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer())
.put(`/notifications`)
.send({ ids: [id] });
expect(service.updateAll).toHaveBeenCalledWith(undefined, expect.objectContaining({ ids: [id] }));
});
});
});
describe('GET /notifications/:id', () => {
@@ -60,5 +84,11 @@ describe(NotificationController.name, () => {
await request(ctx.getHttpServer()).put(`/notifications/${factory.uuid()}`).send({ readAt: factory.date() });
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should accept a null readAt', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer()).put(`/notifications/${id}`).send({ readAt: null });
expect(service.update).toHaveBeenCalledWith(undefined, id, expect.objectContaining({ readAt: null }));
});
});
});

View File

@@ -58,6 +58,11 @@ describe(PersonController.name, () => {
await request(ctx.getHttpServer()).post('/people').send({ birthDate: '' });
expect(service.create).toHaveBeenCalledWith(undefined, { birthDate: null });
});
it('should map an empty color to null', async () => {
await request(ctx.getHttpServer()).post('/people').send({ color: '' });
expect(service.create).toHaveBeenCalledWith(undefined, { color: null });
});
});
describe('DELETE /people', () => {

View File

@@ -0,0 +1,34 @@
import { SharedLinkController } from 'src/controllers/shared-link.controller';
import { SharedLinkType } from 'src/enum';
import { SharedLinkService } from 'src/services/shared-link.service';
import request from 'supertest';
import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
describe(SharedLinkController.name, () => {
let ctx: ControllerContext;
const service = mockBaseService(SharedLinkService);
beforeAll(async () => {
ctx = await controllerSetup(SharedLinkController, [{ provide: SharedLinkService, useValue: service }]);
return () => ctx.close();
});
beforeEach(() => {
service.resetAllMocks();
ctx.reset();
});
describe('POST /shared-links', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).post('/shared-links');
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should allow an null expiresAt', async () => {
await request(ctx.getHttpServer())
.post('/shared-links')
.send({ expiresAt: null, type: SharedLinkType.Individual });
expect(service.create).toHaveBeenCalledWith(undefined, expect.objectContaining({ expiresAt: null }));
});
});
});

View File

@@ -0,0 +1,73 @@
import { TagController } from 'src/controllers/tag.controller';
import { TagService } from 'src/services/tag.service';
import request from 'supertest';
import { errorDto } from 'test/medium/responses';
import { factory } from 'test/small.factory';
import { ControllerContext, controllerSetup, mockBaseService } from 'test/utils';
describe(TagController.name, () => {
let ctx: ControllerContext;
const service = mockBaseService(TagService);
beforeAll(async () => {
ctx = await controllerSetup(TagController, [{ provide: TagService, useValue: service }]);
return () => ctx.close();
});
beforeEach(() => {
service.resetAllMocks();
ctx.reset();
});
describe('GET /tags', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).get('/tags');
expect(ctx.authenticate).toHaveBeenCalled();
});
});
describe('POST /tags', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).post('/tags');
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should a null parentId', async () => {
await request(ctx.getHttpServer()).post(`/tags`).send({ name: 'tag', parentId: null });
expect(service.create).toHaveBeenCalledWith(undefined, expect.objectContaining({ parentId: null }));
});
});
describe('PUT /tags', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).put('/tags');
expect(ctx.authenticate).toHaveBeenCalled();
});
});
describe('GET /tags/:id', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).get(`/tags/${factory.uuid()}`);
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should require a valid uuid', async () => {
const { status, body } = await request(ctx.getHttpServer()).get(`/tags/123`);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest([expect.stringContaining('id must be a UUID')]));
});
});
describe('PUT /tags/:id', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).put(`/tags/${factory.uuid()}`);
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should allow setting a null color via an empty string', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer()).put(`/tags/${id}`).send({ color: '' });
expect(service.update).toHaveBeenCalledWith(undefined, id, expect.objectContaining({ color: null }));
});
});
});

View File

@@ -31,12 +31,55 @@ describe(UserAdminController.name, () => {
});
});
describe('PUT /admin/users/:id', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).put(`/admin/users/${factory.uuid()}`);
expect(ctx.authenticate).toHaveBeenCalled();
});
});
describe('POST /admin/users', () => {
it('should be an authenticated route', async () => {
await request(ctx.getHttpServer()).post('/admin/users');
expect(ctx.authenticate).toHaveBeenCalled();
});
it('should allow a null pinCode', async () => {
await request(ctx.getHttpServer()).post(`/admin/users`).send({
name: 'Test user',
email: 'test@immich.cloud',
password: 'password',
pinCode: null,
});
expect(service.create).toHaveBeenCalledWith(expect.objectContaining({ pinCode: null }));
});
it('should allow a null avatarColor', async () => {
await request(ctx.getHttpServer()).post(`/admin/users`).send({
name: 'Test user',
email: 'test@immich.cloud',
password: 'password',
avatarColor: null,
});
expect(service.create).toHaveBeenCalledWith(expect.objectContaining({ avatarColor: null }));
});
it(`should `, async () => {
const dto: UserAdminCreateDto = {
email: 'user@immich.app',
password: 'test',
name: 'Test User',
quotaSizeInBytes: 1.2,
};
const { status, body } = await request(ctx.getHttpServer())
.post(`/admin/users`)
.set('Authorization', `Bearer token`)
.send(dto);
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(expect.arrayContaining(['quotaSizeInBytes must be an integer number'])));
});
it(`should not allow decimal quota`, async () => {
const dto: UserAdminCreateDto = {
email: 'user@immich.app',
@@ -75,5 +118,17 @@ describe(UserAdminController.name, () => {
expect(status).toBe(400);
expect(body).toEqual(errorDto.badRequest(expect.arrayContaining(['quotaSizeInBytes must be an integer number'])));
});
it('should allow a null pinCode', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer()).put(`/admin/users/${id}`).send({ pinCode: null });
expect(service.update).toHaveBeenCalledWith(undefined, id, expect.objectContaining({ pinCode: null }));
});
it('should allow a null avatarColor', async () => {
const id = factory.uuid();
await request(ctx.getHttpServer()).put(`/admin/users/${id}`).send({ avatarColor: null });
expect(service.update).toHaveBeenCalledWith(undefined, id, expect.objectContaining({ avatarColor: null }));
});
});
});

View File

@@ -54,6 +54,14 @@ describe(UserController.name, () => {
expect(body).toEqual(errorDto.badRequest());
});
}
it('should allow an empty avatarColor', async () => {
await request(ctx.getHttpServer())
.put(`/users/me`)
.set('Authorization', `Bearer token`)
.send({ avatarColor: null });
expect(service.updateMe).toHaveBeenCalledWith(undefined, expect.objectContaining({ avatarColor: null }));
});
});
describe('GET /users/:id', () => {

View File

@@ -73,8 +73,7 @@ export class AssetBulkUpdateDto extends UpdateAssetBase {
@ValidateUUID({ each: true, description: 'Asset IDs to update' })
ids!: string[];
@ApiProperty({ description: 'Duplicate asset ID' })
@Optional()
@ValidateString({ optional: true, nullable: true, description: 'Duplicate ID' })
duplicateId?: string | null;
@ApiProperty({ description: 'Relative time offset in seconds' })

View File

@@ -1,7 +1,7 @@
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
import { IsString } from 'class-validator';
import { ArrayMinSize, IsString } from 'class-validator';
import { NotificationLevel, NotificationType } from 'src/enum';
import { Optional, ValidateBoolean, ValidateDate, ValidateEnum, ValidateUUID } from 'src/validation';
import { Optional, ValidateBoolean, ValidateDate, ValidateEnum, ValidateString, ValidateUUID } from 'src/validation';
export class TestEmailResponseDto {
@ApiProperty({ description: 'Email message ID' })
@@ -75,20 +75,17 @@ export class NotificationCreateDto {
@ValidateEnum({ enum: NotificationType, name: 'NotificationType', optional: true, description: 'Notification type' })
type?: NotificationType;
@ApiProperty({ description: 'Notification title' })
@IsString()
@ValidateString({ description: 'Notification title' })
title!: string;
@ApiPropertyOptional({ description: 'Notification description' })
@IsString()
@Optional({ nullable: true })
@ValidateString({ optional: true, nullable: true, description: 'Notification description' })
description?: string | null;
@ApiPropertyOptional({ description: 'Additional notification data' })
@Optional({ nullable: true })
data?: any;
@ValidateDate({ optional: true, description: 'Date when notification was read' })
@ValidateDate({ optional: true, nullable: true, description: 'Date when notification was read' })
readAt?: Date | null;
@ValidateUUID({ description: 'User ID to send notification to' })
@@ -96,20 +93,22 @@ export class NotificationCreateDto {
}
export class NotificationUpdateDto {
@ValidateDate({ optional: true, description: 'Date when notification was read' })
@ValidateDate({ optional: true, nullable: true, description: 'Date when notification was read' })
readAt?: Date | null;
}
export class NotificationUpdateAllDto {
@ValidateUUID({ each: true, optional: true, description: 'Notification IDs to update' })
@ValidateUUID({ each: true, description: 'Notification IDs to update' })
@ArrayMinSize(1)
ids!: string[];
@ValidateDate({ optional: true, description: 'Date when notifications were read' })
@ValidateDate({ optional: true, nullable: true, description: 'Date when notifications were read' })
readAt?: Date | null;
}
export class NotificationDeleteAllDto {
@ValidateUUID({ each: true, description: 'Notification IDs to delete' })
@ArrayMinSize(1)
ids!: string[];
}

View File

@@ -44,7 +44,7 @@ export class SharedLinkCreateDto {
@IsString()
slug?: string | null;
@ValidateDate({ optional: true, description: 'Expiration date' })
@ValidateDate({ optional: true, nullable: true, description: 'Expiration date' })
expiresAt?: Date | null = null;
@ValidateBoolean({ optional: true, description: 'Allow uploads' })

View File

@@ -9,7 +9,7 @@ export class TagCreateDto {
@IsNotEmpty()
name!: string;
@ValidateUUID({ optional: true, description: 'Parent tag ID' })
@ValidateUUID({ nullable: true, optional: true, description: 'Parent tag ID' })
parentId?: string | null;
@ApiPropertyOptional({ description: 'Tag color (hex)' })
@@ -20,7 +20,7 @@ export class TagCreateDto {
export class TagUpdateDto {
@ApiPropertyOptional({ description: 'Tag color (hex)' })
@Optional({ emptyToNull: true })
@Optional({ nullable: true, emptyToNull: true })
@ValidateHexColor()
color?: string | null;
}

View File

@@ -26,7 +26,13 @@ export class UserUpdateMeDto {
@IsNotEmpty()
name?: string;
@ValidateEnum({ enum: UserAvatarColor, name: 'UserAvatarColor', optional: true, description: 'Avatar color' })
@ValidateEnum({
enum: UserAvatarColor,
name: 'UserAvatarColor',
optional: true,
nullable: true,
description: 'Avatar color',
})
avatarColor?: UserAvatarColor | null;
}
@@ -96,9 +102,19 @@ export class UserAdminCreateDto {
@IsString()
name!: string;
@ValidateEnum({ enum: UserAvatarColor, name: 'UserAvatarColor', optional: true, description: 'Avatar color' })
@ValidateEnum({
enum: UserAvatarColor,
name: 'UserAvatarColor',
optional: true,
nullable: true,
description: 'Avatar color',
})
avatarColor?: UserAvatarColor | null;
@ApiPropertyOptional({ description: 'PIN code' })
@PinCode({ optional: true, nullable: true, emptyToNull: true })
pinCode?: string | null;
@ApiPropertyOptional({ description: 'Storage label' })
@Optional({ nullable: true })
@IsString()
@@ -135,7 +151,7 @@ export class UserAdminUpdateDto {
password?: string;
@ApiPropertyOptional({ description: 'PIN code' })
@PinCode({ optional: true, emptyToNull: true })
@PinCode({ optional: true, nullable: true, emptyToNull: true })
pinCode?: string | null;
@ApiPropertyOptional({ description: 'User name' })
@@ -144,7 +160,13 @@ export class UserAdminUpdateDto {
@IsNotEmpty()
name?: string;
@ValidateEnum({ enum: UserAvatarColor, name: 'UserAvatarColor', optional: true, description: 'Avatar color' })
@ValidateEnum({
enum: UserAvatarColor,
name: 'UserAvatarColor',
optional: true,
nullable: true,
description: 'Avatar color',
})
avatarColor?: UserAvatarColor | null;
@ApiPropertyOptional({ description: 'Storage label' })

View File

@@ -232,19 +232,20 @@ export const ValidateHexColor = () => {
return applyDecorators(...decorators);
};
type DateOptions = { optional?: boolean; nullable?: boolean; format?: 'date' | 'date-time' };
type DateOptions = OptionalOptions & { optional?: boolean; format?: 'date' | 'date-time' };
export const ValidateDate = (options?: DateOptions & ApiPropertyOptions) => {
const { optional, nullable, format, ...apiPropertyOptions } = {
optional: false,
nullable: false,
format: 'date-time',
...options,
};
const {
optional,
nullable = false,
emptyToNull = false,
format = 'date-time',
...apiPropertyOptions
} = options || {};
const decorators = [
return applyDecorators(
ApiProperty({ format, ...apiPropertyOptions }),
IsDate(),
optional ? Optional({ nullable: true }) : IsNotEmpty(),
optional ? Optional({ nullable, emptyToNull }) : IsNotEmpty(),
Transform(({ key, value }) => {
if (value === null || value === undefined) {
return value;
@@ -256,19 +257,17 @@ export const ValidateDate = (options?: DateOptions & ApiPropertyOptions) => {
return new Date(value as string);
}),
];
if (optional) {
decorators.push(Optional({ nullable }));
}
return applyDecorators(...decorators);
);
};
type StringOptions = { optional?: boolean; nullable?: boolean; trim?: boolean };
type StringOptions = OptionalOptions & { optional?: boolean; trim?: boolean };
export const ValidateString = (options?: StringOptions & ApiPropertyOptions) => {
const { optional, nullable, trim, ...apiPropertyOptions } = options || {};
const decorators = [ApiProperty(apiPropertyOptions), IsString(), optional ? Optional({ nullable }) : IsNotEmpty()];
const { optional, nullable, emptyToNull, trim, ...apiPropertyOptions } = options || {};
const decorators = [
ApiProperty(apiPropertyOptions),
IsString(),
optional ? Optional({ nullable, emptyToNull }) : IsNotEmpty(),
];
if (trim) {
decorators.push(Transform(({ value }: { value: string }) => value?.trim()));
@@ -277,9 +276,9 @@ export const ValidateString = (options?: StringOptions & ApiPropertyOptions) =>
return applyDecorators(...decorators);
};
type BooleanOptions = { optional?: boolean; nullable?: boolean };
type BooleanOptions = OptionalOptions & { optional?: boolean };
export const ValidateBoolean = (options?: BooleanOptions & PropertyOptions) => {
const { optional, nullable, ...apiPropertyOptions } = options || {};
const { optional, nullable, emptyToNull, ...apiPropertyOptions } = options || {};
const decorators = [
Property(apiPropertyOptions),
IsBoolean(),
@@ -291,7 +290,7 @@ export const ValidateBoolean = (options?: BooleanOptions & PropertyOptions) => {
}
return value;
}),
optional ? Optional({ nullable }) : IsNotEmpty(),
optional ? Optional({ nullable, emptyToNull }) : IsNotEmpty(),
];
return applyDecorators(...decorators);