[PM-21090] Vault - Repeated Syncs (#14740)

* move `fullSync` contents to private methods in prep to storing the respective promise

* store in-flight sync so multiple calls to the sync service are avoided

* Revert "store in-flight sync so multiple calls to the sync service are avoided"

This reverts commit 233c8e9d4b.

* Revert "move `fullSync` contents to private methods in prep to storing the respective promise"

This reverts commit 3f686ac6a4.

* store inflight API calls for sync service

- This avoids duplicate network requests in a relatively short amount of time but still allows consumers to call `fullSync` if needed

* add debug log for duplicate sync

libs/common/src/platform/sync/default-sync.service.spec.ts

@@ -130,12 +130,7 @@ describe("DefaultSyncService", () => {
 
   const user1 = "user1" as UserId;
 
-  describe("fullSync", () => {
+  const emptySyncResponse = new SyncResponse({
-    beforeEach(() => {
-      accountService.activeAccount$ = of({ id: user1 } as Account);
-      Matrix.autoMockMethod(authService.authStatusFor$, () => of(AuthenticationStatus.Unlocked));
-      apiService.getSync.mockResolvedValue(
-        new SyncResponse({
     profile: {
       id: user1,
     },
@@ -145,8 +140,13 @@ describe("DefaultSyncService", () => {
     sends: [],
     domains: [],
     policies: [],
-        }),
+  });
-      );
+
+  describe("fullSync", () => {
+    beforeEach(() => {
+      accountService.activeAccount$ = of({ id: user1 } as Account);
+      Matrix.autoMockMethod(authService.authStatusFor$, () => of(AuthenticationStatus.Unlocked));
+      apiService.getSync.mockResolvedValue(emptySyncResponse);
       Matrix.autoMockMethod(userDecryptionOptionsService.userDecryptionOptionsById$, () =>
         of({ hasMasterPassword: true } satisfies UserDecryptionOptions),
       );
@@ -201,5 +201,44 @@ describe("DefaultSyncService", () => {
       expect(apiService.refreshIdentityToken).toHaveBeenCalledTimes(1);
       expect(apiService.getSync).toHaveBeenCalledTimes(1);
     });
+
+    describe("in-flight syncs", () => {
+      beforeEach(() => {
+        jest.useFakeTimers();
+      });
+
+      afterEach(() => {
+        jest.useRealTimers();
+      });
+
+      it("does not call getSync when one is already in progress", async () => {
+        const fullSyncPromises = [sut.fullSync(true), sut.fullSync(false), sut.fullSync(false)];
+
+        jest.advanceTimersByTime(100);
+
+        await Promise.all(fullSyncPromises);
+
+        expect(apiService.getSync).toHaveBeenCalledTimes(1);
+      });
+
+      it("does not call refreshIdentityToken when one is already in progress", async () => {
+        const fullSyncPromises = [sut.fullSync(true), sut.fullSync(false), sut.fullSync(false)];
+
+        jest.advanceTimersByTime(100);
+
+        await Promise.all(fullSyncPromises);
+
+        expect(apiService.refreshIdentityToken).toHaveBeenCalledTimes(1);
+      });
+
+      it("resets the in-flight properties when the complete", async () => {
+        const fullSyncPromises = [sut.fullSync(true), sut.fullSync(true)];
+
+        await Promise.all(fullSyncPromises);
+
+        expect(sut["inFlightApiCalls"].refreshToken).toBeNull();
+        expect(sut["inFlightApiCalls"].sync).toBeNull();
+      });
+    });
   });
 });

libs/common/src/platform/sync/default-sync.service.ts

@@ -58,11 +58,21 @@ import { MessageSender } from "../messaging";
 import { StateProvider } from "../state";
 
 import { CoreSyncService } from "./core-sync.service";
+import { SyncResponse } from "./sync.response";
 import { SyncOptions } from "./sync.service";
 
 export class DefaultSyncService extends CoreSyncService {
   syncInProgress = false;
 
+  /** The promises associated with any in-flight api calls. */
+  private inFlightApiCalls: {
+    refreshToken: Promise<void> | null;
+    sync: Promise<SyncResponse> | null;
+  } = {
+    refreshToken: null,
+    sync: null,
+  };
+
   constructor(
     private masterPasswordService: InternalMasterPasswordServiceAbstraction,
     accountService: AccountService,
@@ -141,9 +151,24 @@ export class DefaultSyncService extends CoreSyncService {
 
     try {
       if (!skipTokenRefresh) {
-        await this.apiService.refreshIdentityToken();
+        // Store the promise so multiple calls to refresh the token are not made
+        if (this.inFlightApiCalls.refreshToken === null) {
+          this.inFlightApiCalls.refreshToken = this.apiService.refreshIdentityToken();
         }
-      const response = await this.apiService.getSync();
+
+        await this.inFlightApiCalls.refreshToken;
+      }
+
+      // Store the promise so multiple calls to sync are not made
+      if (this.inFlightApiCalls.sync === null) {
+        this.inFlightApiCalls.sync = this.apiService.getSync();
+      } else {
+        this.logService.debug(
+          "Sync: Sync network call already in progress, returning existing promise",
+        );
+      }
+
+      const response = await this.inFlightApiCalls.sync;
 
       await this.syncProfile(response.profile);
       await this.syncFolders(response.folders, response.profile.id);
@@ -162,6 +187,9 @@ export class DefaultSyncService extends CoreSyncService {
       } else {
         return this.syncCompleted(false, userId);
       }
+    } finally {
+      this.inFlightApiCalls.refreshToken = null;
+      this.inFlightApiCalls.sync = null;
     }
   }