fix: sync backfill (#29267)

server/src/queries/sync.repository.sql

@@ -88,7 +88,7 @@ from
 where
   "album_asset"."updateId" < $3
   and "album_asset"."updateId" <= $4
-  and "album_asset"."updateId" >= $5
+  and "album_asset"."updateId" > $5
   and "album_asset"."albumId" = $6
 order by
   "album_asset"."updateId" asc
@@ -202,7 +202,7 @@ from
 where
   "album_asset"."updateId" < $1
   and "album_asset"."updateId" <= $2
-  and "album_asset"."updateId" >= $3
+  and "album_asset"."updateId" > $3
   and "album_asset"."albumId" = $4
 order by
   "album_asset"."updateId" asc
@@ -297,7 +297,7 @@ from
 where
   "album_asset"."updateId" < $1
   and "album_asset"."updateId" <= $2
-  and "album_asset"."updateId" >= $3
+  and "album_asset"."updateId" > $3
   and "album_asset"."albumId" = $4
 order by
   "album_asset"."updateId" asc
@@ -349,7 +349,7 @@ from
 where
   "album_user"."updateId" < $1
   and "album_user"."updateId" <= $2
-  and "album_user"."updateId" >= $3
+  and "album_user"."updateId" > $3
   and "albumId" = $4
 order by
   "album_user"."updateId" asc
@@ -810,7 +810,7 @@ from
 where
   "asset"."updateId" < $2
   and "asset"."updateId" <= $3
-  and "asset"."updateId" >= $4
+  and "asset"."updateId" > $4
   and "ownerId" = $5
 order by
   "asset"."updateId" asc
@@ -908,7 +908,7 @@ from
 where
   "asset_exif"."updateId" < $1
   and "asset_exif"."updateId" <= $2
-  and "asset_exif"."updateId" >= $3
+  and "asset_exif"."updateId" > $3
   and "asset"."ownerId" = $4
 order by
   "asset_exif"."updateId" asc
@@ -997,7 +997,7 @@ from
 where
   "stack"."updateId" < $1
   and "stack"."updateId" <= $2
-  and "stack"."updateId" >= $3
+  and "stack"."updateId" > $3
   and "ownerId" = $4
 order by
   "stack"."updateId" asc

server/src/repositories/sync.repository.ts

@@ -106,7 +106,7 @@ export class BaseSync {
       .selectFrom(table(t).as(t))
       .where(updateIdRef, '<', nowId)
       .where(updateIdRef, '<=', beforeUpdateId)
-      .$if(!!afterUpdateId, (qb) => qb.where(updateIdRef, '>=', afterUpdateId!))
+      .$if(!!afterUpdateId, (qb) => qb.where(updateIdRef, '>', afterUpdateId!))
       .orderBy(updateIdRef, 'asc');
   }
 

server/test/medium/specs/sync/sync-album-to-asset.spec.ts

@@ -155,6 +155,57 @@ describe(SyncRequestType.AlbumToAssetsV1, () => {
     await ctx.assertSyncIsComplete(auth, [SyncRequestType.AlbumToAssetsV1]);
   });
 
+  it('should not resend an already-acked item when backfill resumes', async () => {
+    const { auth, ctx } = await setup();
+    const { user: user2 } = await ctx.newUser();
+
+    // backfill needs assets with an older updateId
+    const { asset: sharedAsset1 } = await ctx.newAsset({ ownerId: user2.id });
+    const { asset: sharedAsset2 } = await ctx.newAsset({ ownerId: user2.id });
+
+    await wait(2);
+
+    const { album: sharedAlbum } = await ctx.newAlbum({ ownerId: user2.id });
+    await ctx.newAlbumAsset({ albumId: sharedAlbum.id, assetId: sharedAsset1.id });
+    await ctx.newAlbumAsset({ albumId: sharedAlbum.id, assetId: sharedAsset2.id });
+
+    await wait(2);
+
+    // backfill needs an initial ack, otherwise it syncs everything
+    const { asset: ownedAsset } = await ctx.newAsset({ ownerId: auth.user.id });
+    const { album: ownedAlbum } = await ctx.newAlbum({ ownerId: auth.user.id });
+    await ctx.newAlbumAsset({ albumId: ownedAlbum.id, assetId: ownedAsset.id });
+
+    const setupResponse = await ctx.syncStream(auth, [SyncRequestType.AlbumToAssetsV1]);
+    await ctx.syncAckAll(auth, setupResponse);
+
+    // share album to trigger backfill
+    await ctx.newAlbumUser({ albumId: sharedAlbum.id, userId: auth.user.id, role: AlbumUserRole.Editor });
+
+    const response1 = await ctx.syncStream(auth, [SyncRequestType.AlbumToAssetsV1]);
+    expect(response1).toEqual([
+      // receive both
+      expect.objectContaining({ data: { albumId: sharedAlbum.id, assetId: sharedAsset1.id } }),
+      expect.objectContaining({ data: { albumId: sharedAlbum.id, assetId: sharedAsset2.id } }),
+      expect.objectContaining({ type: SyncEntityType.SyncAckV1 }),
+      expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
+    ]);
+
+    // ack 1st
+    await ctx.sut.setAcks(auth, { acks: [response1[0].ack] });
+
+    const response2 = await ctx.syncStream(auth, [SyncRequestType.AlbumToAssetsV1]);
+    expect(response2).toEqual([
+      // receive 2nd
+      expect.objectContaining({ data: { albumId: sharedAlbum.id, assetId: sharedAsset2.id } }),
+      expect.objectContaining({ type: SyncEntityType.SyncAckV1 }),
+      expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
+    ]);
+
+    await ctx.syncAckAll(auth, response2);
+    await ctx.assertSyncIsComplete(auth, [SyncRequestType.AlbumToAssetsV1]);
+  });
+
   it('should detect and sync a deleted album to asset relation', async () => {
     const { auth, ctx } = await setup();
     const albumRepo = ctx.get(AlbumRepository);

server/test/medium/specs/sync/sync-partner-asset.spec.ts

@@ -279,6 +279,68 @@ describe(SyncRequestType.PartnerAssetsV2, () => {
     await ctx.assertSyncIsComplete(auth, [SyncRequestType.PartnerAssetsV2]);
   });
 
+  it('should not resend an already-acked item when backfill resumes', async () => {
+    const { auth, ctx } = await setup();
+    const { user: user2 } = await ctx.newUser();
+    const { user: user3 } = await ctx.newUser();
+
+    // backfill needs assets with an older updateId
+    const { asset: partnerAsset1 } = await ctx.newAsset({ ownerId: user3.id });
+    await wait(2);
+    const { asset: partnerAsset2 } = await ctx.newAsset({ ownerId: user3.id });
+
+    await wait(2);
+
+    // backfill needs an initial ack, otherwise it syncs everything
+    const { asset: initialAsset } = await ctx.newAsset({ ownerId: user2.id });
+    await ctx.newPartner({ sharedById: user2.id, sharedWithId: auth.user.id });
+
+    const setupResponse = await ctx.syncStream(auth, [SyncRequestType.PartnerAssetsV2]);
+    expect(setupResponse).toEqual([
+      expect.objectContaining({
+        data: expect.objectContaining({ id: initialAsset.id }),
+        type: SyncEntityType.PartnerAssetV2,
+      }),
+      expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
+    ]);
+    await ctx.syncAckAll(auth, setupResponse);
+
+    // partner share to trigger backfill
+    await ctx.newPartner({ sharedById: user3.id, sharedWithId: auth.user.id });
+
+    const response1 = await ctx.syncStream(auth, [SyncRequestType.PartnerAssetsV2]);
+    expect(response1).toEqual([
+      // receive both
+      expect.objectContaining({
+        data: expect.objectContaining({ id: partnerAsset1.id }),
+        type: SyncEntityType.PartnerAssetBackfillV2,
+      }),
+      expect.objectContaining({
+        data: expect.objectContaining({ id: partnerAsset2.id }),
+        type: SyncEntityType.PartnerAssetBackfillV2,
+      }),
+      expect.objectContaining({ type: SyncEntityType.SyncAckV1 }),
+      expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
+    ]);
+
+    // ack 1st
+    await ctx.sut.setAcks(auth, { acks: [response1[0].ack] });
+
+    const response2 = await ctx.syncStream(auth, [SyncRequestType.PartnerAssetsV2]);
+    expect(response2).toEqual([
+      // receive 2nd
+      expect.objectContaining({
+        data: expect.objectContaining({ id: partnerAsset2.id }),
+        type: SyncEntityType.PartnerAssetBackfillV2,
+      }),
+      expect.objectContaining({ type: SyncEntityType.SyncAckV1 }),
+      expect.objectContaining({ type: SyncEntityType.SyncCompleteV1 }),
+    ]);
+
+    await ctx.syncAckAll(auth, response2);
+    await ctx.assertSyncIsComplete(auth, [SyncRequestType.PartnerAssetsV2]);
+  });
+
   it('should hide isFavorite for partner assets', async () => {
     const { auth, ctx } = await setup();
     const { user: user2 } = await ctx.newUser();