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>
2024-11-15 09:59:00 -07:00 · 2024-06-09 21:19:28 +02:00 · 2024-06-09 21:19:28 +02:00 · 972c66d467
commit 972c66d467
parent 69795a3763
11 changed files with 38 additions and 121 deletions
--- 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;
--- a/mobile/openapi/lib/model/asset_full_sync_dto.dart
+++ b/mobile/openapi/lib/model/asset_full_sync_dto.dart
@ -13,21 +13,12 @@ part of openapi.api;
 class AssetFullSyncDto {
  /// Returns a new [AssetFullSyncDto] instance.
  AssetFullSyncDto({
-    this.lastCreationDate,
    this.lastId,
    required this.limit,
    required this.updatedUntil,
    this.userId,
  });

-  ///
-  /// Please note: This property should have been non-nullable! Since the specification file
-  /// does not include a default value (using the "default:" property), however, the generated
-  /// source code must fall back to having a nullable type.
-  /// Consider adding a "default:" property in the specification file to hide this note.
-  ///
-  DateTime? lastCreationDate;
-
  ///
  /// Please note: This property should have been non-nullable! Since the specification file
  /// does not include a default value (using the "default:" property), however, the generated
@ -51,7 +42,6 @@ class AssetFullSyncDto {

  @override
  bool operator ==(Object other) => identical(this, other) || other is AssetFullSyncDto &&
-    other.lastCreationDate == lastCreationDate &&
    other.lastId == lastId &&
    other.limit == limit &&
    other.updatedUntil == updatedUntil &&
@ -60,22 +50,16 @@ class AssetFullSyncDto {
  @override
  int get hashCode =>
    // ignore: unnecessary_parenthesis
-    (lastCreationDate == null ? 0 : lastCreationDate!.hashCode) +
    (lastId == null ? 0 : lastId!.hashCode) +
    (limit.hashCode) +
    (updatedUntil.hashCode) +
    (userId == null ? 0 : userId!.hashCode);

  @override
-  String toString() => 'AssetFullSyncDto[lastCreationDate=$lastCreationDate, lastId=$lastId, limit=$limit, updatedUntil=$updatedUntil, userId=$userId]';
+  String toString() => 'AssetFullSyncDto[lastId=$lastId, limit=$limit, updatedUntil=$updatedUntil, userId=$userId]';

  Map<String, dynamic> toJson() {
    final json = <String, dynamic>{};
-    if (this.lastCreationDate != null) {
-      json[r'lastCreationDate'] = this.lastCreationDate!.toUtc().toIso8601String();
-    } else {
-    //  json[r'lastCreationDate'] = null;
-    }
    if (this.lastId != null) {
      json[r'lastId'] = this.lastId;
    } else {
@ -99,7 +83,6 @@ class AssetFullSyncDto {
      final json = value.cast<String, dynamic>();

      return AssetFullSyncDto(
-        lastCreationDate: mapDateTime(json, r'lastCreationDate', r''),
        lastId: mapValueOfType<String>(json, r'lastId'),
        limit: mapValueOfType<int>(json, r'limit')!,
        updatedUntil: mapDateTime(json, r'updatedUntil', r'')!,
--- 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"
--- 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;
--- 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,
--- 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;

--- 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;
 }
--- 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;
--- 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)
--- 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();
  }
 }
--- 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,