From 5c33eb320408e6f692278dc2cfcf89b6e249b8fb Mon Sep 17 00:00:00 2001 From: Timon Date: Thu, 4 Jun 2026 00:16:53 +0200 Subject: [PATCH] refactor(server)!: drop empty string to null conversion (#28808) refactor(server): drop empty string to null conversion --- e2e/src/specs/server/api/search.e2e-spec.ts | 34 ------------------- .../src/controllers/person.controller.spec.ts | 16 --------- server/src/controllers/tag.controller.spec.ts | 6 ---- server/src/dtos/person.dto.ts | 10 +++--- server/src/dtos/search.dto.ts | 14 ++++---- server/src/dtos/shared-link.dto.ts | 14 ++++---- server/src/dtos/tag.dto.ts | 6 ++-- server/src/dtos/user.dto.ts | 12 ++----- server/src/validation.ts | 12 ------- 9 files changed, 26 insertions(+), 98 deletions(-) diff --git a/e2e/src/specs/server/api/search.e2e-spec.ts b/e2e/src/specs/server/api/search.e2e-spec.ts index 09d33b735b..0b86053f78 100644 --- a/e2e/src/specs/server/api/search.e2e-spec.ts +++ b/e2e/src/specs/server/api/search.e2e-spec.ts @@ -259,17 +259,6 @@ describe('/search', () => { assets: [assetHeic], }), }, - { - should: "should search city ('')", - deferred: () => ({ - dto: { - city: '', - visibility: AssetVisibility.Timeline, - includeNull: true, - }, - assets: [assetLast], - }), - }, { should: 'should search city (null)', deferred: () => ({ @@ -291,18 +280,6 @@ describe('/search', () => { assets: [assetDensity], }), }, - { - should: "should search state ('')", - deferred: () => ({ - dto: { - state: '', - visibility: AssetVisibility.Timeline, - withExif: true, - includeNull: true, - }, - assets: [assetLast, assetNotocactus], - }), - }, { should: 'should search state (null)', deferred: () => ({ @@ -324,17 +301,6 @@ describe('/search', () => { assets: [assetFalcon], }), }, - { - should: "should search country ('')", - deferred: () => ({ - dto: { - country: '', - visibility: AssetVisibility.Timeline, - includeNull: true, - }, - assets: [assetLast], - }), - }, { should: 'should search country (null)', deferred: () => ({ diff --git a/server/src/controllers/person.controller.spec.ts b/server/src/controllers/person.controller.spec.ts index cf3a5e56b0..64fe4f3554 100644 --- a/server/src/controllers/person.controller.spec.ts +++ b/server/src/controllers/person.controller.spec.ts @@ -53,16 +53,6 @@ describe(PersonController.name, () => { await request(ctx.getHttpServer()).post('/people'); expect(ctx.authenticate).toHaveBeenCalled(); }); - - it('should map an empty birthDate to null', async () => { - 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', () => { @@ -153,12 +143,6 @@ describe(PersonController.name, () => { ); }); - it('should map an empty birthDate to null', async () => { - const id = factory.uuid(); - await request(ctx.getHttpServer()).put(`/people/${id}`).send({ birthDate: '' }); - expect(service.update).toHaveBeenCalledWith(undefined, id, { birthDate: null }); - }); - it('should not accept an invalid birth date (false)', async () => { const { status, body } = await request(ctx.getHttpServer()) .put(`/people/${factory.uuid()}`) diff --git a/server/src/controllers/tag.controller.spec.ts b/server/src/controllers/tag.controller.spec.ts index 907e99bb43..c2a2de95bd 100644 --- a/server/src/controllers/tag.controller.spec.ts +++ b/server/src/controllers/tag.controller.spec.ts @@ -63,11 +63,5 @@ describe(TagController.name, () => { 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 })); - }); }); }); diff --git a/server/src/dtos/person.dto.ts b/server/src/dtos/person.dto.ts index f39cfd1c88..dcbbc677b9 100644 --- a/server/src/dtos/person.dto.ts +++ b/server/src/dtos/person.dto.ts @@ -9,20 +9,22 @@ import { AssetFaceTable } from 'src/schema/tables/asset-face.table'; import { ImageDimensions, MaybeDehydrated } from 'src/types'; import { asBirthDateString, asDateString } from 'src/utils/date'; import { transformFaceBoundingBox } from 'src/utils/transform'; -import { emptyStringToNull, hexColor, stringToBool } from 'src/validation'; +import { hexColor, stringToBool } from 'src/validation'; import z from 'zod'; const PersonCreateSchema = z .object({ name: z.string().optional().describe('Person name'), - // Note: the mobile app cannot currently set the birth date to null. - birthDate: emptyStringToNull(z.string().meta({ format: 'date' }).nullable()) + birthDate: z + .string() + .meta({ format: 'date' }) + .nullable() .optional() .refine((val) => (val ? new Date(val) <= new Date() : true), { error: 'Birth date cannot be in the future' }) .describe('Person date of birth'), isHidden: z.boolean().optional().describe('Person visibility (hidden)'), isFavorite: z.boolean().optional().describe('Mark as favorite'), - color: emptyStringToNull(hexColor.nullable()).optional().describe('Person color (hex)'), + color: hexColor.nullable().optional().describe('Person color (hex)'), }) .meta({ id: 'PersonCreateDto' }); diff --git a/server/src/dtos/search.dto.ts b/server/src/dtos/search.dto.ts index c9a92b165f..39276bffe0 100644 --- a/server/src/dtos/search.dto.ts +++ b/server/src/dtos/search.dto.ts @@ -4,7 +4,7 @@ import { HistoryBuilder } from 'src/decorators'; import { AlbumResponseSchema } from 'src/dtos/album.dto'; import { AssetResponseSchema } from 'src/dtos/asset-response.dto'; import { AssetOrder, AssetOrderSchema, AssetTypeSchema, AssetVisibilitySchema } from 'src/enum'; -import { emptyStringToNull, isoDatetimeToDate, stringToBool } from 'src/validation'; +import { isoDatetimeToDate, stringToBool } from 'src/validation'; import z from 'zod'; const BaseSearchSchema = z.object({ @@ -23,12 +23,12 @@ const BaseSearchSchema = z.object({ trashedAfter: isoDatetimeToDate.optional().describe('Filter by trash date (after)'), takenBefore: isoDatetimeToDate.optional().describe('Filter by taken date (before)'), takenAfter: isoDatetimeToDate.optional().describe('Filter by taken date (after)'), - city: emptyStringToNull(z.string().nullable()).optional().describe('Filter by city name'), - state: emptyStringToNull(z.string().nullable()).optional().describe('Filter by state/province name'), - country: emptyStringToNull(z.string().nullable()).optional().describe('Filter by country name'), - make: emptyStringToNull(z.string().nullable()).optional().describe('Filter by camera make'), - model: emptyStringToNull(z.string().nullable()).optional().describe('Filter by camera model'), - lensModel: emptyStringToNull(z.string().nullable()).optional().describe('Filter by lens model'), + city: z.string().nullable().optional().describe('Filter by city name'), + state: z.string().nullable().optional().describe('Filter by state/province name'), + country: z.string().nullable().optional().describe('Filter by country name'), + make: z.string().nullable().optional().describe('Filter by camera make'), + model: z.string().nullable().optional().describe('Filter by camera model'), + lensModel: z.string().nullable().optional().describe('Filter by lens model'), isNotInAlbum: z.boolean().optional().describe('Filter assets not in any album'), personIds: z.array(z.uuidv4()).optional().describe('Filter by person IDs'), tagIds: z.array(z.uuidv4()).nullish().describe('Filter by tag IDs'), diff --git a/server/src/dtos/shared-link.dto.ts b/server/src/dtos/shared-link.dto.ts index 2e466c5014..f6253cf7b1 100644 --- a/server/src/dtos/shared-link.dto.ts +++ b/server/src/dtos/shared-link.dto.ts @@ -4,7 +4,7 @@ import { HistoryBuilder } from 'src/decorators'; import { AlbumResponseSchema, mapAlbum } from 'src/dtos/album.dto'; import { AssetResponseSchema, mapAsset } from 'src/dtos/asset-response.dto'; import { SharedLinkTypeSchema } from 'src/enum'; -import { emptyStringToNull, isoDatetimeToDate } from 'src/validation'; +import { isoDatetimeToDate } from 'src/validation'; import z from 'zod'; const SharedLinkSearchSchema = z @@ -23,9 +23,9 @@ const SharedLinkCreateSchema = z type: SharedLinkTypeSchema, assetIds: z.array(z.uuidv4()).optional().describe('Asset IDs (for individual assets)'), albumId: z.uuidv4().optional().describe('Album ID (for album sharing)'), - description: emptyStringToNull(z.string().nullable()).optional().describe('Link description'), - password: emptyStringToNull(z.string().nullable()).optional().describe('Link password'), - slug: emptyStringToNull(z.string().nullable()).optional().describe('Custom URL slug'), + description: z.string().nullable().optional().describe('Link description'), + password: z.string().nullable().optional().describe('Link password'), + slug: z.string().nullable().optional().describe('Custom URL slug'), expiresAt: isoDatetimeToDate.nullable().describe('Expiration date').default(null).optional(), allowUpload: z.boolean().optional().describe('Allow uploads'), allowDownload: z.boolean().default(true).optional().describe('Allow downloads'), @@ -35,9 +35,9 @@ const SharedLinkCreateSchema = z const SharedLinkEditSchema = z .object({ - description: emptyStringToNull(z.string().nullable()).optional().describe('Link description'), - password: emptyStringToNull(z.string().nullable()).optional().describe('Link password'), - slug: emptyStringToNull(z.string().nullable()).optional().describe('Custom URL slug'), + description: z.string().nullable().optional().describe('Link description'), + password: z.string().nullable().optional().describe('Link password'), + slug: z.string().nullable().optional().describe('Custom URL slug'), expiresAt: isoDatetimeToDate.nullish().describe('Expiration date'), allowUpload: z.boolean().optional().describe('Allow uploads'), allowDownload: z.boolean().optional().describe('Allow downloads'), diff --git a/server/src/dtos/tag.dto.ts b/server/src/dtos/tag.dto.ts index 67dbca9914..2fd860db13 100644 --- a/server/src/dtos/tag.dto.ts +++ b/server/src/dtos/tag.dto.ts @@ -2,20 +2,20 @@ import { createZodDto } from 'nestjs-zod'; import { Tag } from 'src/database'; import { MaybeDehydrated } from 'src/types'; import { asDateString } from 'src/utils/date'; -import { emptyStringToNull, hexColor } from 'src/validation'; +import { hexColor } from 'src/validation'; import z from 'zod'; const TagCreateSchema = z .object({ name: z.string().describe('Tag name'), parentId: z.uuidv4().nullish().describe('Parent tag ID'), - color: emptyStringToNull(hexColor.nullable()).optional().describe('Tag color (hex)'), + color: hexColor.nullable().optional().describe('Tag color (hex)'), }) .meta({ id: 'TagCreateDto' }); const TagUpdateSchema = z .object({ - color: emptyStringToNull(hexColor.nullable()).optional().describe('Tag color (hex)'), + color: hexColor.nullable().optional().describe('Tag color (hex)'), }) .meta({ id: 'TagUpdateDto' }); diff --git a/server/src/dtos/user.dto.ts b/server/src/dtos/user.dto.ts index 75256b9e1a..8d71db6618 100644 --- a/server/src/dtos/user.dto.ts +++ b/server/src/dtos/user.dto.ts @@ -4,7 +4,7 @@ import { pinCodeRegex } from 'src/dtos/auth.dto'; import { UserAvatarColor, UserAvatarColorSchema, UserMetadataKey, UserStatusSchema } from 'src/enum'; import { MaybeDehydrated, UserMetadataItem } from 'src/types'; import { asDateString } from 'src/utils/date'; -import { emptyStringToNull, isoDatetimeToDate, sanitizeFilename, stringToBool, toEmail } from 'src/validation'; +import { isoDatetimeToDate, sanitizeFilename, stringToBool, toEmail } from 'src/validation'; import z from 'zod'; export const UserUpdateMeSchema = z @@ -80,10 +80,7 @@ export const UserAdminCreateSchema = z password: z.string().describe('User password'), name: z.string().describe('User name'), avatarColor: UserAvatarColorSchema.nullish(), - pinCode: emptyStringToNull(z.string().regex(pinCodeRegex).nullable()) - .optional() - .describe('PIN code') - .meta({ example: '123456' }), + pinCode: z.string().regex(pinCodeRegex).nullable().optional().describe('PIN code').meta({ example: '123456' }), storageLabel: z.string().pipe(sanitizeFilename).nullish().describe('Storage label'), quotaSizeInBytes: z.int().min(0).nullish().describe('Storage quota in bytes'), shouldChangePassword: z.boolean().optional().describe('Require password change on next login'), @@ -98,10 +95,7 @@ const UserAdminUpdateSchema = z .object({ email: toEmail.optional().describe('User email'), password: z.string().optional().describe('User password'), - pinCode: emptyStringToNull(z.string().regex(pinCodeRegex).nullable()) - .optional() - .describe('PIN code') - .meta({ example: '123456' }), + pinCode: z.string().regex(pinCodeRegex).nullable().optional().describe('PIN code').meta({ example: '123456' }), name: z.string().optional().describe('User name'), avatarColor: UserAvatarColorSchema.nullish(), storageLabel: z.string().pipe(sanitizeFilename).nullish().describe('Storage label'), diff --git a/server/src/validation.ts b/server/src/validation.ts index 59131b3abe..97d4b71964 100644 --- a/server/src/validation.ts +++ b/server/src/validation.ts @@ -251,16 +251,4 @@ export const hexColor = z .regex(hexColorRegex) .transform((val) => (val.startsWith('#') ? val : `#${val}`)); -/** - * Transform empty strings to null. Inner schema passed to this function must accept null. - * @docs https://zod.dev/api?id=preprocess - * @example emptyStringToNull(z.string().nullable()).optional() // [encouraged] final schema is optional - * @example emptyStringToNull(z.string().nullable()) // [encouraged] same as the one above, but final schema is not optional - * @example emptyStringToNull(z.string().nullish()) // [discouraged] same as the one above, might be confusing - * @example emptyStringToNull(z.string().optional()) // fails: string schema rejects null - * @example emptyStringToNull(z.string().nullable()).nullish() // [discouraged] passes, null is duplicated. use the first example instead - */ -export const emptyStringToNull = (schema: T) => - z.preprocess((val) => (val === '' ? null : val), schema); - export const sanitizeFilename = z.string().transform((val) => sanitize(val.replaceAll('.', '')));