From 972c66d467dc723bc2fd501ccc38a24e5d0e2d0e Mon Sep 17 00:00:00 2001 From: Fynn Petersen-Frey <10599762+fyfrey@users.noreply.github.com> Date: Sun, 9 Jun 2024 21:19:28 +0200 Subject: [PATCH] fix(server): proper asset sync (#10019) * fix(server,mobile): proper asset sync * fix CI issues * only use id instead of createdAt+id * remove createdAt index * fix typo * cleanup createdAt usage --------- Co-authored-by: Alex Tran <alex.tran1502@gmail.com> --- mobile/lib/services/asset.service.dart | 9 ++- .../lib/model/asset_full_sync_dto.dart | Bin 5233 -> 4417 bytes open-api/immich-openapi-specs.json | 4 - open-api/typescript-sdk/src/fetch-client.ts | 1 - server/src/dtos/asset-response.dto.ts | 6 +- server/src/dtos/sync.dto.ts | 3 - server/src/entities/asset-stack.entity.ts | 2 + server/src/interfaces/asset.interface.ts | 1 - server/src/queries/asset.repository.sql | 73 ++---------------- server/src/repositories/asset.repository.ts | 40 +++++----- server/src/services/sync.service.ts | 1 - 11 files changed, 37 insertions(+), 103 deletions(-) diff --git a/mobile/lib/services/asset.service.dart b/mobile/lib/services/asset.service.dart index 13090fe822..5751c00b47 100644 --- a/mobile/lib/services/asset.service.dart +++ b/mobile/lib/services/asset.service.dart @@ -101,7 +101,6 @@ class AssetService { const int chunkSize = 10000; try { final List<Asset> allAssets = []; - DateTime? lastCreationDate; String? lastId; // will break on error or once all assets are loaded while (true) { @@ -109,15 +108,17 @@ class AssetService { limit: chunkSize, updatedUntil: until, lastId: lastId, - lastCreationDate: lastCreationDate, userId: user.id, ); + log.fine("Requesting $chunkSize assets from $lastId"); final List<AssetResponseDto>? assets = await _apiService.syncApi.getFullSyncForUser(dto); if (assets == null) return null; + log.fine( + "Received ${assets.length} assets from ${assets.firstOrNull?.id} to ${assets.lastOrNull?.id}", + ); allAssets.addAll(assets.map(Asset.remote)); - if (assets.isEmpty) break; - lastCreationDate = assets.last.fileCreatedAt; + if (assets.length != chunkSize) break; lastId = assets.last.id; } return allAssets; diff --git a/mobile/openapi/lib/model/asset_full_sync_dto.dart b/mobile/openapi/lib/model/asset_full_sync_dto.dart index e08bdaddf357645142d0f8fd810980348dd56234..e80638f6b0869cba839e686ab4d4e93202affdd2 100644 GIT binary patch delta 39 xcmV+?0NDTWD8VAIjRCW$0ki?L*aS5KlRpQ=lST-^lPd|Lvxf@20<($@=LSk+5A*;4 delta 339 zcmX@8^ig9&FC(vWQEFmIW`3SaVo9pb=6c3Mj8Y(GNM>%Ty+Tf6aS5`5&G#9nGb<pA zDA?L!sG6L@dfftDQC?|Ij)J{{fr1r=dPTjA#NrI+{FGEp1?|a(Y^PCduvI~Kg^t4H zA8gBbkZqW}fL&G{SwKBj!B(LpBePf!UAN-oFb;*uSJ`D`Fw}$GXw9Vn1ht!c*v~Pc ZT5*9(Oak37bsdEwb#+ah&Awb;*Z{(Ac&-2d diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index ed3f1ce403..031e53ada7 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -7432,10 +7432,6 @@ }, "AssetFullSyncDto": { "properties": { - "lastCreationDate": { - "format": "date-time", - "type": "string" - }, "lastId": { "format": "uuid", "type": "string" diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index 3929afc0bb..c30d0a93e7 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -904,7 +904,6 @@ export type AssetDeltaSyncResponseDto = { upserted: AssetResponseDto[]; }; export type AssetFullSyncDto = { - lastCreationDate?: string; lastId?: string; limit: number; updatedUntil: string; diff --git a/server/src/dtos/asset-response.dto.ts b/server/src/dtos/asset-response.dto.ts index 755ae8e1d7..a0095f9543 100644 --- a/server/src/dtos/asset-response.dto.ts +++ b/server/src/dtos/asset-response.dto.ts @@ -127,10 +127,10 @@ export function mapAsset(entity: AssetEntity, options: AssetMapOptions = {}): As stackParentId: withStack ? entity.stack?.primaryAssetId : undefined, stack: withStack ? entity.stack?.assets - .filter((a) => a.id !== entity.stack?.primaryAssetId) - .map((a) => mapAsset(a, { stripMetadata, auth: options.auth })) + ?.filter((a) => a.id !== entity.stack?.primaryAssetId) + ?.map((a) => mapAsset(a, { stripMetadata, auth: options.auth })) : undefined, - stackCount: entity.stack?.assets?.length ?? null, + stackCount: entity.stack?.assetCount ?? entity.stack?.assets?.length ?? null, isOffline: entity.isOffline, hasMetadata: true, duplicateId: entity.duplicateId, diff --git a/server/src/dtos/sync.dto.ts b/server/src/dtos/sync.dto.ts index 1a02ba5ca0..820de8d6c3 100644 --- a/server/src/dtos/sync.dto.ts +++ b/server/src/dtos/sync.dto.ts @@ -7,9 +7,6 @@ export class AssetFullSyncDto { @ValidateUUID({ optional: true }) lastId?: string; - @ValidateDate({ optional: true }) - lastCreationDate?: Date; - @ValidateDate() updatedUntil!: Date; diff --git a/server/src/entities/asset-stack.entity.ts b/server/src/entities/asset-stack.entity.ts index 28607e7ab6..6397bded3a 100644 --- a/server/src/entities/asset-stack.entity.ts +++ b/server/src/entities/asset-stack.entity.ts @@ -16,4 +16,6 @@ export class AssetStackEntity { @Column({ nullable: false }) primaryAssetId!: string; + + assetCount?: number; } diff --git a/server/src/interfaces/asset.interface.ts b/server/src/interfaces/asset.interface.ts index 80a7538e0c..43886a5534 100644 --- a/server/src/interfaces/asset.interface.ts +++ b/server/src/interfaces/asset.interface.ts @@ -122,7 +122,6 @@ export interface AssetExploreOptions extends AssetExploreFieldOptions { export interface AssetFullSyncOptions { ownerId: string; - lastCreationDate?: Date; lastId?: string; updatedUntil: Date; limit: number; diff --git a/server/src/queries/asset.repository.sql b/server/src/queries/asset.repository.sql index 3ee46c4c13..6968a2b8f5 100644 --- a/server/src/queries/asset.repository.sql +++ b/server/src/queries/asset.repository.sql @@ -1049,50 +1049,18 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", - "stack"."primaryAssetId" AS "stack_primaryAssetId", - "stackedAssets"."id" AS "stackedAssets_id", - "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", - "stackedAssets"."ownerId" AS "stackedAssets_ownerId", - "stackedAssets"."libraryId" AS "stackedAssets_libraryId", - "stackedAssets"."deviceId" AS "stackedAssets_deviceId", - "stackedAssets"."type" AS "stackedAssets_type", - "stackedAssets"."originalPath" AS "stackedAssets_originalPath", - "stackedAssets"."previewPath" AS "stackedAssets_previewPath", - "stackedAssets"."thumbnailPath" AS "stackedAssets_thumbnailPath", - "stackedAssets"."thumbhash" AS "stackedAssets_thumbhash", - "stackedAssets"."encodedVideoPath" AS "stackedAssets_encodedVideoPath", - "stackedAssets"."createdAt" AS "stackedAssets_createdAt", - "stackedAssets"."updatedAt" AS "stackedAssets_updatedAt", - "stackedAssets"."deletedAt" AS "stackedAssets_deletedAt", - "stackedAssets"."fileCreatedAt" AS "stackedAssets_fileCreatedAt", - "stackedAssets"."localDateTime" AS "stackedAssets_localDateTime", - "stackedAssets"."fileModifiedAt" AS "stackedAssets_fileModifiedAt", - "stackedAssets"."isFavorite" AS "stackedAssets_isFavorite", - "stackedAssets"."isArchived" AS "stackedAssets_isArchived", - "stackedAssets"."isExternal" AS "stackedAssets_isExternal", - "stackedAssets"."isOffline" AS "stackedAssets_isOffline", - "stackedAssets"."checksum" AS "stackedAssets_checksum", - "stackedAssets"."duration" AS "stackedAssets_duration", - "stackedAssets"."isVisible" AS "stackedAssets_isVisible", - "stackedAssets"."livePhotoVideoId" AS "stackedAssets_livePhotoVideoId", - "stackedAssets"."originalFileName" AS "stackedAssets_originalFileName", - "stackedAssets"."sidecarPath" AS "stackedAssets_sidecarPath", - "stackedAssets"."stackId" AS "stackedAssets_stackId", - "stackedAssets"."duplicateId" AS "stackedAssets_duplicateId" + "stack"."primaryAssetId" AS "stack_primaryAssetId" FROM "assets" "asset" LEFT JOIN "exif" "exifInfo" ON "exifInfo"."assetId" = "asset"."id" LEFT JOIN "asset_stack" "stack" ON "stack"."id" = "asset"."stackId" - LEFT JOIN "assets" "stackedAssets" ON "stackedAssets"."stackId" = "stack"."id" - AND ("stackedAssets"."deletedAt" IS NULL) WHERE "asset"."isVisible" = true AND "asset"."ownerId" IN ($1) - AND ("asset"."fileCreatedAt", "asset"."id") < ($2, $3) - AND "asset"."updatedAt" <= $4 + AND "asset"."id" > $2 + AND "asset"."updatedAt" <= $3 ORDER BY - "asset"."fileCreatedAt" DESC, - "asset"."id" DESC + "asset"."id" ASC LIMIT 10 @@ -1156,42 +1124,11 @@ SELECT "exifInfo"."bitsPerSample" AS "exifInfo_bitsPerSample", "exifInfo"."fps" AS "exifInfo_fps", "stack"."id" AS "stack_id", - "stack"."primaryAssetId" AS "stack_primaryAssetId", - "stackedAssets"."id" AS "stackedAssets_id", - "stackedAssets"."deviceAssetId" AS "stackedAssets_deviceAssetId", - "stackedAssets"."ownerId" AS "stackedAssets_ownerId", - "stackedAssets"."libraryId" AS "stackedAssets_libraryId", - "stackedAssets"."deviceId" AS "stackedAssets_deviceId", - "stackedAssets"."type" AS "stackedAssets_type", - "stackedAssets"."originalPath" AS "stackedAssets_originalPath", - "stackedAssets"."previewPath" AS "stackedAssets_previewPath", - "stackedAssets"."thumbnailPath" AS "stackedAssets_thumbnailPath", - "stackedAssets"."thumbhash" AS "stackedAssets_thumbhash", - "stackedAssets"."encodedVideoPath" AS "stackedAssets_encodedVideoPath", - "stackedAssets"."createdAt" AS "stackedAssets_createdAt", - "stackedAssets"."updatedAt" AS "stackedAssets_updatedAt", - "stackedAssets"."deletedAt" AS "stackedAssets_deletedAt", - "stackedAssets"."fileCreatedAt" AS "stackedAssets_fileCreatedAt", - "stackedAssets"."localDateTime" AS "stackedAssets_localDateTime", - "stackedAssets"."fileModifiedAt" AS "stackedAssets_fileModifiedAt", - "stackedAssets"."isFavorite" AS "stackedAssets_isFavorite", - "stackedAssets"."isArchived" AS "stackedAssets_isArchived", - "stackedAssets"."isExternal" AS "stackedAssets_isExternal", - "stackedAssets"."isOffline" AS "stackedAssets_isOffline", - "stackedAssets"."checksum" AS "stackedAssets_checksum", - "stackedAssets"."duration" AS "stackedAssets_duration", - "stackedAssets"."isVisible" AS "stackedAssets_isVisible", - "stackedAssets"."livePhotoVideoId" AS "stackedAssets_livePhotoVideoId", - "stackedAssets"."originalFileName" AS "stackedAssets_originalFileName", - "stackedAssets"."sidecarPath" AS "stackedAssets_sidecarPath", - "stackedAssets"."stackId" AS "stackedAssets_stackId", - "stackedAssets"."duplicateId" AS "stackedAssets_duplicateId" + "stack"."primaryAssetId" AS "stack_primaryAssetId" FROM "assets" "asset" LEFT JOIN "exif" "exifInfo" ON "exifInfo"."assetId" = "asset"."id" LEFT JOIN "asset_stack" "stack" ON "stack"."id" = "asset"."stackId" - LEFT JOIN "assets" "stackedAssets" ON "stackedAssets"."stackId" = "stack"."id" - AND ("stackedAssets"."deletedAt" IS NULL) WHERE "asset"."isVisible" = true AND "asset"."ownerId" IN ($1) diff --git a/server/src/repositories/asset.repository.ts b/server/src/repositories/asset.repository.ts index d1efa8b197..15ff5ced7a 100644 --- a/server/src/repositories/asset.repository.ts +++ b/server/src/repositories/asset.repository.ts @@ -763,36 +763,40 @@ export class AssetRepository implements IAssetRepository { ], }) getAllForUserFullSync(options: AssetFullSyncOptions): Promise<AssetEntity[]> { - const { ownerId, lastCreationDate, lastId, updatedUntil, limit } = options; + const { ownerId, lastId, updatedUntil, limit } = options; const builder = this.getBuilder({ userIds: [ownerId], - exifInfo: true, // also joins stack information + exifInfo: false, // need to do this manually because `exifInfo: true` also loads stacked assets messing with `limit` withStacked: false, // return all assets individually as expected by the app - }); + }) + .leftJoinAndSelect('asset.exifInfo', 'exifInfo') + .leftJoinAndSelect('asset.stack', 'stack') + .loadRelationCountAndMap('stack.assetCount', 'stack.assets', 'stackedAssetsCount'); - if (lastCreationDate !== undefined && lastId !== undefined) { - builder.andWhere('(asset.fileCreatedAt, asset.id) < (:lastCreationDate, :lastId)', { - lastCreationDate, - lastId, - }); + if (lastId !== undefined) { + builder.andWhere('asset.id > :lastId', { lastId }); } - - return builder + builder .andWhere('asset.updatedAt <= :updatedUntil', { updatedUntil }) - .orderBy('asset.fileCreatedAt', 'DESC') - .addOrderBy('asset.id', 'DESC') - .limit(limit) - .withDeleted() - .getMany(); + .orderBy('asset.id', 'ASC') + .limit(limit) // cannot use `take` for performance reasons + .withDeleted(); + return builder.getMany(); } @GenerateSql({ params: [{ userIds: [DummyValue.UUID], updatedAfter: DummyValue.DATE }] }) getChangedDeltaSync(options: AssetDeltaSyncOptions): Promise<AssetEntity[]> { - const builder = this.getBuilder({ userIds: options.userIds, exifInfo: true, withStacked: false }) + const builder = this.getBuilder({ + userIds: options.userIds, + exifInfo: false, // need to do this manually because `exifInfo: true` also loads stacked assets messing with `limit` + withStacked: false, // return all assets individually as expected by the app + }) + .leftJoinAndSelect('asset.exifInfo', 'exifInfo') + .leftJoinAndSelect('asset.stack', 'stack') + .loadRelationCountAndMap('stack.assetCount', 'stack.assets', 'stackedAssetsCount') .andWhere({ updatedAt: MoreThan(options.updatedAfter) }) - .limit(options.limit) + .limit(options.limit) // cannot use `take` for performance reasons .withDeleted(); - return builder.getMany(); } } diff --git a/server/src/services/sync.service.ts b/server/src/services/sync.service.ts index c0ac362d89..228ad65603 100644 --- a/server/src/services/sync.service.ts +++ b/server/src/services/sync.service.ts @@ -32,7 +32,6 @@ export class SyncService { await this.access.requirePermission(auth, Permission.TIMELINE_READ, userId); const assets = await this.assetRepository.getAllForUserFullSync({ ownerId: userId, - lastCreationDate: dto.lastCreationDate, updatedUntil: dto.updatedUntil, lastId: dto.lastId, limit: dto.limit,