diff --git a/src/app/features/tag/store/tag.effects.spec.ts b/src/app/features/tag/store/tag.effects.spec.ts index 4546f1f743..43791e4d5e 100644 --- a/src/app/features/tag/store/tag.effects.spec.ts +++ b/src/app/features/tag/store/tag.effects.spec.ts @@ -11,6 +11,7 @@ import { } from '../../work-context/store/work-context.selectors'; import { selectAllTasksDueToday } from '../../planner/store/planner.selectors'; import { HydrationStateService } from '../../../op-log/apply/hydration-state.service'; +import { SyncTriggerService } from '../../../imex/sync/sync-trigger.service'; import { SnackService } from '../../../core/snack/snack.service'; import { TagService } from '../tag.service'; import { WorkContextService } from '../../work-context/work-context.service'; @@ -32,6 +33,7 @@ describe('TagEffects', () => { let actions$: Observable; let store: MockStore; let hydrationStateServiceSpy: jasmine.SpyObj; + let syncTriggerServiceSpy: jasmine.SpyObj; beforeEach(() => { actions$ = EMPTY; @@ -44,6 +46,12 @@ describe('TagEffects', () => { hydrationStateServiceSpy.isApplyingRemoteOps.and.returnValue(false); hydrationStateServiceSpy.isInSyncWindow.and.returnValue(false); + syncTriggerServiceSpy = jasmine.createSpyObj('SyncTriggerService', [ + 'isInitialSyncDoneSync', + ]); + // Default: initial sync is done (so effect should fire) + syncTriggerServiceSpy.isInitialSyncDoneSync.and.returnValue(true); + const snackServiceSpy = jasmine.createSpyObj('SnackService', ['open']); const tagServiceSpy = jasmine.createSpyObj('TagService', ['updateTag']); const workContextServiceSpy = jasmine.createSpyObj('WorkContextService', [], { @@ -83,6 +91,7 @@ describe('TagEffects', () => { ], }), { provide: HydrationStateService, useValue: hydrationStateServiceSpy }, + { provide: SyncTriggerService, useValue: syncTriggerServiceSpy }, { provide: SnackService, useValue: snackServiceSpy }, { provide: TagService, useValue: tagServiceSpy }, { provide: WorkContextService, useValue: workContextServiceSpy }, @@ -179,6 +188,27 @@ describe('TagEffects', () => { }, 50); }); + it('should not dispatch when initial sync is not done', (done) => { + // Simulate app startup before first sync + syncTriggerServiceSpy.isInitialSyncDoneSync.and.returnValue(false); + + store.overrideSelector(selectTodayTagRepair, { + needsRepair: true, + repairedTaskIds: ['task1'], + }); + store.refreshState(); + + let emitted = false; + effects.repairTodayTagConsistency$.subscribe(() => { + emitted = true; + }); + + setTimeout(() => { + expect(emitted).toBe(false); + done(); + }, 50); + }); + it('should handle empty repairedTaskIds', (done) => { store.overrideSelector(selectTodayTagRepair, { needsRepair: true, @@ -298,6 +328,27 @@ describe('TagEffects', () => { }, 50); }); + it('should not dispatch when initial sync is not done', (done) => { + syncTriggerServiceSpy.isInitialSyncDoneSync.and.returnValue(false); + + const parent = createTask('parent-1'); + const child = createTask('child-1', 'parent-1'); + + store.overrideSelector(selectTodayTaskIds, ['parent-1', 'child-1']); + store.overrideSelector(selectAllTasksDueToday, [parent, child]); + store.refreshState(); + + let emitted = false; + effects.preventParentAndSubTaskInTodayList$.subscribe(() => { + emitted = true; + }); + + setTimeout(() => { + expect(emitted).toBe(false); + done(); + }, 50); + }); + it('should not dispatch when todayTaskIds is empty', (done) => { // Empty today list should not trigger the effect store.overrideSelector(selectTodayTaskIds, []); diff --git a/src/app/imex/sync/sync-trigger.service.spec.ts b/src/app/imex/sync/sync-trigger.service.spec.ts new file mode 100644 index 0000000000..4256d440b5 --- /dev/null +++ b/src/app/imex/sync/sync-trigger.service.spec.ts @@ -0,0 +1,115 @@ +import { TestBed } from '@angular/core/testing'; +import { SyncTriggerService } from './sync-trigger.service'; +import { GlobalConfigService } from '../../features/config/global-config.service'; +import { DataInitStateService } from '../../core/data-init/data-init-state.service'; +import { IdleService } from '../../features/idle/idle.service'; +import { PfapiService } from '../../pfapi/pfapi.service'; +import { SyncWrapperService } from './sync-wrapper.service'; +import { Store } from '@ngrx/store'; +import { EMPTY, of, ReplaySubject } from 'rxjs'; + +describe('SyncTriggerService', () => { + let service: SyncTriggerService; + let globalConfigService: jasmine.SpyObj; + let dataInitStateService: jasmine.SpyObj; + let idleService: jasmine.SpyObj; + let pfapiService: jasmine.SpyObj; + let syncWrapperService: jasmine.SpyObj; + let store: jasmine.SpyObj; + + beforeEach(() => { + const isAllDataLoadedSubject = new ReplaySubject(1); + isAllDataLoadedSubject.next(true); + + globalConfigService = jasmine.createSpyObj('GlobalConfigService', [], { + cfg$: of({ sync: { isEnabled: false } }), + idle$: of({ isEnableIdleTimeTracking: false }), + }); + + dataInitStateService = jasmine.createSpyObj('DataInitStateService', [], { + isAllDataLoadedInitially$: isAllDataLoadedSubject.asObservable(), + }); + + idleService = jasmine.createSpyObj('IdleService', [], { + isIdle$: of(false), + }); + + pfapiService = jasmine.createSpyObj('PfapiService', [], { + onLocalMetaUpdate$: EMPTY, + }); + + syncWrapperService = jasmine.createSpyObj('SyncWrapperService', [], { + syncProviderId$: of(null), + isWaitingForUserInput$: of(false), + }); + + store = jasmine.createSpyObj('Store', ['select']); + store.select.and.returnValue(of(null)); + + TestBed.configureTestingModule({ + providers: [ + SyncTriggerService, + { provide: GlobalConfigService, useValue: globalConfigService }, + { provide: DataInitStateService, useValue: dataInitStateService }, + { provide: IdleService, useValue: idleService }, + { provide: PfapiService, useValue: pfapiService }, + { provide: SyncWrapperService, useValue: syncWrapperService }, + { provide: Store, useValue: store }, + ], + }); + + service = TestBed.inject(SyncTriggerService); + }); + + describe('isInitialSyncDoneSync', () => { + it('should return false initially', () => { + expect(service.isInitialSyncDoneSync()).toBe(false); + }); + + it('should return true after setInitialSyncDone(true)', () => { + service.setInitialSyncDone(true); + expect(service.isInitialSyncDoneSync()).toBe(true); + }); + + it('should return false after setInitialSyncDone(false)', () => { + service.setInitialSyncDone(true); + expect(service.isInitialSyncDoneSync()).toBe(true); + + service.setInitialSyncDone(false); + expect(service.isInitialSyncDoneSync()).toBe(false); + }); + + it('should track multiple state changes', () => { + expect(service.isInitialSyncDoneSync()).toBe(false); + + service.setInitialSyncDone(true); + expect(service.isInitialSyncDoneSync()).toBe(true); + + service.setInitialSyncDone(false); + expect(service.isInitialSyncDoneSync()).toBe(false); + + service.setInitialSyncDone(true); + expect(service.isInitialSyncDoneSync()).toBe(true); + }); + }); + + describe('setInitialSyncDone', () => { + it('should update both sync flag and observable', (done) => { + let observedValue: boolean | undefined; + + // Subscribe to the observable (it's a ReplaySubject internally) + service['_isInitialSyncDoneManual$'].subscribe((val) => { + observedValue = val; + }); + + service.setInitialSyncDone(true); + + // Check sync getter + expect(service.isInitialSyncDoneSync()).toBe(true); + + // Check observable received the value + expect(observedValue).toBe(true); + done(); + }); + }); +}); diff --git a/src/app/imex/sync/sync-trigger.service.ts b/src/app/imex/sync/sync-trigger.service.ts index 3bab4aaa7a..eb8e2493c3 100644 --- a/src/app/imex/sync/sync-trigger.service.ts +++ b/src/app/imex/sync/sync-trigger.service.ts @@ -150,6 +150,7 @@ export class SyncTriggerService { private _isInitialSyncDoneManual$: ReplaySubject = new ReplaySubject( 1, ); + private _isInitialSyncDoneSync = false; private _isInitialSyncDone$: Observable = this._isInitialSyncEnabled$.pipe( switchMap((isActive) => { if (!isActive) { @@ -240,6 +241,16 @@ export class SyncTriggerService { } setInitialSyncDone(val: boolean): void { + this._isInitialSyncDoneSync = val; this._isInitialSyncDoneManual$.next(val); } + + /** + * Synchronous getter for initial sync done state. + * Used by operators like skipDuringSyncWindow() that need + * to check state synchronously in filter callbacks. + */ + isInitialSyncDoneSync(): boolean { + return this._isInitialSyncDoneSync; + } } diff --git a/src/app/util/debounce-during-startup.operator.spec.ts b/src/app/util/debounce-during-startup.operator.spec.ts new file mode 100644 index 0000000000..af6c7f7370 --- /dev/null +++ b/src/app/util/debounce-during-startup.operator.spec.ts @@ -0,0 +1,348 @@ +import { TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { Subject } from 'rxjs'; +import { debounceDuringStartup } from './debounce-during-startup.operator'; +import { SyncTriggerService } from '../imex/sync/sync-trigger.service'; + +describe('debounceDuringStartup', () => { + let syncTriggerService: jasmine.SpyObj; + let source$: Subject; + + beforeEach(() => { + syncTriggerService = jasmine.createSpyObj('SyncTriggerService', [ + 'isInitialSyncDoneSync', + ]); + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + + TestBed.configureTestingModule({ + providers: [{ provide: SyncTriggerService, useValue: syncTriggerService }], + }); + + source$ = new Subject(); + }); + + describe('when initial sync is done', () => { + beforeEach(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + }); + + it('should pass emissions immediately (0ms debounce)', fakeAsync(() => { + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup()).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(0); // Minimal tick for the of(0) debounce + expect(result).toBe(1); + + source$.next(2); + tick(0); + expect(result).toBe(2); + })); + + it('should pass through all emissions when sync is done (0ms debounce)', fakeAsync(() => { + const results: number[] = []; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup()).subscribe((val) => { + results.push(val); + }); + }); + + source$.next(1); + tick(0); + source$.next(2); + tick(0); + source$.next(3); + tick(0); + + // All values pass through immediately when sync is done + expect(results).toEqual([1, 2, 3]); + })); + }); + + describe('when initial sync is not done', () => { + beforeEach(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + }); + + it('should debounce emissions by default 500ms', fakeAsync(() => { + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup()).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(100); + expect(result).toBeUndefined(); + + tick(400); // Total 500ms + expect(result).toBe(1); + })); + + it('should debounce emissions by custom time', fakeAsync(() => { + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(200)).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(100); + expect(result).toBeUndefined(); + + tick(100); // Total 200ms + expect(result).toBe(1); + })); + + it('should reset debounce timer on new emissions', fakeAsync(() => { + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(200)).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(100); + expect(result).toBeUndefined(); + + // New emission resets the timer + source$.next(2); + tick(100); + expect(result).toBeUndefined(); + + tick(100); // 200ms after last emission + expect(result).toBe(2); + })); + + it('should emit only the latest value after debounce', fakeAsync(() => { + const results: number[] = []; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(200)).subscribe((val) => { + results.push(val); + }); + }); + + source$.next(1); + tick(50); + source$.next(2); + tick(50); + source$.next(3); + tick(200); + + expect(results).toEqual([3]); + })); + }); + + describe('state transitions', () => { + it('should switch from debounced to immediate when sync completes', fakeAsync(() => { + const results: number[] = []; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(200)).subscribe((val) => { + results.push(val); + }); + }); + + // Initial sync not done - debounce + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + source$.next(1); + tick(200); + expect(results).toEqual([1]); + + // Sync completes - immediate + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + source$.next(2); + tick(0); + expect(results).toEqual([1, 2]); + })); + }); + + describe('type preservation', () => { + it('should work with different value types', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + const stringSource$ = new Subject(); + const results: string[] = []; + + TestBed.runInInjectionContext(() => { + stringSource$.pipe(debounceDuringStartup()).subscribe((val) => { + results.push(val); + }); + }); + + stringSource$.next('a'); + tick(0); + stringSource$.next('b'); + tick(0); + + expect(results).toEqual(['a', 'b']); + })); + + it('should work with object types', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + const objectSource$ = new Subject<{ id: number }>(); + const results: { id: number }[] = []; + + TestBed.runInInjectionContext(() => { + objectSource$.pipe(debounceDuringStartup()).subscribe((val) => { + results.push(val); + }); + }); + + objectSource$.next({ id: 1 }); + tick(0); + objectSource$.next({ id: 2 }); + tick(0); + + expect(results).toEqual([{ id: 1 }, { id: 2 }]); + })); + }); + + describe('edge cases', () => { + it('should handle zero debounce time parameter', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(0)).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(0); + + expect(result).toBe(1); + })); + + it('should handle very long debounce time', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + let result: number | undefined; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(5000)).subscribe((val) => { + result = val; + }); + }); + + source$.next(1); + tick(2500); + expect(result).toBeUndefined(); + + tick(2500); // Total 5000ms + expect(result).toBe(1); + })); + + it('should emit last value when source completes during debounce', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + const results: number[] = []; + let completed = false; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(500)).subscribe({ + next: (val) => results.push(val), + complete: () => { + completed = true; + }, + }); + }); + + source$.next(1); + tick(100); + source$.complete(); + tick(500); + + // RxJS debounce emits the last value on source completion + expect(results).toEqual([1]); + expect(completed).toBe(true); + })); + }); + + describe('realistic scenarios', () => { + it('should debounce rapid state changes during startup', fakeAsync(() => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + const results: string[] = []; + + TestBed.runInInjectionContext(() => { + const events$ = new Subject(); + events$.pipe(debounceDuringStartup(200)).subscribe((val) => { + results.push(val); + }); + + // Rapid selector emissions during data load + events$.next('state-1'); + tick(50); + events$.next('state-2'); + tick(50); + events$.next('state-3'); + tick(200); + + // Only the final settled state should emit + expect(results).toEqual(['state-3']); + }); + })); + + it('should allow immediate emissions after sync completes', fakeAsync(() => { + const results: string[] = []; + + TestBed.runInInjectionContext(() => { + const events$ = new Subject(); + events$.pipe(debounceDuringStartup(200)).subscribe((val) => { + results.push(val); + }); + + // During startup - debounced + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + events$.next('startup-event'); + tick(200); + expect(results).toEqual(['startup-event']); + + // After sync - immediate + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + events$.next('user-action-1'); + tick(0); + events$.next('user-action-2'); + tick(0); + + expect(results).toEqual(['startup-event', 'user-action-1', 'user-action-2']); + }); + })); + + it('should handle sync state toggle during debounce period', fakeAsync(() => { + const results: number[] = []; + + TestBed.runInInjectionContext(() => { + source$.pipe(debounceDuringStartup(300)).subscribe((val) => { + results.push(val); + }); + }); + + // Start with sync not done - start debouncing + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + source$.next(1); + tick(100); + + // Sync completes mid-debounce + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + tick(200); // Complete the original debounce + + expect(results).toEqual([1]); + + // Now emissions should be immediate + source$.next(2); + tick(0); + expect(results).toEqual([1, 2]); + })); + }); +}); diff --git a/src/app/util/debounce-during-startup.operator.ts b/src/app/util/debounce-during-startup.operator.ts new file mode 100644 index 0000000000..60f50ee030 --- /dev/null +++ b/src/app/util/debounce-during-startup.operator.ts @@ -0,0 +1,60 @@ +import { inject } from '@angular/core'; +import { MonoTypeOperatorFunction, of, timer } from 'rxjs'; +import { debounce } from 'rxjs/operators'; +import { SyncTriggerService } from '../imex/sync/sync-trigger.service'; + +/** + * RxJS operator that debounces emissions during startup, then passes through immediately. + * + * ## Why This Exists + * + * Unlike `skipDuringSyncWindow()` which completely skips emissions during startup, + * this operator allows emissions through after a debounce period. Use this for + * effects that: + * - Should eventually run during startup (not be skipped entirely) + * - But should wait for state to settle before firing + * + * ## How It Works + * + * - Before `setInitialSyncDone(true)`: Emissions are debounced by `debounceMs` + * - After `setInitialSyncDone(true)`: Emissions pass through immediately (0ms debounce) + * + * ## When to Use + * + * Use `debounceDuringStartup()` for effects that: + * - Need to run during startup but can wait + * - Should let state settle before reacting + * - Are not sync-sensitive (won't conflict with remote operations) + * + * Use `skipDuringSyncWindow()` instead for effects that: + * - Modify shared entities (TODAY_TAG, etc.) + * - Could create operations that conflict with sync + * - Should never run before sync completes + * + * ## Usage + * + * ```typescript + * import { debounceDuringStartup } from '../../../util/debounce-during-startup.operator'; + * + * @Injectable() + * export class MyEffects { + * startupEffect$ = createEffect(() => + * this.store.select(mySelector).pipe( + * debounceDuringStartup(500), // Debounce 500ms during startup + * // ... effect logic + * ) + * ); + * } + * ``` + * + * @param debounceMs Debounce time in milliseconds during startup (default: 500ms) + */ +export const debounceDuringStartup = ( + debounceMs = 500, +): MonoTypeOperatorFunction => { + const syncTrigger = inject(SyncTriggerService); + + return debounce(() => + syncTrigger.isInitialSyncDoneSync() ? of(0) : timer(debounceMs), + ); +}; diff --git a/src/app/util/skip-during-sync-window.operator.spec.ts b/src/app/util/skip-during-sync-window.operator.spec.ts new file mode 100644 index 0000000000..520e915f22 --- /dev/null +++ b/src/app/util/skip-during-sync-window.operator.spec.ts @@ -0,0 +1,339 @@ +import { TestBed } from '@angular/core/testing'; +import { Subject } from 'rxjs'; +import { skipDuringSyncWindow } from './skip-during-sync-window.operator'; +import { HydrationStateService } from '../op-log/apply/hydration-state.service'; +import { SyncTriggerService } from '../imex/sync/sync-trigger.service'; + +describe('skipDuringSyncWindow', () => { + let hydrationStateService: jasmine.SpyObj; + let syncTriggerService: jasmine.SpyObj; + let source$: Subject; + + beforeEach(() => { + hydrationStateService = jasmine.createSpyObj('HydrationStateService', [ + 'isInSyncWindow', + ]); + syncTriggerService = jasmine.createSpyObj('SyncTriggerService', [ + 'isInitialSyncDoneSync', + ]); + + // Default: initial sync done and not in sync window + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.configureTestingModule({ + providers: [ + { provide: HydrationStateService, useValue: hydrationStateService }, + { provide: SyncTriggerService, useValue: syncTriggerService }, + ], + }); + + source$ = new Subject(); + }); + + describe('initial sync check', () => { + it('should skip emissions when initial sync is not done', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([]); + done(); + }, + }); + }); + + source$.next(1); + source$.next(2); + source$.next(3); + source$.complete(); + }); + + it('should pass emissions when initial sync is done and not in sync window', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([1, 2, 3]); + done(); + }, + }); + }); + + source$.next(1); + source$.next(2); + source$.next(3); + source$.complete(); + }); + }); + + describe('sync window check', () => { + it('should skip emissions when in sync window', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(true); + + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([]); + done(); + }, + }); + }); + + source$.next(1); + source$.next(2); + source$.next(3); + source$.complete(); + }); + + it('should skip emissions when both initial sync not done AND in sync window', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + hydrationStateService.isInSyncWindow.and.returnValue(true); + + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([]); + done(); + }, + }); + }); + + source$.next(1); + source$.next(2); + source$.next(3); + source$.complete(); + }); + }); + + describe('dynamic state changes', () => { + it('should dynamically filter based on initial sync state', (done) => { + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([2, 3]); + done(); + }, + }); + }); + + // Initial sync not done - should skip + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + source$.next(1); + + // Initial sync done - should pass + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + source$.next(2); + source$.next(3); + + source$.complete(); + }); + + it('should dynamically filter based on sync window state', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([1, 3, 5]); + done(); + }, + }); + }); + + // Not in sync window - should pass + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(1); + + // Enter sync window - should skip + hydrationStateService.isInSyncWindow.and.returnValue(true); + source$.next(2); + + // Exit sync window - should pass + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(3); + + // Enter sync window again - should skip + hydrationStateService.isInSyncWindow.and.returnValue(true); + source$.next(4); + + // Exit sync window - should pass + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(5); + + source$.complete(); + }); + }); + + describe('type preservation', () => { + it('should work with different value types', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.runInInjectionContext(() => { + const stringSource$ = new Subject(); + const results: string[] = []; + + stringSource$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual(['a', 'b', 'c']); + done(); + }, + }); + + stringSource$.next('a'); + stringSource$.next('b'); + stringSource$.next('c'); + stringSource$.complete(); + }); + }); + + it('should work with object types', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.runInInjectionContext(() => { + const objectSource$ = new Subject<{ id: number }>(); + const results: { id: number }[] = []; + + objectSource$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([{ id: 1 }, { id: 2 }]); + done(); + }, + }); + + objectSource$.next({ id: 1 }); + objectSource$.next({ id: 2 }); + objectSource$.complete(); + }); + }); + }); + + describe('realistic scenarios', () => { + it('should block during initial startup until first sync completes', (done) => { + // Simulate app startup scenario: initial sync not done, not in sync window + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + hydrationStateService.isInSyncWindow.and.returnValue(false); + + TestBed.runInInjectionContext(() => { + const results: string[] = []; + const events$ = new Subject(); + + events$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + // Only events after initial sync should pass + expect(results).toEqual(['user-interaction-3', 'user-interaction-4']); + done(); + }, + }); + + // Effects fire during data load - should be blocked + events$.next('effect-during-load-1'); + events$.next('effect-during-load-2'); + + // Initial sync completes + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + + // User interactions after sync - should pass + events$.next('user-interaction-3'); + events$.next('user-interaction-4'); + + events$.complete(); + }); + }); + + it('should block during active sync operations', (done) => { + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + + TestBed.runInInjectionContext(() => { + const results: string[] = []; + const events$ = new Subject(); + + events$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual(['before-sync', 'after-sync']); + done(); + }, + }); + + // Normal operation + hydrationStateService.isInSyncWindow.and.returnValue(false); + events$.next('before-sync'); + + // Sync starts (e.g., tab focus triggers sync) + hydrationStateService.isInSyncWindow.and.returnValue(true); + events$.next('during-sync-1'); + events$.next('during-sync-2'); + + // Sync finishes + hydrationStateService.isInSyncWindow.and.returnValue(false); + events$.next('after-sync'); + + events$.complete(); + }); + }); + + it('should handle rapid state changes correctly', (done) => { + TestBed.runInInjectionContext(() => { + const results: number[] = []; + source$.pipe(skipDuringSyncWindow()).subscribe({ + next: (val) => results.push(val), + complete: () => { + expect(results).toEqual([1, 4, 6]); + done(); + }, + }); + + // Allowed + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(1); + + // Blocked (initial sync flag flipped) + syncTriggerService.isInitialSyncDoneSync.and.returnValue(false); + source$.next(2); + + // Blocked (both flags wrong) + hydrationStateService.isInSyncWindow.and.returnValue(true); + source$.next(3); + + // Allowed (both flags correct) + syncTriggerService.isInitialSyncDoneSync.and.returnValue(true); + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(4); + + // Blocked (sync window only) + hydrationStateService.isInSyncWindow.and.returnValue(true); + source$.next(5); + + // Allowed again + hydrationStateService.isInSyncWindow.and.returnValue(false); + source$.next(6); + + source$.complete(); + }); + }); + }); +}); diff --git a/src/app/util/skip-during-sync-window.operator.ts b/src/app/util/skip-during-sync-window.operator.ts index 841fdfeedf..b8898641dc 100644 --- a/src/app/util/skip-during-sync-window.operator.ts +++ b/src/app/util/skip-during-sync-window.operator.ts @@ -2,24 +2,34 @@ import { inject } from '@angular/core'; import { MonoTypeOperatorFunction } from 'rxjs'; import { filter } from 'rxjs/operators'; import { HydrationStateService } from '../op-log/apply/hydration-state.service'; +import { SyncTriggerService } from '../imex/sync/sync-trigger.service'; /** * RxJS operator that skips emissions during the full sync window, - * including the post-sync cooldown period. + * including: + * - Initial app startup (before first sync completes) + * - While remote operations are being applied + * - Post-sync cooldown period * * ## Why This Exists * - * `skipWhileApplyingRemoteOps()` only blocks during `isApplyingRemoteOps()` - the window - * when operations are being applied to the store. However, there's a timing gap: + * There are two timing gaps where selector-based effects could fire and + * create operations that conflict with sync: * + * ### Gap 1: Initial Startup + * 1. App loads → data loaded from IndexedDB + * 2. Selectors evaluate → effects fire before sync happens + * 3. Operations created with stale vector clocks + * 4. First sync happens → conflicts with remote operations + * + * ### Gap 2: Post-Sync * 1. Tab gains focus → sync triggers * 2. Remote ops applied (isApplyingRemoteOps = true) * 3. Sync finishes, isApplyingRemoteOps = false * 4. Selectors immediately re-evaluate with new state * 5. Effects fire and create operations that conflict with just-synced state * - * This operator extends the suppression window to include a cooldown period - * after sync, preventing step 5. + * This operator blocks during both gaps. * * ## When to Use * @@ -49,9 +59,12 @@ import { HydrationStateService } from '../op-log/apply/hydration-state.service'; * | Operator | Suppresses during | Use case | * |----------|-------------------|----------| * | skipWhileApplyingRemoteOps() | isApplyingRemoteOps only | General selector-based effects | - * | skipDuringSyncWindow() | isApplyingRemoteOps + cooldown | Effects that modify shared entities | + * | skipDuringSyncWindow() | startup + isApplyingRemoteOps + cooldown | Effects that modify shared entities | */ export const skipDuringSyncWindow = (): MonoTypeOperatorFunction => { const hydrationState = inject(HydrationStateService); - return filter(() => !hydrationState.isInSyncWindow()); + const syncTrigger = inject(SyncTriggerService); + return filter( + () => syncTrigger.isInitialSyncDoneSync() && !hydrationState.isInSyncWindow(), + ); };