From 144a57ddffd34a5edd8d0b5d71092e365de80f5b Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:51:00 +0100 Subject: [PATCH] refactor(server): use helpers for shared link queries (#27088) * fix(server): prevent album shared link from breaking after uploads * update test * add withSharedAssets helper * remove options * add more helpers * update selects --- server/src/queries/shared.link.repository.sql | 174 +++++++++++++----- .../repositories/shared-link.repository.ts | 154 +++++----------- .../services/shared-link.service.spec.ts | 37 ++++ 3 files changed, 212 insertions(+), 153 deletions(-) diff --git a/server/src/queries/shared.link.repository.sql b/server/src/queries/shared.link.repository.sql index f002110735..e1177bba28 100644 --- a/server/src/queries/shared.link.repository.sql +++ b/server/src/queries/shared.link.repository.sql @@ -3,37 +3,64 @@ -- SharedLinkRepository.get select "shared_link".*, - coalesce( - json_agg("a") filter ( - where - "a"."id" is not null - ), - '[]' + ( + select + coalesce(json_agg(agg), '[]') + from + ( + select + "asset".*, + to_json("exifInfo") as "exifInfo" + from + "shared_link_asset" + inner join "asset" on "asset"."id" = "shared_link_asset"."assetId" + inner join lateral ( + select + "asset_exif"."assetId", + "asset_exif"."autoStackId", + "asset_exif"."bitsPerSample", + "asset_exif"."city", + "asset_exif"."colorspace", + "asset_exif"."country", + "asset_exif"."dateTimeOriginal", + "asset_exif"."description", + "asset_exif"."exifImageHeight", + "asset_exif"."exifImageWidth", + "asset_exif"."exposureTime", + "asset_exif"."fileSizeInByte", + "asset_exif"."fNumber", + "asset_exif"."focalLength", + "asset_exif"."fps", + "asset_exif"."iso", + "asset_exif"."latitude", + "asset_exif"."lensModel", + "asset_exif"."livePhotoCID", + "asset_exif"."longitude", + "asset_exif"."make", + "asset_exif"."model", + "asset_exif"."modifyDate", + "asset_exif"."orientation", + "asset_exif"."profileDescription", + "asset_exif"."projectionType", + "asset_exif"."rating", + "asset_exif"."state", + "asset_exif"."tags", + "asset_exif"."timeZone" + from + "asset_exif" + where + "asset_exif"."assetId" = "asset"."id" + ) as "exifInfo" on true + where + "shared_link"."id" = "shared_link_asset"."sharedLinkId" + and "asset"."deletedAt" is null + order by + "asset"."fileCreatedAt" asc + ) as agg ) as "assets", to_json("album") as "album" from "shared_link" - left join lateral ( - select - "asset".*, - to_json("exifInfo") as "exifInfo" - from - "shared_link_asset" - inner join "asset" on "asset"."id" = "shared_link_asset"."assetId" - inner join lateral ( - select - "asset_exif".* - from - "asset_exif" - where - "asset_exif"."assetId" = "asset"."id" - ) as "exifInfo" on true - where - "shared_link"."id" = "shared_link_asset"."sharedLinkId" - and "asset"."deletedAt" is null - order by - "asset"."fileCreatedAt" asc - ) as "a" on true left join lateral ( select "album".*, @@ -60,7 +87,36 @@ from "asset" inner join lateral ( select - "asset_exif".* + "asset_exif"."assetId", + "asset_exif"."autoStackId", + "asset_exif"."bitsPerSample", + "asset_exif"."city", + "asset_exif"."colorspace", + "asset_exif"."country", + "asset_exif"."dateTimeOriginal", + "asset_exif"."description", + "asset_exif"."exifImageHeight", + "asset_exif"."exifImageWidth", + "asset_exif"."exposureTime", + "asset_exif"."fileSizeInByte", + "asset_exif"."fNumber", + "asset_exif"."focalLength", + "asset_exif"."fps", + "asset_exif"."iso", + "asset_exif"."latitude", + "asset_exif"."lensModel", + "asset_exif"."livePhotoCID", + "asset_exif"."longitude", + "asset_exif"."make", + "asset_exif"."model", + "asset_exif"."modifyDate", + "asset_exif"."orientation", + "asset_exif"."profileDescription", + "asset_exif"."projectionType", + "asset_exif"."rating", + "asset_exif"."state", + "asset_exif"."tags", + "asset_exif"."timeZone" from "asset_exif" where @@ -74,7 +130,12 @@ from ) as "assets" on true inner join lateral ( select - "user".* + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt" from "user" where @@ -95,9 +156,6 @@ where "shared_link"."type" = $3 or "album"."id" is not null ) -group by - "shared_link"."id", - "album".* order by "shared_link"."createdAt" desc @@ -134,21 +192,12 @@ from "album" inner join lateral ( select - "user"."id", - "user"."email", - "user"."createdAt", - "user"."profileImagePath", - "user"."isAdmin", - "user"."shouldChangePassword", - "user"."deletedAt", - "user"."oauthId", - "user"."updatedAt", - "user"."storageLabel", - "user"."name", - "user"."quotaSizeInBytes", - "user"."quotaUsageInBytes", - "user"."status", - "user"."profileChangedAt" + "id", + "name", + "email", + "avatarColor", + "profileImagePath", + "profileChangedAt" from "user" where @@ -267,7 +316,36 @@ from "asset" inner join lateral ( select - * + "asset_exif"."assetId", + "asset_exif"."autoStackId", + "asset_exif"."bitsPerSample", + "asset_exif"."city", + "asset_exif"."colorspace", + "asset_exif"."country", + "asset_exif"."dateTimeOriginal", + "asset_exif"."description", + "asset_exif"."exifImageHeight", + "asset_exif"."exifImageWidth", + "asset_exif"."exposureTime", + "asset_exif"."fileSizeInByte", + "asset_exif"."fNumber", + "asset_exif"."focalLength", + "asset_exif"."fps", + "asset_exif"."iso", + "asset_exif"."latitude", + "asset_exif"."lensModel", + "asset_exif"."livePhotoCID", + "asset_exif"."longitude", + "asset_exif"."make", + "asset_exif"."model", + "asset_exif"."modifyDate", + "asset_exif"."orientation", + "asset_exif"."profileDescription", + "asset_exif"."projectionType", + "asset_exif"."rating", + "asset_exif"."state", + "asset_exif"."tags", + "asset_exif"."timeZone" from "asset_exif" where diff --git a/server/src/repositories/shared-link.repository.ts b/server/src/repositories/shared-link.repository.ts index 1ad5d7bd77..ddfe37ef35 100644 --- a/server/src/repositories/shared-link.repository.ts +++ b/server/src/repositories/shared-link.repository.ts @@ -1,5 +1,5 @@ import { Injectable } from '@nestjs/common'; -import { Insertable, Kysely, Selectable, ShallowDehydrateObject, sql, Updateable } from 'kysely'; +import { ExpressionBuilder, Insertable, Kysely, Selectable, ShallowDehydrateObject, sql, Updateable } from 'kysely'; import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres'; import _ from 'lodash'; import { InjectKysely } from 'nestjs-kysely'; @@ -17,6 +17,41 @@ export type SharedLinkSearchOptions = { albumId?: string; }; +const withSharedAssets = (eb: ExpressionBuilder) => { + return eb + .selectFrom('shared_link_asset') + .whereRef('shared_link.id', '=', 'shared_link_asset.sharedLinkId') + .innerJoin('asset', 'asset.id', 'shared_link_asset.assetId') + .where('asset.deletedAt', 'is', null) + .selectAll('asset') + .orderBy('asset.fileCreatedAt', 'asc'); +}; + +export const withExifInfo = (eb: ExpressionBuilder) => { + return eb + .selectFrom('asset_exif') + .select(columns.exif) + .whereRef('asset_exif.assetId', '=', 'asset.id') + .as('exifInfo'); +}; + +const withAlbumOwner = (eb: ExpressionBuilder) => { + return eb + .selectFrom('user') + .select(columns.user) + .whereRef('user.id', '=', 'album.ownerId') + .where('user.deletedAt', 'is', null) + .as('owner'); +}; + +const withSharedLinkAlbum = (eb: ExpressionBuilder) => { + return eb + .selectFrom('album') + .selectAll('album') + .whereRef('album.id', '=', 'shared_link.albumId') + .where('album.deletedAt', 'is', null); +}; + @Injectable() export class SharedLinkRepository { constructor(@InjectKysely() private db: Kysely) {} @@ -26,35 +61,16 @@ export class SharedLinkRepository { return this.db .selectFrom('shared_link') .selectAll('shared_link') - .leftJoinLateral( - (eb) => - eb - .selectFrom('shared_link_asset') - .whereRef('shared_link.id', '=', 'shared_link_asset.sharedLinkId') - .innerJoin('asset', 'asset.id', 'shared_link_asset.assetId') - .where('asset.deletedAt', 'is', null) - .selectAll('asset') - .innerJoinLateral( - (eb) => - eb - .selectFrom('asset_exif') - .selectAll('asset_exif') - .whereRef('asset_exif.assetId', '=', 'asset.id') - .as('exifInfo'), - (join) => join.onTrue(), - ) - .select((eb) => eb.fn.toJson('exifInfo').as('exifInfo')) - .orderBy('asset.fileCreatedAt', 'asc') - .as('a'), - (join) => join.onTrue(), + .select((eb) => + jsonArrayFrom( + withSharedAssets(eb) + .innerJoinLateral(withExifInfo, (join) => join.onTrue()) + .select((eb) => eb.fn.toJson('exifInfo').as('exifInfo')), + ).as('assets'), ) .leftJoinLateral( (eb) => - eb - .selectFrom('album') - .selectAll('album') - .whereRef('album.id', '=', 'shared_link.albumId') - .where('album.deletedAt', 'is', null) + withSharedLinkAlbum(eb) .leftJoin('album_asset', 'album_asset.albumId', 'album.id') .leftJoinLateral( (eb) => @@ -63,30 +79,13 @@ export class SharedLinkRepository { .selectAll('asset') .whereRef('album_asset.assetId', '=', 'asset.id') .where('asset.deletedAt', 'is', null) - .innerJoinLateral( - (eb) => - eb - .selectFrom('asset_exif') - .selectAll('asset_exif') - .whereRef('asset_exif.assetId', '=', 'asset.id') - .as('exifInfo'), - (join) => join.onTrue(), - ) + .innerJoinLateral(withExifInfo, (join) => join.onTrue()) .select((eb) => eb.fn.toJson(eb.table('exifInfo')).as('exifInfo')) .orderBy('asset.fileCreatedAt', 'asc') .as('assets'), (join) => join.onTrue(), ) - .innerJoinLateral( - (eb) => - eb - .selectFrom('user') - .selectAll('user') - .whereRef('user.id', '=', 'album.ownerId') - .where('user.deletedAt', 'is', null) - .as('owner'), - (join) => join.onTrue(), - ) + .innerJoinLateral(withAlbumOwner, (join) => join.onTrue()) .select((eb) => eb.fn .coalesce( @@ -104,17 +103,6 @@ export class SharedLinkRepository { .as('album'), (join) => join.onTrue(), ) - .select((eb) => - eb.fn - .coalesce(eb.fn.jsonAgg('a').filterWhere('a.id', 'is not', null), sql`'[]'`) - .$castTo< - (ShallowDehydrateObject> & { - exifInfo: ShallowDehydrateObject>; - })[] - >() - .as('assets'), - ) - .groupBy(['shared_link.id', sql`"album".*`]) .select((eb) => eb.fn.toJson(eb.table('album')).$castTo | null>().as('album')) .where('shared_link.id', '=', id) .where('shared_link.userId', '=', userId) @@ -128,53 +116,13 @@ export class SharedLinkRepository { return this.db .selectFrom('shared_link') .selectAll('shared_link') + .select((eb) => jsonArrayFrom(withSharedAssets(eb).limit(1)).as('assets')) .where('shared_link.userId', '=', userId) - .select((eb) => - jsonArrayFrom( - eb - .selectFrom('shared_link_asset') - .whereRef('shared_link.id', '=', 'shared_link_asset.sharedLinkId') - .innerJoin('asset', 'asset.id', 'shared_link_asset.assetId') - .where('asset.deletedAt', 'is', null) - .selectAll('asset') - .orderBy('asset.fileCreatedAt', 'asc') - .limit(1), - ).as('assets'), - ) .leftJoinLateral( (eb) => - eb - .selectFrom('album') - .selectAll('album') - .whereRef('album.id', '=', 'shared_link.albumId') - .innerJoinLateral( - (eb) => - eb - .selectFrom('user') - .select([ - 'user.id', - 'user.email', - 'user.createdAt', - 'user.profileImagePath', - 'user.isAdmin', - 'user.shouldChangePassword', - 'user.deletedAt', - 'user.oauthId', - 'user.updatedAt', - 'user.storageLabel', - 'user.name', - 'user.quotaSizeInBytes', - 'user.quotaUsageInBytes', - 'user.status', - 'user.profileChangedAt', - ]) - .whereRef('user.id', '=', 'album.ownerId') - .where('user.deletedAt', 'is', null) - .as('owner'), - (join) => join.onTrue(), - ) + withSharedLinkAlbum(eb) + .innerJoinLateral(withAlbumOwner, (join) => join.onTrue()) .select((eb) => eb.fn.toJson('owner').as('owner')) - .where('album.deletedAt', 'is', null) .as('album'), (join) => join.onTrue(), ) @@ -283,11 +231,7 @@ export class SharedLinkRepository { .selectFrom('asset') .whereRef('asset.id', '=', 'shared_link_asset.assetId') .selectAll('asset') - .innerJoinLateral( - (eb) => - eb.selectFrom('asset_exif').whereRef('asset_exif.assetId', '=', 'asset.id').selectAll().as('exifInfo'), - (join) => join.onTrue(), - ) + .innerJoinLateral(withExifInfo, (join) => join.onTrue()) .as('assets'), (join) => join.onTrue(), ) diff --git a/server/test/medium/specs/services/shared-link.service.spec.ts b/server/test/medium/specs/services/shared-link.service.spec.ts index 5873d469a5..347e2e9506 100644 --- a/server/test/medium/specs/services/shared-link.service.spec.ts +++ b/server/test/medium/specs/services/shared-link.service.spec.ts @@ -372,6 +372,43 @@ describe(SharedLinkService.name, () => { }); describe('get', () => { + it('should return an album shared link with assets', async () => { + const { sut, ctx } = setup(); + const { user } = await ctx.newUser(); + const auth = factory.auth({ user }); + const { album } = await ctx.newAlbum({ ownerId: user.id }); + + const [{ asset: asset1 }, { asset: asset2 }] = await Promise.all([ + ctx.newAsset({ ownerId: user.id }), + ctx.newAsset({ ownerId: user.id }), + ]); + await Promise.all([ + ctx.newExif({ assetId: asset1.id, make: 'Canon' }), + ctx.newExif({ assetId: asset2.id, make: 'Canon' }), + ]); + + const sharedLinkRepo = ctx.get(SharedLinkRepository); + const sharedLink = await sharedLinkRepo.create({ + key: randomBytes(16), + id: factory.uuid(), + userId: user.id, + albumId: album.id, + allowUpload: true, + type: SharedLinkType.Album, + }); + + await sharedLinkRepo.addAssets(sharedLink.id, [asset1.id, asset2.id]); + const result = await sut.get(auth, sharedLink.id); + const assetIds = result.assets.map((asset) => asset.id); + + expect(result).toMatchObject({ + id: sharedLink.id, + album: expect.objectContaining({ id: album.id }), + }); + expect(assetIds).toHaveLength(2); + expect(assetIds).toEqual(expect.arrayContaining([asset1.id, asset2.id])); + }); + it('should not return trashed assets for an individual shared link', async () => { const { sut, ctx } = setup(); const { user } = await ctx.newUser();