mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
refactor(sync): use discriminated unions for sync orchestrator results
Replace flag-based result objects with DownloadOutcome and UploadOutcome discriminated unions at the orchestrator level. This eliminates null returns, optional boolean checks, and ambiguous flag combinations. - Add DownloadOutcome (5 variants) and UploadOutcome (3 variants) types - Update OperationLogSyncService to return discriminated unions - Update SyncWrapperService and ImmediateUploadService consumers - Use exhaustive switch in downloadCallback adapter - Replace piggybackedOps array pass-through with piggybackedOpsCount
This commit is contained in:
Johannes Millan
parent
88d57e8e8b
commit
efaffe97a5
7 changed files with 326 additions and 303 deletions
|
|
@ -92,18 +92,18 @@ describe('SyncWrapperService', () => {
|
|||
]);
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -335,10 +335,7 @@ describe('SyncWrapperService', () => {
|
|||
it('should NOT update lastSyncedProviderId when download is cancelled', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
cancelled: true,
|
||||
kind: 'cancelled' as const,
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -353,16 +350,18 @@ describe('SyncWrapperService', () => {
|
|||
const callOrder: string[] = [];
|
||||
mockSyncService.downloadRemoteOps.and.callFake(async () => {
|
||||
callOrder.push('download');
|
||||
return { newOpsCount: 0, serverMigrationHandled: false, localWinOpsCreated: 0 };
|
||||
return { kind: 'no_new_ops' as const };
|
||||
});
|
||||
mockSyncService.uploadPendingOps.and.callFake(async () => {
|
||||
callOrder.push('upload');
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -374,18 +373,20 @@ describe('SyncWrapperService', () => {
|
|||
it('should re-upload when localWinOpsCreated > 0 from download', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'ops_processed' as const,
|
||||
newOpsCount: 5,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 3, // LWW created 3 local-win ops
|
||||
}),
|
||||
);
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 3,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -398,9 +399,7 @@ describe('SyncWrapperService', () => {
|
|||
it('should re-upload when localWinOpsCreated > 0 from upload', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
// First upload returns localWinOpsCreated > 0
|
||||
|
|
@ -409,19 +408,23 @@ describe('SyncWrapperService', () => {
|
|||
uploadCallCount++;
|
||||
if (uploadCallCount === 1) {
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 2,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 2, // LWW created ops from piggybacked
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
}
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 2,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -434,18 +437,20 @@ describe('SyncWrapperService', () => {
|
|||
it('should NOT re-upload when no localWinOpsCreated', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'ops_processed' as const,
|
||||
newOpsCount: 5,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
}),
|
||||
);
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 3,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -477,11 +482,13 @@ describe('SyncWrapperService', () => {
|
|||
it('should set ERROR and return HANDLED_ERROR when upload has rejected ops with "Payload too complex"', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 1,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Payload too complex (max depth 50)' }],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 1,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Payload too complex (max depth 50)' }],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -495,11 +502,13 @@ describe('SyncWrapperService', () => {
|
|||
it('should set ERROR and return HANDLED_ERROR when upload has rejected ops with "Payload too large"', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 1,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Payload too large' }],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 1,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Payload too large' }],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -513,11 +522,13 @@ describe('SyncWrapperService', () => {
|
|||
it('should set ERROR for non-payload rejected ops', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 1,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Some other rejection' }],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 1,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [{ opId: 'test-op', error: 'Some other rejection' }],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -530,11 +541,13 @@ describe('SyncWrapperService', () => {
|
|||
it('should set IN_SYNC when some ops uploaded and none rejected', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 5,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -547,14 +560,16 @@ describe('SyncWrapperService', () => {
|
|||
it('should set ERROR when multiple ops rejected', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 3,
|
||||
rejectedCount: 2,
|
||||
piggybackedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 2,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [
|
||||
{ opId: 'op1', error: 'Conflict' },
|
||||
{ opId: 'op2', error: 'Validation failed' },
|
||||
],
|
||||
localWinOpsCreated: 0,
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -564,8 +579,10 @@ describe('SyncWrapperService', () => {
|
|||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
|
||||
});
|
||||
|
||||
it('should set IN_SYNC when uploadResult is null (fresh client)', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(Promise.resolve(null));
|
||||
it('should set IN_SYNC when uploadResult is blocked_fresh_client', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({ kind: 'blocked_fresh_client' as const }),
|
||||
);
|
||||
|
||||
const result = await service.sync();
|
||||
|
||||
|
|
@ -573,14 +590,16 @@ describe('SyncWrapperService', () => {
|
|||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('IN_SYNC');
|
||||
});
|
||||
|
||||
it('should set IN_SYNC when rejectedCount is 0 even with empty rejectedOps array', async () => {
|
||||
it('should set IN_SYNC when permanentRejectionCount is 0 even with empty rejectedOps array', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 0,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -1088,7 +1107,7 @@ describe('SyncWrapperService', () => {
|
|||
callOrder.push('sync-download-start');
|
||||
await syncPromise;
|
||||
callOrder.push('sync-download-end');
|
||||
return { newOpsCount: 0, serverMigrationHandled: false, localWinOpsCreated: 0 };
|
||||
return { kind: 'no_new_ops' as const };
|
||||
});
|
||||
|
||||
// Start sync
|
||||
|
|
@ -1415,19 +1434,19 @@ describe('SyncWrapperService', () => {
|
|||
it('should stop after MAX_LWW_REUPLOAD_RETRIES when upload always returns localWinOpsCreated', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
// Upload always returns localWinOpsCreated: 2 (never resolves)
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 2,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 2,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -1448,9 +1467,7 @@ describe('SyncWrapperService', () => {
|
|||
it('should exit early when retry returns localWinOpsCreated: 0', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -1458,12 +1475,14 @@ describe('SyncWrapperService', () => {
|
|||
mockSyncService.uploadPendingOps.and.callFake(async () => {
|
||||
uploadCallCount++;
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 2,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
// First call returns 1, second call returns 0 -> exits loop
|
||||
localWinOpsCreated: uploadCallCount <= 1 ? 1 : 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -1475,12 +1494,10 @@ describe('SyncWrapperService', () => {
|
|||
expect(result).toBe(SyncStatus.InSync);
|
||||
});
|
||||
|
||||
it('should treat null reupload result as 0 localWinOpsCreated and exit loop', async () => {
|
||||
it('should treat blocked_fresh_client reupload result as 0 localWinOpsCreated and exit loop', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
newOpsCount: 0,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -1489,20 +1506,22 @@ describe('SyncWrapperService', () => {
|
|||
uploadCallCount++;
|
||||
if (uploadCallCount === 1) {
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 2,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
}
|
||||
// Second call returns null (e.g., fresh client scenario)
|
||||
return null;
|
||||
// Second call returns blocked_fresh_client (treated as 0 localWinOpsCreated)
|
||||
return { kind: 'blocked_fresh_client' as const };
|
||||
});
|
||||
|
||||
const result = await service.sync();
|
||||
|
||||
// 1 initial + 1 retry (returns null -> treated as 0) = 2 total
|
||||
// 1 initial + 1 retry (returns blocked_fresh_client -> treated as 0) = 2 total
|
||||
expect(mockSyncService.uploadPendingOps).toHaveBeenCalledTimes(2);
|
||||
expect(result).toBe(SyncStatus.InSync);
|
||||
});
|
||||
|
|
@ -1510,8 +1529,8 @@ describe('SyncWrapperService', () => {
|
|||
it('should enter while loop when both download and upload produce LWW ops', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
kind: 'ops_processed' as const,
|
||||
newOpsCount: 5,
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 2, // download produced LWW ops
|
||||
}),
|
||||
);
|
||||
|
|
@ -1520,12 +1539,14 @@ describe('SyncWrapperService', () => {
|
|||
mockSyncService.uploadPendingOps.and.callFake(async () => {
|
||||
uploadCallCount++;
|
||||
return {
|
||||
kind: 'completed' as const,
|
||||
uploadedCount: 3,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
piggybackedOpsCount: 0,
|
||||
// First upload also produces LWW ops, subsequent do not
|
||||
localWinOpsCreated: uploadCallCount === 1 ? 1 : 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -324,14 +324,12 @@ export class SyncWrapperService {
|
|||
syncCapableProvider,
|
||||
isProviderSwitch ? { forceFromSeq0: true } : undefined,
|
||||
);
|
||||
SyncLog.log(
|
||||
`SyncWrapperService: Download complete. newOps=${downloadResult.newOpsCount}, migration=${downloadResult.serverMigrationHandled}`,
|
||||
);
|
||||
SyncLog.log(`SyncWrapperService: Download complete. kind=${downloadResult.kind}`);
|
||||
|
||||
// If user cancelled the sync import conflict dialog, skip upload entirely.
|
||||
// This keeps the local state unchanged and doesn't push it to the server.
|
||||
// Don't update lastSyncedProvider so the next sync retries with forceFromSeq0.
|
||||
if (downloadResult.cancelled) {
|
||||
if (downloadResult.kind === 'cancelled') {
|
||||
SyncLog.log('SyncWrapperService: Sync cancelled by user. Skipping upload phase.');
|
||||
this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED');
|
||||
return 'HANDLED_ERROR';
|
||||
|
|
@ -343,14 +341,14 @@ export class SyncWrapperService {
|
|||
// 2. Upload pending local ops
|
||||
const uploadResult =
|
||||
await this._opLogSyncService.uploadPendingOps(syncCapableProvider);
|
||||
if (uploadResult) {
|
||||
if (uploadResult.kind === 'completed') {
|
||||
SyncLog.log(
|
||||
`SyncWrapperService: Upload complete. uploaded=${uploadResult.uploadedCount}, piggybacked=${uploadResult.piggybackedOps.length}`,
|
||||
`SyncWrapperService: Upload complete. uploaded=${uploadResult.uploadedCount}, piggybacked=${uploadResult.piggybackedOpsCount}`,
|
||||
);
|
||||
}
|
||||
|
||||
// If upload was cancelled (piggybacked SYNC_IMPORT conflict dialog), skip LWW re-upload
|
||||
if (uploadResult?.cancelled) {
|
||||
if (uploadResult.kind === 'cancelled') {
|
||||
SyncLog.log(
|
||||
'SyncWrapperService: Upload cancelled by user (piggybacked SYNC_IMPORT). Skipping LWW re-upload.',
|
||||
);
|
||||
|
|
@ -359,10 +357,12 @@ export class SyncWrapperService {
|
|||
}
|
||||
|
||||
// 3. If LWW created local-win ops, upload them (with retry limit to prevent infinite loops)
|
||||
const downloadLwwOps =
|
||||
downloadResult.kind === 'ops_processed' ? downloadResult.localWinOpsCreated : 0;
|
||||
const uploadLwwOps =
|
||||
uploadResult.kind === 'completed' ? uploadResult.localWinOpsCreated : 0;
|
||||
let lwwRetries = 0;
|
||||
let pendingLwwOps =
|
||||
(downloadResult.localWinOpsCreated ?? 0) +
|
||||
(uploadResult?.localWinOpsCreated ?? 0);
|
||||
let pendingLwwOps = downloadLwwOps + uploadLwwOps;
|
||||
while (pendingLwwOps > 0 && lwwRetries < MAX_LWW_REUPLOAD_RETRIES) {
|
||||
lwwRetries++;
|
||||
SyncLog.log(
|
||||
|
|
@ -371,7 +371,8 @@ export class SyncWrapperService {
|
|||
);
|
||||
const reuploadResult =
|
||||
await this._opLogSyncService.uploadPendingOps(syncCapableProvider);
|
||||
pendingLwwOps = reuploadResult?.localWinOpsCreated ?? 0;
|
||||
pendingLwwOps =
|
||||
reuploadResult.kind === 'completed' ? reuploadResult.localWinOpsCreated : 0;
|
||||
}
|
||||
if (pendingLwwOps > 0) {
|
||||
SyncLog.warn(
|
||||
|
|
@ -383,22 +384,9 @@ export class SyncWrapperService {
|
|||
return SyncStatus.UpdateRemote;
|
||||
}
|
||||
|
||||
// 4. Check for permanent rejection failures - these are critical failures that should
|
||||
// NOT be reported as "in sync" to the user.
|
||||
//
|
||||
// IMPORTANT: We check permanentRejectionCount, NOT rejectedCount.
|
||||
// - Transient errors (INTERNAL_ERROR) will retry on next sync
|
||||
// - Resolved conflicts (CONFLICT_CONCURRENT merged successfully) are not failures
|
||||
// - Duplicate operations (already synced) are not failures
|
||||
// Only permanent rejections (VALIDATION_ERROR, payload too large) should block success.
|
||||
//
|
||||
// Fall back to rejectedCount for backward compatibility if permanentRejectionCount is undefined.
|
||||
const permanentFailures =
|
||||
uploadResult?.permanentRejectionCount ?? uploadResult?.rejectedCount ?? 0;
|
||||
|
||||
if (permanentFailures > 0) {
|
||||
// Check for payload errors first (special handling with alert)
|
||||
const hasPayloadError = uploadResult?.rejectedOps?.some(
|
||||
// 4. Check for permanent rejection failures
|
||||
if (uploadResult.kind === 'completed' && uploadResult.permanentRejectionCount > 0) {
|
||||
const hasPayloadError = uploadResult.rejectedOps.some(
|
||||
(r) =>
|
||||
r.error?.includes('Payload too complex') ||
|
||||
r.error?.includes('Payload too large'),
|
||||
|
|
@ -407,25 +395,22 @@ export class SyncWrapperService {
|
|||
if (hasPayloadError) {
|
||||
SyncLog.err(
|
||||
'SyncWrapperService: Upload rejected - payload too large/complex',
|
||||
uploadResult?.rejectedOps,
|
||||
uploadResult.rejectedOps,
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
// Use alertDialog for maximum visibility - this is a critical error
|
||||
alertDialog(this._translateService.instant(T.F.SYNC.S.ERROR_PAYLOAD_TOO_LARGE));
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
|
||||
// Other permanent rejections - still shouldn't claim success
|
||||
SyncLog.err(
|
||||
`SyncWrapperService: Upload had ${permanentFailures} permanent rejection(s), not marking as IN_SYNC`,
|
||||
uploadResult?.rejectedOps,
|
||||
`SyncWrapperService: Upload had ${uploadResult.permanentRejectionCount} permanent rejection(s), not marking as IN_SYNC`,
|
||||
uploadResult.rejectedOps,
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
|
||||
// Mark as in-sync for all providers after successful sync
|
||||
// This indicates the sync operation completed successfully
|
||||
this._providerManager.setSyncStatus('IN_SYNC');
|
||||
SyncLog.log('SyncWrapperService: Sync complete, status=IN_SYNC');
|
||||
return SyncStatus.InSync;
|
||||
|
|
|
|||
|
|
@ -157,3 +157,71 @@ export interface DownloadResultForRejection {
|
|||
export type DownloadCallback = (options?: {
|
||||
forceFromSeq0?: boolean;
|
||||
}) => Promise<DownloadResultForRejection>;
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Orchestrator-level result types (discriminated unions)
|
||||
//
|
||||
// These replace the ad-hoc return types from OperationLogSyncService methods.
|
||||
// Transport-level types (DownloadResult, UploadResult) remain unchanged.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Outcome of OperationLogSyncService.downloadRemoteOps().
|
||||
*
|
||||
* Each variant represents a distinct terminal state — callers switch on `kind`
|
||||
* instead of checking boolean flag combinations.
|
||||
*/
|
||||
export type DownloadOutcome =
|
||||
| {
|
||||
/** Server was empty/reset — a SYNC_IMPORT was created. Caller must upload. */
|
||||
kind: 'server_migration_handled';
|
||||
}
|
||||
| {
|
||||
/** No new operations on server. */
|
||||
kind: 'no_new_ops';
|
||||
allOpClocks?: VectorClock[];
|
||||
snapshotVectorClock?: VectorClock;
|
||||
}
|
||||
| {
|
||||
/** Incremental ops were downloaded and processed. */
|
||||
kind: 'ops_processed';
|
||||
newOpsCount: number;
|
||||
localWinOpsCreated: number;
|
||||
allOpClocks?: VectorClock[];
|
||||
snapshotVectorClock?: VectorClock;
|
||||
}
|
||||
| {
|
||||
/** File-based snapshot was hydrated (fresh download from file provider). */
|
||||
kind: 'snapshot_hydrated';
|
||||
allOpClocks?: VectorClock[];
|
||||
snapshotVectorClock?: VectorClock;
|
||||
}
|
||||
| {
|
||||
/** User cancelled a SYNC_IMPORT conflict dialog. */
|
||||
kind: 'cancelled';
|
||||
};
|
||||
|
||||
/**
|
||||
* Outcome of OperationLogSyncService.uploadPendingOps().
|
||||
*
|
||||
* Each variant represents a distinct terminal state.
|
||||
*/
|
||||
export type UploadOutcome =
|
||||
| {
|
||||
/** Upload was blocked because this is a fresh client with no history. */
|
||||
kind: 'blocked_fresh_client';
|
||||
}
|
||||
| {
|
||||
/** Upload completed (ops may have been accepted, rejected, or piggybacked). */
|
||||
kind: 'completed';
|
||||
uploadedCount: number;
|
||||
piggybackedOpsCount: number;
|
||||
localWinOpsCreated: number;
|
||||
permanentRejectionCount: number;
|
||||
hasMorePiggyback: boolean;
|
||||
rejectedOps: RejectedOpInfo[];
|
||||
}
|
||||
| {
|
||||
/** User cancelled a piggybacked SYNC_IMPORT conflict dialog. */
|
||||
kind: 'cancelled';
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2,11 +2,11 @@ import { TestBed, fakeAsync, tick } from '@angular/core/testing';
|
|||
import { ImmediateUploadService } from './immediate-upload.service';
|
||||
import { SyncProviderManager } from '../sync-providers/provider-manager.service';
|
||||
import { OperationLogSyncService } from './operation-log-sync.service';
|
||||
import { ActionType, Operation, OpType } from '../core/operation.types';
|
||||
import { SyncProviderId } from '../sync-providers/provider.const';
|
||||
import { DataInitStateService } from '../../core/data-init/data-init-state.service';
|
||||
import { SyncWrapperService } from '../../imex/sync/sync-wrapper.service';
|
||||
import { BehaviorSubject } from 'rxjs';
|
||||
import { RejectedOpInfo } from '../core/types/sync-results.types';
|
||||
|
||||
describe('ImmediateUploadService', () => {
|
||||
let service: ImmediateUploadService;
|
||||
|
|
@ -16,17 +16,32 @@ describe('ImmediateUploadService', () => {
|
|||
let mockSyncWrapperService: { isEncryptionOperationInProgress: boolean };
|
||||
let mockProvider: any;
|
||||
|
||||
const createMockOp = (id: string): Operation => ({
|
||||
id,
|
||||
clientId: 'clientA',
|
||||
actionType: '[Task] Add' as ActionType,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: `task-${id}`,
|
||||
payload: {},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
const completedResult = (
|
||||
overrides: Partial<{
|
||||
uploadedCount: number;
|
||||
piggybackedOpsCount: number;
|
||||
localWinOpsCreated: number;
|
||||
permanentRejectionCount: number;
|
||||
hasMorePiggyback: boolean;
|
||||
rejectedOps: RejectedOpInfo[];
|
||||
}> = {},
|
||||
): {
|
||||
kind: 'completed';
|
||||
uploadedCount: number;
|
||||
piggybackedOpsCount: number;
|
||||
localWinOpsCreated: number;
|
||||
permanentRejectionCount: number;
|
||||
hasMorePiggyback: boolean;
|
||||
rejectedOps: RejectedOpInfo[];
|
||||
} => ({
|
||||
kind: 'completed',
|
||||
uploadedCount: 0,
|
||||
piggybackedOpsCount: 0,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
...overrides,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
@ -86,12 +101,7 @@ describe('ImmediateUploadService', () => {
|
|||
describe('checkmark (IN_SYNC) behavior', () => {
|
||||
it('should show checkmark when upload succeeds and no piggybacked ops', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 3,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [], // No remote ops = confirmed in sync
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 3 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -102,14 +112,8 @@ describe('ImmediateUploadService', () => {
|
|||
}));
|
||||
|
||||
it('should NOT show checkmark when piggybacked ops exist', fakeAsync(() => {
|
||||
const piggybackedOp = createMockOp('piggybacked-1');
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 2,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [piggybackedOp], // Remote ops exist = may not be fully in sync
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 2, piggybackedOpsCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -123,12 +127,7 @@ describe('ImmediateUploadService', () => {
|
|||
|
||||
it('should NOT show checkmark when nothing was uploaded', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 0, // Nothing uploaded
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 0 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -154,18 +153,8 @@ describe('ImmediateUploadService', () => {
|
|||
});
|
||||
|
||||
it('should NOT show checkmark when piggybacked ops exist (multiple)', fakeAsync(() => {
|
||||
const piggybackedOps = [
|
||||
createMockOp('piggybacked-1'),
|
||||
createMockOp('piggybacked-2'),
|
||||
createMockOp('piggybacked-3'),
|
||||
];
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 5,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps,
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 5, piggybackedOpsCount: 3 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -182,12 +171,7 @@ describe('ImmediateUploadService', () => {
|
|||
// Need to re-create the mock with isSyncInProgress = true
|
||||
Object.defineProperty(mockProviderManager, 'isSyncInProgress', { value: true });
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -201,12 +185,7 @@ describe('ImmediateUploadService', () => {
|
|||
// Set encryption operation in progress (e.g., password change)
|
||||
mockSyncWrapperService.isEncryptionOperationInProgress = true;
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -218,14 +197,16 @@ describe('ImmediateUploadService', () => {
|
|||
|
||||
it('should handle fresh client (syncService returns null)', fakeAsync(() => {
|
||||
// Fresh client handling is now done inside syncService.uploadPendingOps()
|
||||
// which returns null for fresh clients
|
||||
mockSyncService.uploadPendingOps.and.returnValue(Promise.resolve(null));
|
||||
// which returns { kind: 'blocked_fresh_client' } for fresh clients
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({ kind: 'blocked_fresh_client' as const }),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
service.trigger();
|
||||
tick(2100);
|
||||
|
||||
// Upload was called, but returned null - no checkmark shown
|
||||
// Upload was called, but returned blocked_fresh_client - no checkmark shown
|
||||
expect(mockSyncService.uploadPendingOps).toHaveBeenCalled();
|
||||
expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalled();
|
||||
}));
|
||||
|
|
@ -233,12 +214,7 @@ describe('ImmediateUploadService', () => {
|
|||
it('should skip upload when provider is not ready', fakeAsync(() => {
|
||||
mockProvider.isReady.and.returnValue(Promise.resolve(false));
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -252,12 +228,7 @@ describe('ImmediateUploadService', () => {
|
|||
// File-based providers use periodic sync, not immediate upload
|
||||
mockProvider.id = SyncProviderId.Dropbox;
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -270,12 +241,7 @@ describe('ImmediateUploadService', () => {
|
|||
it('should skip upload for WebDAV (file-based provider)', fakeAsync(() => {
|
||||
mockProvider.id = SyncProviderId.WebDAV;
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -288,12 +254,7 @@ describe('ImmediateUploadService', () => {
|
|||
it('should skip upload for LocalFile (file-based provider)', fakeAsync(() => {
|
||||
mockProvider.id = SyncProviderId.LocalFile;
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -307,12 +268,7 @@ describe('ImmediateUploadService', () => {
|
|||
describe('debouncing', () => {
|
||||
it('should debounce rapid triggers into single upload', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
|
|
@ -334,12 +290,7 @@ describe('ImmediateUploadService', () => {
|
|||
describe('constructor initialization', () => {
|
||||
it('should auto-initialize when data is loaded', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
// Simulate data loading complete
|
||||
|
|
@ -355,12 +306,7 @@ describe('ImmediateUploadService', () => {
|
|||
|
||||
it('should not auto-initialize before data is loaded', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
// Data not loaded (still false)
|
||||
|
|
@ -373,12 +319,7 @@ describe('ImmediateUploadService', () => {
|
|||
|
||||
it('should queue triggers before initialization and replay them when initialized', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
Promise.resolve(completedResult({ uploadedCount: 1 })),
|
||||
);
|
||||
|
||||
// Trigger multiple times before initialization (data not loaded)
|
||||
|
|
|
|||
|
|
@ -182,27 +182,24 @@ export class ImmediateUploadService implements OnDestroy {
|
|||
|
||||
// Use sync service's uploadPendingOps which includes migration detection callback.
|
||||
// This ensures SYNC_IMPORT is created when switching to a new/empty server.
|
||||
// Returns null if fresh client (blocked from upload).
|
||||
const result = await this._syncService.uploadPendingOps(syncCapableProvider);
|
||||
if (!result) {
|
||||
OpLog.verbose('ImmediateUploadService: Upload returned null (fresh client)');
|
||||
if (result.kind === 'blocked_fresh_client') {
|
||||
OpLog.verbose('ImmediateUploadService: Upload blocked (fresh client)');
|
||||
return;
|
||||
}
|
||||
|
||||
// If cancelled (piggybacked SYNC_IMPORT conflict dialog), skip post-upload logic
|
||||
if (result.cancelled) {
|
||||
if (result.kind === 'cancelled') {
|
||||
OpLog.verbose(
|
||||
'ImmediateUploadService: Upload cancelled (piggybacked SYNC_IMPORT conflict)',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Note: piggybacked ops and rejected ops are already handled by _syncService.uploadPendingOps()
|
||||
// We just need to handle the sync status here.
|
||||
// result.kind === 'completed' from here
|
||||
|
||||
// If LWW local-wins created new update ops from piggybacked ops,
|
||||
// do a follow-up upload to push them to the server immediately
|
||||
if ((result.localWinOpsCreated ?? 0) > 0) {
|
||||
if (result.localWinOpsCreated > 0) {
|
||||
OpLog.verbose(
|
||||
`ImmediateUploadService: LWW created ${result.localWinOpsCreated} local-win op(s), re-uploading`,
|
||||
);
|
||||
|
|
@ -211,17 +208,17 @@ export class ImmediateUploadService implements OnDestroy {
|
|||
|
||||
// Don't show checkmark when piggybacked ops exist - there may be more
|
||||
// remote ops pending. Let normal sync cycle confirm full sync state.
|
||||
if (result.piggybackedOps.length > 0) {
|
||||
if (result.piggybackedOpsCount > 0) {
|
||||
OpLog.verbose(
|
||||
`ImmediateUploadService: Uploaded ${result.uploadedCount} ops, ` +
|
||||
`processed ${result.piggybackedOps.length} piggybacked (checkmark deferred)`,
|
||||
`processed ${result.piggybackedOpsCount} piggybacked (checkmark deferred)`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Show checkmark ONLY when server confirms no pending remote ops
|
||||
// (empty piggybackedOps means we're confirmed in sync)
|
||||
if (result.uploadedCount > 0 || (result.localWinOpsCreated ?? 0) > 0) {
|
||||
if (result.uploadedCount > 0 || result.localWinOpsCreated > 0) {
|
||||
this._providerManager.setSyncStatus('IN_SYNC');
|
||||
OpLog.verbose(
|
||||
`ImmediateUploadService: Uploaded ${result.uploadedCount} ops, confirmed in sync`,
|
||||
|
|
|
|||
|
|
@ -267,7 +267,10 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.uploadPendingOps(mockProvider);
|
||||
|
||||
expect(result?.localWinOpsCreated).toBe(0);
|
||||
expect(result.kind).toBe('completed');
|
||||
if (result.kind === 'completed') {
|
||||
expect(result.localWinOpsCreated).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('should return localWinOpsCreated count from piggybacked ops processing', async () => {
|
||||
|
|
@ -309,7 +312,10 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.uploadPendingOps(mockProvider);
|
||||
|
||||
expect(result?.localWinOpsCreated).toBe(2);
|
||||
expect(result.kind).toBe('completed');
|
||||
if (result.kind === 'completed') {
|
||||
expect(result.localWinOpsCreated).toBe(2);
|
||||
}
|
||||
});
|
||||
|
||||
describe('rejected ops handling delegation', () => {
|
||||
|
|
@ -373,9 +379,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const downloadSpy = spyOn(service, 'downloadRemoteOps').and.returnValue(
|
||||
Promise.resolve({
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0,
|
||||
kind: 'no_new_ops' as const,
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
@ -439,7 +443,10 @@ describe('OperationLogSyncService', () => {
|
|||
const result = await service.uploadPendingOps(mockProvider);
|
||||
|
||||
// Total should be 2 + 3 = 5
|
||||
expect(result?.localWinOpsCreated).toBe(5);
|
||||
expect(result.kind).toBe('completed');
|
||||
if (result.kind === 'completed') {
|
||||
expect(result.localWinOpsCreated).toBe(5);
|
||||
}
|
||||
});
|
||||
|
||||
it('should not call handleRejectedOps if processRemoteOps throws', async () => {
|
||||
|
|
@ -518,8 +525,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(result.localWinOpsCreated).toBe(0);
|
||||
expect(result.newOpsCount).toBe(0);
|
||||
expect(result.kind).toBe('no_new_ops');
|
||||
});
|
||||
|
||||
it('should return localWinOpsCreated count and newOpsCount from processing remote ops', async () => {
|
||||
|
|
@ -563,8 +569,11 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(result.localWinOpsCreated).toBe(1);
|
||||
expect(result.newOpsCount).toBe(1);
|
||||
expect(result.kind).toBe('ops_processed');
|
||||
if (result.kind === 'ops_processed') {
|
||||
expect(result.localWinOpsCreated).toBe(1);
|
||||
expect(result.newOpsCount).toBe(1);
|
||||
}
|
||||
});
|
||||
|
||||
it('should return localWinOpsCreated: 0 and newOpsCount: 0 on server migration', async () => {
|
||||
|
|
@ -587,9 +596,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(result.serverMigrationHandled).toBe(true);
|
||||
expect(result.localWinOpsCreated).toBe(0);
|
||||
expect(result.newOpsCount).toBe(0);
|
||||
expect(result.kind).toBe('server_migration_handled');
|
||||
});
|
||||
|
||||
describe('lastServerSeq persistence', () => {
|
||||
|
|
@ -1736,7 +1743,7 @@ describe('OperationLogSyncService', () => {
|
|||
mockProvider,
|
||||
{ syncImportReason: 'SERVER_MIGRATION' },
|
||||
);
|
||||
expect(result.serverMigrationHandled).toBe(true);
|
||||
expect(result.kind).toBe('server_migration_handled');
|
||||
});
|
||||
|
||||
it('should NOT create SYNC_IMPORT when fresh client has no meaningful data on empty server', async () => {
|
||||
|
|
@ -1764,7 +1771,7 @@ describe('OperationLogSyncService', () => {
|
|||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled();
|
||||
expect(result.serverMigrationHandled).toBe(false);
|
||||
expect(result.kind).not.toBe('server_migration_handled');
|
||||
});
|
||||
|
||||
it('should NOT create SYNC_IMPORT when server is not empty (latestServerSeq > 0)', async () => {
|
||||
|
|
@ -1792,7 +1799,7 @@ describe('OperationLogSyncService', () => {
|
|||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled();
|
||||
expect(result.serverMigrationHandled).toBe(false);
|
||||
expect(result.kind).not.toBe('server_migration_handled');
|
||||
});
|
||||
|
||||
it('should NOT create SYNC_IMPORT when client is not fresh (has op-log history)', async () => {
|
||||
|
|
@ -1830,7 +1837,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
// Should NOT call handleServerMigration - client is not fresh
|
||||
expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled();
|
||||
expect(result.serverMigrationHandled).toBe(false);
|
||||
expect(result.kind).not.toBe('server_migration_handled');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -1907,7 +1914,7 @@ describe('OperationLogSyncService', () => {
|
|||
syncImportReason: 'SERVER_MIGRATION',
|
||||
}),
|
||||
);
|
||||
expect(result?.cancelled).toBe(true);
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
|
||||
it('should show conflict dialog when piggybacked ops contain SYNC_IMPORT and client has meaningful data', async () => {
|
||||
|
|
@ -1948,7 +1955,7 @@ describe('OperationLogSyncService', () => {
|
|||
const result = await service.uploadPendingOps(mockProvider);
|
||||
|
||||
expect(syncImportConflictDialogServiceSpy.showConflictDialog).toHaveBeenCalled();
|
||||
expect(result?.cancelled).toBe(true);
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
|
||||
it('should process piggybacked SYNC_IMPORT silently when no meaningful local data', async () => {
|
||||
|
|
@ -1994,7 +2001,7 @@ describe('OperationLogSyncService', () => {
|
|||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
piggybackedSyncImport,
|
||||
]);
|
||||
expect(result?.cancelled).toBeUndefined();
|
||||
expect(result.kind).not.toBe('cancelled');
|
||||
});
|
||||
|
||||
it('should process piggybacked ops normally when no SYNC_IMPORT present', async () => {
|
||||
|
|
@ -2034,7 +2041,7 @@ describe('OperationLogSyncService', () => {
|
|||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
piggybackedOp,
|
||||
]);
|
||||
expect(result?.cancelled).toBeUndefined();
|
||||
expect(result.kind).not.toBe('cancelled');
|
||||
});
|
||||
|
||||
it('should call forceUploadLocalState when user chooses USE_LOCAL', async () => {
|
||||
|
|
|
|||
|
|
@ -2,19 +2,15 @@ import { inject, Injectable } from '@angular/core';
|
|||
import { Store } from '@ngrx/store';
|
||||
import { TranslateService } from '@ngx-translate/core';
|
||||
import { OperationLogStoreService } from '../persistence/operation-log-store.service';
|
||||
import {
|
||||
OperationLogEntry,
|
||||
OpType,
|
||||
VectorClock,
|
||||
FULL_STATE_OP_TYPES,
|
||||
} from '../core/operation.types';
|
||||
import { OperationLogEntry, OpType, FULL_STATE_OP_TYPES } from '../core/operation.types';
|
||||
import { OpLog } from '../../core/log';
|
||||
import {
|
||||
OperationSyncCapable,
|
||||
SyncProviderBase,
|
||||
} from '../sync-providers/provider.interface';
|
||||
import { SyncProviderId } from '../sync-providers/provider.const';
|
||||
import { OperationLogUploadService, UploadResult } from './operation-log-upload.service';
|
||||
import { OperationLogUploadService } from './operation-log-upload.service';
|
||||
import { DownloadOutcome, UploadOutcome } from '../core/types/sync-results.types';
|
||||
import { OperationLogDownloadService } from './operation-log-download.service';
|
||||
import { SnackService } from '../../core/snack/snack.service';
|
||||
import { T } from '../../t.const';
|
||||
|
|
@ -248,7 +244,7 @@ export class OperationLogSyncService {
|
|||
async uploadPendingOps(
|
||||
syncProvider: OperationSyncCapable,
|
||||
options?: { skipPiggybackProcessing?: boolean; skipServerMigrationCheck?: boolean },
|
||||
): Promise<UploadResult | null> {
|
||||
): Promise<UploadOutcome> {
|
||||
// CRITICAL: Ensure all pending write operations have completed before uploading.
|
||||
// The effect that writes operations uses concatMap for sequential processing,
|
||||
// but if sync is triggered before all operations are written to IndexedDB,
|
||||
|
|
@ -264,7 +260,7 @@ export class OperationLogSyncService {
|
|||
'OperationLogSyncService: Upload blocked - this is a fresh client with no history. ' +
|
||||
'Download remote data first before uploading.',
|
||||
);
|
||||
return null;
|
||||
return { kind: 'blocked_fresh_client' };
|
||||
}
|
||||
|
||||
// SERVER MIGRATION CHECK: Passed as callback to execute INSIDE the upload lock.
|
||||
|
|
@ -332,11 +328,18 @@ export class OperationLogSyncService {
|
|||
},
|
||||
'OperationLogSyncService (piggybacked SYNC_IMPORT)',
|
||||
);
|
||||
if (resolution === 'CANCEL') {
|
||||
return { kind: 'cancelled' };
|
||||
}
|
||||
// USE_LOCAL or USE_REMOTE was handled — report as completed with no further work
|
||||
return {
|
||||
...result,
|
||||
kind: 'completed',
|
||||
uploadedCount: result.uploadedCount,
|
||||
piggybackedOpsCount: result.piggybackedOps.length,
|
||||
localWinOpsCreated: 0,
|
||||
permanentRejectionCount: 0,
|
||||
cancelled: resolution === 'CANCEL',
|
||||
hasMorePiggyback: false,
|
||||
rejectedOps: [],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
@ -354,10 +357,29 @@ export class OperationLogSyncService {
|
|||
//
|
||||
// NOTE: This must NOT run after a SYNC_IMPORT conflict dialog resolution (USE_LOCAL,
|
||||
// USE_REMOTE, CANCEL) — those paths return early above to avoid stale rejection handling.
|
||||
const downloadCallback = (downloadOptions?: {
|
||||
const downloadCallback = async (downloadOptions?: {
|
||||
forceFromSeq0?: boolean;
|
||||
}): Promise<DownloadResultForRejection> =>
|
||||
this.downloadRemoteOps(syncProvider, downloadOptions);
|
||||
}): Promise<DownloadResultForRejection> => {
|
||||
const outcome = await this.downloadRemoteOps(syncProvider, downloadOptions);
|
||||
switch (outcome.kind) {
|
||||
case 'ops_processed':
|
||||
return {
|
||||
newOpsCount: outcome.newOpsCount,
|
||||
allOpClocks: outcome.allOpClocks,
|
||||
snapshotVectorClock: outcome.snapshotVectorClock,
|
||||
};
|
||||
case 'no_new_ops':
|
||||
case 'snapshot_hydrated':
|
||||
return {
|
||||
newOpsCount: 0,
|
||||
allOpClocks: outcome.allOpClocks,
|
||||
snapshotVectorClock: outcome.snapshotVectorClock,
|
||||
};
|
||||
case 'server_migration_handled':
|
||||
case 'cancelled':
|
||||
return { newOpsCount: 0 };
|
||||
}
|
||||
};
|
||||
try {
|
||||
rejectionResult = await this.rejectedOpsHandlerService.handleRejectedOps(
|
||||
result.rejectedOps,
|
||||
|
|
@ -379,9 +401,13 @@ export class OperationLogSyncService {
|
|||
);
|
||||
|
||||
return {
|
||||
...result,
|
||||
kind: 'completed',
|
||||
uploadedCount: result.uploadedCount,
|
||||
piggybackedOpsCount: result.piggybackedOps.length,
|
||||
localWinOpsCreated,
|
||||
permanentRejectionCount: rejectionResult.permanentRejectionCount,
|
||||
hasMorePiggyback: result.hasMorePiggyback ?? false,
|
||||
rejectedOps: result.rejectedOps,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -401,14 +427,7 @@ export class OperationLogSyncService {
|
|||
async downloadRemoteOps(
|
||||
syncProvider: OperationSyncCapable,
|
||||
options?: { forceFromSeq0?: boolean },
|
||||
): Promise<{
|
||||
serverMigrationHandled: boolean;
|
||||
localWinOpsCreated: number;
|
||||
newOpsCount: number;
|
||||
allOpClocks?: VectorClock[];
|
||||
snapshotVectorClock?: VectorClock;
|
||||
cancelled?: boolean;
|
||||
}> {
|
||||
): Promise<DownloadOutcome> {
|
||||
const result = await this.downloadService.downloadRemoteOps(syncProvider, options);
|
||||
|
||||
// Server migration detected: gap on empty server
|
||||
|
|
@ -419,8 +438,7 @@ export class OperationLogSyncService {
|
|||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
// Return with flag indicating migration was handled - caller should upload the SYNC_IMPORT
|
||||
return { serverMigrationHandled: true, localWinOpsCreated: 0, newOpsCount: 0 };
|
||||
return { kind: 'server_migration_handled' };
|
||||
}
|
||||
|
||||
// FILE-BASED SYNC: Handle full state snapshot from fresh download
|
||||
|
|
@ -499,11 +517,7 @@ export class OperationLogSyncService {
|
|||
this.snackService.open({
|
||||
msg: T.F.SYNC.S.FRESH_CLIENT_SYNC_CANCELLED,
|
||||
});
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0,
|
||||
};
|
||||
return { kind: 'cancelled' };
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
|
|
@ -549,9 +563,7 @@ export class OperationLogSyncService {
|
|||
OpLog.normal('OperationLogSyncService: Snapshot hydration complete.');
|
||||
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0, // Snapshot applied, not incremental ops
|
||||
kind: 'snapshot_hydrated',
|
||||
allOpClocks: result.allOpClocks,
|
||||
snapshotVectorClock: result.snapshotVectorClock,
|
||||
};
|
||||
|
|
@ -577,7 +589,7 @@ export class OperationLogSyncService {
|
|||
});
|
||||
// After SYNC_IMPORT is created, isWhollyFreshClient() returns false
|
||||
// and upload phase will proceed normally.
|
||||
return { serverMigrationHandled: true, localWinOpsCreated: 0, newOpsCount: 0 };
|
||||
return { kind: 'server_migration_handled' };
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -591,12 +603,8 @@ export class OperationLogSyncService {
|
|||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0,
|
||||
// Include all op clocks from forced download (even though no new ops)
|
||||
kind: 'no_new_ops',
|
||||
allOpClocks: result.allOpClocks,
|
||||
// Include snapshot vector clock for superseded op resolution
|
||||
snapshotVectorClock: result.snapshotVectorClock,
|
||||
};
|
||||
}
|
||||
|
|
@ -627,7 +635,7 @@ export class OperationLogSyncService {
|
|||
this.snackService.open({
|
||||
msg: T.F.SYNC.S.FRESH_CLIENT_SYNC_CANCELLED,
|
||||
});
|
||||
return { serverMigrationHandled: false, localWinOpsCreated: 0, newOpsCount: 0 };
|
||||
return { kind: 'cancelled' };
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
|
|
@ -676,12 +684,10 @@ export class OperationLogSyncService {
|
|||
},
|
||||
'OperationLogSyncService (incoming SYNC_IMPORT)',
|
||||
);
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0,
|
||||
cancelled: resolution === 'CANCEL',
|
||||
};
|
||||
if (resolution === 'CANCEL') {
|
||||
return { kind: 'cancelled' };
|
||||
}
|
||||
return { kind: 'no_new_ops' };
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -718,12 +724,10 @@ export class OperationLogSyncService {
|
|||
},
|
||||
'OperationLogSyncService (local SYNC_IMPORT filters remote)',
|
||||
);
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: 0,
|
||||
newOpsCount: 0,
|
||||
cancelled: resolution === 'CANCEL',
|
||||
};
|
||||
if (resolution === 'CANCEL') {
|
||||
return { kind: 'cancelled' };
|
||||
}
|
||||
return { kind: 'no_new_ops' };
|
||||
} else if (
|
||||
processResult.allOpsFilteredBySyncImport &&
|
||||
processResult.filteredOpCount > 0
|
||||
|
|
@ -757,9 +761,9 @@ export class OperationLogSyncService {
|
|||
);
|
||||
|
||||
return {
|
||||
serverMigrationHandled: false,
|
||||
localWinOpsCreated: processResult.localWinOpsCreated,
|
||||
kind: 'ops_processed',
|
||||
newOpsCount: result.newOps.length,
|
||||
localWinOpsCreated: processResult.localWinOpsCreated,
|
||||
allOpClocks: result.allOpClocks,
|
||||
snapshotVectorClock: result.snapshotVectorClock,
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue