fix: liveSync just starts one sync job at a time (#2534)

Author: tomwwinter

src/app/child-dev-project/notes/dashboard-widgets/notes-dashboard/notes-dashboard.component.spec.ts

@@ -10,6 +10,12 @@ import { ChildrenService } from "../../../children/children.service";
 import { NotesDashboardComponent } from "./notes-dashboard.component";
 import { MockedTestingModule } from "../../../../utils/mocked-testing.module";
 import { TestEntity } from "../../../../utils/test-utils/TestEntity";
+import { EntityRegistry } from "../../../../core/entity/database-entity.decorator";
+import { Entity } from "../../../../core/entity/model/entity";
+
+class Child extends Entity {
+  static ENTITY_TYPE = "Child";
+}
 
 describe("NotesDashboardComponent", () => {
   let component: NotesDashboardComponent;
@@ -29,6 +35,8 @@ describe("NotesDashboardComponent", () => {
       imports: [NotesDashboardComponent, MockedTestingModule.withState()],
       providers: [{ provide: ChildrenService, useValue: mockChildrenService }],
     }).compileComponents();
+
+    TestBed.inject(EntityRegistry).set("Child", Child);
   }));
 
   describe("with recent notes", () => {

src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts

@@ -10,14 +10,14 @@ import { CoreTestingModule } from "../../../../utils/core-testing.module";
 import { EntityFormService } from "../../../common-components/entity-form/entity-form.service";
 import { MatDialog } from "@angular/material/dialog";
 import { FontAwesomeTestingModule } from "@fortawesome/angular-fontawesome/testing";
-import { Note } from "../../../../child-dev-project/notes/model/note";
 import { FormGroup } from "@angular/forms";
 import { NoopAnimationsModule } from "@angular/platform-browser/animations";
 import { CdkDragDrop } from "@angular/cdk/drag-drop";
 import { of } from "rxjs";
 import { AdminModule } from "../../admin.module";
 import { FormConfig } from "../../../entity-details/form/form.component";
 import { ColumnConfig } from "../../../common-components/entity-form/FormConfig";
+import { TestEntity } from "../../../../utils/test-utils/TestEntity";
 
 describe("AdminEntityFormComponent", () => {
   let component: AdminEntityFormComponent;
@@ -31,7 +31,7 @@ describe("AdminEntityFormComponent", () => {
   beforeEach(() => {
     testConfig = {
       fieldGroups: [
-        { header: "Group 1", fields: ["subject", "date"] },
+        { header: "Group 1", fields: ["name", "other"] },
         { fields: ["category"] },
       ],
     };
@@ -64,7 +64,7 @@ describe("AdminEntityFormComponent", () => {
     component = fixture.componentInstance;
 
     component.config = testConfig;
-    component.entityType = Note;
+    component.entityType = TestEntity;
 
     fixture.detectChanges();
 
@@ -85,7 +85,7 @@ describe("AdminEntityFormComponent", () => {
     };
     component.ngOnChanges({ config: true as any });
 
-    const noteUserFacingFields = Array.from(Note.schema.entries())
+    const noteUserFacingFields = Array.from(TestEntity.schema.entries())
       .filter(([key, value]) => value.label)
       .sort(([aId, a], [bId, b]) => a.label.localeCompare(b.label))
       .map(([key]) => key);
@@ -126,7 +126,7 @@ describe("AdminEntityFormComponent", () => {
     tick();
 
     expect(mockDialog.open).toHaveBeenCalled();
-    expect(targetContainer).toEqual(["subject", newFieldId, "date"]);
+    expect(targetContainer).toEqual(["name", newFieldId, "other"]);
     expect(component.availableFields).toContain(
       component.createNewFieldPlaceholder,
     );
@@ -139,7 +139,7 @@ describe("AdminEntityFormComponent", () => {
     component.drop(mockDropNewFieldEvent(targetContainer));
     tick();
 
-    expect(targetContainer).toEqual(["subject", "date"]);
+    expect(targetContainer).toEqual(["name", "other"]);
     expect(mockDialog.open).toHaveBeenCalled();
     expect(component.availableFields).toContain(
       component.createNewFieldPlaceholder,

src/app/core/database/sync.service.spec.ts

@@ -8,13 +8,16 @@ import { LoginState } from "../session/session-states/login-state.enum";
 import { KeycloakAuthService } from "../session/auth/keycloak/keycloak-auth.service";
 import { Subject } from "rxjs";
 import { NAVIGATOR_TOKEN } from "../../utils/di-tokens";
+import { SyncState } from "../session/session-states/sync-state.enum";
 
 describe("SyncService", () => {
   let service: SyncService;
   let loginState: LoginStateSubject;
   let mockAuthService: jasmine.SpyObj<KeycloakAuthService>;
   let mockNavigator;
 
+  let mockSyncStateSubject: SyncStateSubject;
+
   beforeEach(() => {
     mockAuthService = jasmine.createSpyObj(["login", "addAuthHeader"]);
     mockNavigator = { onLine: true };
@@ -30,6 +33,7 @@ describe("SyncService", () => {
     });
     service = TestBed.inject(SyncService);
     loginState = TestBed.inject(LoginStateSubject);
+    mockSyncStateSubject = TestBed.inject(SyncStateSubject);
   });
 
   /**
@@ -45,18 +49,29 @@ describe("SyncService", () => {
     expect(service).toBeTruthy();
   });
 
-  it("should restart the sync if it fails at one point", fakeAsync(() => {
+  /**
+   * Set up a mocked db and localDb for tests and override the TestBed service provider.
+   */
+  function mockPouchDatabaseService(): {
+    mockLocalDb: jasmine.SpyObj<PouchDB.Database>;
+    db: PouchDatabase;
+  } {
+    mockSyncStateSubject.next(SyncState.UNSYNCED);
     const mockLocalDb = jasmine.createSpyObj(["sync"]);
-    spyOn(
-      TestBed.inject(Database) as PouchDatabase,
-      "getPouchDB",
-    ).and.returnValue(mockLocalDb);
+    mockLocalDb.sync.and.resolveTo({});
+
+    const db = TestBed.inject(Database) as PouchDatabase;
+    spyOn(db, "getPouchDB").and.returnValue(mockLocalDb);
+    return { mockLocalDb, db };
+  }
+
+  it("should restart the sync if it fails at one point", fakeAsync(() => {
+    const { mockLocalDb } = mockPouchDatabaseService();
 
     loginState.next(LoginState.LOGGED_IN);
 
     service.startSync();
 
-    mockLocalDb.sync.and.resolveTo({});
     tick(1000);
     expect(mockLocalDb.sync).toHaveBeenCalled();
 
@@ -76,17 +91,14 @@ describe("SyncService", () => {
   }));
 
   it("should sync immediately when local db has changes", fakeAsync(() => {
-    const mockLocalDb = jasmine.createSpyObj(["sync"]);
-    const db = TestBed.inject(Database) as PouchDatabase;
-    spyOn(db, "getPouchDB").and.returnValue(mockLocalDb);
+    const { mockLocalDb, db } = mockPouchDatabaseService();
     const mockChanges = new Subject();
     spyOn(db, "changes").and.returnValue(mockChanges);
 
     loginState.next(LoginState.LOGGED_IN);
 
     service.startSync();
 
-    mockLocalDb.sync.and.resolveTo({});
     tick(1000);
     expect(mockLocalDb.sync).toHaveBeenCalled();
     mockLocalDb.sync.calls.reset();
@@ -101,10 +113,7 @@ describe("SyncService", () => {
   }));
 
   it("should skip sync calls when offline", fakeAsync(() => {
-    const mockLocalDb = jasmine.createSpyObj(["sync"]);
-    mockLocalDb.sync.and.resolveTo({});
-    const db = TestBed.inject(Database) as PouchDatabase;
-    spyOn(db, "getPouchDB").and.returnValue(mockLocalDb);
+    const { mockLocalDb } = mockPouchDatabaseService();
 
     mockNavigator.onLine = false;
 
@@ -119,4 +128,29 @@ describe("SyncService", () => {
 
     stopPeriodicTimer();
   }));
+
+  it("should not start additional syncs while a previous sync is still running", fakeAsync(() => {
+    const LONG_SYNC_TIME = 100000;
+
+    const { mockLocalDb } = mockPouchDatabaseService();
+    mockLocalDb.sync.and.callFake(
+      // @ts-ignore
+      async () => await new Promise((r) => setTimeout(r, LONG_SYNC_TIME)),
+    );
+
+    service.startSync();
+
+    tick(SyncService.SYNC_INTERVAL);
+    expect(mockLocalDb.sync).toHaveBeenCalledTimes(1);
+
+    tick(SyncService.SYNC_INTERVAL);
+    expect(mockLocalDb.sync).toHaveBeenCalledTimes(1);
+
+    tick(LONG_SYNC_TIME);
+    expect(mockLocalDb.sync).toHaveBeenCalledTimes(2);
+
+    // stop periodic timer:
+    service.liveSyncEnabled = false;
+    tick(LONG_SYNC_TIME);
+  }));
 });

src/app/core/database/sync.service.ts

@@ -14,7 +14,7 @@ import {
 import { KeycloakAuthService } from "../session/auth/keycloak/keycloak-auth.service";
 import { Config } from "../config/config";
 import { Entity } from "../entity/model/entity";
-import { from, interval, merge, of } from "rxjs";
+import { from, interval, merge, of, Subscription } from "rxjs";
 import { environment } from "../../../environments/environment";
 import { NAVIGATOR_TOKEN } from "../../utils/di-tokens";
 
@@ -132,7 +132,13 @@ export class SyncService {
     )
       .pipe(
         debounceTime(500),
-        mergeMap(() => from(this.sync())),
+        mergeMap(() => {
+          if (this.syncStateSubject.value == SyncState.STARTED) {
+            return of();
+          } else {
+            return from(this.sync());
+          }
+        }),
         retry({ delay: SyncService.SYNC_INTERVAL }),
         takeWhile(() => this.liveSyncEnabled),
       )