Mirrors/super-productivity
1
0
Fork 0
mirror of https://github.com/johannesjo/super-productivity.git synced 2026-07-17 16:37:43 +00:00
Code Issues Projects Releases Wiki Activity

feat(sync): add local data conflict handling for file-based sync

Browse source 
Add conflict resolution when file-based sync (WebDAV/Dropbox) detects
local unsynced changes that would be lost if remote snapshot is applied.

Changes:
- Add LocalDataConflictError to signal first-sync conflicts
- Add _handleLocalDataConflict in SyncWrapperService to show dialog
- User can choose USE_LOCAL (upload) or USE_REMOTE (download)
- Add forceUploadLocalState and forceDownloadRemoteState methods
- Fix test mocks and update test expectations for new behavior

The conflict dialog allows users to explicitly choose whether to keep
their local changes or accept the remote state during first sync.
This commit is contained in:
Johannes Millan 2026-01-08 10:10:48 +01:00
parent c9b3116882
commit 2b5fafccae
12 changed files with 870 additions and 110 deletions
Download patch file Download diff file Expand all files Collapse all files

233
e2e/tests/sync/webdav-first-sync-conflict.spec.ts Normal file
View file

@ -0,0 +1,233 @@
import { test, expect } from '../../fixtures/test.fixture';
import { SyncPage } from '../../pages/sync.page';
import { WorkViewPage } from '../../pages/work-view.page';
import { waitForStatePersistence } from '../../utils/waits';
import { isWebDavServerUp } from '../../utils/check-webdav';
import {
WEBDAV_CONFIG_TEMPLATE,
setupSyncClient,
createSyncFolder,
waitForSyncComplete,
generateSyncFolderName,
dismissTourIfVisible,
} from '../../utils/sync-helpers';
import { waitForAppReady } from '../../utils/waits';
/**
* Tests for first sync conflict when both clients have existing data.
*
* Scenario:
* 1. Client A has data, sets up WebDAV sync → uploads data
* 2. Client B has DIFFERENT data, sets up WebDAV sync to same folder
* 3. Expected: Conflict dialog with "Use Local" vs "Use Remote" options
*/
test.describe('WebDAV First Sync Conflict', () => {
test.describe.configure({ mode: 'serial' });
test.beforeAll(async () => {
const isUp = await isWebDavServerUp(WEBDAV_CONFIG_TEMPLATE.baseUrl);
if (!isUp) {
test.skip(true, 'WebDAV server not reachable');
}
});
test('should show conflict dialog and allow USE_LOCAL to upload local data', async ({
browser,
baseURL,
request,
}) => {
test.slow();
const SYNC_FOLDER_NAME = generateSyncFolderName('e2e-first-conflict-local');
await createSyncFolder(request, SYNC_FOLDER_NAME);
const WEBDAV_CONFIG = {
...WEBDAV_CONFIG_TEMPLATE,
syncFolderPath: `/${SYNC_FOLDER_NAME}`,
};
const url = baseURL || 'http://localhost:4242';
// --- Client A: Create task and upload ---
const { context: contextA, page: pageA } = await setupSyncClient(browser, url);
const syncPageA = new SyncPage(pageA);
const workViewPageA = new WorkViewPage(pageA);
await workViewPageA.waitForTaskList();
// Setup sync and create task
await syncPageA.setupWebdavSync(WEBDAV_CONFIG);
const taskA = 'Task from Client A - ' + Date.now();
await workViewPageA.addTask(taskA);
await waitForStatePersistence(pageA);
// Upload to remote
await syncPageA.triggerSync();
await waitForSyncComplete(pageA, syncPageA);
console.log('[Test] Client A uploaded task');
// --- Client B: Create local task BEFORE setting up sync ---
// IMPORTANT: Do NOT use setupSyncClient - we need to NOT auto-accept dialogs
const contextB = await browser.newContext({ baseURL: url });
const pageB = await contextB.newPage();
// Don't add dialog handler here - we want the conflict dialog to appear
await pageB.goto('/');
await waitForAppReady(pageB);
await dismissTourIfVisible(pageB);
const syncPageB = new SyncPage(pageB);
const workViewPageB = new WorkViewPage(pageB);
await workViewPageB.waitForTaskList();
// Create a local task BEFORE setting up sync
const taskB = 'Task from Client B - ' + Date.now();
await workViewPageB.addTask(taskB);
await waitForStatePersistence(pageB);
console.log('[Test] Client B created local task');
// Now setup sync - this triggers auto-sync which should show conflict dialog
await syncPageB.setupWebdavSync(WEBDAV_CONFIG);
console.log('[Test] Client B set up sync, waiting for conflict dialog...');
// Wait for conflict dialog to appear (auto-sync triggers after save)
// The dialog contains "Sync: Conflicting Data" title and "Keep local"/"Keep remote" buttons
const conflictDialog = pageB.locator('mat-dialog-container', {
hasText: 'Conflicting Data',
});
await expect(conflictDialog).toBeVisible({ timeout: 30000 });
console.log('[Test] Conflict dialog appeared on Client B');
// Click "Keep local" button
const useLocalBtn = conflictDialog.locator('button', { hasText: /Keep local/i });
await expect(useLocalBtn).toBeVisible();
await useLocalBtn.click();
console.log('[Test] Clicked Use Local on Client B');
// Handle potential confirmation dialog (for overwrites with many changes)
const confirmDialog = pageB.locator('dialog-confirm');
try {
await confirmDialog.waitFor({ state: 'visible', timeout: 3000 });
// Click the confirm/OK button
await confirmDialog
.locator('button[color="warn"], button:has-text("OK")')
.first()
.click();
} catch {
// Confirmation might not appear - that's fine
}
// Wait for sync to complete
await waitForSyncComplete(pageB, syncPageB, 30000);
console.log('[Test] Sync completed on Client B');
// Verify Client B still has its local task
await expect(pageB.locator('task', { hasText: taskB })).toBeVisible({
timeout: 5000,
});
// Client A's task should NOT be visible (we chose USE_LOCAL, which replaced remote)
await expect(pageB.locator('task', { hasText: taskA })).not.toBeVisible();
console.log('[Test] Verified Client B has local task, not remote task');
// --- Verify Client A now gets Client B's data ---
await syncPageA.triggerSync();
await waitForSyncComplete(pageA, syncPageA);
console.log('[Test] Client A synced');
// Client A should now have Client B's task (remote was replaced by B's local data)
await expect(pageA.locator('task', { hasText: taskB })).toBeVisible({
timeout: 5000,
});
// Client A's original task is gone (replaced by B's upload)
await expect(pageA.locator('task', { hasText: taskA })).not.toBeVisible();
console.log('[Test] Verified Client A received Client B data');
await contextA.close();
await contextB.close();
});
test('should show conflict dialog and allow USE_REMOTE to download remote data', async ({
browser,
baseURL,
request,
}) => {
test.slow();
const SYNC_FOLDER_NAME = generateSyncFolderName('e2e-first-conflict-remote');
await createSyncFolder(request, SYNC_FOLDER_NAME);
const WEBDAV_CONFIG = {
...WEBDAV_CONFIG_TEMPLATE,
syncFolderPath: `/${SYNC_FOLDER_NAME}`,
};
const url = baseURL || 'http://localhost:4242';
// --- Client A: Create task and upload ---
const { context: contextA, page: pageA } = await setupSyncClient(browser, url);
const syncPageA = new SyncPage(pageA);
const workViewPageA = new WorkViewPage(pageA);
await workViewPageA.waitForTaskList();
await syncPageA.setupWebdavSync(WEBDAV_CONFIG);
const taskA = 'Task from Client A - ' + Date.now();
await workViewPageA.addTask(taskA);
await waitForStatePersistence(pageA);
await syncPageA.triggerSync();
await waitForSyncComplete(pageA, syncPageA);
console.log('[Test] Client A uploaded task');
// --- Client B: Create local task, then connect ---
const contextB = await browser.newContext({ baseURL: url });
const pageB = await contextB.newPage();
await pageB.goto('/');
await waitForAppReady(pageB);
await dismissTourIfVisible(pageB);
const syncPageB = new SyncPage(pageB);
const workViewPageB = new WorkViewPage(pageB);
await workViewPageB.waitForTaskList();
const taskB = 'Task from Client B - ' + Date.now();
await workViewPageB.addTask(taskB);
await waitForStatePersistence(pageB);
console.log('[Test] Client B created local task');
await syncPageB.setupWebdavSync(WEBDAV_CONFIG);
console.log('[Test] Client B set up sync, waiting for conflict dialog...');
// Wait for conflict dialog (auto-sync triggers after save)
const conflictDialog = pageB.locator('mat-dialog-container', {
hasText: 'Conflicting Data',
});
await expect(conflictDialog).toBeVisible({ timeout: 30000 });
console.log('[Test] Conflict dialog appeared');
// Click "Keep remote" button
const useRemoteBtn = conflictDialog.locator('button', { hasText: /Keep remote/i });
await expect(useRemoteBtn).toBeVisible();
await useRemoteBtn.click();
console.log('[Test] Clicked Use Remote');
// Handle potential confirmation dialog
const confirmDialog = pageB.locator('dialog-confirm');
try {
await confirmDialog.waitFor({ state: 'visible', timeout: 3000 });
await confirmDialog
.locator('button[color="warn"], button:has-text("OK")')
.first()
.click();
} catch {
// Confirmation might not appear
}
await waitForSyncComplete(pageB, syncPageB, 30000);
console.log('[Test] Sync completed');
// Verify Client B now has remote data (Client A's task)
await expect(pageB.locator('task', { hasText: taskA })).toBeVisible({
timeout: 5000,
});
// Client B's local task should be gone
await expect(pageB.locator('task', { hasText: taskB })).not.toBeVisible();
console.log('[Test] Verified Client B has remote task, not local task');
await contextA.close();
await contextB.close();
});
});

118
src/app/imex/sync/sync-wrapper.service.spec.ts
View file

@ -20,7 +20,10 @@ import {
SyncProviderId,
SyncStatus,
} from '../../op-log/sync-exports';
import { SyncAlreadyInProgressError } from '../../op-log/core/errors/sync-errors';
import {
SyncAlreadyInProgressError,
LocalDataConflictError,
} from '../../op-log/core/errors/sync-errors';
import { LegacySyncProvider } from './legacy-sync-provider.model';
describe('SyncWrapperService', () => {
@ -127,9 +130,15 @@ describe('SyncWrapperService', () => {
]);
mockReminderService = jasmine.createSpyObj('ReminderService', ['reloadFromDatabase']);
mockUserInputWaitState = jasmine.createSpyObj('UserInputWaitStateService', [], {
isWaitingForUserInput$: of(false),
});
mockUserInputWaitState = jasmine.createSpyObj(
'UserInputWaitStateService',
['startWaiting'],
{
isWaitingForUserInput$: of(false),
},
);
// startWaiting returns a stopWaiting function
mockUserInputWaitState.startWaiting.and.returnValue(() => {});
mockSuperSyncStatusService = jasmine.createSpyObj('SuperSyncStatusService', [], {
isConfirmedInSync: { value: false },
@ -427,6 +436,107 @@ describe('SyncWrapperService', () => {
expect(mockSnackService.open).not.toHaveBeenCalled();
});
describe('LocalDataConflictError handling', () => {
beforeEach(() => {
mockOpLogStore.getVectorClockEntry.and.returnValue(
Promise.resolve({
clock: { clientA: 5 },
lastUpdate: Date.now(),
}),
);
});
it('should catch LocalDataConflictError and open conflict dialog', async () => {
const conflictError = new LocalDataConflictError(
3, // unsyncedCount
{ tasks: [{ id: 'remote-task' }] }, // remoteSnapshotState
{ clientB: 5 }, // remoteVectorClock
);
mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError));
// Mock dialog to return USE_LOCAL
mockMatDialog.open.and.returnValue({
afterClosed: () => of('USE_LOCAL'),
} as any);
mockSyncService.forceUploadLocalState = jasmine
.createSpy('forceUploadLocalState')
.and.resolveTo();
await service.sync();
// Should open conflict dialog
expect(mockMatDialog.open).toHaveBeenCalled();
});
it('should call forceUploadLocalState when user chooses USE_LOCAL', async () => {
const conflictError = new LocalDataConflictError(
2,
{ tasks: [] },
{ clientB: 3 },
);
mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError));
mockMatDialog.open.and.returnValue({
afterClosed: () => of('USE_LOCAL'),
} as any);
mockSyncService.forceUploadLocalState = jasmine
.createSpy('forceUploadLocalState')
.and.resolveTo();
const result = await service.sync();
expect(mockSyncService.forceUploadLocalState).toHaveBeenCalledWith(
mockSyncCapableProvider,
);
expect(result).toBe(SyncStatus.InSync);
});
it('should call forceDownloadRemoteState when user chooses USE_REMOTE', async () => {
const conflictError = new LocalDataConflictError(
2,
{ tasks: [] },
{ clientB: 3 },
);
mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError));
mockMatDialog.open.and.returnValue({
afterClosed: () => of('USE_REMOTE'),
} as any);
mockSyncService.forceDownloadRemoteState = jasmine
.createSpy('forceDownloadRemoteState')
.and.resolveTo();
const result = await service.sync();
expect(mockSyncService.forceDownloadRemoteState).toHaveBeenCalledWith(
mockSyncCapableProvider,
);
expect(result).toBe(SyncStatus.InSync);
});
it('should return HANDLED_ERROR when user cancels conflict dialog', async () => {
const conflictError = new LocalDataConflictError(
2,
{ tasks: [] },
{ clientB: 3 },
);
mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError));
// User cancels dialog (returns undefined)
mockMatDialog.open.and.returnValue({
afterClosed: () => of(undefined),
} as any);
const result = await service.sync();
expect(result).toBe('HANDLED_ERROR');
expect(mockSnackService.open).toHaveBeenCalled();
});
});
it('should handle permission errors with appropriate message', async () => {
mockSyncService.downloadRemoteOps.and.returnValue(
Promise.reject(new Error('EACCES: permission denied')),

101
src/app/imex/sync/sync-wrapper.service.ts
View file

@ -12,7 +12,10 @@ import {
timeout,
} from 'rxjs/operators';
import { toObservable } from '@angular/core/rxjs-interop';
import { SyncAlreadyInProgressError } from '../../op-log/core/errors/sync-errors';
import {
SyncAlreadyInProgressError,
LocalDataConflictError,
} from '../../op-log/core/errors/sync-errors';
import { SyncConfig } from '../../features/config/global-config.model';
import { TranslateService } from '@ngx-translate/core';
import { MatDialog, MatDialogRef } from '@angular/material/dialog';
@ -21,6 +24,7 @@ import {
AuthFailSPError,
CanNotMigrateMajorDownError,
ConflictData,
ConflictReason,
DecryptError,
DecryptNoPasswordError,
LockPresentError,
@ -299,6 +303,10 @@ export class SyncWrapperService {
// Silently ignore concurrent sync attempts (using proper error class)
SyncLog.log('Sync already in progress, skipping concurrent sync attempt');
return 'HANDLED_ERROR';
} else if (error instanceof LocalDataConflictError) {
// File-based sync: Local data exists and remote snapshot would overwrite it
// Show conflict dialog to let user choose between local and remote data
return this._handleLocalDataConflict(error);
} else if (this._isPermissionError(error)) {
this._snackService.open({
msg: this._getPermissionErrorMessage(),
@ -478,6 +486,97 @@ export class SyncWrapperService {
});
}
/**
* Handles LocalDataConflictError by showing a conflict resolution dialog.
* This occurs when file-based sync (Dropbox, WebDAV) detects local unsynced
* changes that would be lost if the remote snapshot is applied.
*
* User can choose:
* - USE_LOCAL: Upload local data, overwriting remote (uses forceUploadLocalState)
* - USE_REMOTE: Download remote data, discarding local (uses forceDownloadRemoteState)
*/
private async _handleLocalDataConflict(
error: LocalDataConflictError,
): Promise<SyncStatus | 'HANDLED_ERROR'> {
// Signal that we're waiting for user input (prevents sync timeout)
const stopWaiting = this._userInputWaitState.startWaiting('local-data-conflict');
try {
// Build ConflictData for the dialog
const vcEntry = await this._opLogStore.getVectorClockEntry();
const localClock = vcEntry?.clock;
const localLastUpdate = vcEntry?.lastUpdate || Date.now();
const conflictData: ConflictData = {
reason: ConflictReason.NoLastSync,
remote: {
lastUpdate: Date.now(), // Remote snapshot doesn't have a timestamp, use now
lastUpdateAction: 'Remote data',
revMap: {},
crossModelVersion: 1,
mainModelData: error.remoteSnapshotState,
isFullData: true,
vectorClock: error.remoteVectorClock,
},
local: {
lastUpdate: localLastUpdate,
lastUpdateAction: `${error.unsyncedCount} local changes pending`,
revMap: {},
crossModelVersion: 1,
lastSyncedUpdate: null,
metaRev: null,
vectorClock: localClock,
lastSyncedVectorClock: null,
},
};
SyncLog.log(
`SyncWrapperService: Showing conflict dialog for ${error.unsyncedCount} local changes vs remote snapshot`,
);
const resolution = await firstValueFrom(this._openConflictDialog$(conflictData));
// Get sync provider for the resolution operation
const rawProvider = this._providerManager.getActiveProvider();
const syncCapableProvider =
await this._wrappedProvider.getOperationSyncCapable(rawProvider);
if (!syncCapableProvider) {
SyncLog.err(
'SyncWrapperService: Cannot resolve conflict - provider not available',
);
return 'HANDLED_ERROR';
}
if (resolution === 'USE_LOCAL') {
// User chose to keep local data and upload it to remote
SyncLog.log(
'SyncWrapperService: User chose USE_LOCAL - uploading local state to overwrite remote',
);
await this._opLogSyncService.forceUploadLocalState(syncCapableProvider);
this._providerManager.setSyncStatus('IN_SYNC');
return SyncStatus.InSync;
} else if (resolution === 'USE_REMOTE') {
// User chose to discard local data and download remote
SyncLog.log(
'SyncWrapperService: User chose USE_REMOTE - downloading remote state, discarding local',
);
await this._opLogSyncService.forceDownloadRemoteState(syncCapableProvider);
this._providerManager.setSyncStatus('IN_SYNC');
return SyncStatus.InSync;
} else {
// User cancelled the dialog
SyncLog.log('SyncWrapperService: User cancelled first sync conflict dialog');
this._snackService.open({
msg: T.F.SYNC.S.LOCAL_DATA_REPLACE_CANCELLED,
});
return 'HANDLED_ERROR';
}
} finally {
stopWaiting();
}
}
private async _reInitAppAfterDataModelChange(
downloadedMainModelData?: Record<string, unknown>,
): Promise<void> {

17
src/app/op-log/core/errors/sync-errors.ts
View file

@ -250,6 +250,23 @@ export class NoSyncProviderSetError extends Error {
override name = 'NoSyncProviderSetError';
}
/**
* Thrown when file-based sync detects local unsynced changes that would be
* lost if remote snapshot is applied. Caught by SyncWrapperService to show
* conflict resolution dialog.
*/
export class LocalDataConflictError extends Error {
override name = 'LocalDataConflictError';
constructor(
public readonly unsyncedCount: number,
public readonly remoteSnapshotState: Record<string, unknown>,
public readonly remoteVectorClock?: Record<string, number>,
) {
super(`Local data conflict: ${unsyncedCount} unsynced changes would be lost`);
}
}
export class SyncAlreadyInProgressError extends Error {
override name = 'SyncAlreadyInProgressError';

2
src/app/op-log/persistence/operation-log-hydrator.service.spec.ts
View file

@ -1147,6 +1147,7 @@ describe('OperationLogHydratorService', () => {
expect(mockSyncHydrationService.hydrateFromRemoteSync).toHaveBeenCalledWith(
downloadedData,
undefined,
);
});
@ -1155,6 +1156,7 @@ describe('OperationLogHydratorService', () => {
expect(mockSyncHydrationService.hydrateFromRemoteSync).toHaveBeenCalledWith(
undefined,
undefined,
);
});
});

39
src/app/op-log/persistence/operation-log-migration.service.spec.ts
View file

@ -140,7 +140,7 @@ describe('OperationLogMigrationService', () => {
expect(mockOpLogStore.deleteOpsWhere).not.toHaveBeenCalled();
});
it('should clear orphan operations and check for legacy data', async () => {
it('should clear orphan operations when legacy data exists', async () => {
mockOpLogStore.getOpsAfterSeq.and.resolveTo([
{
seq: 1,
@ -176,16 +176,49 @@ describe('OperationLogMigrationService', () => {
},
]);
mockOpLogStore.deleteOpsWhere.and.resolveTo();
mockLegacyPfDb.hasUsableEntityData.and.resolveTo(false);
// Legacy data exists - orphan ops should be cleared before migration
mockLegacyPfDb.hasUsableEntityData.and.resolveTo(true);
// Lock acquisition fails - prevents migration from proceeding (test focuses on clearing)
mockLegacyPfDb.acquireMigrationLock.and.resolveTo(false);
await service.checkAndMigrate();
expect(OpLog.warn).toHaveBeenCalledWith(
jasmine.stringContaining('Found 2 orphan operations without Genesis'),
jasmine.stringContaining('Found 2 orphan operations'),
);
expect(mockOpLogStore.deleteOpsWhere).toHaveBeenCalled();
expect(mockLegacyPfDb.hasUsableEntityData).toHaveBeenCalled();
});
it('should NOT clear orphan operations when no legacy data exists (fresh install)', async () => {
mockOpLogStore.getOpsAfterSeq.and.resolveTo([
{
seq: 1,
op: {
id: 'orphan-op-1',
entityType: 'TASK',
actionType: '[Task] Update Task' as ActionType,
opType: OpType.Update,
clientId: 'client1',
vectorClock: { client1: 1 },
timestamp: Date.now(),
payload: { id: 't1', title: 'Test' },
schemaVersion: 1,
},
appliedAt: Date.now(),
source: 'local',
},
]);
// No legacy data - orphan ops are kept (fresh install scenario)
mockLegacyPfDb.hasUsableEntityData.and.resolveTo(false);
await service.checkAndMigrate();
expect(OpLog.normal).toHaveBeenCalledWith(
jasmine.stringContaining('fresh install'),
);
expect(mockOpLogStore.deleteOpsWhere).not.toHaveBeenCalled();
});
});
describe('when no snapshot and no operations exist (fresh install)', () => {

177
src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.spec.ts
View file

@ -11,7 +11,10 @@ import {
FileBasedSyncData,
SyncDataCorruptedError,
} from './file-based-sync.types';
import { RemoteFileNotFoundAPIError } from '../../core/errors/sync-errors';
import {
RemoteFileNotFoundAPIError,
UploadRevToMatchMismatchAPIError,
} from '../../core/errors/sync-errors';
import { EncryptAndCompressCfg } from '../../core/types/sync.types';
import { getSyncFilePrefix } from '../../util/sync-file-prefix';
import { ArchiveDbAdapter } from '../../../core/persistence/archive-db-adapter.service';
@ -204,11 +207,12 @@ describe('FileBasedSyncAdapterService', () => {
const result = await adapter.uploadOps([], 'client1');
// Should still create the sync file with current state
// Uses conditional upload (isForceOverwrite: false) with null rev for new file
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
jasmine.any(String),
null,
true,
false, // Conditional upload - provider handles null rev for new file creation
);
expect(result.results).toEqual([]);
});
@ -224,11 +228,12 @@ describe('FileBasedSyncAdapterService', () => {
expect(result.results.length).toBe(1);
expect(result.results[0].accepted).toBe(true);
// Uses conditional upload with null rev for new file
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
jasmine.any(String),
null,
true,
false, // Conditional upload - provider handles null rev for new file creation
);
});
@ -283,52 +288,6 @@ describe('FileBasedSyncAdapterService', () => {
expect(result.newOps![0].op.id).toBe('other-op');
});
it('should create backup before uploading', async () => {
// First download to set expected version
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
await adapter.downloadOps(0); // Sets expected version to 1
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-2' }));
const op = createMockSyncOp();
await adapter.uploadOps([op], 'client1');
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
jasmine.any(String),
null,
true,
);
});
it('should continue even if backup fails', async () => {
// First download to set expected version
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
await adapter.downloadOps(0); // Sets expected version to 1
// First upload (backup) fails, second (main) succeeds
let uploadCalls = 0;
mockProvider.uploadFile.and.callFake(() => {
uploadCalls++;
if (uploadCalls === 1) {
return Promise.reject(new Error('Backup failed'));
}
return Promise.resolve({ rev: 'rev-2' });
});
const op = createMockSyncOp();
const result = await adapter.uploadOps([op], 'client1');
// Should still succeed
expect(result.results[0].accepted).toBe(true);
});
it('should merge vector clocks from all ops', async () => {
mockProvider.downloadFile.and.throwError(
new RemoteFileNotFoundAPIError('sync-data.json'),
@ -354,6 +313,105 @@ describe('FileBasedSyncAdapterService', () => {
const uploadedData = parseWithPrefix(uploadedDataStr);
expect(uploadedData.vectorClock).toEqual({ client1: 3, client2: 2, client3: 1 });
});
it('should use cached data from downloadOps when uploading (avoids redundant download)', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-2' }));
// First, download to populate cache
await adapter.downloadOps(0);
// Reset download spy call count
mockProvider.downloadFile.calls.reset();
// Upload should use cached data, not download again
const op = createMockSyncOp();
await adapter.uploadOps([op], 'client1');
// Download should NOT be called again during upload (cache hit)
expect(mockProvider.downloadFile).not.toHaveBeenCalled();
});
it('should use conditional upload with revToMatch from cache', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-123' }),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-124' }));
// Download to populate cache
await adapter.downloadOps(0);
mockProvider.downloadFile.calls.reset();
// Upload should pass revToMatch parameter
const op = createMockSyncOp();
await adapter.uploadOps([op], 'client1');
// Verify upload was called with revToMatch (cached rev)
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
jasmine.any(String),
'rev-123', // revToMatch from cached download
false,
);
});
it('should retry upload once when UploadRevToMatchMismatchAPIError occurs', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
// First upload fails with rev mismatch, second succeeds
let uploadCalls = 0;
mockProvider.uploadFile.and.callFake(() => {
uploadCalls++;
if (uploadCalls === 1) {
return Promise.reject(new UploadRevToMatchMismatchAPIError('Rev mismatch'));
}
return Promise.resolve({ rev: 'rev-3' });
});
// Download to populate cache
await adapter.downloadOps(0);
const op = createMockSyncOp();
const result = await adapter.uploadOps([op], 'client1');
// Should succeed after retry
expect(result.results[0].accepted).toBe(true);
// Upload was called twice (initial + retry)
expect(uploadCalls).toBe(2);
// Download was called twice (initial cache + re-download on retry)
expect(mockProvider.downloadFile).toHaveBeenCalledTimes(2);
});
it('should clear cache after successful upload', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-2' }));
// Download to populate cache
await adapter.downloadOps(0);
// Upload (should clear cache)
const op = createMockSyncOp();
await adapter.uploadOps([op], 'client1');
mockProvider.downloadFile.calls.reset();
// Another upload should re-download since cache was cleared
const op2 = createMockSyncOp({ id: 'op-2' });
await adapter.uploadOps([op2], 'client1');
// Download should be called because cache was cleared after first upload
expect(mockProvider.downloadFile).toHaveBeenCalledTimes(1);
});
});
describe('downloadOps', () => {
@ -589,20 +647,27 @@ describe('FileBasedSyncAdapterService', () => {
expect(uploadedData.recentOps).toEqual([]); // Fresh start
});
it('should backup existing file before snapshot upload', async () => {
const existingData = createMockSyncData();
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(existingData), rev: 'rev-1' }),
it('should upload snapshot directly without backup (snapshots replace state completely)', async () => {
mockProvider.downloadFile.and.throwError(
new RemoteFileNotFoundAPIError('sync-data.json'),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-2' }));
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-1' }));
await adapter.uploadSnapshot({}, 'client1', 'recovery', { client1: 1 }, 1);
// Should upload to main sync file with force overwrite (no backup created)
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
jasmine.any(String),
null,
true,
true, // Force overwrite - snapshots replace state completely
);
// Should NOT create backup file
expect(mockProvider.uploadFile).not.toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
jasmine.any(String),
jasmine.anything(),
jasmine.anything(),
);
});

18
src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts
View file

@ -613,6 +613,20 @@ export class FileBasedSyncAdapterService {
throw e;
}
// Detect syncVersion reset (e.g., another client uploaded a snapshot).
// When syncVersion resets to a lower value, we need to signal this to trigger
// a re-download from seq 0 so the caller can get the snapshotState.
const previousExpectedVersion = this._expectedSyncVersions.get(providerKey) ?? 0;
const versionWasReset =
previousExpectedVersion > 0 && syncData.syncVersion < previousExpectedVersion;
if (versionWasReset) {
OpLog.warn(
`FileBasedSyncAdapter: Sync version reset detected (${previousExpectedVersion} → ${syncData.syncVersion}). ` +
'Another client may have uploaded a snapshot. Signaling gap detection.',
);
}
// Update expected version for next upload
this._expectedSyncVersions.set(providerKey, syncData.syncVersion);
@ -689,6 +703,10 @@ export class FileBasedSyncAdapterService {
hasMore,
latestSeq,
snapshotVectorClock: syncData.vectorClock,
// Signal gap detection when sync version was reset (another client uploaded snapshot).
// This triggers the download service to re-download from seq 0, which will include
// the snapshotState for proper state replacement.
gapDetected: versionWasReset,
// Include full state snapshot for fresh downloads (sinceSeq === 0)
// This allows new clients to bootstrap with complete state, not just recent ops
...(isForceFromZero && syncData.state ? { snapshotState: syncData.state } : {}),

169
src/app/op-log/sync/operation-log-sync.service.spec.ts
View file

@ -20,8 +20,15 @@ import { RejectedOpsHandlerService } from './rejected-ops-handler.service';
import { OperationWriteFlushService } from './operation-write-flush.service';
import { SuperSyncStatusService } from './super-sync-status.service';
import { provideMockStore } from '@ngrx/store/testing';
import { ActionType, Operation, OpType } from '../core/operation.types';
import {
ActionType,
Operation,
OperationLogEntry,
OpType,
} from '../core/operation.types';
import { TranslateService } from '@ngx-translate/core';
import { LocalDataConflictError } from '../core/errors/sync-errors';
import { SyncHydrationService } from '../persistence/sync-hydration.service';
describe('OperationLogSyncService', () => {
let service: OperationLogSyncService;
@ -157,6 +164,12 @@ describe('OperationLogSyncService', () => {
{ provide: RejectedOpsHandlerService, useValue: rejectedOpsHandlerServiceSpy },
{ provide: OperationWriteFlushService, useValue: writeFlushServiceSpy },
{ provide: SuperSyncStatusService, useValue: superSyncStatusServiceSpy },
{
provide: SyncHydrationService,
useValue: jasmine.createSpyObj('SyncHydrationService', [
'hydrateFromRemoteSync',
]),
},
],
});
@ -676,6 +689,159 @@ describe('OperationLogSyncService', () => {
expect(setLastServerSeqSpy).toHaveBeenCalledWith(100);
});
});
describe('LocalDataConflictError for file-based sync', () => {
it('should throw LocalDataConflictError when client has unsynced ops and receives snapshotState', async () => {
// Mock unsynced local ops as OperationLogEntry
const unsyncedEntry: OperationLogEntry = {
seq: 1,
op: {
id: 'local-op-1',
clientId: 'client-A',
actionType: 'test' as ActionType,
opType: OpType.Update,
entityType: 'TASK',
entityId: 'task-1',
payload: { title: 'Local Title' },
vectorClock: { clientA: 1 },
timestamp: Date.now(),
schemaVersion: 1,
},
appliedAt: Date.now(),
source: 'local',
};
opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve([unsyncedEntry]));
// Mock download service returning snapshotState (file-based sync scenario)
downloadServiceSpy.downloadRemoteOps.and.returnValue(
Promise.resolve({
newOps: [],
hasMore: false,
latestSeq: 0,
needsFullStateUpload: false,
success: true,
failedFileCount: 0,
snapshotState: { tasks: [{ id: 'remote-task' }] }, // Remote snapshot
snapshotVectorClock: { clientB: 5 },
}),
);
const mockProvider = {
isReady: () => Promise.resolve(true),
supportsOperationSync: true,
} as any;
// Should throw LocalDataConflictError
await expectAsync(service.downloadRemoteOps(mockProvider)).toBeRejectedWith(
jasmine.any(LocalDataConflictError),
);
});
it('should include correct context in LocalDataConflictError', async () => {
const unsyncedEntries: OperationLogEntry[] = [
{
seq: 1,
op: {
id: 'local-op-1',
clientId: 'client-A',
actionType: 'test' as ActionType,
opType: OpType.Update,
entityType: 'TASK',
entityId: 'task-1',
payload: {},
vectorClock: { clientA: 1 },
timestamp: Date.now(),
schemaVersion: 1,
},
appliedAt: Date.now(),
source: 'local',
},
{
seq: 2,
op: {
id: 'local-op-2',
clientId: 'client-A',
actionType: 'test' as ActionType,
opType: OpType.Create,
entityType: 'TASK',
entityId: 'task-2',
payload: {},
vectorClock: { clientA: 2 },
timestamp: Date.now(),
schemaVersion: 1,
},
appliedAt: Date.now(),
source: 'local',
},
];
opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve(unsyncedEntries));
const remoteSnapshot = { tasks: [{ id: 'remote-task' }] };
const remoteVectorClock = { clientB: 5, clientC: 3 };
downloadServiceSpy.downloadRemoteOps.and.returnValue(
Promise.resolve({
newOps: [],
hasMore: false,
latestSeq: 0,
needsFullStateUpload: false,
success: true,
failedFileCount: 0,
snapshotState: remoteSnapshot,
snapshotVectorClock: remoteVectorClock,
}),
);
const mockProvider = {
isReady: () => Promise.resolve(true),
supportsOperationSync: true,
} as any;
try {
await service.downloadRemoteOps(mockProvider);
fail('Expected LocalDataConflictError to be thrown');
} catch (error) {
expect(error).toBeInstanceOf(LocalDataConflictError);
const conflictError = error as LocalDataConflictError;
expect(conflictError.unsyncedCount).toBe(2);
expect(conflictError.remoteSnapshotState).toEqual(remoteSnapshot);
expect(conflictError.remoteVectorClock).toEqual(remoteVectorClock);
}
});
it('should NOT throw LocalDataConflictError when client has no unsynced ops', async () => {
// No unsynced ops
opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve([]));
const syncHydrationServiceSpy = TestBed.inject(
SyncHydrationService,
) as jasmine.SpyObj<SyncHydrationService>;
syncHydrationServiceSpy.hydrateFromRemoteSync.and.resolveTo();
downloadServiceSpy.downloadRemoteOps.and.returnValue(
Promise.resolve({
newOps: [],
hasMore: false,
latestSeq: 0,
needsFullStateUpload: false,
success: true,
failedFileCount: 0,
snapshotState: { tasks: [] },
snapshotVectorClock: { clientB: 5 },
}),
);
const mockProvider = {
isReady: () => Promise.resolve(true),
supportsOperationSync: true,
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
} as any;
// Should NOT throw - should hydrate from snapshot instead
await expectAsync(service.downloadRemoteOps(mockProvider)).toBeResolved();
expect(syncHydrationServiceSpy.hydrateFromRemoteSync).toHaveBeenCalled();
});
});
});
});
@ -713,6 +879,7 @@ describe('OperationLogSyncService', () => {
expect(serverMigrationServiceSpy.handleServerMigration).toHaveBeenCalledWith(
mockProvider,
{ skipServerEmptyCheck: true }, // Force upload skips the server empty check
);
});

48
src/app/op-log/sync/operation-log-sync.service.ts
View file

@ -8,6 +8,7 @@ import { OperationLogUploadService, UploadResult } from './operation-log-upload.
import { OperationLogDownloadService } from './operation-log-download.service';
import { SnackService } from '../../core/snack/snack.service';
import { T } from '../../t.const';
import { LocalDataConflictError } from '../core/errors/sync-errors';
import { SuperSyncStatusService } from './super-sync-status.service';
import { ServerMigrationService } from './server-migration.service';
import { OperationWriteFlushService } from './operation-write-flush.service';
@ -259,27 +260,17 @@ export class OperationLogSyncService {
const hasLocalChanges = unsyncedOps.length > 0;
if (hasLocalChanges) {
// Non-fresh client with local changes - ask for confirmation
// Non-fresh client with local changes - throw error to trigger conflict dialog in SyncWrapperService
// This allows the user to choose between keeping local data (upload) or remote data (download)
OpLog.warn(
`OperationLogSyncService: Client has ${unsyncedOps.length} unsynced local ops. ` +
'Requesting confirmation before replacing with snapshot.',
'Throwing LocalDataConflictError for conflict resolution dialog.',
);
const confirmed = this._showLocalDataReplaceConfirmation(unsyncedOps.length);
if (!confirmed) {
OpLog.normal(
'OperationLogSyncService: User cancelled sync. Local data preserved.',
);
this.snackService.open({
msg: T.F.SYNC.S.LOCAL_DATA_REPLACE_CANCELLED,
});
return { serverMigrationHandled: false, localWinOpsCreated: 0, newOpsCount: 0 };
}
// User chose to accept remote - clear unsynced ops since they'll be replaced
await this.opLogStore.clearUnsyncedOps();
OpLog.normal(
'OperationLogSyncService: User confirmed. Cleared unsynced ops and proceeding with snapshot.',
throw new LocalDataConflictError(
unsyncedOps.length,
result.snapshotState as Record<string, unknown>,
result.snapshotVectorClock,
);
} else {
// Show fresh client confirmation if this is a wholly fresh client
@ -424,24 +415,6 @@ export class OperationLogSyncService {
return window.confirm(`${title}\n\n${message}`);
}
/**
* Shows a confirmation dialog when local data would be replaced by remote snapshot.
* Used when a non-fresh client (with unsynced local ops) receives a snapshot.
* Uses synchronous window.confirm() to prevent race conditions.
*/
private _showLocalDataReplaceConfirmation(unsyncedCount: number): boolean {
const title = this.translateService.instant(
T.F.SYNC.D_LOCAL_DATA_REPLACE_CONFIRM.TITLE,
);
const message = this.translateService.instant(
T.F.SYNC.D_LOCAL_DATA_REPLACE_CONFIRM.MESSAGE,
{
count: unsyncedCount,
},
);
return window.confirm(`${title}\n\n${message}`);
}
/**
* Force upload local state as a SYNC_IMPORT, replacing all remote data.
* This is used when user explicitly chooses "USE_LOCAL" in conflict resolution.
@ -457,7 +430,10 @@ export class OperationLogSyncService {
);
// Create SYNC_IMPORT with current local state
await this.serverMigrationService.handleServerMigration(syncProvider);
// Pass skipServerEmptyCheck=true because we're forcing upload even if server has data
await this.serverMigrationService.handleServerMigration(syncProvider, {
skipServerEmptyCheck: true,
});
// Upload the SYNC_IMPORT (and any pending ops)
await this.uploadPendingOps(syncProvider);

26
src/app/op-log/sync/server-migration.service.ts
View file

@ -106,6 +106,7 @@ export class ServerMigrationService {
*
* ## Process
* 1. Double-check server is still empty (in case another client just uploaded)
* - Unless skipServerEmptyCheck is true (for force upload scenarios)
* 2. Get current state from NgRx store
* 3. Skip if state is empty (nothing to migrate)
* 4. Validate and repair state (prevent propagating corruption)
@ -123,17 +124,26 @@ export class ServerMigrationService {
* compares operations, all pre-import ops are LESS_THAN the import.
*
* @param syncProvider - The sync provider to use for double-check
* @param options - Optional configuration
* @param options.skipServerEmptyCheck - If true, creates SYNC_IMPORT even if server has data.
* Used for "USE_LOCAL" conflict resolution to force overwrite remote with local state.
*/
async handleServerMigration(syncProvider: OperationSyncCapable): Promise<void> {
async handleServerMigration(
syncProvider: OperationSyncCapable,
options?: { skipServerEmptyCheck?: boolean },
): Promise<void> {
// Double-check server is still empty (in case another client just uploaded)
// This is called inside the upload lock, but network timing could still race
const freshCheck = await syncProvider.downloadOps(0, undefined, 1);
if (freshCheck.latestSeq !== 0) {
OpLog.warn(
'ServerMigrationService: Server no longer empty, aborting SYNC_IMPORT. ' +
'Another client may have just uploaded.',
);
return;
// Skip this check when forcing upload (conflict resolution "USE_LOCAL")
if (!options?.skipServerEmptyCheck) {
const freshCheck = await syncProvider.downloadOps(0, undefined, 1);
if (freshCheck.latestSeq !== 0) {
OpLog.warn(
'ServerMigrationService: Server no longer empty, aborting SYNC_IMPORT. ' +
'Another client may have just uploaded.',
);
return;
}
}
OpLog.warn(

32
src/app/op-log/testing/integration/legacy-data-migration.integration.spec.ts
View file

@ -129,7 +129,7 @@ describe('Legacy Data Migration Integration', () => {
});
describe('Orphan Operations Handling', () => {
it('should clear orphan operations', async () => {
it('should clear orphan operations when legacy data exists', async () => {
// Pre-create orphan operations (e.g., from effects that ran before migration)
await opLogStore.append({
id: 'orphan-op-1',
@ -144,6 +144,11 @@ describe('Legacy Data Migration Integration', () => {
schemaVersion: 1,
});
// Legacy data exists - orphan ops should be cleared before migration
mockLegacyPfDb.hasUsableEntityData.and.returnValue(Promise.resolve(true));
// Lock acquisition fails - prevents migration from proceeding (test focuses on clearing)
mockLegacyPfDb.acquireMigrationLock.and.returnValue(Promise.resolve(false));
await migrationService.checkAndMigrate();
// Should have cleared orphan ops
@ -151,6 +156,31 @@ describe('Legacy Data Migration Integration', () => {
expect(ops.length).toBe(0);
});
it('should NOT clear orphan operations when no legacy data exists (fresh install)', async () => {
// Pre-create orphan operations
await opLogStore.append({
id: 'orphan-op-1',
actionType: '[Task] Update Task' as ActionType,
opType: OpType.Update,
entityType: 'TASK',
entityId: 'task-1',
payload: { title: 'Updated' },
clientId: 'orphanClient',
vectorClock: { orphanClient: 1 },
timestamp: Date.now() - 50000,
schemaVersion: 1,
});
// No legacy data - orphan ops should be kept (fresh install scenario)
mockLegacyPfDb.hasUsableEntityData.and.returnValue(Promise.resolve(false));
await migrationService.checkAndMigrate();
// Should NOT clear orphan ops - they will be replayed by hydrator
const ops = await opLogStore.getOpsAfterSeq(0);
expect(ops.length).toBe(1);
});
it('should not clear operations if first op is Genesis', async () => {
// Pre-create a Genesis operation followed by normal operations
await opLogStore.append({