From 5f6bd4ae7ec7fbd22d06d1182815057c10483d4b Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Thu, 28 Dec 2023 00:50:54 -0500 Subject: [PATCH] fix(server): Reduce number of bound parameters in Access queries (#6015) * fix(server): Reduce number of bound parameters in Access queries According to https://github.com/typeorm/typeorm/issues/7565, the introduction of bulk queries for permission checks could quickly reach the limit of 65536 bound parameters allowed by the PostgreSQL connection. To avoid reaching that limit, this first change refactors the Access queries that are expanding the set of ids multiple times. For example, `asset.checkSharedLinkAccess` expands the ids 4 times, so providing just ~16400 ids is enough to break the query. Refactored queries: * activity.checkCreateAccess ```sql -- Before SELECT "AlbumEntity"."id" AS "AlbumEntity_id" FROM "albums" "AlbumEntity" LEFT JOIN "albums_shared_users_users" "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers" ON "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."albumsId"="AlbumEntity"."id" LEFT JOIN "users" "AlbumEntity__AlbumEntity_sharedUsers" ON "AlbumEntity__AlbumEntity_sharedUsers"."id"="AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."usersId" AND ("AlbumEntity__AlbumEntity_sharedUsers"."deletedAt" IS NULL) WHERE ( ( "AlbumEntity"."id" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) AND "AlbumEntity"."isActivityEnabled" = $11 AND "AlbumEntity__AlbumEntity_sharedUsers"."id" = $12 ) OR ( "AlbumEntity"."id" IN ($13, $14, $15, $16, $17, $18, $19, $20, $21, $22) AND "AlbumEntity"."isActivityEnabled" = $23 AND "AlbumEntity"."ownerId" = $24 ) ) AND "AlbumEntity"."deletedAt" IS NULL -- After SELECT "album"."id" AS "album_id" FROM "albums" "album" LEFT JOIN "albums_shared_users_users" "album_sharedUsers" ON "album_sharedUsers"."albumsId"="album"."id" LEFT JOIN "users" "sharedUsers" ON "sharedUsers"."id"="album_sharedUsers"."usersId" AND "sharedUsers"."deletedAt" IS NULL WHERE "album"."id" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) AND "album"."isActivityEnabled" = true AND ( "album"."ownerId" = $11 OR "sharedUsers"."id" = $12 ) AND "album"."deletedAt" IS NULL ``` * asset.checkAlbumAccess ```sql -- Before SELECT "asset"."id" AS "assetId", "asset"."livePhotoVideoId" AS "livePhotoVideoId" FROM "albums" "album" INNER JOIN "albums_assets_assets" "album_asset" ON "album_asset"."albumsId"="album"."id" INNER JOIN "assets" "asset" ON "asset"."id"="album_asset"."assetsId" AND "asset"."deletedAt" IS NULL LEFT JOIN "albums_shared_users_users" "album_sharedUsers" ON "album_sharedUsers"."albumsId"="album"."id" LEFT JOIN "users" "sharedUsers" ON "sharedUsers"."id"="album_sharedUsers"."usersId" AND "sharedUsers"."deletedAt" IS NULL WHERE ( "album"."ownerId" = $1 OR "sharedUsers"."id" = $2 ) AND ( "asset"."id" IN ($3, $4, $5, $6, $7, $8, $9, $10, $11, $12) OR "asset"."livePhotoVideoId" IN ($13, $14, $15, $16, $17, $18, $19, $20, $21, $22) ) AND "album"."deletedAt" IS NULL -- After WITH "assetIds" AS ( SELECT unnest(array[$1, $2, $3, $4, $5, $6, $7, $8, $9, $10])::uuid AS "id" FROM (SELECT 1 AS dummy_column) "dummy_table" ) SELECT "asset"."id" AS "assetId", "asset"."livePhotoVideoId" AS "livePhotoVideoId" FROM "albums" "album" INNER JOIN "albums_assets_assets" "album_asset" ON "album_asset"."albumsId"="album"."id" INNER JOIN "assets" "asset" ON "asset"."id"="album_asset"."assetsId" AND "asset"."deletedAt" IS NULL LEFT JOIN "albums_shared_users_users" "album_sharedUsers" ON "album_sharedUsers"."albumsId"="album"."id" LEFT JOIN "users" "sharedUsers" ON "sharedUsers"."id"="album_sharedUsers"."usersId" AND "sharedUsers"."deletedAt" IS NULL WHERE ( "album"."ownerId" = $11 OR "sharedUsers"."id" = $12 ) AND ( "asset"."id" IN (SELECT id FROM "assetIds") OR "asset"."livePhotoVideoId" IN (SELECT id FROM "assetIds") ) AND "album"."deletedAt" IS NULL ``` * asset.checkSharedLinkAccess ```sql -- Before SELECT "assets"."id" AS "assetId", "assets"."livePhotoVideoId" AS "assetLivePhotoVideoId", "albumAssets"."id" AS "albumAssetId", "albumAssets"."livePhotoVideoId" AS "albumAssetLivePhotoVideoId" FROM "shared_links" "sharedLink" LEFT JOIN "albums" "album" ON "album"."id"="sharedLink"."albumId" AND "album"."deletedAt" IS NULL LEFT JOIN "shared_link__asset" "assets_sharedLink" ON "assets_sharedLink"."sharedLinksId"="sharedLink"."id" LEFT JOIN "assets" "assets" ON "assets"."id"="assets_sharedLink"."assetsId" AND "assets"."deletedAt" IS NULL LEFT JOIN "albums_assets_assets" "album_albumAssets" ON "album_albumAssets"."albumsId"="album"."id" LEFT JOIN "assets" "albumAssets" ON "albumAssets"."id"="album_albumAssets"."assetsId" AND "albumAssets"."deletedAt" IS NULL WHERE "sharedLink"."id" = $1 AND ( "assets"."id" IN ($2, $3, $4, $5, $6, $7, $8, $9, $10, $11) OR "albumAssets"."id" IN ($12, $13, $14, $15, $16, $17, $18, $19, $20, $21) OR "assets"."livePhotoVideoId" IN ($22, $23, $24, $25, $26, $27, $28, $29, $30, $31) OR "albumAssets"."livePhotoVideoId" IN ($32, $33, $34, $35, $36, $37, $38, $39, $40, $41) ) -- After WITH "assetIds" AS ( SELECT unnest(array[$1, $2, $3, $4, $5, $6, $7, $8, $9, $10])::uuid AS "id" FROM (SELECT 1 AS dummy_column) "dummy_table" ) SELECT "assets"."id" AS "assetId", "assets"."livePhotoVideoId" AS "assetLivePhotoVideoId", "albumAssets"."id" AS "albumAssetId", "albumAssets"."livePhotoVideoId" AS "albumAssetLivePhotoVideoId" FROM "shared_links" "sharedLink" LEFT JOIN "albums" "album" ON "album"."id"="sharedLink"."albumId" AND "album"."deletedAt" IS NULL LEFT JOIN "shared_link__asset" "assets_sharedLink" ON "assets_sharedLink"."sharedLinksId"="sharedLink"."id" LEFT JOIN "assets" "assets" ON "assets"."id"="assets_sharedLink"."assetsId" AND "assets"."deletedAt" IS NULL LEFT JOIN "albums_assets_assets" "album_albumAssets" ON "album_albumAssets"."albumsId"="album"."id" LEFT JOIN "assets" "albumAssets" ON "albumAssets"."id"="album_albumAssets"."assetsId" AND "albumAssets"."deletedAt" IS NULL WHERE "sharedLink"."id" = $11 AND ( "assets"."id" IN (SELECT id FROM "assetIds") OR "albumAssets"."id" IN (SELECT id FROM "assetIds") OR "assets"."livePhotoVideoId" IN (SELECT id FROM "assetIds") OR "albumAssets"."livePhotoVideoId" IN (SELECT id FROM "assetIds") ) ``` * fix: Use array overlapping instead of CTEs --- .../infra/repositories/access.repository.ts | 51 +++++++------------ 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/server/src/infra/repositories/access.repository.ts b/server/src/infra/repositories/access.repository.ts index eb01c93bed..6c5b24aaca 100644 --- a/server/src/infra/repositories/access.repository.ts +++ b/server/src/infra/repositories/access.repository.ts @@ -67,23 +67,17 @@ export class AccessRepository implements IAccessRepository { } return this.albumRepository - .find({ - select: { id: true }, - where: [ - { - id: In([...albumIds]), - isActivityEnabled: true, - sharedUsers: { - id: userId, - }, - }, - { - id: In([...albumIds]), - isActivityEnabled: true, - ownerId: userId, - }, - ], - }) + .createQueryBuilder('album') + .select('album.id') + .leftJoin('album.sharedUsers', 'sharedUsers') + .where('album.id IN (:...albumIds)', { albumIds: [...albumIds] }) + .andWhere('album.isActivityEnabled = true') + .andWhere( + new Brackets((qb) => { + qb.where('album.ownerId = :userId', { userId }).orWhere('sharedUsers.id = :userId', { userId }); + }), + ) + .getMany() .then((albums) => new Set(albums.map((album) => album.id))); }, }; @@ -148,16 +142,12 @@ export class AccessRepository implements IAccessRepository { .leftJoin('album.sharedUsers', 'sharedUsers') .select('asset.id', 'assetId') .addSelect('asset.livePhotoVideoId', 'livePhotoVideoId') - .where( - new Brackets((qb) => { - qb.where('album.ownerId = :userId', { userId }).orWhere('sharedUsers.id = :userId', { userId }); - }), - ) + .where('array["asset"."id", "asset"."livePhotoVideoId"] && array[:...assetIds]::uuid[]', { + assetIds: [...assetIds], + }) .andWhere( new Brackets((qb) => { - qb.where('asset.id IN (:...assetIds)', { assetIds: [...assetIds] }) - // still part of a live photo is in an album - .orWhere('asset.livePhotoVideoId IN (:...assetIds)', { assetIds: [...assetIds] }); + qb.where('album.ownerId = :userId', { userId }).orWhere('sharedUsers.id = :userId', { userId }); }), ) .getRawMany() @@ -224,13 +214,10 @@ export class AccessRepository implements IAccessRepository { .addSelect('albumAssets.livePhotoVideoId', 'albumAssetLivePhotoVideoId') .where('sharedLink.id = :sharedLinkId', { sharedLinkId }) .andWhere( - new Brackets((qb) => { - qb.where('assets.id IN (:...assetIds)', { assetIds: [...assetIds] }) - .orWhere('albumAssets.id IN (:...assetIds)', { assetIds: [...assetIds] }) - // still part of a live photo is in a shared link - .orWhere('assets.livePhotoVideoId IN (:...assetIds)', { assetIds: [...assetIds] }) - .orWhere('albumAssets.livePhotoVideoId IN (:...assetIds)', { assetIds: [...assetIds] }); - }), + 'array["assets"."id", "assets"."livePhotoVideoId", "albumAssets"."id", "albumAssets"."livePhotoVideoId"] && array[:...assetIds]::uuid[]', + { + assetIds: [...assetIds], + }, ) .getRawMany() .then((rows) => {