mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): harden replay and conflict recovery
Make remote replay and authoritative state replacement crash-resumable and atomic. Preserve pending changes until piggyback processing commits, and stop cleanly on incompatible operations. Validate sync identities and schemas while strengthening request deduplication and privacy-safe diagnostics.
This commit is contained in:
parent
6e6ce34c15
commit
f38342eeb9
63 changed files with 2170 additions and 621 deletions
|
|
@ -40,7 +40,6 @@ const createClickCounter = async (
|
|||
): Promise<void> => {
|
||||
await client.page.goto('/#/habits');
|
||||
await client.page.waitForURL(/habits/);
|
||||
await client.page.waitForTimeout(500);
|
||||
|
||||
const addBtn = client.page.locator('.add-habit-btn');
|
||||
await addBtn.waitFor({ state: 'visible', timeout: 10000 });
|
||||
|
|
@ -55,28 +54,51 @@ const createClickCounter = async (
|
|||
|
||||
const typeSelect = dialog.locator('mat-select').first();
|
||||
await typeSelect.waitFor({ state: 'visible', timeout: 5000 });
|
||||
await client.page.waitForTimeout(500);
|
||||
await typeSelect.click();
|
||||
await client.page.waitForTimeout(300);
|
||||
await client.page.locator('mat-option:has-text("Click Counter")').click();
|
||||
await client.page.waitForTimeout(300);
|
||||
const clickCounterOption = client.page.locator('mat-option:has-text("Click Counter")');
|
||||
await clickCounterOption.waitFor({ state: 'visible', timeout: 5000 });
|
||||
await clickCounterOption.click();
|
||||
|
||||
await dialog.locator('button[type="submit"]').click();
|
||||
await dialog.waitFor({ state: 'hidden', timeout: 10000 });
|
||||
await client.page.waitForTimeout(500);
|
||||
|
||||
await client.page.goto('/#/tag/TODAY/tasks');
|
||||
await client.page.waitForURL(/(active\/tasks|tag\/TODAY\/tasks)/);
|
||||
await client.page.waitForTimeout(500);
|
||||
};
|
||||
|
||||
const incrementClickCounter = async (client: SimulatedE2EClient): Promise<void> => {
|
||||
const getNamedClickCounter = async (
|
||||
client: SimulatedE2EClient,
|
||||
title: string,
|
||||
): Promise<ReturnType<typeof client.page.locator>> => {
|
||||
const counter = client.page
|
||||
.locator('.counters-action-group simple-counter-button')
|
||||
.last();
|
||||
await counter.waitFor({ state: 'visible', timeout: 15000 });
|
||||
|
||||
// Counter buttons render only their initial, so verify the exact counter via
|
||||
// its accessible Material tooltip before interacting with it.
|
||||
await counter.hover();
|
||||
await expect(client.page.getByRole('tooltip').filter({ hasText: title })).toBeVisible();
|
||||
return counter;
|
||||
};
|
||||
|
||||
const expectClickCounterValue = async (
|
||||
client: SimulatedE2EClient,
|
||||
title: string,
|
||||
expectedValue: number,
|
||||
): Promise<void> => {
|
||||
const counter = await getNamedClickCounter(client, title);
|
||||
await expect(counter.locator('.label')).toHaveText(String(expectedValue));
|
||||
};
|
||||
|
||||
const incrementClickCounter = async (
|
||||
client: SimulatedE2EClient,
|
||||
title: string,
|
||||
expectedValue: number,
|
||||
): Promise<void> => {
|
||||
const counter = await getNamedClickCounter(client, title);
|
||||
await counter.locator('.main-btn').click();
|
||||
await client.page.waitForTimeout(200);
|
||||
await expect(counter.locator('.label')).toHaveText(String(expectedValue));
|
||||
};
|
||||
|
||||
test.describe.configure({ mode: 'serial' });
|
||||
|
|
@ -101,7 +123,7 @@ test.describe('@supersync incoming SYNC_IMPORT preserves non-task local work', (
|
|||
clientA = await createSimulatedClient(browser, baseURL!, 'A', testRunId);
|
||||
await clientA.sync.setupSuperSync(syncConfig);
|
||||
await createClickCounter(clientA, counterTitle);
|
||||
await incrementClickCounter(clientA);
|
||||
await incrementClickCounter(clientA, counterTitle, 1);
|
||||
await clientA.sync.syncAndWait();
|
||||
console.log('[Gate] Client A created + synced a simple counter');
|
||||
|
||||
|
|
@ -111,10 +133,11 @@ test.describe('@supersync incoming SYNC_IMPORT preserves non-task local work', (
|
|||
await clientB.sync.syncAndWait();
|
||||
await clientB.page.goto('/#/tag/TODAY/tasks');
|
||||
await clientB.page.waitForLoadState('networkidle');
|
||||
await expectClickCounterValue(clientB, counterTitle, 1);
|
||||
console.log('[Gate] Client B synced and received the counter');
|
||||
|
||||
// ===== PHASE 3: B makes a pending NON-task change (does NOT sync) =====
|
||||
await incrementClickCounter(clientB);
|
||||
await incrementClickCounter(clientB, counterTitle, 2);
|
||||
console.log('[Gate] Client B incremented the counter locally (pending, unsynced)');
|
||||
|
||||
// ===== PHASE 4: A imports a backup (SYNC_IMPORT) and syncs it =====
|
||||
|
|
@ -138,6 +161,17 @@ test.describe('@supersync incoming SYNC_IMPORT preserves non-task local work', (
|
|||
state: 'hidden',
|
||||
timeout: 10000,
|
||||
});
|
||||
await clientB.sync.waitForSyncToComplete({ timeout: 30000 });
|
||||
|
||||
// The dialog itself is not the guarantee: prove the exact pending counter
|
||||
// value won, then run another sync and prove it remains durable.
|
||||
await clientB.page.goto('/#/tag/TODAY/tasks');
|
||||
await clientB.page.waitForURL(/(active\/tasks|tag\/TODAY\/tasks)/);
|
||||
await expectClickCounterValue(clientB, counterTitle, 2);
|
||||
|
||||
await clientB.sync.syncAndWait();
|
||||
await expectClickCounterValue(clientB, counterTitle, 2);
|
||||
console.log('[Gate] ✓ Pending counter value survived resolution and re-sync');
|
||||
} finally {
|
||||
if (clientA) await closeClient(clientA);
|
||||
if (clientB) await closeClient(clientB);
|
||||
|
|
|
|||
|
|
@ -97,6 +97,7 @@ export const MiscToTasksSettingsMigration_v1v2: SchemaMigration = {
|
|||
result.push({
|
||||
...op,
|
||||
id: `${op.id}_misc`,
|
||||
entityIds: ['misc'],
|
||||
payload: buildPayload(miscCfg, 'misc'),
|
||||
});
|
||||
}
|
||||
|
|
@ -106,6 +107,7 @@ export const MiscToTasksSettingsMigration_v1v2: SchemaMigration = {
|
|||
...op,
|
||||
id: `${op.id}_tasks`,
|
||||
entityId: 'tasks',
|
||||
entityIds: ['tasks'],
|
||||
payload: buildPayload(tasksCfg, 'tasks'),
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ export const SuperSyncOperationSchema = z.object({
|
|||
payload: z.unknown(),
|
||||
vectorClock: SuperSyncVectorClockSchema,
|
||||
timestamp: z.number(),
|
||||
schemaVersion: z.number(),
|
||||
schemaVersion: z.number().int().min(1).max(100),
|
||||
isPayloadEncrypted: z.boolean().optional(),
|
||||
syncImportReason: z.enum(SUPER_SYNC_IMPORT_REASONS).optional(),
|
||||
});
|
||||
|
|
@ -92,7 +92,7 @@ export const SuperSyncUploadSnapshotRequestSchema = z.object({
|
|||
clientId: SuperSyncClientIdSchema,
|
||||
reason: z.enum(SUPER_SYNC_SNAPSHOT_REASONS),
|
||||
vectorClock: SuperSyncVectorClockSchema,
|
||||
schemaVersion: z.number().optional(),
|
||||
schemaVersion: z.number().int().min(1).max(100).optional(),
|
||||
isPayloadEncrypted: z.boolean().optional(),
|
||||
syncImportReason: z.enum(SUPER_SYNC_IMPORT_REASONS).optional(),
|
||||
opId: z.string().uuid().optional(),
|
||||
|
|
|
|||
|
|
@ -72,6 +72,33 @@ describe('SuperSync HTTP contract schemas', () => {
|
|||
).toThrow();
|
||||
});
|
||||
|
||||
it.each([0, 1.5, -1, 101])(
|
||||
'rejects malformed operation schema version %s',
|
||||
(schemaVersion) => {
|
||||
expect(() =>
|
||||
SuperSyncUploadOpsRequestSchema.parse({
|
||||
ops: [{ ...createValidOperation(), schemaVersion }],
|
||||
clientId: 'client_1',
|
||||
}),
|
||||
).toThrow();
|
||||
},
|
||||
);
|
||||
|
||||
it.each([0, 1.5, -1, 101])(
|
||||
'rejects malformed snapshot schema version %s',
|
||||
(schemaVersion) => {
|
||||
expect(() =>
|
||||
SuperSyncUploadSnapshotRequestSchema.parse({
|
||||
state: {},
|
||||
clientId: 'client_1',
|
||||
reason: 'recovery',
|
||||
vectorClock: { client_1: 1 },
|
||||
schemaVersion,
|
||||
}),
|
||||
).toThrow();
|
||||
},
|
||||
);
|
||||
|
||||
it('coerces download query numbers like the route-level schema', () => {
|
||||
const parsed = SuperSyncDownloadOpsQuerySchema.parse({
|
||||
sinceSeq: '5',
|
||||
|
|
|
|||
|
|
@ -265,6 +265,34 @@ export const isSameDuplicateOperation = (
|
|||
);
|
||||
};
|
||||
|
||||
/** Complete identity comparison for two validated operations in one request. */
|
||||
export const isSameIncomingOperation = (
|
||||
first: Operation,
|
||||
second: Operation,
|
||||
firstOriginalTimestamp: number = first.timestamp,
|
||||
secondOriginalTimestamp: number = second.timestamp,
|
||||
): boolean => {
|
||||
const bothEncrypted =
|
||||
(first.isPayloadEncrypted ?? false) && (second.isPayloadEncrypted ?? false);
|
||||
return (
|
||||
first.clientId === second.clientId &&
|
||||
first.actionType === second.actionType &&
|
||||
first.opType === second.opType &&
|
||||
first.entityType === second.entityType &&
|
||||
first.entityId === second.entityId &&
|
||||
areJsonValuesEqual(getStoredEntityIds(first), getStoredEntityIds(second)) &&
|
||||
(bothEncrypted || areJsonValuesEqual(first.payload, second.payload)) &&
|
||||
areJsonValuesEqual(
|
||||
limitVectorClockSize(first.vectorClock, [first.clientId]),
|
||||
limitVectorClockSize(second.vectorClock, [second.clientId]),
|
||||
) &&
|
||||
first.schemaVersion === second.schemaVersion &&
|
||||
firstOriginalTimestamp === secondOriginalTimestamp &&
|
||||
(first.isPayloadEncrypted ?? false) === (second.isPayloadEncrypted ?? false) &&
|
||||
(first.syncImportReason ?? null) === (second.syncImportReason ?? null)
|
||||
);
|
||||
};
|
||||
|
||||
export const isSameDuplicateTimestamp = (
|
||||
existingTimestamp: bigint | number | string,
|
||||
existingReceivedAt: bigint | number | string,
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ const OPERATION_DOWNLOAD_SELECT = {
|
|||
opType: true,
|
||||
entityType: true,
|
||||
entityId: true,
|
||||
entityIds: true,
|
||||
payload: true,
|
||||
vectorClock: true,
|
||||
schemaVersion: true,
|
||||
|
|
@ -54,6 +55,7 @@ type OperationDownloadRow = {
|
|||
opType: string;
|
||||
entityType: string;
|
||||
entityId: string | null;
|
||||
entityIds: string[];
|
||||
payload: unknown;
|
||||
vectorClock: unknown;
|
||||
schemaVersion: number;
|
||||
|
|
@ -72,6 +74,7 @@ const mapOperationRow = (row: OperationDownloadRow): ServerOperation => ({
|
|||
opType: row.opType as Operation['opType'],
|
||||
entityType: row.entityType,
|
||||
entityId: row.entityId ?? undefined,
|
||||
entityIds: row.entityIds.length > 0 ? row.entityIds : undefined,
|
||||
payload: row.payload,
|
||||
vectorClock: row.vectorClock as VectorClock,
|
||||
schemaVersion: row.schemaVersion,
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import {
|
|||
getStoredEntityIds,
|
||||
getEntityConflictKey,
|
||||
isSameDuplicateOperation,
|
||||
isSameIncomingOperation,
|
||||
prefetchLatestEntityOpsForBatch,
|
||||
pruneVectorClockForStorage,
|
||||
resolveConflictForExistingOp,
|
||||
|
|
@ -317,10 +318,9 @@ export class OperationUploadService {
|
|||
}
|
||||
|
||||
/**
|
||||
* Stage 2: within a single batch, accept the first op for an id and reject
|
||||
* every later op sharing that id as DUPLICATE_OPERATION (by id, not content
|
||||
* — see plan §1a step 2 / the C4 divergence note). Must run before sequence
|
||||
* reservation so a duplicate never consumes a server_seq.
|
||||
* Stage 2: within a single batch, accept the first op for an id. Exact retries
|
||||
* are DUPLICATE_OPERATION; same-id operations with different complete identity
|
||||
* are INVALID_OP_ID. Must run before sequence reservation.
|
||||
*/
|
||||
private rejectIntraBatchDuplicates(
|
||||
userId: number,
|
||||
|
|
@ -328,20 +328,31 @@ export class OperationUploadService {
|
|||
validatedCandidates: BatchUploadCandidate[],
|
||||
results: UploadResult[],
|
||||
): BatchUploadCandidate[] {
|
||||
const seenOpIds = new Set<string>();
|
||||
const firstCandidateByOpId = new Map<string, BatchUploadCandidate>();
|
||||
const uniqueCandidates: BatchUploadCandidate[] = [];
|
||||
for (const candidate of validatedCandidates) {
|
||||
if (seenOpIds.has(candidate.op.id)) {
|
||||
const firstCandidate = firstCandidateByOpId.get(candidate.op.id);
|
||||
if (firstCandidate) {
|
||||
const isExactRetry = isSameIncomingOperation(
|
||||
firstCandidate.op,
|
||||
candidate.op,
|
||||
firstCandidate.originalTimestamp,
|
||||
candidate.originalTimestamp,
|
||||
);
|
||||
results[candidate.resultIndex] = this.rejectedUploadResult(
|
||||
userId,
|
||||
clientId,
|
||||
candidate.op,
|
||||
'Duplicate operation ID',
|
||||
SYNC_ERROR_CODES.DUPLICATE_OPERATION,
|
||||
isExactRetry
|
||||
? 'Duplicate operation ID'
|
||||
: 'Operation ID already belongs to a different operation',
|
||||
isExactRetry
|
||||
? SYNC_ERROR_CODES.DUPLICATE_OPERATION
|
||||
: SYNC_ERROR_CODES.INVALID_OP_ID,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
seenOpIds.add(candidate.op.id);
|
||||
firstCandidateByOpId.set(candidate.op.id, candidate);
|
||||
uniqueCandidates.push(candidate);
|
||||
}
|
||||
return uniqueCandidates;
|
||||
|
|
|
|||
|
|
@ -8,6 +8,9 @@
|
|||
* Entries expire after 5 minutes.
|
||||
*/
|
||||
import type { UploadResult } from '../sync.types';
|
||||
import type { Operation } from '../sync.types';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { stableJsonStringify } from '../conflict';
|
||||
|
||||
/**
|
||||
* Maximum entries in deduplication cache to prevent unbounded memory growth.
|
||||
|
|
@ -38,8 +41,18 @@ export interface SnapshotDedupResponse {
|
|||
* even if the same `requestId` string is reused across namespaces.
|
||||
*/
|
||||
type RequestDeduplicationEntry =
|
||||
| { namespace: 'ops'; processedAt: number; results: UploadResult[] }
|
||||
| { namespace: 'snapshot'; processedAt: number; results: SnapshotDedupResponse };
|
||||
| {
|
||||
namespace: 'ops';
|
||||
processedAt: number;
|
||||
results: UploadResult[];
|
||||
fingerprint?: string;
|
||||
}
|
||||
| {
|
||||
namespace: 'snapshot';
|
||||
processedAt: number;
|
||||
results: SnapshotDedupResponse;
|
||||
fingerprint?: string;
|
||||
};
|
||||
|
||||
type DedupPayload<N extends RequestDedupNamespace> = N extends 'ops'
|
||||
? UploadResult[]
|
||||
|
|
@ -56,6 +69,7 @@ export class RequestDeduplicationService {
|
|||
userId: number,
|
||||
namespace: N,
|
||||
requestId: string,
|
||||
fingerprint?: string,
|
||||
): DedupPayload<N> | null {
|
||||
const key = this._key(userId, namespace, requestId);
|
||||
const entry = this.cache.get(key);
|
||||
|
|
@ -66,6 +80,7 @@ export class RequestDeduplicationService {
|
|||
}
|
||||
// Defensive: keying already isolates namespaces, but verify before casting.
|
||||
if (entry.namespace !== namespace) return null;
|
||||
if (fingerprint !== undefined && entry.fingerprint !== fingerprint) return null;
|
||||
return entry.results as DedupPayload<N>;
|
||||
}
|
||||
|
||||
|
|
@ -77,6 +92,7 @@ export class RequestDeduplicationService {
|
|||
namespace: N,
|
||||
requestId: string,
|
||||
results: DedupPayload<N>,
|
||||
fingerprint?: string,
|
||||
): void {
|
||||
const key = this._key(userId, namespace, requestId);
|
||||
if (this.cache.size >= MAX_CACHE_SIZE) {
|
||||
|
|
@ -88,6 +104,7 @@ export class RequestDeduplicationService {
|
|||
namespace,
|
||||
processedAt: Date.now(),
|
||||
results,
|
||||
fingerprint,
|
||||
} as RequestDeduplicationEntry);
|
||||
}
|
||||
|
||||
|
|
@ -138,3 +155,41 @@ export class RequestDeduplicationService {
|
|||
return `${userId}:${namespace}:${requestId}`;
|
||||
}
|
||||
}
|
||||
|
||||
export const createOpsRequestFingerprint = (
|
||||
clientId: string,
|
||||
ops: Operation[],
|
||||
): string => {
|
||||
const logicalOps = ops.map((op) => ({
|
||||
...op,
|
||||
payload: op.isPayloadEncrypted ? '[encrypted-payload]' : op.payload,
|
||||
}));
|
||||
return createHash('sha256')
|
||||
.update(stableJsonStringify({ clientId, ops: logicalOps }))
|
||||
.digest('base64url');
|
||||
};
|
||||
|
||||
export interface SnapshotRequestFingerprintInput {
|
||||
state: unknown;
|
||||
clientId: string;
|
||||
reason: string;
|
||||
vectorClock: Record<string, number>;
|
||||
schemaVersion?: number;
|
||||
isPayloadEncrypted?: boolean;
|
||||
syncImportReason?: string;
|
||||
opId?: string;
|
||||
isCleanSlate?: boolean;
|
||||
snapshotOpType?: string;
|
||||
}
|
||||
|
||||
export const createSnapshotRequestFingerprint = (
|
||||
request: SnapshotRequestFingerprintInput,
|
||||
): string => {
|
||||
const logicalRequest = {
|
||||
...request,
|
||||
state: request.isPayloadEncrypted ? '[encrypted-state]' : request.state,
|
||||
};
|
||||
return createHash('sha256')
|
||||
.update(stableJsonStringify(logicalRequest))
|
||||
.digest('base64url');
|
||||
};
|
||||
|
|
|
|||
|
|
@ -158,7 +158,11 @@ export class ValidationService {
|
|||
};
|
||||
}
|
||||
if (op.schemaVersion !== undefined) {
|
||||
if (op.schemaVersion < 1 || op.schemaVersion > 100) {
|
||||
if (
|
||||
!Number.isInteger(op.schemaVersion) ||
|
||||
op.schemaVersion < 1 ||
|
||||
op.schemaVersion > 100
|
||||
) {
|
||||
return {
|
||||
valid: false,
|
||||
error: `Invalid schema version: ${op.schemaVersion}`,
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ import {
|
|||
getRawOpsCount,
|
||||
sendOpsBatchTooLargeReply,
|
||||
} from './sync.routes.quota';
|
||||
import { createOpsRequestFingerprint } from './services/request-deduplication.service';
|
||||
|
||||
export const uploadOpsHandler = async (
|
||||
req: FastifyRequest<{ Body: UploadOpsRequest }>,
|
||||
|
|
@ -86,6 +87,9 @@ export const uploadOpsHandler = async (
|
|||
}
|
||||
|
||||
const { ops, clientId, lastKnownServerSeq, requestId } = parseResult.data;
|
||||
const requestFingerprint = requestId
|
||||
? createOpsRequestFingerprint(clientId, ops as unknown as Operation[])
|
||||
: undefined;
|
||||
const syncService = getSyncService();
|
||||
|
||||
Logger.info(
|
||||
|
|
@ -112,7 +116,11 @@ export const uploadOpsHandler = async (
|
|||
// This ensures retries after successful uploads don't fail with 413
|
||||
// if the original upload pushed the user over quota
|
||||
if (requestId) {
|
||||
const cachedResults = syncService.checkOpsRequestDedup(userId, requestId);
|
||||
const cachedResults = syncService.checkOpsRequestDedup(
|
||||
userId,
|
||||
requestId,
|
||||
requestFingerprint,
|
||||
);
|
||||
if (cachedResults) {
|
||||
Logger.info(`[user:${userId}] Returning cached results for request ${requestId}`);
|
||||
|
||||
|
|
@ -214,7 +222,7 @@ export const uploadOpsHandler = async (
|
|||
requestId &&
|
||||
!results.some((r) => r.errorCode === SYNC_ERROR_CODES.INTERNAL_ERROR)
|
||||
) {
|
||||
syncService.cacheOpsRequestResults(userId, requestId, results);
|
||||
syncService.cacheOpsRequestResults(userId, requestId, results, requestFingerprint);
|
||||
}
|
||||
|
||||
const accepted = results.filter((r) => r.accepted).length;
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import {
|
|||
sendQuotaExceededReply,
|
||||
sendSyncImportExistsReply,
|
||||
} from './sync.routes.quota';
|
||||
import { createSnapshotRequestFingerprint } from './services/request-deduplication.service';
|
||||
|
||||
export const uploadSnapshotHandler = async (
|
||||
req: FastifyRequest<{ Body: unknown }>,
|
||||
|
|
@ -99,9 +100,27 @@ export const uploadSnapshotHandler = async (
|
|||
requestId,
|
||||
} = snapshotRequest;
|
||||
const syncService = getSyncService();
|
||||
const requestFingerprint = requestId
|
||||
? createSnapshotRequestFingerprint({
|
||||
state,
|
||||
clientId,
|
||||
reason,
|
||||
vectorClock,
|
||||
schemaVersion,
|
||||
isPayloadEncrypted,
|
||||
syncImportReason,
|
||||
opId,
|
||||
isCleanSlate,
|
||||
snapshotOpType,
|
||||
})
|
||||
: undefined;
|
||||
|
||||
if (requestId) {
|
||||
const cachedResponse = syncService.checkSnapshotRequestDedup(userId, requestId);
|
||||
const cachedResponse = syncService.checkSnapshotRequestDedup(
|
||||
userId,
|
||||
requestId,
|
||||
requestFingerprint,
|
||||
);
|
||||
if (cachedResponse) {
|
||||
Logger.info(
|
||||
`[user:${userId}] Returning cached snapshot result for request ${requestId}`,
|
||||
|
|
@ -340,7 +359,12 @@ export const uploadSnapshotHandler = async (
|
|||
finalResult.errorCode !== SYNC_ERROR_CODES.DUPLICATE_OPERATION &&
|
||||
finalResult.errorCode !== SYNC_ERROR_CODES.INTERNAL_ERROR
|
||||
) {
|
||||
syncService.cacheSnapshotRequestResult(userId, requestId, responseBody);
|
||||
syncService.cacheSnapshotRequestResult(
|
||||
userId,
|
||||
requestId,
|
||||
responseBody,
|
||||
requestFingerprint,
|
||||
);
|
||||
}
|
||||
|
||||
return reply.send(responseBody);
|
||||
|
|
|
|||
|
|
@ -454,26 +454,44 @@ export class SyncService {
|
|||
return this.rateLimitService.cleanupExpiredCounters();
|
||||
}
|
||||
|
||||
checkOpsRequestDedup(userId: number, requestId: string): UploadResult[] | null {
|
||||
return this.requestDeduplicationService.checkDeduplication(userId, 'ops', requestId);
|
||||
checkOpsRequestDedup(
|
||||
userId: number,
|
||||
requestId: string,
|
||||
fingerprint?: string,
|
||||
): UploadResult[] | null {
|
||||
return this.requestDeduplicationService.checkDeduplication(
|
||||
userId,
|
||||
'ops',
|
||||
requestId,
|
||||
fingerprint,
|
||||
);
|
||||
}
|
||||
|
||||
cacheOpsRequestResults(
|
||||
userId: number,
|
||||
requestId: string,
|
||||
results: UploadResult[],
|
||||
fingerprint?: string,
|
||||
): void {
|
||||
this.requestDeduplicationService.cacheResults(userId, 'ops', requestId, results);
|
||||
this.requestDeduplicationService.cacheResults(
|
||||
userId,
|
||||
'ops',
|
||||
requestId,
|
||||
results,
|
||||
fingerprint,
|
||||
);
|
||||
}
|
||||
|
||||
checkSnapshotRequestDedup(
|
||||
userId: number,
|
||||
requestId: string,
|
||||
fingerprint?: string,
|
||||
): SnapshotDedupResponse | null {
|
||||
return this.requestDeduplicationService.checkDeduplication(
|
||||
userId,
|
||||
'snapshot',
|
||||
requestId,
|
||||
fingerprint,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -481,12 +499,14 @@ export class SyncService {
|
|||
userId: number,
|
||||
requestId: string,
|
||||
response: SnapshotDedupResponse,
|
||||
fingerprint?: string,
|
||||
): void {
|
||||
this.requestDeduplicationService.cacheResults(
|
||||
userId,
|
||||
'snapshot',
|
||||
requestId,
|
||||
response,
|
||||
fingerprint,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
|
||||
import type { Prisma } from '@prisma/client';
|
||||
import { OperationDownloadService } from '../src/sync/services/operation-download.service';
|
||||
|
||||
// Mock prisma
|
||||
|
|
@ -35,6 +36,7 @@ const EXPECTED_OPERATION_DOWNLOAD_SELECT = {
|
|||
opType: true,
|
||||
entityType: true,
|
||||
entityId: true,
|
||||
entityIds: true,
|
||||
payload: true,
|
||||
vectorClock: true,
|
||||
schemaVersion: true,
|
||||
|
|
@ -54,6 +56,7 @@ const createMockOpRow = (
|
|||
actionType: string;
|
||||
entityType: string;
|
||||
entityId: string | null;
|
||||
entityIds: string[];
|
||||
payload: unknown;
|
||||
vectorClock: Record<string, number>;
|
||||
schemaVersion: number;
|
||||
|
|
@ -71,6 +74,7 @@ const createMockOpRow = (
|
|||
entityType: overrides.entityType ?? 'Task',
|
||||
// Use 'in' check to allow null to be explicitly set
|
||||
entityId: 'entityId' in overrides ? overrides.entityId : `task-${serverSeq}`,
|
||||
entityIds: overrides.entityIds ?? [],
|
||||
payload: overrides.payload ?? { title: `Task ${serverSeq}` },
|
||||
vectorClock: overrides.vectorClock ?? { [clientId]: serverSeq },
|
||||
schemaVersion: overrides.schemaVersion ?? 1,
|
||||
|
|
@ -358,6 +362,30 @@ describe('OperationDownloadService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should round-trip batch entityIds in downloaded operations', async () => {
|
||||
vi.mocked(prisma.$transaction).mockImplementation(
|
||||
async (fn: (tx: Prisma.TransactionClient) => Promise<unknown>) =>
|
||||
fn({
|
||||
operation: {
|
||||
findFirst: vi.fn().mockResolvedValue(null),
|
||||
findMany: vi.fn().mockResolvedValue([
|
||||
createMockOpRow(1, 'batch-client', {
|
||||
entityId: 'task-1',
|
||||
entityIds: ['task-1', 'task-2'],
|
||||
}),
|
||||
]),
|
||||
},
|
||||
userSyncState: {
|
||||
findUnique: vi.fn().mockResolvedValue({ lastSeq: 1 }),
|
||||
},
|
||||
} as unknown as Prisma.TransactionClient),
|
||||
);
|
||||
|
||||
const result = await service.getOpsSinceWithSeq(1, 0);
|
||||
|
||||
expect(result.ops[0].op.entityIds).toEqual(['task-1', 'task-2']);
|
||||
});
|
||||
|
||||
it('should detect gap when client is ahead of server', async () => {
|
||||
vi.mocked(prisma.$transaction).mockImplementation(async (fn: any) => {
|
||||
const mockTx = {
|
||||
|
|
|
|||
|
|
@ -196,6 +196,7 @@ describe('Request dedup — transaction-failure results are not cached (#8332)',
|
|||
userId,
|
||||
'ops-v1-success',
|
||||
results,
|
||||
expect.any(String),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -218,6 +219,7 @@ describe('Request dedup — transaction-failure results are not cached (#8332)',
|
|||
userId,
|
||||
'ops-v1-conflict',
|
||||
results,
|
||||
expect.any(String),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -245,6 +247,7 @@ describe('Request dedup — transaction-failure results are not cached (#8332)',
|
|||
userId,
|
||||
'snapshot-v1-success',
|
||||
{ accepted: true, serverSeq: 5, error: undefined },
|
||||
expect.any(String),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
|
||||
import { RequestDeduplicationService } from '../src/sync/services/request-deduplication.service';
|
||||
import {
|
||||
RequestDeduplicationService,
|
||||
createSnapshotRequestFingerprint,
|
||||
} from '../src/sync/services/request-deduplication.service';
|
||||
import { UploadResult } from '../src/sync/sync.types';
|
||||
|
||||
describe('RequestDeduplicationService', () => {
|
||||
|
|
@ -36,6 +39,18 @@ describe('RequestDeduplicationService', () => {
|
|||
expect(result).toEqual(mockResults);
|
||||
});
|
||||
|
||||
it('should treat the same requestId with a different body fingerprint as a cache miss', () => {
|
||||
const mockResults = createMockResults(1);
|
||||
service.cacheResults(1, 'ops', 'request-123', mockResults, 'fingerprint-a');
|
||||
|
||||
expect(
|
||||
service.checkDeduplication(1, 'ops', 'request-123', 'fingerprint-a'),
|
||||
).toEqual(mockResults);
|
||||
expect(
|
||||
service.checkDeduplication(1, 'ops', 'request-123', 'fingerprint-b'),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('should return null for expired request', () => {
|
||||
const mockResults = createMockResults(1);
|
||||
service.cacheResults(1, 'ops', 'request-123', mockResults);
|
||||
|
|
@ -114,6 +129,41 @@ describe('RequestDeduplicationService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('snapshot request fingerprints', () => {
|
||||
const request = {
|
||||
state: { task: { ids: ['task-1'] } },
|
||||
clientId: 'client-1',
|
||||
reason: 'recovery',
|
||||
vectorClock: { 'client-1': 1 },
|
||||
schemaVersion: 4,
|
||||
};
|
||||
|
||||
it('changes when an unencrypted snapshot body changes', () => {
|
||||
expect(createSnapshotRequestFingerprint(request)).not.toBe(
|
||||
createSnapshotRequestFingerprint({
|
||||
...request,
|
||||
state: { task: { ids: ['task-2'] } },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('stays stable across encrypted snapshot IV changes', () => {
|
||||
expect(
|
||||
createSnapshotRequestFingerprint({
|
||||
...request,
|
||||
state: 'ciphertext-a',
|
||||
isPayloadEncrypted: true,
|
||||
}),
|
||||
).toBe(
|
||||
createSnapshotRequestFingerprint({
|
||||
...request,
|
||||
state: 'ciphertext-b',
|
||||
isPayloadEncrypted: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('cleanupExpiredEntries', () => {
|
||||
it('should remove all expired entries', () => {
|
||||
service.cacheResults(1, 'ops', 'request-1', createMockResults(1));
|
||||
|
|
|
|||
|
|
@ -810,6 +810,7 @@ describe('Sync compressed body routes', () => {
|
|||
expect(mocks.syncService.checkSnapshotRequestDedup).toHaveBeenCalledWith(
|
||||
1,
|
||||
'snapshot-v1-retry',
|
||||
expect.any(String),
|
||||
);
|
||||
expect(mocks.syncService.checkOpsRequestDedup).not.toHaveBeenCalled();
|
||||
expect(mocks.prisma.operation.findFirst).not.toHaveBeenCalled();
|
||||
|
|
@ -879,6 +880,7 @@ describe('Sync compressed body routes', () => {
|
|||
1,
|
||||
'snapshot-v1-dup-test',
|
||||
{ accepted: true, serverSeq: 77 },
|
||||
expect.any(String),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -708,15 +708,7 @@ describe('SyncService', () => {
|
|||
expect(testState.operations.size).toBe(25);
|
||||
});
|
||||
|
||||
it('rejects an intra-batch same-id op as DUPLICATE_OPERATION even when its content differs (deliberate divergence from the legacy per-op path, which yields INVALID_OP_ID)', async () => {
|
||||
// C4: the batch path dedups intra-batch purely by op.id (plan §1a step 2)
|
||||
// and rejects the later op as DUPLICATE_OPERATION regardless of content.
|
||||
// The legacy per-op path inserts the first then catches the second at the
|
||||
// DB and returns INVALID_OP_ID for differing content. Both are terminal
|
||||
// rejections with no row and no sequence gap, so sync invariants hold;
|
||||
// the client treats DUPLICATE_OPERATION as a silent success and
|
||||
// INVALID_OP_ID as a hard rejection. This test pins the chosen batch
|
||||
// semantics so the divergence stays intentional, not accidental drift.
|
||||
it('rejects an intra-batch same-id collision as INVALID_OP_ID', async () => {
|
||||
const service = new SyncService({ batchUpload: true });
|
||||
const opId = uuidv7();
|
||||
const first = makeOp({
|
||||
|
|
@ -747,7 +739,7 @@ describe('SyncService', () => {
|
|||
expect(results[1]).toEqual(
|
||||
expect.objectContaining({
|
||||
accepted: false,
|
||||
errorCode: SYNC_ERROR_CODES.DUPLICATE_OPERATION,
|
||||
errorCode: SYNC_ERROR_CODES.INVALID_OP_ID,
|
||||
}),
|
||||
);
|
||||
// No sequence gap: lastSeq advanced by exactly 1, exactly one row.
|
||||
|
|
@ -755,6 +747,28 @@ describe('SyncService', () => {
|
|||
expect(testState.operations.size).toBe(1);
|
||||
});
|
||||
|
||||
it('preserves an exact intra-batch retry as DUPLICATE_OPERATION', async () => {
|
||||
const service = new SyncService({ batchUpload: true });
|
||||
const retry = makeOp({
|
||||
id: uuidv7(),
|
||||
entityId: 'task-1',
|
||||
entityIds: ['task-1', 'task-2'],
|
||||
vectorClock: { [clientId]: 1 },
|
||||
});
|
||||
|
||||
const results = await service.uploadOps(userId, clientId, [retry, { ...retry }]);
|
||||
|
||||
expect(results[0]).toEqual(
|
||||
expect.objectContaining({ accepted: true, serverSeq: 1 }),
|
||||
);
|
||||
expect(results[1]).toEqual(
|
||||
expect.objectContaining({
|
||||
accepted: false,
|
||||
errorCode: SYNC_ERROR_CODES.DUPLICATE_OPERATION,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject intra-batch entity conflicts in order', async () => {
|
||||
const service = new SyncService({ batchUpload: true });
|
||||
const ops: Operation[] = [
|
||||
|
|
|
|||
|
|
@ -315,6 +315,16 @@ describe('ValidationService', () => {
|
|||
expect(result.errorCode).toBe(SYNC_ERROR_CODES.INVALID_SCHEMA_VERSION);
|
||||
});
|
||||
|
||||
it('should reject non-integer schema versions', () => {
|
||||
const result = validationService.validateOp(
|
||||
createValidOp({ schemaVersion: 1.5 }),
|
||||
clientId,
|
||||
);
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errorCode).toBe(SYNC_ERROR_CODES.INVALID_SCHEMA_VERSION);
|
||||
});
|
||||
|
||||
it('should accept valid schema versions 1-100', () => {
|
||||
for (const schemaVersion of [1, 50, 100]) {
|
||||
const op = createValidOp({ schemaVersion });
|
||||
|
|
|
|||
|
|
@ -151,7 +151,7 @@ export interface OperationLogEntry<TOperation extends Operation<string> = Operat
|
|||
* For remote ops only: tracks whether the op was successfully applied to local state.
|
||||
* Used for crash recovery: on startup, any 'pending' or 'failed' remote ops are re-dispatched.
|
||||
*/
|
||||
applicationStatus?: 'pending' | 'applied' | 'failed';
|
||||
applicationStatus?: 'pending' | 'archive_pending' | 'applied' | 'failed';
|
||||
|
||||
/**
|
||||
* For 'failed' ops: number of retry attempts.
|
||||
|
|
|
|||
|
|
@ -20,7 +20,10 @@ export interface SyncActionLike {
|
|||
export interface OperationApplyPort<TOperation extends Operation<string> = Operation> {
|
||||
applyOperations(
|
||||
ops: TOperation[],
|
||||
options?: ApplyOperationsOptions,
|
||||
options?: ApplyOperationsOptions & {
|
||||
/** Persist reducer-commit bookkeeping before post-dispatch side effects start. */
|
||||
onReducersCommitted?: (ops: TOperation[]) => Promise<void>;
|
||||
},
|
||||
): Promise<ApplyOperationsResult<TOperation>>;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ export interface RemoteOperationApplyStorePort<
|
|||
source: 'remote',
|
||||
options: { pendingApply: true },
|
||||
): Promise<RemoteOperationAppendResult<TOperation>>;
|
||||
markArchivePending(seqs: number[]): Promise<void>;
|
||||
markApplied(seqs: number[]): Promise<void>;
|
||||
markFailed(opIds: string[]): Promise<void>;
|
||||
mergeRemoteOpClocks(ops: TOperation[]): Promise<void>;
|
||||
|
|
@ -111,7 +112,25 @@ export const applyRemoteOperations = async <
|
|||
}
|
||||
});
|
||||
|
||||
const applyResult = await applier.applyOperations(appendResult.writtenOps);
|
||||
const applyResult = await applier.applyOperations(appendResult.writtenOps, {
|
||||
onReducersCommitted: async (reducerCommittedOps) => {
|
||||
const reducerCommittedSeqs = reducerCommittedOps
|
||||
.map((op) => opIdToSeq.get(op.id))
|
||||
.filter((seq): seq is number => seq !== undefined);
|
||||
if (reducerCommittedSeqs.length !== reducerCommittedOps.length) {
|
||||
throw new Error(
|
||||
'applyRemoteOperations: reducer commit contained an operation outside the appended batch.',
|
||||
);
|
||||
}
|
||||
await store.markArchivePending(reducerCommittedSeqs);
|
||||
await store.mergeRemoteOpClocks(reducerCommittedOps);
|
||||
},
|
||||
});
|
||||
if (applyResult.failedOp && !opIdToSeq.has(applyResult.failedOp.op.id)) {
|
||||
throw new Error(
|
||||
`applyRemoteOperations: applier reported unknown failed op ${applyResult.failedOp.op.id}.`,
|
||||
);
|
||||
}
|
||||
const appliedSeqs = applyResult.appliedOps
|
||||
.map((op) => opIdToSeq.get(op.id))
|
||||
.filter((seq): seq is number => seq !== undefined);
|
||||
|
|
@ -120,7 +139,6 @@ export const applyRemoteOperations = async <
|
|||
|
||||
if (appliedSeqs.length > 0) {
|
||||
await store.markApplied(appliedSeqs);
|
||||
await store.mergeRemoteOpClocks(applyResult.appliedOps);
|
||||
|
||||
const appliedFullStateOpIds = applyResult.appliedOps
|
||||
.filter(isFullStateOperation)
|
||||
|
|
@ -132,9 +150,7 @@ export const applyRemoteOperations = async <
|
|||
}
|
||||
}
|
||||
|
||||
const failedOpIds = applyResult.failedOp
|
||||
? getFailedOpIds(appendResult.writtenOps, applyResult.failedOp.op)
|
||||
: [];
|
||||
const failedOpIds = applyResult.failedOp ? [applyResult.failedOp.op.id] : [];
|
||||
|
||||
if (failedOpIds.length > 0) {
|
||||
await store.markFailed(failedOpIds);
|
||||
|
|
@ -150,12 +166,3 @@ export const applyRemoteOperations = async <
|
|||
failedOpIds,
|
||||
};
|
||||
};
|
||||
|
||||
const getFailedOpIds = <TOperation extends Operation<string>>(
|
||||
opsToApply: TOperation[],
|
||||
failedOp: TOperation,
|
||||
): string[] => {
|
||||
const failedOpIndex = opsToApply.findIndex((op) => op.id === failedOp.id);
|
||||
const failedOps = failedOpIndex === -1 ? [failedOp] : opsToApply.slice(failedOpIndex);
|
||||
return failedOps.map((op) => op.id);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ export interface OperationReplayCoordinatorOptions<
|
|||
operationToAction?: (op: TOperation) => TReplayAction;
|
||||
isArchiveAffectingAction?: (action: TReplayAction) => boolean;
|
||||
onRemoteArchiveDataApplied?: () => void;
|
||||
onReducersCommitted?: (ops: TOperation[]) => Promise<void>;
|
||||
onArchiveSideEffectError?: (
|
||||
context: OperationReplayArchiveFailureContext<TOperation>,
|
||||
) => void;
|
||||
|
|
@ -94,6 +95,7 @@ export const replayOperationBatch = async <
|
|||
operationToAction,
|
||||
isArchiveAffectingAction = () => false,
|
||||
onRemoteArchiveDataApplied,
|
||||
onReducersCommitted,
|
||||
onArchiveSideEffectError,
|
||||
onPostSyncCooldownError,
|
||||
yieldToEventLoop: waitForEventLoop = yieldToEventLoop,
|
||||
|
|
@ -118,6 +120,7 @@ export const replayOperationBatch = async <
|
|||
if (!skipReducerDispatch) {
|
||||
dispatcher.dispatch(createBulkApplyAction(ops));
|
||||
await waitForEventLoop();
|
||||
await onReducersCommitted?.(ops);
|
||||
}
|
||||
|
||||
if (!isLocalHydration && archiveSideEffects !== undefined && operationToAction) {
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ const createStore = (appendResult: {
|
|||
skippedCount: number;
|
||||
}): RemoteOperationApplyStorePort<Operation<string>> => ({
|
||||
appendBatchSkipDuplicates: vi.fn().mockResolvedValue(appendResult),
|
||||
markArchivePending: vi.fn().mockResolvedValue(undefined),
|
||||
markApplied: vi.fn().mockResolvedValue(undefined),
|
||||
markFailed: vi.fn().mockResolvedValue(undefined),
|
||||
mergeRemoteOpClocks: vi.fn().mockResolvedValue(undefined),
|
||||
|
|
@ -31,7 +32,10 @@ const createStore = (appendResult: {
|
|||
const createApplier = (
|
||||
result: Awaited<ReturnType<OperationApplyPort<Operation<string>>['applyOperations']>>,
|
||||
): OperationApplyPort<Operation<string>> => ({
|
||||
applyOperations: vi.fn().mockResolvedValue(result),
|
||||
applyOperations: vi.fn(async (ops, options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return result;
|
||||
}),
|
||||
});
|
||||
|
||||
describe('applyRemoteOperations', () => {
|
||||
|
|
@ -50,7 +54,11 @@ describe('applyRemoteOperations', () => {
|
|||
expect(store.appendBatchSkipDuplicates).toHaveBeenCalledWith([op1, op2], 'remote', {
|
||||
pendingApply: true,
|
||||
});
|
||||
expect(applier.applyOperations).toHaveBeenCalledWith([op2]);
|
||||
expect(applier.applyOperations).toHaveBeenCalledWith(
|
||||
[op2],
|
||||
jasmineLikeObjectContainingFunction(),
|
||||
);
|
||||
expect(store.markArchivePending).toHaveBeenCalledWith([10]);
|
||||
expect(store.markApplied).toHaveBeenCalledWith([10]);
|
||||
expect(store.mergeRemoteOpClocks).toHaveBeenCalledWith([op2]);
|
||||
expect(result).toEqual({
|
||||
|
|
@ -104,7 +112,7 @@ describe('applyRemoteOperations', () => {
|
|||
expect(result.clearedFullStateOpCount).toBe(3);
|
||||
});
|
||||
|
||||
it('marks the failed op and remaining unapplied ops as failed', async () => {
|
||||
it('checkpoints the whole reducer batch but charges only the attempted archive failure', async () => {
|
||||
const op1 = createOperation('op-1');
|
||||
const op2 = createOperation('op-2');
|
||||
const op3 = createOperation('op-3');
|
||||
|
|
@ -126,13 +134,14 @@ describe('applyRemoteOperations', () => {
|
|||
});
|
||||
|
||||
expect(store.markApplied).toHaveBeenCalledWith([1]);
|
||||
expect(store.mergeRemoteOpClocks).toHaveBeenCalledWith([op1]);
|
||||
expect(store.markFailed).toHaveBeenCalledWith(['op-2', 'op-3']);
|
||||
expect(store.markArchivePending).toHaveBeenCalledWith([1, 2, 3]);
|
||||
expect(store.mergeRemoteOpClocks).toHaveBeenCalledWith([op1, op2, op3]);
|
||||
expect(store.markFailed).toHaveBeenCalledWith(['op-2']);
|
||||
expect(result.failedOp).toEqual({ op: op2, error });
|
||||
expect(result.failedOpIds).toEqual(['op-2', 'op-3']);
|
||||
expect(result.failedOpIds).toEqual(['op-2']);
|
||||
});
|
||||
|
||||
it('marks only the reported failed op if it is not in the appended batch', async () => {
|
||||
it('rejects an applier result whose failed op is not in the appended batch', async () => {
|
||||
const op1 = createOperation('op-1');
|
||||
const unknownFailedOp = createOperation('unknown-failed-op');
|
||||
const store = createStore({ seqs: [1], writtenOps: [op1], skippedCount: 0 });
|
||||
|
|
@ -141,9 +150,12 @@ describe('applyRemoteOperations', () => {
|
|||
failedOp: { op: unknownFailedOp, error: new Error('unexpected') },
|
||||
});
|
||||
|
||||
const result = await applyRemoteOperations({ ops: [op1], store, applier });
|
||||
|
||||
expect(store.markFailed).toHaveBeenCalledWith(['unknown-failed-op']);
|
||||
expect(result.failedOpIds).toEqual(['unknown-failed-op']);
|
||||
await expect(applyRemoteOperations({ ops: [op1], store, applier })).rejects.toThrow(
|
||||
'unknown-failed-op',
|
||||
);
|
||||
expect(store.markFailed).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
const jasmineLikeObjectContainingFunction = (): object =>
|
||||
expect.objectContaining({ onReducersCommitted: expect.any(Function) });
|
||||
|
|
|
|||
|
|
@ -69,6 +69,9 @@ describe('replayOperationBatch', () => {
|
|||
const onRemoteArchiveDataApplied = vi.fn(() => {
|
||||
callOrder.push('remoteArchiveDataApplied');
|
||||
});
|
||||
const onReducersCommitted = vi.fn(async () => {
|
||||
callOrder.push('reducersCommitted');
|
||||
});
|
||||
|
||||
const result = await replayOperationBatch({
|
||||
ops,
|
||||
|
|
@ -87,6 +90,7 @@ describe('replayOperationBatch', () => {
|
|||
}),
|
||||
isArchiveAffectingAction: (action) => action.archiveAffecting === true,
|
||||
onRemoteArchiveDataApplied,
|
||||
onReducersCommitted,
|
||||
yieldToEventLoop,
|
||||
});
|
||||
|
||||
|
|
@ -101,6 +105,7 @@ describe('replayOperationBatch', () => {
|
|||
'startApplyingRemoteOps',
|
||||
'dispatchBulk',
|
||||
'yield',
|
||||
'reducersCommitted',
|
||||
'archive:op-1',
|
||||
'archive:op-2',
|
||||
'yield',
|
||||
|
|
|
|||
|
|
@ -583,6 +583,19 @@ describe('SyncWrapperService', () => {
|
|||
|
||||
expect(mockProviderManager.setLastSyncedProviderId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should stop with ERROR when download is blocked by an incompatible op', async () => {
|
||||
mockSyncService.downloadRemoteOps.and.resolveTo({
|
||||
kind: 'blocked_incompatible',
|
||||
});
|
||||
|
||||
const result = await service.sync();
|
||||
|
||||
expect(result).toBe('HANDLED_ERROR');
|
||||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
|
||||
expect(mockProviderManager.setLastSyncedProviderId).not.toHaveBeenCalled();
|
||||
expect(mockSyncService.uploadPendingOps).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('_sync() - Sync flow', () => {
|
||||
|
|
@ -1000,6 +1013,17 @@ describe('SyncWrapperService', () => {
|
|||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('IN_SYNC');
|
||||
});
|
||||
|
||||
it('should stop with ERROR when upload piggyback is blocked by an incompatible op', async () => {
|
||||
mockSyncService.uploadPendingOps.and.resolveTo({
|
||||
kind: 'blocked_incompatible',
|
||||
});
|
||||
|
||||
const result = await service.sync();
|
||||
|
||||
expect(result).toBe('HANDLED_ERROR');
|
||||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
|
||||
});
|
||||
|
||||
it('should set IN_SYNC when permanentRejectionCount is 0 even with empty rejectedOps array', async () => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
|
|
|
|||
|
|
@ -506,6 +506,13 @@ export class SyncWrapperService {
|
|||
this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
if (downloadResult.kind === 'blocked_incompatible') {
|
||||
SyncLog.warn(
|
||||
'SyncWrapperService: Download blocked by an incompatible operation.',
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
|
||||
// Track the successfully synced provider for switch detection on next sync
|
||||
this._providerManager.setLastSyncedProviderId(providerId);
|
||||
|
|
@ -515,6 +522,13 @@ export class SyncWrapperService {
|
|||
syncCapableProvider,
|
||||
{ isNeverSynced: isNeverSyncedAtSyncStart },
|
||||
);
|
||||
if (uploadResult.kind === 'blocked_incompatible') {
|
||||
SyncLog.warn(
|
||||
'SyncWrapperService: Upload piggyback blocked by an incompatible operation.',
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
const completedUploadResults: CompletedUploadOutcome[] =
|
||||
uploadResult.kind === 'completed' ? [uploadResult] : [];
|
||||
if (uploadResult.kind === 'completed') {
|
||||
|
|
@ -582,6 +596,13 @@ export class SyncWrapperService {
|
|||
this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
if (reuploadResult.kind === 'blocked_incompatible') {
|
||||
SyncLog.warn(
|
||||
'SyncWrapperService: LWW re-upload blocked by an incompatible operation.',
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return 'HANDLED_ERROR';
|
||||
}
|
||||
if (reuploadResult.kind === 'completed') {
|
||||
completedUploadResults.push(reuploadResult);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -129,6 +129,7 @@ export class OperationApplierService implements OperationApplyPort<Operation> {
|
|||
// this action is now disabled to prevent UI freezes during bulk archive sync.
|
||||
this.store.dispatch(remoteArchiveDataApplied());
|
||||
},
|
||||
onReducersCommitted: options.onReducersCommitted,
|
||||
onArchiveSideEffectError: ({ op, processedCount, error }) => {
|
||||
OpLog.err(
|
||||
`OperationApplierService: Failed archive handling for operation ${op.id}. ` +
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import { selectPluginMetadataFeatureState } from '../../plugins/store/plugin-met
|
|||
import { selectReminderFeatureState } from '../../features/reminder/store/reminder.reducer';
|
||||
import { ArchiveModel } from '../../features/time-tracking/time-tracking.model';
|
||||
import { initialTimeTrackingState } from '../../features/time-tracking/store/time-tracking.reducer';
|
||||
import { TaskState } from '../../features/tasks/task.model';
|
||||
|
||||
describe('StateSnapshotService', () => {
|
||||
let service: StateSnapshotService;
|
||||
|
|
@ -225,6 +226,32 @@ describe('StateSnapshotService', () => {
|
|||
expect(archiveDbAdapterSpy.loadArchiveYoung).toHaveBeenCalledTimes(1);
|
||||
expect(archiveDbAdapterSpy.loadArchiveOld).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should capture NgRx state before awaiting archive I/O', async () => {
|
||||
let releaseArchives!: () => void;
|
||||
const archiveGate = new Promise<void>((resolve) => {
|
||||
releaseArchives = resolve;
|
||||
});
|
||||
archiveDbAdapterSpy.loadArchiveYoung.and.callFake(async () => {
|
||||
await archiveGate;
|
||||
return mockArchiveYoung;
|
||||
});
|
||||
archiveDbAdapterSpy.loadArchiveOld.and.callFake(async () => {
|
||||
await archiveGate;
|
||||
return mockArchiveOld;
|
||||
});
|
||||
|
||||
const snapshotPromise = service.getStateSnapshotAsync();
|
||||
store.overrideSelector(selectTaskFeatureState, {
|
||||
...mockTaskState,
|
||||
ids: ['later-task'],
|
||||
} as unknown as TaskState);
|
||||
store.refreshState();
|
||||
releaseArchives();
|
||||
|
||||
const snapshot = await snapshotPromise;
|
||||
expect((snapshot.task as TaskState).ids).toEqual(['task1']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('backward compatibility aliases', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { Selector, Store } from '@ngrx/store';
|
||||
import { combineLatest, firstValueFrom } from 'rxjs';
|
||||
import { first } from 'rxjs/operators';
|
||||
|
||||
import { selectBoardsState } from '../../features/boards/store/boards.selectors';
|
||||
|
|
@ -135,24 +134,18 @@ export class StateSnapshotService {
|
|||
* ⚠️ Returns live store references — never mutate (clone first). See class doc.
|
||||
*/
|
||||
async getStateSnapshotAsync(): Promise<AppStateSnapshot> {
|
||||
// Capture NgRx synchronously before the first await. Callers that hold the
|
||||
// operation-log barrier can now establish an exact reducer-state cutoff:
|
||||
// actions dispatched while archive I/O is pending are not half-included in
|
||||
// the snapshot and will be persisted after it.
|
||||
const ngRxData = this._getNgRxDataSync();
|
||||
const [archiveYoung, archiveOld] = await Promise.all([
|
||||
this._loadArchive('archiveYoung'),
|
||||
this._loadArchive('archiveOld'),
|
||||
]);
|
||||
|
||||
const ngRxValues = await firstValueFrom(
|
||||
combineLatest(SNAPSHOT_SELECTORS.map((s) => this._store.select(s.selector))).pipe(
|
||||
first(),
|
||||
),
|
||||
);
|
||||
|
||||
const result: Partial<Record<NgRxModelKey, unknown>> = {};
|
||||
for (let i = 0; i < SNAPSHOT_SELECTORS.length; i++) {
|
||||
result[SNAPSHOT_SELECTORS[i].key] = ngRxValues[i];
|
||||
}
|
||||
|
||||
return {
|
||||
...this._normalizeNgRxData(result),
|
||||
...ngRxData,
|
||||
archiveYoung,
|
||||
archiveOld,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ describe('operationCaptureMetaReducer', () => {
|
|||
expect(buffered).toEqual([]);
|
||||
});
|
||||
|
||||
it('should clear buffer after returning actions', () => {
|
||||
it('should return a non-destructive snapshot until actions are acknowledged', () => {
|
||||
const action = createMockAction();
|
||||
bufferDeferredAction(action);
|
||||
|
||||
|
|
@ -257,7 +257,7 @@ describe('operationCaptureMetaReducer', () => {
|
|||
const secondCall = getDeferredActions();
|
||||
|
||||
expect(firstCall).toEqual([action]);
|
||||
expect(secondCall).toEqual([]);
|
||||
expect(secondCall).toEqual([action]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -329,7 +329,7 @@ describe('operationCaptureMetaReducer', () => {
|
|||
expect(getDeferredActions()).toEqual(actions);
|
||||
});
|
||||
|
||||
it('should drop the oldest action only past the pathological hard cap', () => {
|
||||
it('should retain every accepted action past the pathological warning cap', () => {
|
||||
const confirmSpy = spyNativeDialogs();
|
||||
const actions = createManyActions(DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP);
|
||||
const dropErrorCalls = (): unknown[][] =>
|
||||
|
|
@ -345,9 +345,8 @@ describe('operationCaptureMetaReducer', () => {
|
|||
expect(dropErrorCalls().length).toBe(1);
|
||||
|
||||
const buffered = getDeferredActions();
|
||||
expect(buffered.length).toBe(DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP);
|
||||
// Oldest action was dropped, remaining order intact
|
||||
expect(buffered[0]).toBe(actions[1]);
|
||||
expect(buffered.length).toBe(DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP + 1);
|
||||
expect(buffered[0]).toBe(actions[0]);
|
||||
expect(buffered[buffered.length - 1]).toBe(extraAction);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -148,13 +148,12 @@ const DEFERRED_ACTIONS_SOFT_WARNING_THRESHOLD = 10;
|
|||
export const DEFERRED_ACTIONS_RELOAD_WARNING_THRESHOLD = 100;
|
||||
|
||||
/**
|
||||
* Pathological hard cap for the deferred actions buffer — purely a memory
|
||||
* backstop against a runaway dispatch loop. ~100 buffered actions are
|
||||
* Pathological high-water mark for the deferred actions buffer. ~100 buffered actions are
|
||||
* plausibly reachable by a real user during a stuck/very long remote-apply
|
||||
* window, so we must never drop there (see reload-warning threshold above,
|
||||
* which tells the user to reload long before this cap can be hit through
|
||||
* real interaction). Only past this cap is drop-oldest the lesser evil vs
|
||||
* unbounded memory growth.
|
||||
* which tells the user to reload long before this mark can be hit through
|
||||
* real interaction). Crossing it logs loudly but still cannot discard an
|
||||
* action whose reducer change has already been accepted.
|
||||
* Exported for tests.
|
||||
*/
|
||||
export const DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP = 5000;
|
||||
|
|
@ -164,19 +163,15 @@ export const DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP = 5000;
|
|||
* Called by the meta-reducer when a persistent action arrives during sync.
|
||||
*/
|
||||
export const bufferDeferredAction = (action: PersistentAction): void => {
|
||||
// Pathological cap only: dropping an accepted persistent action means its
|
||||
// state mutation survives in NgRx while no operation will ever represent
|
||||
// it → permanent, unsyncable divergence. So this drop exists solely as a
|
||||
// memory backstop against a runaway dispatch loop, never as flow control.
|
||||
// NOTE: The shifted action remains in deferredActionSet (WeakSet has no
|
||||
// delete-by-value). The effect filters it via isDeferredAction(), and
|
||||
// getDeferredActions() won't return it, so it is silently lost.
|
||||
// Never drop an accepted persistent action: its reducer mutation already
|
||||
// exists in NgRx, so removal here would create permanent unsyncable state.
|
||||
// The cap remains a loud runaway-loop diagnostic, not a lossy flow-control
|
||||
// mechanism.
|
||||
if (deferredActions.length >= DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP) {
|
||||
devError(
|
||||
`[operationCaptureMetaReducer] Deferred actions buffer exceeded pathological cap of ${DEFERRED_ACTIONS_PATHOLOGICAL_HARD_CAP} items. ` +
|
||||
`Dropping oldest action - this local change will NOT sync. Likely a runaway dispatch loop; reload the app.`,
|
||||
'Retaining all actions to preserve sync correctness. Likely a runaway dispatch loop; reload the app.',
|
||||
);
|
||||
deferredActions.shift();
|
||||
}
|
||||
|
||||
deferredActions.push(action);
|
||||
|
|
@ -195,25 +190,32 @@ export const bufferDeferredAction = (action: PersistentAction): void => {
|
|||
};
|
||||
|
||||
/**
|
||||
* Gets and clears the deferred actions buffer.
|
||||
* Called after sync completes to process buffered actions.
|
||||
* Returns a stable snapshot of the deferred actions buffer. Entries remain queued
|
||||
* until their operation is durably written and explicitly acknowledged.
|
||||
*/
|
||||
export const getDeferredActions = (): PersistentAction[] => {
|
||||
const actions = deferredActions;
|
||||
deferredActions = [];
|
||||
return actions;
|
||||
return [...deferredActions];
|
||||
};
|
||||
|
||||
export const acknowledgeDeferredAction = (action: PersistentAction): void => {
|
||||
const index = deferredActions.indexOf(action);
|
||||
if (index === -1) {
|
||||
return;
|
||||
}
|
||||
deferredActions.splice(index, 1);
|
||||
deferredActionSet.delete(action);
|
||||
};
|
||||
|
||||
/**
|
||||
* Clears the deferred actions buffer without processing.
|
||||
* Used for cleanup during testing or error recovery.
|
||||
*
|
||||
* Note: deferredActionSet (WeakSet) is not cleared here because WeakSet has
|
||||
* no .clear() method. Entries are garbage-collected when action references are
|
||||
* released. In practice, cleared actions are not reused by reference, so stale
|
||||
* entries in the WeakSet are harmless.
|
||||
* WeakSet has no clear(), so delete the known buffered references individually.
|
||||
*/
|
||||
export const clearDeferredActions = (): void => {
|
||||
for (const action of deferredActions) {
|
||||
deferredActionSet.delete(action);
|
||||
}
|
||||
deferredActions = [];
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import { COMPACTION_THRESHOLD } from '../core/operation-log.const';
|
|||
import {
|
||||
bufferDeferredAction,
|
||||
clearDeferredActions,
|
||||
getDeferredActions,
|
||||
} from './operation-capture.meta-reducer';
|
||||
import { ClientIdService } from '../../core/util/client-id.service';
|
||||
import { OperationCaptureService } from './operation-capture.service';
|
||||
|
|
@ -876,8 +877,31 @@ describe('OperationLogEffects', () => {
|
|||
// Should not throw - errors are logged but don't stop processing
|
||||
await expectAsync(effects.processDeferredActions()).toBeResolved();
|
||||
|
||||
// Both actions should have been attempted
|
||||
expect(mockOpLogStore.appendWithVectorClockUpdate).toHaveBeenCalledTimes(2);
|
||||
// The failed write is retried once, then processing continues to action 2.
|
||||
expect(mockOpLogStore.appendWithVectorClockUpdate).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('should keep an exhausted deferred write queued while acknowledging later successes', async () => {
|
||||
const failedAction = createPersistentAction(ActionType.TASK_SHARED_ADD);
|
||||
const successfulAction = createPersistentAction(ActionType.TASK_SHARED_UPDATE);
|
||||
bufferDeferredAction(failedAction);
|
||||
bufferDeferredAction(successfulAction);
|
||||
let attempts = 0;
|
||||
mockOpLogStore.appendWithVectorClockUpdate.and.callFake(() => {
|
||||
attempts++;
|
||||
return attempts <= 3
|
||||
? Promise.reject(new Error('persistent failure'))
|
||||
: Promise.resolve(1);
|
||||
});
|
||||
|
||||
await effects.processDeferredActions();
|
||||
|
||||
expect(getDeferredActions()).toEqual([failedAction]);
|
||||
|
||||
mockOpLogStore.appendWithVectorClockUpdate.and.resolveTo(2);
|
||||
await effects.processDeferredActions();
|
||||
|
||||
expect(getDeferredActions()).toEqual([]);
|
||||
});
|
||||
|
||||
it('should use fresh vector clock for deferred actions', async () => {
|
||||
|
|
|
|||
|
|
@ -28,7 +28,11 @@ import {
|
|||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
import { OperationCaptureService } from './operation-capture.service';
|
||||
import { ImmediateUploadService } from '../sync/immediate-upload.service';
|
||||
import { getDeferredActions, isDeferredAction } from './operation-capture.meta-reducer';
|
||||
import {
|
||||
acknowledgeDeferredAction,
|
||||
getDeferredActions,
|
||||
isDeferredAction,
|
||||
} from './operation-capture.meta-reducer';
|
||||
import { ClientIdService } from '../../core/util/client-id.service';
|
||||
import { SuperSyncStatusService } from '../sync/super-sync-status.service';
|
||||
|
||||
|
|
@ -181,6 +185,9 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
devError(
|
||||
`[OperationLogEffects] Action ${action.type} has invalid entityId/entityIds (${action.meta.entityId}) - skipping persistence`,
|
||||
);
|
||||
if (isDeferredWrite) {
|
||||
throw new Error(`Deferred action ${action.type} has invalid entity identifiers.`);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -292,6 +299,9 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
window.location.reload();
|
||||
},
|
||||
});
|
||||
if (isDeferredWrite) {
|
||||
throw new Error(`Deferred action ${action.type} has an invalid payload.`);
|
||||
}
|
||||
return; // Skip persisting invalid operation
|
||||
}
|
||||
|
||||
|
|
@ -378,10 +388,12 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
this.notifyUserAndTriggerRollback();
|
||||
} else {
|
||||
await this.handleQuotaExceeded(action, isDeferredWrite, options);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
this.notifyUserAndTriggerRollback();
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -512,6 +524,7 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
|
||||
// Use lock for cross-tab coordination - only one tab handles quota at a time
|
||||
let bailReason: Error | null = null;
|
||||
let recovered = false;
|
||||
await this.lockService.request('sp_quota_exceeded', async () => {
|
||||
if (options.callerHoldsOperationLogLock) {
|
||||
OpLog.err(
|
||||
|
|
@ -541,6 +554,7 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
// extractEntityChanges() inside writeOperation, so the retry simply
|
||||
// re-extracts the same changes. Pass isDeferredWrite through unchanged.
|
||||
await this.writeOperation(action, isDeferredWrite, options);
|
||||
recovered = true;
|
||||
this.snackService.open({
|
||||
type: 'SUCCESS',
|
||||
msg: T.F.SYNC.S.STORAGE_RECOVERED_AFTER_COMPACTION,
|
||||
|
|
@ -562,6 +576,10 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
if (bailReason !== null) {
|
||||
throw bailReason;
|
||||
}
|
||||
if (recovered) {
|
||||
return;
|
||||
}
|
||||
throw new Error('Storage quota recovery failed; operation was not persisted.');
|
||||
}
|
||||
|
||||
private showStorageQuotaExceededError(): void {
|
||||
|
|
@ -646,8 +664,13 @@ export class OperationLogEffects implements DeferredLocalActionsPort {
|
|||
// Log error after all retries exhausted, continue processing remaining actions
|
||||
OpLog.err(
|
||||
`OperationLogEffects: Failed to process deferred action after ${MAX_RETRIES} retries`,
|
||||
{ actionType: action.type, error: lastError },
|
||||
{
|
||||
actionType: action.type,
|
||||
errorName: (lastError as Error | undefined)?.name,
|
||||
},
|
||||
);
|
||||
} else {
|
||||
acknowledgeDeferredAction(action);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -41,4 +41,11 @@ export interface ApplyOperationsOptions {
|
|||
* increaseSimpleCounterCounterToday.
|
||||
*/
|
||||
skipReducerDispatch?: boolean;
|
||||
|
||||
/**
|
||||
* Called after the bulk reducer dispatch commits and before archive side effects.
|
||||
* Remote apply uses this to persist its reducer/archive checkpoint and merge the
|
||||
* causal frontier before deferred local actions can be written.
|
||||
*/
|
||||
onReducersCommitted?: (ops: Operation[]) => Promise<void>;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Operation, VectorClock } from '../operation.types';
|
||||
import { Operation, OperationLogEntry, VectorClock } from '../operation.types';
|
||||
import { OperationSyncProviderMode } from '../../sync-providers/provider.interface';
|
||||
|
||||
/**
|
||||
|
|
@ -106,6 +106,13 @@ export interface UploadResult {
|
|||
piggybackedOps: Operation[];
|
||||
rejectedCount: number;
|
||||
rejectedOps: RejectedOpInfo[];
|
||||
/** Exact in-lock pending set considered by this upload round. */
|
||||
selectedPendingOps?: OperationLogEntry[];
|
||||
/**
|
||||
* Accepted/local-only operation sequences whose acknowledgement was deliberately
|
||||
* deferred until the caller has resolved and applied piggybacked operations.
|
||||
*/
|
||||
pendingAcknowledgementSeqs?: number[];
|
||||
/**
|
||||
* Number of local-win update ops created during LWW conflict resolution.
|
||||
* These ops need to be uploaded to propagate local state to other clients.
|
||||
|
|
@ -171,6 +178,13 @@ export interface UploadOptions {
|
|||
*/
|
||||
preUploadCallback?: () => Promise<void>;
|
||||
|
||||
/**
|
||||
* Return accepted sequence numbers to the caller instead of marking them synced
|
||||
* immediately. Required when piggybacked full-state operations may need user
|
||||
* resolution before the upload round is considered committed locally.
|
||||
*/
|
||||
deferAcknowledgement?: boolean;
|
||||
|
||||
/**
|
||||
* If true, instructs server to delete all existing user data before accepting uploaded operations.
|
||||
* Used for clean slate operations like encryption password changes or full imports.
|
||||
|
|
@ -254,6 +268,10 @@ export type DownloadOutcome =
|
|||
| {
|
||||
/** User cancelled a SYNC_IMPORT conflict dialog. */
|
||||
kind: 'cancelled';
|
||||
}
|
||||
| {
|
||||
/** Processing stopped at an op this app version cannot interpret safely. */
|
||||
kind: 'blocked_incompatible';
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -284,4 +302,8 @@ export type UploadOutcome =
|
|||
| {
|
||||
/** User cancelled a piggybacked SYNC_IMPORT conflict dialog. */
|
||||
kind: 'cancelled';
|
||||
}
|
||||
| {
|
||||
/** Piggyback processing stopped at an incompatible operation. */
|
||||
kind: 'blocked_incompatible';
|
||||
};
|
||||
|
|
|
|||
|
|
@ -65,6 +65,6 @@ export interface CompactOperationLogEntry {
|
|||
source: 'local' | 'remote';
|
||||
syncedAt?: number;
|
||||
rejectedAt?: number;
|
||||
applicationStatus?: 'pending' | 'applied' | 'failed';
|
||||
applicationStatus?: 'pending' | 'archive_pending' | 'applied' | 'failed';
|
||||
retryCount?: number;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1450,7 +1450,7 @@ describe('OperationLogHydratorService', () => {
|
|||
expect(mockOpLogStore.markApplied).toHaveBeenCalledWith([40, 41, 42]);
|
||||
});
|
||||
|
||||
it('should mark the failed op and every op after it (in seq order) as still-failing', async () => {
|
||||
it('should charge retry budget only to the attempted archive failure', async () => {
|
||||
const entries = [
|
||||
failedEntry(40, 'op-a'),
|
||||
failedEntry(41, 'op-b'),
|
||||
|
|
@ -1469,10 +1469,9 @@ describe('OperationLogHydratorService', () => {
|
|||
await service.retryFailedRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markApplied).toHaveBeenCalledWith([40]);
|
||||
// op-b (failed) and op-c (after it) are both marked failed, with the
|
||||
// retry-cap so a permanently-failing op is eventually rejected.
|
||||
// op-c remains archive-pending and has not consumed a retry attempt.
|
||||
expect(mockOpLogStore.markFailed).toHaveBeenCalledWith(
|
||||
['op-b', 'op-c'],
|
||||
['op-b'],
|
||||
MAX_CONFLICT_RETRY_ATTEMPTS,
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@ import { ValidateStateService } from '../validation/validate-state.service';
|
|||
import { OperationApplierService } from '../apply/operation-applier.service';
|
||||
import { HydrationStateService } from '../apply/hydration-state.service';
|
||||
import { bulkApplyOperations } from '../apply/bulk-hydration.action';
|
||||
import { getFailedOpIdsFromBatch } from '../apply/failed-op-ids.util';
|
||||
import { VectorClockService } from '../sync/vector-clock.service';
|
||||
import { MAX_CONFLICT_RETRY_ATTEMPTS } from '../core/operation-log.const';
|
||||
import { AppDataComplete } from '../model/model-config';
|
||||
|
|
@ -629,22 +628,20 @@ export class OperationLogHydratorService {
|
|||
);
|
||||
}
|
||||
|
||||
// On a partial failure the batch applier stops at the first op whose archive
|
||||
// side effect throws and returns it; that op and every op after it in seq
|
||||
// order stay unapplied (slice-from-failure, shared with the primary path).
|
||||
// markFailed bumps the retry count (rejecting ops past
|
||||
// MAX_CONFLICT_RETRY_ATTEMPTS), so a permanently-failing op can't be retried
|
||||
// forever.
|
||||
// On a partial failure the batch applier stops at the first archive error.
|
||||
// Charge only that attempted operation: successors remain archive-pending
|
||||
// without consuming retry budget and will run after the blocker succeeds or
|
||||
// becomes terminally rejected.
|
||||
if (result.failedOp) {
|
||||
const stillFailedOpIds = getFailedOpIdsFromBatch(opsToApply, result.failedOp.op);
|
||||
const failedOpIds = [result.failedOp.op.id];
|
||||
|
||||
OpLog.warn(
|
||||
`OperationLogHydratorService: Failed to retry op ${result.failedOp.op.id}`,
|
||||
result.failedOp.error,
|
||||
);
|
||||
await this.opLogStore.markFailed(stillFailedOpIds, MAX_CONFLICT_RETRY_ATTEMPTS);
|
||||
await this.opLogStore.markFailed(failedOpIds, MAX_CONFLICT_RETRY_ATTEMPTS);
|
||||
OpLog.warn(
|
||||
`OperationLogHydratorService: ${stillFailedOpIds.length} ops still failing after retry`,
|
||||
'OperationLogHydratorService: Archive operation still failing after retry',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ describe('OperationLogRecoveryService', () => {
|
|||
'getPendingRemoteOps',
|
||||
'markRejected',
|
||||
'markApplied',
|
||||
'markArchivePending',
|
||||
'getUnsynced',
|
||||
]);
|
||||
mockOpLogStore.setVectorClock.and.resolveTo(undefined);
|
||||
|
|
@ -183,22 +184,23 @@ describe('OperationLogRecoveryService', () => {
|
|||
|
||||
await service.recoverPendingRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markApplied).not.toHaveBeenCalled();
|
||||
expect(mockOpLogStore.markArchivePending).not.toHaveBeenCalled();
|
||||
expect(mockOpLogStore.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should mark valid pending ops as applied', async () => {
|
||||
it('should mark valid crash-interrupted ops as archive-pending', async () => {
|
||||
const now = Date.now();
|
||||
const pendingOps = [
|
||||
{ seq: 1, op: { id: 'op1' }, appliedAt: now - 1000, source: 'remote' },
|
||||
{ seq: 2, op: { id: 'op2' }, appliedAt: now - 2000, source: 'remote' },
|
||||
] as any;
|
||||
mockOpLogStore.getPendingRemoteOps.and.resolveTo(pendingOps);
|
||||
mockOpLogStore.markApplied.and.resolveTo(undefined);
|
||||
mockOpLogStore.markArchivePending.and.resolveTo(undefined);
|
||||
|
||||
await service.recoverPendingRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markApplied).toHaveBeenCalledWith([1, 2]);
|
||||
expect(mockOpLogStore.markArchivePending).toHaveBeenCalledWith([1, 2]);
|
||||
expect(mockOpLogStore.markApplied).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject ops that exceed PENDING_OPERATION_EXPIRY_MS', async () => {
|
||||
|
|
@ -213,12 +215,12 @@ describe('OperationLogRecoveryService', () => {
|
|||
}, // Expired
|
||||
] as any;
|
||||
mockOpLogStore.getPendingRemoteOps.and.resolveTo(pendingOps);
|
||||
mockOpLogStore.markApplied.and.resolveTo(undefined);
|
||||
mockOpLogStore.markArchivePending.and.resolveTo(undefined);
|
||||
mockOpLogStore.markRejected.and.resolveTo(undefined);
|
||||
|
||||
await service.recoverPendingRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markApplied).toHaveBeenCalledWith([1]);
|
||||
expect(mockOpLogStore.markArchivePending).toHaveBeenCalledWith([1]);
|
||||
expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['expired']);
|
||||
});
|
||||
|
||||
|
|
@ -235,7 +237,7 @@ describe('OperationLogRecoveryService', () => {
|
|||
await service.recoverPendingRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['old1', 'old2']);
|
||||
expect(mockOpLogStore.markApplied).not.toHaveBeenCalled();
|
||||
expect(mockOpLogStore.markArchivePending).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle mixed valid and expired ops correctly', async () => {
|
||||
|
|
@ -257,12 +259,12 @@ describe('OperationLogRecoveryService', () => {
|
|||
},
|
||||
] as any;
|
||||
mockOpLogStore.getPendingRemoteOps.and.resolveTo(pendingOps);
|
||||
mockOpLogStore.markApplied.and.resolveTo(undefined);
|
||||
mockOpLogStore.markArchivePending.and.resolveTo(undefined);
|
||||
mockOpLogStore.markRejected.and.resolveTo(undefined);
|
||||
|
||||
await service.recoverPendingRemoteOps();
|
||||
|
||||
expect(mockOpLogStore.markApplied).toHaveBeenCalledWith([1, 3]);
|
||||
expect(mockOpLogStore.markArchivePending).toHaveBeenCalledWith([1, 3]);
|
||||
expect(mockOpLogStore.markRejected).toHaveBeenCalledWith(['expired1', 'expired2']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -136,8 +136,10 @@ export class OperationLogRecoveryService {
|
|||
|
||||
/**
|
||||
* Recovers from pending remote ops that were stored but not applied (crash recovery).
|
||||
* These ops are in the log and will be replayed during normal hydration, so we just
|
||||
* need to mark them as applied to prevent them appearing as orphaned.
|
||||
* These ops are replayed through reducers during normal hydration, but a crash may
|
||||
* have happened before their archive side effects completed. Move them to the
|
||||
* archive-pending checkpoint so hydration retries archive work without double-applying
|
||||
* reducers.
|
||||
*
|
||||
* Operations pending for longer than PENDING_OPERATION_EXPIRY_MS are considered
|
||||
* superseded (likely due to data corruption or repeated failures) and are rejected
|
||||
|
|
@ -169,13 +171,13 @@ export class OperationLogRecoveryService {
|
|||
);
|
||||
}
|
||||
|
||||
// Mark valid ops as applied - they'll be replayed during normal hydration
|
||||
// Reducers are replayed status-blind during hydration; archive work is retried after.
|
||||
if (validOps.length > 0) {
|
||||
const seqs = validOps.map((e) => e.seq);
|
||||
await this.opLogStore.markApplied(seqs);
|
||||
await this.opLogStore.markArchivePending(seqs);
|
||||
OpLog.warn(
|
||||
`OperationLogRecoveryService: Found ${validOps.length} pending remote ops from previous crash. ` +
|
||||
`Marking as applied (they will be replayed during hydration).`,
|
||||
'Marking archive work pending (reducers will replay during hydration).',
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ import {
|
|||
} from '../core/operation-log.const';
|
||||
import { IndexedDBOpenError } from '../core/errors/indexed-db-open.error';
|
||||
import { FULL_STATE_OPS_META_KEY, SINGLETON_KEY, STORE_NAMES } from './db-keys.const';
|
||||
import { ArchiveModel } from '../../features/time-tracking/time-tracking.model';
|
||||
|
||||
describe('OperationLogStoreService', () => {
|
||||
let service: OperationLogStoreService;
|
||||
|
|
@ -1423,6 +1424,20 @@ describe('OperationLogStoreService', () => {
|
|||
});
|
||||
|
||||
describe('markApplied', () => {
|
||||
it('should checkpoint reducer-committed operations as archive-pending', async () => {
|
||||
const op = createTestOperation();
|
||||
const seq = await service.append(op, 'remote', { pendingApply: true });
|
||||
|
||||
await service.markArchivePending([seq]);
|
||||
|
||||
const [stored] = await service.getOpsAfterSeq(0);
|
||||
expect(stored.applicationStatus).toBe('archive_pending');
|
||||
expect((await service.getPendingRemoteOps()).length).toBe(0);
|
||||
expect((await service.getFailedRemoteOps()).map((entry) => entry.op.id)).toEqual([
|
||||
op.id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should update applicationStatus from pending to applied', async () => {
|
||||
const op = createTestOperation();
|
||||
const seq = await service.append(op, 'remote', { pendingApply: true });
|
||||
|
|
@ -1478,6 +1493,17 @@ describe('OperationLogStoreService', () => {
|
|||
expect(afterMarkApplied[0].applicationStatus).toBe('applied');
|
||||
});
|
||||
|
||||
it('should update applicationStatus from archive-pending to applied', async () => {
|
||||
const op = createTestOperation();
|
||||
const seq = await service.append(op, 'remote', { pendingApply: true });
|
||||
await service.markArchivePending([seq]);
|
||||
|
||||
await service.markApplied([seq]);
|
||||
|
||||
const [stored] = await service.getOpsAfterSeq(0);
|
||||
expect(stored.applicationStatus).toBe('applied');
|
||||
});
|
||||
|
||||
it('should remove failed ops from getFailedRemoteOps after markApplied is called', async () => {
|
||||
// Create an op and mark it as pending
|
||||
const op = createTestOperation();
|
||||
|
|
@ -2301,6 +2327,139 @@ describe('OperationLogStoreService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('runRemoteStateReplacement', () => {
|
||||
const createArchive = (taskId: string): ArchiveModel =>
|
||||
({
|
||||
task: {
|
||||
ids: [taskId],
|
||||
entities: { [taskId]: { id: taskId, title: taskId } },
|
||||
},
|
||||
timeTracking: { project: {}, tag: {} },
|
||||
lastTimeTrackingFlush: 0,
|
||||
}) as unknown as ArchiveModel;
|
||||
|
||||
it('atomically replaces ops, cache, clock, metadata, and both archives', async () => {
|
||||
await service.append(
|
||||
createTestOperation({
|
||||
opType: OpType.SyncImport,
|
||||
entityType: 'ALL' as EntityType,
|
||||
}),
|
||||
);
|
||||
const baselineState = { task: { ids: [], entities: {} } };
|
||||
const archiveYoung = createArchive('remote-young');
|
||||
const archiveOld = createArchive('remote-old');
|
||||
|
||||
await service.runRemoteStateReplacement({
|
||||
baselineState,
|
||||
vectorClock: { remote: 4 },
|
||||
schemaVersion: 4,
|
||||
snapshotEntityKeys: ['TASK:remote-task'],
|
||||
archiveYoung,
|
||||
archiveOld,
|
||||
});
|
||||
|
||||
expect(await service.getOpsAfterSeq(0)).toEqual([]);
|
||||
expect(await service.getLatestFullStateOpEntry()).toBeUndefined();
|
||||
expect(await service.loadStateCache()).toEqual(
|
||||
jasmine.objectContaining({
|
||||
state: baselineState,
|
||||
lastAppliedOpSeq: 0,
|
||||
vectorClock: { remote: 4 },
|
||||
schemaVersion: 4,
|
||||
snapshotEntityKeys: ['TASK:remote-task'],
|
||||
}),
|
||||
);
|
||||
expect(await service.getVectorClock()).toEqual({ remote: 4 });
|
||||
|
||||
const db = (
|
||||
service as unknown as {
|
||||
db: IDBPDatabase<unknown>;
|
||||
}
|
||||
).db;
|
||||
expect((await db.get(STORE_NAMES.ARCHIVE_YOUNG, SINGLETON_KEY)).data).toEqual(
|
||||
archiveYoung,
|
||||
);
|
||||
expect((await db.get(STORE_NAMES.ARCHIVE_OLD, SINGLETON_KEY)).data).toEqual(
|
||||
archiveOld,
|
||||
);
|
||||
});
|
||||
|
||||
it('rolls back every store if one archive write fails', async () => {
|
||||
const priorOp = createTestOperation({ entityId: 'prior-task' });
|
||||
const priorState = { sentinel: 'prior-state' };
|
||||
const priorYoung = createArchive('prior-young');
|
||||
const priorOld = createArchive('prior-old');
|
||||
await service.append(priorOp);
|
||||
await service.saveStateCache({
|
||||
state: priorState,
|
||||
lastAppliedOpSeq: 1,
|
||||
vectorClock: { testClient: 1 },
|
||||
compactedAt: Date.now(),
|
||||
});
|
||||
await service.setVectorClock({ testClient: 1 });
|
||||
|
||||
const db = (
|
||||
service as unknown as {
|
||||
db: IDBPDatabase<unknown>;
|
||||
}
|
||||
).db;
|
||||
await db.put(STORE_NAMES.ARCHIVE_YOUNG, {
|
||||
id: SINGLETON_KEY,
|
||||
data: priorYoung,
|
||||
lastModified: 1,
|
||||
});
|
||||
await db.put(STORE_NAMES.ARCHIVE_OLD, {
|
||||
id: SINGLETON_KEY,
|
||||
data: priorOld,
|
||||
lastModified: 1,
|
||||
});
|
||||
|
||||
const realTransaction = db.transaction.bind(db);
|
||||
spyOn(db, 'transaction').and.callFake(((
|
||||
stores: Parameters<typeof db.transaction>[0],
|
||||
mode: Parameters<typeof db.transaction>[1],
|
||||
) => {
|
||||
const tx = realTransaction(stores, mode);
|
||||
if (Array.isArray(stores) && stores.includes(STORE_NAMES.ARCHIVE_OLD)) {
|
||||
const realObjectStore = tx.objectStore.bind(tx);
|
||||
tx.objectStore = ((storeName: string) => {
|
||||
const store = realObjectStore(storeName);
|
||||
if (storeName === STORE_NAMES.ARCHIVE_OLD) {
|
||||
store.put = async () => {
|
||||
throw new Error('Simulated archive write failure');
|
||||
};
|
||||
}
|
||||
return store;
|
||||
}) as typeof tx.objectStore;
|
||||
}
|
||||
return tx;
|
||||
}) as typeof db.transaction);
|
||||
|
||||
await expectAsync(
|
||||
service.runRemoteStateReplacement({
|
||||
baselineState: { sentinel: 'new-state' },
|
||||
vectorClock: { remote: 2 },
|
||||
schemaVersion: 4,
|
||||
snapshotEntityKeys: [],
|
||||
archiveYoung: createArchive('new-young'),
|
||||
archiveOld: createArchive('new-old'),
|
||||
}),
|
||||
).toBeRejected();
|
||||
|
||||
expect((await service.getOpsAfterSeq(0)).map((entry) => entry.op.id)).toEqual([
|
||||
priorOp.id,
|
||||
]);
|
||||
expect((await service.loadStateCache())!.state).toEqual(priorState);
|
||||
expect(await service.getVectorClock()).toEqual({ testClient: 1 });
|
||||
expect((await db.get(STORE_NAMES.ARCHIVE_YOUNG, SINGLETON_KEY)).data).toEqual(
|
||||
priorYoung,
|
||||
);
|
||||
expect((await db.get(STORE_NAMES.ARCHIVE_OLD, SINGLETON_KEY)).data).toEqual(
|
||||
priorOld,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('appendWithVectorClockUpdate', () => {
|
||||
it('should append operation and update vector clock atomically for local ops', async () => {
|
||||
const op = createTestOperation({
|
||||
|
|
|
|||
|
|
@ -90,7 +90,7 @@ interface StoredOperationLogEntry {
|
|||
source: 'local' | 'remote';
|
||||
syncedAt?: number;
|
||||
rejectedAt?: number;
|
||||
applicationStatus?: 'pending' | 'applied' | 'failed';
|
||||
applicationStatus?: 'pending' | 'archive_pending' | 'applied' | 'failed';
|
||||
retryCount?: number;
|
||||
}
|
||||
|
||||
|
|
@ -679,6 +679,24 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort<O
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Records that reducers committed for remote operations and only their archive
|
||||
* side effects remain. This checkpoint is persisted before archive I/O starts,
|
||||
* allowing startup recovery to retry archive work without double-dispatching.
|
||||
*/
|
||||
async markArchivePending(seqs: number[]): Promise<void> {
|
||||
await this._ensureInit();
|
||||
await this._adapter.transaction([STORE_NAMES.OPS], 'readwrite', async (tx) => {
|
||||
for (const seq of seqs) {
|
||||
const entry = await tx.get<StoredOperationLogEntry>(STORE_NAMES.OPS, seq);
|
||||
if (entry?.applicationStatus === 'pending') {
|
||||
entry.applicationStatus = 'archive_pending';
|
||||
await tx.put(STORE_NAMES.OPS, entry);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks operations as successfully applied.
|
||||
* Called after remote operations have been dispatched to NgRx.
|
||||
|
|
@ -689,11 +707,12 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort<O
|
|||
await this._adapter.transaction([STORE_NAMES.OPS], 'readwrite', async (tx) => {
|
||||
for (const seq of seqs) {
|
||||
const entry = await tx.get<StoredOperationLogEntry>(STORE_NAMES.OPS, seq);
|
||||
// Allow transitioning from 'pending' or 'failed' to 'applied'
|
||||
// 'failed' ops can be retried and need to be cleared when successful
|
||||
// Failed/archive-pending ops can be retried and cleared when successful.
|
||||
if (
|
||||
entry &&
|
||||
(entry.applicationStatus === 'pending' || entry.applicationStatus === 'failed')
|
||||
(entry.applicationStatus === 'pending' ||
|
||||
entry.applicationStatus === 'archive_pending' ||
|
||||
entry.applicationStatus === 'failed')
|
||||
) {
|
||||
entry.applicationStatus = 'applied';
|
||||
await tx.put(STORE_NAMES.OPS, entry);
|
||||
|
|
@ -1100,20 +1119,31 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort<O
|
|||
}
|
||||
|
||||
/**
|
||||
* Gets remote operations that failed and can be retried.
|
||||
* These are ops that were attempted but failed (e.g., missing dependency).
|
||||
* Gets remote operations whose archive work is incomplete and can be retried.
|
||||
* Includes both attempted failures and reducer-committed successors that have
|
||||
* not attempted their archive handler yet.
|
||||
* PERF: Uses compound index to reduce scan scope, then filters by rejectedAt.
|
||||
*/
|
||||
async getFailedRemoteOps(): Promise<OperationLogEntry[]> {
|
||||
await this._ensureInit();
|
||||
let storedEntries: StoredOperationLogEntry[];
|
||||
try {
|
||||
// Exact compound-key match expressed as a degenerate [k, k] range.
|
||||
storedEntries = await this._adapter.getAllFromIndex<StoredOperationLogEntry>(
|
||||
STORE_NAMES.OPS,
|
||||
OPS_INDEXES.BY_SOURCE_AND_STATUS,
|
||||
{ lower: ['remote', 'failed'], upper: ['remote', 'failed'] },
|
||||
);
|
||||
const [archivePendingEntries, failedEntries] = await Promise.all([
|
||||
this._adapter.getAllFromIndex<StoredOperationLogEntry>(
|
||||
STORE_NAMES.OPS,
|
||||
OPS_INDEXES.BY_SOURCE_AND_STATUS,
|
||||
{
|
||||
lower: ['remote', 'archive_pending'],
|
||||
upper: ['remote', 'archive_pending'],
|
||||
},
|
||||
),
|
||||
this._adapter.getAllFromIndex<StoredOperationLogEntry>(
|
||||
STORE_NAMES.OPS,
|
||||
OPS_INDEXES.BY_SOURCE_AND_STATUS,
|
||||
{ lower: ['remote', 'failed'], upper: ['remote', 'failed'] },
|
||||
),
|
||||
]);
|
||||
storedEntries = [...archivePendingEntries, ...failedEntries];
|
||||
} catch (e) {
|
||||
// Fallback for databases created before version 3 index migration
|
||||
Log.warn(
|
||||
|
|
@ -1121,7 +1151,10 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort<O
|
|||
);
|
||||
const allOps = await this._adapter.getAll<StoredOperationLogEntry>(STORE_NAMES.OPS);
|
||||
storedEntries = allOps.filter(
|
||||
(entry) => entry.source === 'remote' && entry.applicationStatus === 'failed',
|
||||
(entry) =>
|
||||
entry.source === 'remote' &&
|
||||
(entry.applicationStatus === 'archive_pending' ||
|
||||
entry.applicationStatus === 'failed'),
|
||||
);
|
||||
}
|
||||
// Decode and filter out rejected ops
|
||||
|
|
@ -1504,6 +1537,83 @@ export class OperationLogStoreService implements RemoteOperationApplyStorePort<O
|
|||
this._invalidateAppliedAndUnsyncedCaches();
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomically prepares the local persistence baseline for an authoritative
|
||||
* remote rebuild. The remote operations are replayed after this transaction,
|
||||
* but every committed intermediate state is self-consistent: an empty op-log,
|
||||
* the supplied baseline snapshot, its vector clock, and authoritative archive
|
||||
* contents all become visible together.
|
||||
*
|
||||
* If replay is interrupted, startup hydrates this baseline and the next sync
|
||||
* resumes from server cursor 0. It can never combine a cleared op-log with the
|
||||
* stale pre-replacement state cache or archives.
|
||||
*/
|
||||
async runRemoteStateReplacement(opts: {
|
||||
baselineState: unknown;
|
||||
vectorClock: VectorClock;
|
||||
schemaVersion: number;
|
||||
snapshotEntityKeys: string[];
|
||||
archiveYoung: ArchiveStoreEntry['data'];
|
||||
archiveOld: ArchiveStoreEntry['data'];
|
||||
}): Promise<void> {
|
||||
await this._ensureInit();
|
||||
|
||||
const now = Date.now();
|
||||
try {
|
||||
await this._adapter.transaction(
|
||||
[
|
||||
STORE_NAMES.OPS,
|
||||
STORE_NAMES.META,
|
||||
STORE_NAMES.STATE_CACHE,
|
||||
STORE_NAMES.VECTOR_CLOCK,
|
||||
STORE_NAMES.ARCHIVE_YOUNG,
|
||||
STORE_NAMES.ARCHIVE_OLD,
|
||||
],
|
||||
'readwrite',
|
||||
async (tx) => {
|
||||
await tx.clear(STORE_NAMES.OPS);
|
||||
await tx.put(
|
||||
STORE_NAMES.META,
|
||||
buildFullStateOpsMeta([]),
|
||||
FULL_STATE_OPS_META_KEY,
|
||||
);
|
||||
await tx.put(STORE_NAMES.STATE_CACHE, {
|
||||
id: SINGLETON_KEY,
|
||||
state: opts.baselineState,
|
||||
lastAppliedOpSeq: 0,
|
||||
vectorClock: opts.vectorClock,
|
||||
compactedAt: now,
|
||||
schemaVersion: opts.schemaVersion,
|
||||
snapshotEntityKeys: opts.snapshotEntityKeys,
|
||||
});
|
||||
await tx.put(
|
||||
STORE_NAMES.VECTOR_CLOCK,
|
||||
{ clock: opts.vectorClock, lastUpdate: now },
|
||||
SINGLETON_KEY,
|
||||
);
|
||||
await tx.put(STORE_NAMES.ARCHIVE_YOUNG, {
|
||||
id: SINGLETON_KEY,
|
||||
data: opts.archiveYoung,
|
||||
lastModified: now,
|
||||
});
|
||||
await tx.put(STORE_NAMES.ARCHIVE_OLD, {
|
||||
id: SINGLETON_KEY,
|
||||
data: opts.archiveOld,
|
||||
lastModified: now,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
this._invalidateAppliedAndUnsyncedCaches();
|
||||
this._vectorClockCache = { ...opts.vectorClock };
|
||||
} catch (e) {
|
||||
if (e instanceof DOMException && e.name === 'QuotaExceededError') {
|
||||
throw new StorageQuotaExceededError();
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// Vector Clock Management (Performance Optimization)
|
||||
// ============================================================
|
||||
|
|
|
|||
|
|
@ -147,6 +147,13 @@ describe('SchemaMigrationService', () => {
|
|||
// Should return the operation (with undefined treated as version 1)
|
||||
expect(result).not.toBeNull();
|
||||
});
|
||||
|
||||
it('should reject a present non-integer schema version at the migration boundary', () => {
|
||||
const op = createMockOperation('malformed-op');
|
||||
op.schemaVersion = '2' as unknown as number;
|
||||
|
||||
expect(() => service.migrateOperation(op)).toThrowError(/schemaVersion/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('migrateOperations', () => {
|
||||
|
|
@ -181,6 +188,33 @@ describe('SchemaMigrationService', () => {
|
|||
|
||||
expect(result.map((op) => op.id)).toEqual(['op-a', 'op-b', 'op-c']);
|
||||
});
|
||||
|
||||
it('should preserve unique migrated identities when a v1 config operation splits', () => {
|
||||
const op = createMockOperation('config-op', 1);
|
||||
op.actionType = ActionType.GLOBAL_CONFIG_UPDATE_SECTION;
|
||||
op.entityType = 'GLOBAL_CONFIG';
|
||||
op.entityId = 'misc';
|
||||
op.entityIds = ['misc'];
|
||||
op.payload = {
|
||||
actionPayload: {
|
||||
sectionKey: 'misc',
|
||||
sectionCfg: {
|
||||
isConfirmBeforeTaskDelete: true,
|
||||
unrelatedMiscSetting: 'keep-me',
|
||||
},
|
||||
},
|
||||
entityChanges: [],
|
||||
};
|
||||
|
||||
const result = service.migrateOperations([op]);
|
||||
|
||||
expect(result.map((migrated) => migrated.id)).toEqual([
|
||||
'config-op_misc',
|
||||
'config-op_tasks',
|
||||
]);
|
||||
expect(result.map((migrated) => migrated.entityId)).toEqual(['misc', 'tasks']);
|
||||
expect(result.map((migrated) => migrated.entityIds)).toEqual([['misc'], ['tasks']]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('migrateIfNeeded (deprecated)', () => {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,22 @@ export const MIN_SUPPORTED_SCHEMA_VERSION = SHARED_MIN_SUPPORTED_SCHEMA_VERSION;
|
|||
// Re-export types
|
||||
export type { SchemaMigration };
|
||||
|
||||
export const getOperationSchemaVersion = (op: { schemaVersion?: unknown }): number => {
|
||||
if (op.schemaVersion === undefined) {
|
||||
return 1;
|
||||
}
|
||||
if (
|
||||
typeof op.schemaVersion !== 'number' ||
|
||||
!Number.isInteger(op.schemaVersion) ||
|
||||
op.schemaVersion < 0
|
||||
) {
|
||||
throw new Error(
|
||||
'Operation schemaVersion must be a non-negative integer when present.',
|
||||
);
|
||||
}
|
||||
return op.schemaVersion;
|
||||
};
|
||||
|
||||
/**
|
||||
* Interface for state cache that may need migration.
|
||||
*/
|
||||
|
|
@ -133,7 +149,7 @@ export class SchemaMigrationService {
|
|||
* @returns The migrated operation(s), or null if it should be dropped
|
||||
*/
|
||||
migrateOperation(op: Operation): Operation | Operation[] | null {
|
||||
const opVersion = op.schemaVersion ?? 1;
|
||||
const opVersion = getOperationSchemaVersion(op);
|
||||
|
||||
if (opVersion >= CURRENT_SCHEMA_VERSION) {
|
||||
return op;
|
||||
|
|
@ -164,6 +180,7 @@ export class SchemaMigrationService {
|
|||
if (Array.isArray(result.data)) {
|
||||
return result.data.map((migratedOpLike) => ({
|
||||
...op,
|
||||
id: migratedOpLike.id,
|
||||
opType: migratedOpLike.opType as Operation['opType'],
|
||||
entityType: migratedOpLike.entityType as Operation['entityType'],
|
||||
entityId: migratedOpLike.entityId,
|
||||
|
|
@ -176,9 +193,11 @@ export class SchemaMigrationService {
|
|||
// Merge migrated fields back into the original operation
|
||||
return {
|
||||
...op,
|
||||
id: result.data.id,
|
||||
opType: result.data.opType as Operation['opType'],
|
||||
entityType: result.data.entityType as Operation['entityType'],
|
||||
entityId: result.data.entityId,
|
||||
entityIds: result.data.entityIds,
|
||||
payload: result.data.payload,
|
||||
schemaVersion: result.data.schemaVersion,
|
||||
};
|
||||
|
|
@ -225,13 +244,14 @@ export class SchemaMigrationService {
|
|||
* Returns true if the operation needs migration.
|
||||
*/
|
||||
operationNeedsMigration(op: Operation): boolean {
|
||||
const schemaVersion = getOperationSchemaVersion(op);
|
||||
return sharedOperationNeedsMigration(
|
||||
{
|
||||
id: op.id,
|
||||
opType: op.opType,
|
||||
entityType: op.entityType,
|
||||
payload: op.payload,
|
||||
schemaVersion: op.schemaVersion ?? 1,
|
||||
schemaVersion,
|
||||
},
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ describe('ConflictResolutionService', () => {
|
|||
'append',
|
||||
'appendBatchSkipDuplicates',
|
||||
'appendWithVectorClockUpdate',
|
||||
'markArchivePending',
|
||||
'markApplied',
|
||||
'markRejected',
|
||||
'markFailed',
|
||||
|
|
@ -1787,8 +1788,9 @@ describe('ConflictResolutionService', () => {
|
|||
|
||||
mockOpLogStore.hasOp.and.resolveTo(false);
|
||||
mockOpLogStore.append.and.resolveTo(1);
|
||||
mockOperationApplier.applyOperations.and.resolveTo({
|
||||
appliedOps: [remoteOp],
|
||||
mockOperationApplier.applyOperations.and.callFake(async (ops, options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return { appliedOps: [remoteOp] };
|
||||
});
|
||||
mockOpLogStore.markApplied.and.resolveTo(undefined);
|
||||
mockOpLogStore.markRejected.and.resolveTo(undefined);
|
||||
|
|
@ -1812,10 +1814,14 @@ describe('ConflictResolutionService', () => {
|
|||
const callOrder: string[] = [];
|
||||
|
||||
mockOpLogStore.hasOp.and.resolveTo(false);
|
||||
mockOperationApplier.applyOperations.and.callFake(async () => {
|
||||
mockOperationApplier.applyOperations.and.callFake(async (ops, options) => {
|
||||
callOrder.push('applyOperations');
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return { appliedOps: [remoteOp] };
|
||||
});
|
||||
mockOpLogStore.markArchivePending.and.callFake(async () => {
|
||||
callOrder.push('markArchivePending');
|
||||
});
|
||||
mockOpLogStore.markApplied.and.callFake(async () => {
|
||||
callOrder.push('markApplied');
|
||||
});
|
||||
|
|
@ -1831,16 +1837,21 @@ describe('ConflictResolutionService', () => {
|
|||
callerHoldsOperationLogLock: true,
|
||||
});
|
||||
|
||||
expect(mockOperationApplier.applyOperations).toHaveBeenCalledWith([remoteOp], {
|
||||
skipDeferredLocalActions: true,
|
||||
});
|
||||
expect(mockOperationApplier.applyOperations).toHaveBeenCalledWith(
|
||||
[remoteOp],
|
||||
jasmine.objectContaining({
|
||||
skipDeferredLocalActions: true,
|
||||
onReducersCommitted: jasmine.any(Function),
|
||||
}),
|
||||
);
|
||||
expect(mockOperationLogEffects.processDeferredActions).toHaveBeenCalledWith({
|
||||
callerHoldsOperationLogLock: true,
|
||||
});
|
||||
expect(callOrder).toEqual([
|
||||
'applyOperations',
|
||||
'markApplied',
|
||||
'markArchivePending',
|
||||
'mergeRemoteOpClocks',
|
||||
'markApplied',
|
||||
'processDeferredActions',
|
||||
]);
|
||||
});
|
||||
|
|
@ -1865,8 +1876,9 @@ describe('ConflictResolutionService', () => {
|
|||
|
||||
mockOpLogStore.hasOp.and.resolveTo(false);
|
||||
mockOpLogStore.append.and.resolveTo(1);
|
||||
mockOperationApplier.applyOperations.and.resolveTo({
|
||||
appliedOps: [conflictRemoteOp, nonConflictingOp],
|
||||
mockOperationApplier.applyOperations.and.callFake(async (ops, options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return { appliedOps: [conflictRemoteOp, nonConflictingOp] };
|
||||
});
|
||||
mockOpLogStore.markApplied.and.resolveTo(undefined);
|
||||
mockOpLogStore.markRejected.and.resolveTo(undefined);
|
||||
|
|
|
|||
|
|
@ -35,7 +35,6 @@ import {
|
|||
} from '../core/operation.types';
|
||||
import { toLwwUpdateActionType } from '../core/lww-update-action-types';
|
||||
import { OperationApplierService } from '../apply/operation-applier.service';
|
||||
import { getFailedOpIdsFromBatch } from '../apply/failed-op-ids.util';
|
||||
import { OperationLogStoreService } from '../persistence/operation-log-store.service';
|
||||
import { OpLog } from '../../core/log';
|
||||
import { toEntityKey } from '../util/entity-key.util';
|
||||
|
|
@ -430,6 +429,18 @@ export class ConflictResolutionService {
|
|||
const opIdToSeq = new Map(allStoredOps.map((o) => [o.id, o.seq]));
|
||||
const applyResult = await this.operationApplier.applyOperations(allOpsToApply, {
|
||||
skipDeferredLocalActions: true,
|
||||
onReducersCommitted: async (reducerCommittedOps) => {
|
||||
const reducerCommittedSeqs = reducerCommittedOps
|
||||
.map((op) => opIdToSeq.get(op.id))
|
||||
.filter((seq): seq is number => seq !== undefined);
|
||||
if (reducerCommittedSeqs.length !== reducerCommittedOps.length) {
|
||||
throw new Error(
|
||||
'ConflictResolutionService: reducer commit contained an unknown operation.',
|
||||
);
|
||||
}
|
||||
await this.opLogStore.markArchivePending(reducerCommittedSeqs);
|
||||
await this.opLogStore.mergeRemoteOpClocks(reducerCommittedOps);
|
||||
},
|
||||
});
|
||||
|
||||
const appliedSeqs = applyResult.appliedOps
|
||||
|
|
@ -439,28 +450,17 @@ export class ConflictResolutionService {
|
|||
if (appliedSeqs.length > 0) {
|
||||
await this.opLogStore.markApplied(appliedSeqs);
|
||||
|
||||
// CRITICAL: Merge remote ops' vector clocks into local clock.
|
||||
// This ensures subsequent local operations have clocks that "dominate"
|
||||
// the applied remote ops (GREATER_THAN instead of CONCURRENT).
|
||||
// Without this, ops created after conflict resolution would have clocks
|
||||
// that are CONCURRENT with the applied ops, causing them to be incorrectly
|
||||
// filtered by SyncImportFilterService or rejected as conflicts on next sync.
|
||||
await this.opLogStore.mergeRemoteOpClocks(applyResult.appliedOps);
|
||||
|
||||
OpLog.normal(
|
||||
`ConflictResolutionService: Successfully applied ${appliedSeqs.length} ops`,
|
||||
);
|
||||
}
|
||||
|
||||
if (applyResult.failedOp) {
|
||||
const failedOpIds = getFailedOpIdsFromBatch(
|
||||
allOpsToApply,
|
||||
applyResult.failedOp.op,
|
||||
);
|
||||
const failedOpIds = [applyResult.failedOp.op.id];
|
||||
|
||||
OpLog.err(
|
||||
`ConflictResolutionService: ${applyResult.appliedOps.length} ops applied before failure. ` +
|
||||
`Marking ${failedOpIds.length} ops as failed.`,
|
||||
'Marking the attempted archive operation as failed.',
|
||||
applyResult.failedOp.error,
|
||||
);
|
||||
await this.opLogStore.markFailed(failedOpIds, MAX_CONFLICT_RETRY_ATTEMPTS);
|
||||
|
|
|
|||
|
|
@ -179,6 +179,19 @@ describe('ImmediateUploadService', () => {
|
|||
expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should report ERROR when piggyback processing is blocked by an incompatible op', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.resolveTo({
|
||||
kind: 'blocked_incompatible',
|
||||
});
|
||||
|
||||
service.initialize();
|
||||
service.trigger();
|
||||
tick(2000);
|
||||
flush();
|
||||
|
||||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
|
||||
}));
|
||||
|
||||
it('should NOT show checkmark when piggybacked ops exist (multiple)', fakeAsync(() => {
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve(completedResult({ uploadedCount: 5, piggybackedOpsCount: 3 })),
|
||||
|
|
|
|||
|
|
@ -237,6 +237,14 @@ export class ImmediateUploadService implements OnDestroy {
|
|||
return;
|
||||
}
|
||||
|
||||
if (result.kind === 'blocked_incompatible') {
|
||||
OpLog.warn(
|
||||
'ImmediateUploadService: Piggyback processing blocked by an incompatible operation',
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return;
|
||||
}
|
||||
|
||||
// result.kind === 'completed' from here
|
||||
|
||||
// If LWW local-wins created new update ops from piggybacked ops,
|
||||
|
|
|
|||
|
|
@ -718,6 +718,51 @@ describe('OperationLogDownloadService', () => {
|
|||
expect(mockApiProvider.downloadOps).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should reject an empty page that claims more data', async () => {
|
||||
mockApiProvider.downloadOps.and.resolveTo({
|
||||
ops: [],
|
||||
hasMore: true,
|
||||
latestSeq: 10,
|
||||
});
|
||||
|
||||
const result = await service.downloadRemoteOps(mockApiProvider);
|
||||
|
||||
expect(result.success).toBeFalse();
|
||||
expect(result.newOps).toEqual([]);
|
||||
expect(mockSuperSyncStatusService.markRemoteChecked).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject a page that does not advance its cursor while claiming more data', async () => {
|
||||
mockApiProvider.getLastServerSeq.and.resolveTo(5);
|
||||
mockApiProvider.downloadOps.and.resolveTo({
|
||||
ops: [
|
||||
{
|
||||
serverSeq: 5,
|
||||
receivedAt: Date.now(),
|
||||
op: {
|
||||
id: 'op-stuck',
|
||||
clientId: 'c1',
|
||||
actionType: '[Task] Update' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
payload: {},
|
||||
vectorClock: {},
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
},
|
||||
],
|
||||
hasMore: true,
|
||||
latestSeq: 10,
|
||||
});
|
||||
|
||||
const result = await service.downloadRemoteOps(mockApiProvider);
|
||||
|
||||
expect(result.success).toBeFalse();
|
||||
expect(result.newOps).toEqual([]);
|
||||
expect(mockApiProvider.downloadOps).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should filter already applied operations', async () => {
|
||||
mockOpLogStore.getAppliedOpIds.and.returnValue(
|
||||
Promise.resolve(new Set(['op-1'])),
|
||||
|
|
|
|||
|
|
@ -229,6 +229,12 @@ export class OperationLogDownloadService implements OnDestroy {
|
|||
}
|
||||
|
||||
if (response.ops.length === 0) {
|
||||
if (response.hasMore) {
|
||||
OpLog.error(
|
||||
'OperationLogDownloadService: Server returned an empty page with hasMore=true. Aborting to avoid accepting a partial download.',
|
||||
);
|
||||
downloadFailed = true;
|
||||
}
|
||||
// No ops to download - caller will persist latestServerSeq after this method returns
|
||||
break;
|
||||
}
|
||||
|
|
@ -316,8 +322,18 @@ export class OperationLogDownloadService implements OnDestroy {
|
|||
break;
|
||||
}
|
||||
|
||||
// Update cursors
|
||||
sinceSeq = response.ops[response.ops.length - 1].serverSeq;
|
||||
// Update cursors. A page that claims more data must advance the cursor;
|
||||
// otherwise accepting the accumulated prefix would silently skip the
|
||||
// unseen suffix (or spin until the iteration cap).
|
||||
const nextSinceSeq = response.ops[response.ops.length - 1].serverSeq;
|
||||
if (response.hasMore && nextSinceSeq <= sinceSeq) {
|
||||
OpLog.error(
|
||||
`OperationLogDownloadService: Non-progressing page cursor (${nextSinceSeq} <= ${sinceSeq}) with hasMore=true. Aborting partial download.`,
|
||||
);
|
||||
downloadFailed = true;
|
||||
break;
|
||||
}
|
||||
sinceSeq = nextSinceSeq;
|
||||
hasMore = response.hasMore;
|
||||
|
||||
// Monotonicity check: warn if server seq decreased (indicates potential server bug)
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import { BackupService } from '../backup/backup.service';
|
|||
import { T } from '../../t.const';
|
||||
import { INBOX_PROJECT } from '../../features/project/project.const';
|
||||
import { TODAY_TAG, SYSTEM_TAG_IDS } from '../../features/tag/tag.const';
|
||||
import { OperationSyncCapable } from '../sync-providers/provider.interface';
|
||||
|
||||
describe('OperationLogSyncService', () => {
|
||||
let service: OperationLogSyncService;
|
||||
|
|
@ -50,6 +51,9 @@ describe('OperationLogSyncService', () => {
|
|||
let stateSnapshotServiceSpy: jasmine.SpyObj<StateSnapshotService>;
|
||||
let backupServiceSpy: jasmine.SpyObj<BackupService>;
|
||||
let syncImportConflictDialogServiceSpy: jasmine.SpyObj<SyncImportConflictDialogService>;
|
||||
let schemaMigrationServiceSpy: jasmine.SpyObj<SchemaMigrationService>;
|
||||
let validateStateServiceSpy: jasmine.SpyObj<ValidateStateService>;
|
||||
let lockServiceSpy: jasmine.SpyObj<LockService>;
|
||||
|
||||
beforeEach(() => {
|
||||
snackServiceSpy = jasmine.createSpyObj('SnackService', ['open']);
|
||||
|
|
@ -58,14 +62,17 @@ describe('OperationLogSyncService', () => {
|
|||
'loadStateCache',
|
||||
'getLastSeq',
|
||||
'getOpById',
|
||||
'markSynced',
|
||||
'markRejected',
|
||||
'setVectorClock',
|
||||
'clearFullStateOps',
|
||||
'getVectorClock',
|
||||
'appendBatchSkipDuplicates',
|
||||
'hasSyncedOps',
|
||||
'runRemoteStateReplacement',
|
||||
]);
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.markSynced.and.resolveTo();
|
||||
opLogStoreSpy.setVectorClock.and.resolveTo();
|
||||
opLogStoreSpy.clearFullStateOps.and.resolveTo();
|
||||
opLogStoreSpy.getVectorClock.and.resolveTo(null);
|
||||
|
|
@ -74,6 +81,26 @@ describe('OperationLogSyncService', () => {
|
|||
writtenOps: [],
|
||||
skippedCount: 0,
|
||||
});
|
||||
opLogStoreSpy.runRemoteStateReplacement.and.resolveTo();
|
||||
|
||||
schemaMigrationServiceSpy = jasmine.createSpyObj('SchemaMigrationService', [
|
||||
'getCurrentVersion',
|
||||
'migrateOperation',
|
||||
'migrateOperations',
|
||||
]);
|
||||
schemaMigrationServiceSpy.migrateOperations.and.callFake((ops) => ops);
|
||||
|
||||
validateStateServiceSpy = jasmine.createSpyObj('ValidateStateService', [
|
||||
'validateAndRepair',
|
||||
'validateAndRepairCurrentState',
|
||||
]);
|
||||
validateStateServiceSpy.validateAndRepair.and.resolveTo({
|
||||
isValid: true,
|
||||
wasRepaired: false,
|
||||
});
|
||||
|
||||
lockServiceSpy = jasmine.createSpyObj('LockService', ['request']);
|
||||
lockServiceSpy.request.and.callFake(async (_name, callback) => callback());
|
||||
serverMigrationServiceSpy = jasmine.createSpyObj('ServerMigrationService', [
|
||||
'checkAndHandleMigration',
|
||||
'handleServerMigration',
|
||||
|
|
@ -137,13 +164,7 @@ describe('OperationLogSyncService', () => {
|
|||
providers: [
|
||||
OperationLogSyncService,
|
||||
provideMockStore(),
|
||||
{
|
||||
provide: SchemaMigrationService,
|
||||
useValue: jasmine.createSpyObj('SchemaMigrationService', [
|
||||
'getCurrentVersion',
|
||||
'migrateOperation',
|
||||
]),
|
||||
},
|
||||
{ provide: SchemaMigrationService, useValue: schemaMigrationServiceSpy },
|
||||
{ provide: SnackService, useValue: snackServiceSpy },
|
||||
{ provide: OperationLogStoreService, useValue: opLogStoreSpy },
|
||||
{
|
||||
|
|
@ -166,13 +187,7 @@ describe('OperationLogSyncService', () => {
|
|||
'checkOpForConflicts',
|
||||
]),
|
||||
},
|
||||
{
|
||||
provide: ValidateStateService,
|
||||
useValue: jasmine.createSpyObj('ValidateStateService', [
|
||||
'validateAndRepair',
|
||||
'validateAndRepairCurrentState',
|
||||
]),
|
||||
},
|
||||
{ provide: ValidateStateService, useValue: validateStateServiceSpy },
|
||||
{
|
||||
provide: RepairOperationService,
|
||||
useValue: jasmine.createSpyObj('RepairOperationService', [
|
||||
|
|
@ -191,10 +206,7 @@ describe('OperationLogSyncService', () => {
|
|||
'downloadRemoteOps',
|
||||
]),
|
||||
},
|
||||
{
|
||||
provide: LockService,
|
||||
useValue: jasmine.createSpyObj('LockService', ['request']),
|
||||
},
|
||||
{ provide: LockService, useValue: lockServiceSpy },
|
||||
{
|
||||
provide: OperationLogCompactionService,
|
||||
useValue: jasmine.createSpyObj('OperationLogCompactionService', ['compact']),
|
||||
|
|
@ -747,6 +759,48 @@ describe('OperationLogSyncService', () => {
|
|||
expect(rejectedOpsHandlerServiceSpy.handleRejectedOps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return a terminal outcome and keep acknowledgements pending when piggyback processing is incompatible', async () => {
|
||||
const piggybackedOp = {
|
||||
id: 'future-op',
|
||||
clientId: 'client-B',
|
||||
actionType: 'test' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK' as const,
|
||||
entityId: 'task-1',
|
||||
payload: {},
|
||||
vectorClock: { clientB: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 99,
|
||||
};
|
||||
uploadServiceSpy.uploadPendingOps.and.resolveTo({
|
||||
uploadedCount: 1,
|
||||
piggybackedOps: [piggybackedOp],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
pendingAcknowledgementSeqs: [1],
|
||||
lastServerSeqToPersist: 9,
|
||||
});
|
||||
remoteOpsProcessingServiceSpy.processRemoteOps.and.resolveTo({
|
||||
localWinOpsCreated: 0,
|
||||
allOpsFilteredBySyncImport: false,
|
||||
filteredOpCount: 0,
|
||||
isLocalUnsyncedImport: false,
|
||||
blockedByIncompatibleOp: true,
|
||||
});
|
||||
const setLastServerSeq = jasmine.createSpy('setLastServerSeq').and.resolveTo();
|
||||
const provider = {
|
||||
...mockProvider,
|
||||
setLastServerSeq,
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
const result = await service.uploadPendingOps(provider);
|
||||
|
||||
expect(result.kind).toBe('blocked_incompatible');
|
||||
expect(opLogStoreSpy.markSynced).not.toHaveBeenCalled();
|
||||
expect(setLastServerSeq).not.toHaveBeenCalled();
|
||||
expect(rejectedOpsHandlerServiceSpy.handleRejectedOps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not call handleRejectedOps when there are no rejected ops', async () => {
|
||||
uploadServiceSpy.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
|
|
@ -942,7 +996,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
// Cursor stays behind the blocked op so it is re-downloaded and retried
|
||||
// after an app update instead of skipped forever.
|
||||
expect(result.kind).toBe('ops_processed');
|
||||
expect(result.kind).toBe('blocked_incompatible');
|
||||
expect(setLastServerSeqSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -1190,10 +1244,7 @@ describe('OperationLogSyncService', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('should NOT throw LocalDataConflictError for clients with only system/config ops (no user data)', async () => {
|
||||
// Clients with only system/config ops (no tasks/projects/tags) should NOT see conflict dialog.
|
||||
// They should just download the remote data.
|
||||
|
||||
it('should protect unsynced user config from file-snapshot replacement', async () => {
|
||||
const unsyncedEntry: OperationLogEntry = {
|
||||
seq: 1,
|
||||
op: {
|
||||
|
|
@ -1239,9 +1290,10 @@ describe('OperationLogSyncService', () => {
|
|||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
|
||||
// Should NOT throw - fresh client should proceed with download
|
||||
await expectAsync(service.downloadRemoteOps(mockProvider)).toBeResolved();
|
||||
expect(syncHydrationServiceSpy.hydrateFromRemoteSync).toHaveBeenCalled();
|
||||
await expectAsync(
|
||||
service.downloadRemoteOps(mockProvider),
|
||||
).toBeRejectedWithError(LocalDataConflictError);
|
||||
expect(syncHydrationServiceSpy.hydrateFromRemoteSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw LocalDataConflictError when only config ops but store has meaningful data (provider switch)', async () => {
|
||||
|
|
@ -2289,7 +2341,7 @@ describe('OperationLogSyncService', () => {
|
|||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await service.forceUploadLocalState(mockProvider);
|
||||
|
||||
|
|
@ -2318,7 +2370,7 @@ describe('OperationLogSyncService', () => {
|
|||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await service.forceUploadLocalState(mockProvider);
|
||||
|
||||
|
|
@ -2331,7 +2383,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
} as any;
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await expectAsync(service.forceUploadLocalState(mockProvider)).toBeRejectedWith(
|
||||
error,
|
||||
|
|
@ -2345,7 +2397,7 @@ describe('OperationLogSyncService', () => {
|
|||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await expectAsync(service.forceUploadLocalState(mockProvider)).toBeRejectedWith(
|
||||
error,
|
||||
|
|
@ -2391,16 +2443,13 @@ describe('OperationLogSyncService', () => {
|
|||
OperationLogDownloadService,
|
||||
) as jasmine.SpyObj<OperationLogDownloadService>;
|
||||
|
||||
opLogStoreSpy.clearAllOperations = jasmine
|
||||
.createSpy('clearAllOperations')
|
||||
.and.resolveTo();
|
||||
opLogStoreSpy.saveStateCache = jasmine.createSpy('saveStateCache').and.resolveTo();
|
||||
opLogStoreSpy.runRemoteStateReplacement.calls.reset();
|
||||
});
|
||||
|
||||
it('should download BEFORE any destructive local mutation', async () => {
|
||||
const callOrder: string[] = [];
|
||||
opLogStoreSpy.clearAllOperations.and.callFake(async () => {
|
||||
callOrder.push('clearAllOperations');
|
||||
opLogStoreSpy.runRemoteStateReplacement.and.callFake(async () => {
|
||||
callOrder.push('runRemoteStateReplacement');
|
||||
});
|
||||
downloadServiceSpy.downloadRemoteOps.and.callFake(async () => {
|
||||
callOrder.push('downloadRemoteOps');
|
||||
|
|
@ -2421,25 +2470,31 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
await service.forceDownloadRemoteState(mockProvider);
|
||||
|
||||
expect(callOrder).toEqual(['downloadRemoteOps', 'clearAllOperations']);
|
||||
expect(callOrder).toEqual(['downloadRemoteOps', 'runRemoteStateReplacement']);
|
||||
});
|
||||
|
||||
it('should capture a safety backup BEFORE clearing local data (#8107)', async () => {
|
||||
it('should capture a safety backup after download but before replacement (#8107)', async () => {
|
||||
const callOrder: string[] = [];
|
||||
downloadServiceSpy.downloadRemoteOps.and.callFake(async () => {
|
||||
callOrder.push('downloadRemoteOps');
|
||||
return {
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
};
|
||||
});
|
||||
backupServiceSpy.captureImportBackup.and.callFake(async () => {
|
||||
callOrder.push('captureImportBackup');
|
||||
return 1;
|
||||
});
|
||||
opLogStoreSpy.clearAllOperations.and.callFake(async () => {
|
||||
callOrder.push('clearAllOperations');
|
||||
writeFlushServiceSpy.flushPendingWrites.and.callFake(async () => {
|
||||
callOrder.push('flushPendingWrites');
|
||||
});
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
opLogStoreSpy.runRemoteStateReplacement.and.callFake(async () => {
|
||||
callOrder.push('runRemoteStateReplacement');
|
||||
});
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
|
|
@ -2448,10 +2503,23 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
await service.forceDownloadRemoteState(mockProvider);
|
||||
|
||||
expect(callOrder).toEqual(['captureImportBackup', 'clearAllOperations']);
|
||||
expect(callOrder).toEqual([
|
||||
'downloadRemoteOps',
|
||||
'flushPendingWrites',
|
||||
'captureImportBackup',
|
||||
'runRemoteStateReplacement',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should ABORT without wiping local data if the safety backup fails (#8107)', async () => {
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
});
|
||||
backupServiceSpy.captureImportBackup.and.rejectWith(new Error('disk full'));
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
|
|
@ -2460,8 +2528,9 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
await expectAsync(service.forceDownloadRemoteState(mockProvider)).toBeRejected();
|
||||
|
||||
expect(opLogStoreSpy.clearAllOperations).not.toHaveBeenCalled();
|
||||
expect(downloadServiceSpy.downloadRemoteOps).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
expect(downloadServiceSpy.downloadRemoteOps).toHaveBeenCalled();
|
||||
expect(backupServiceSpy.captureImportBackup).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should offer to restore the previous data after replacing (#8107)', async () => {
|
||||
|
|
@ -2510,6 +2579,33 @@ describe('OperationLogSyncService', () => {
|
|||
expect(setLastServerSeqSpy).toHaveBeenCalledWith(0);
|
||||
});
|
||||
|
||||
it('should reset the external cursor before committing the local replacement', async () => {
|
||||
const callOrder: string[] = [];
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
});
|
||||
opLogStoreSpy.runRemoteStateReplacement.and.callFake(async () => {
|
||||
callOrder.push('replace');
|
||||
});
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine
|
||||
.createSpy('setLastServerSeq')
|
||||
.and.callFake(async (seq: number) => {
|
||||
if (seq === 0) callOrder.push('cursor-zero');
|
||||
}),
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await service.forceDownloadRemoteState(mockProvider);
|
||||
|
||||
expect(callOrder.slice(0, 2)).toEqual(['cursor-zero', 'replace']);
|
||||
});
|
||||
|
||||
it('should download raw history: forceFromSeq0 AND includeOwnAndAppliedOps', async () => {
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
|
|
@ -2551,7 +2647,7 @@ describe('OperationLogSyncService', () => {
|
|||
service.forceDownloadRemoteState(mockProvider),
|
||||
).toBeRejectedWithError(/Download failed/);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.clearAllOperations).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
expect(setLastServerSeqSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -2573,10 +2669,95 @@ describe('OperationLogSyncService', () => {
|
|||
await expectAsync(
|
||||
service.forceDownloadRemoteState(mockProvider),
|
||||
).toBeRejectedWithError(/newer schema version/);
|
||||
expect(opLogStoreSpy.clearAllOperations).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should run all operation migrations before backup or replacement', async () => {
|
||||
const remoteOp = { ...makeRemoteOp(), schemaVersion: 1 };
|
||||
const migratedOp = { ...remoteOp, schemaVersion: 4 };
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [remoteOp],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
});
|
||||
schemaMigrationServiceSpy.migrateOperations.and.returnValue([migratedOp]);
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await service.forceDownloadRemoteState(mockProvider);
|
||||
|
||||
expect(schemaMigrationServiceSpy.migrateOperations).toHaveBeenCalledOnceWith([
|
||||
remoteOp,
|
||||
]);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith(
|
||||
[migratedOp],
|
||||
{
|
||||
skipConflictDetection: true,
|
||||
callerHoldsOperationLogLock: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should abort before backup and replacement when operation migration fails', async () => {
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'superSyncOps',
|
||||
failedFileCount: 0,
|
||||
latestServerSeq: 1,
|
||||
});
|
||||
schemaMigrationServiceSpy.migrateOperations.and.throwError('bad migration');
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await expectAsync(
|
||||
service.forceDownloadRemoteState(mockProvider),
|
||||
).toBeRejectedWithError(/migration failed/);
|
||||
|
||||
expect(backupServiceSpy.captureImportBackup).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should validate a file snapshot before backup and replacement', async () => {
|
||||
const snapshotState = { task: { ids: ['remote-task'] } };
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [],
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'fileSnapshotOps',
|
||||
failedFileCount: 0,
|
||||
snapshotState,
|
||||
latestServerSeq: 1,
|
||||
});
|
||||
validateStateServiceSpy.validateAndRepair.and.resolveTo({
|
||||
isValid: false,
|
||||
wasRepaired: false,
|
||||
});
|
||||
const mockProvider = {
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as unknown as OperationSyncCapable;
|
||||
|
||||
await expectAsync(
|
||||
service.forceDownloadRemoteState(mockProvider),
|
||||
).toBeRejectedWithError(/snapshot is invalid/);
|
||||
|
||||
expect(validateStateServiceSpy.validateAndRepair).toHaveBeenCalledOnceWith(
|
||||
snapshotState,
|
||||
);
|
||||
expect(backupServiceSpy.captureImportBackup).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should NOT advance the cursor past 0 when replay blocks on a migration failure', async () => {
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
|
|
@ -2641,7 +2822,10 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith(
|
||||
mockOps,
|
||||
{ skipConflictDetection: true },
|
||||
{
|
||||
skipConflictDetection: true,
|
||||
callerHoldsOperationLogLock: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -2703,7 +2887,7 @@ describe('OperationLogSyncService', () => {
|
|||
service.forceDownloadRemoteState(mockProvider),
|
||||
).toBeRejectedWithError(/no data to rebuild from/);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.clearAllOperations).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.runRemoteStateReplacement).not.toHaveBeenCalled();
|
||||
expect(setLastServerSeqSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -2753,9 +2937,9 @@ describe('OperationLogSyncService', () => {
|
|||
expect(setLastServerSeqSpy).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it('should propagate errors from clearAllOperations', async () => {
|
||||
it('should propagate errors from the atomic replacement', async () => {
|
||||
const error = new Error('Failed to clear ops');
|
||||
opLogStoreSpy.clearAllOperations.and.rejectWith(error);
|
||||
opLogStoreSpy.runRemoteStateReplacement.and.rejectWith(error);
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
newOps: [makeRemoteOp()],
|
||||
needsFullStateUpload: false,
|
||||
|
|
@ -2816,7 +3000,10 @@ describe('OperationLogSyncService', () => {
|
|||
await service.forceDownloadRemoteState(mockProvider);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith(
|
||||
mockOps,
|
||||
{ skipConflictDetection: true },
|
||||
{
|
||||
skipConflictDetection: true,
|
||||
callerHoldsOperationLogLock: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -3504,7 +3691,7 @@ describe('OperationLogSyncService', () => {
|
|||
expect(result.kind).toBe('ops_processed');
|
||||
});
|
||||
|
||||
it('should process incoming SYNC_IMPORT when pending ops are config-only', async () => {
|
||||
it('should prompt before replacing pending user config with an incoming SYNC_IMPORT', async () => {
|
||||
const incomingSyncImport = createIncomingSyncImport();
|
||||
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
|
|
@ -3542,19 +3729,14 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(
|
||||
syncImportConflictDialogServiceSpy.showConflictDialog,
|
||||
).not.toHaveBeenCalled();
|
||||
// No example-task ops pending -> nothing is rejected (empty-array guard).
|
||||
expect(syncImportConflictDialogServiceSpy.showConflictDialog).toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
incomingSyncImport,
|
||||
]);
|
||||
expect(mockProvider.setLastServerSeq).toHaveBeenCalledWith(42);
|
||||
expect(result.kind).toBe('ops_processed');
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).not.toHaveBeenCalled();
|
||||
expect(mockProvider.setLastServerSeq).not.toHaveBeenCalled();
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
|
||||
it('should process incoming SYNC_IMPORT when pending ops are only config and startup example tasks', async () => {
|
||||
it('should prompt when pending user config exists alongside startup example tasks', async () => {
|
||||
const incomingSyncImport = createIncomingSyncImport();
|
||||
|
||||
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
|
||||
|
|
@ -3619,20 +3801,11 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.downloadRemoteOps(mockProvider);
|
||||
|
||||
expect(
|
||||
syncImportConflictDialogServiceSpy.showConflictDialog,
|
||||
).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'local-example-task-op-1',
|
||||
'local-example-task-op-2',
|
||||
'local-example-task-op-3',
|
||||
'local-example-task-op-4',
|
||||
]);
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
incomingSyncImport,
|
||||
]);
|
||||
expect(mockProvider.setLastServerSeq).toHaveBeenCalledWith(42);
|
||||
expect(result.kind).toBe('ops_processed');
|
||||
expect(syncImportConflictDialogServiceSpy.showConflictDialog).toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).not.toHaveBeenCalled();
|
||||
expect(mockProvider.setLastServerSeq).not.toHaveBeenCalled();
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -3673,6 +3846,7 @@ describe('OperationLogSyncService', () => {
|
|||
piggybackedOps: [piggybackedSyncImport],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
pendingAcknowledgementSeqs: [1],
|
||||
});
|
||||
|
||||
// Client has pending ops
|
||||
|
|
@ -3709,6 +3883,7 @@ describe('OperationLogSyncService', () => {
|
|||
syncImportReason: 'SERVER_MIGRATION',
|
||||
}),
|
||||
);
|
||||
expect(opLogStoreSpy.markSynced).not.toHaveBeenCalled();
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
|
||||
|
|
@ -3725,13 +3900,6 @@ describe('OperationLogSyncService', () => {
|
|||
schemaVersion: 1,
|
||||
};
|
||||
|
||||
uploadServiceSpy.uploadPendingOps.and.resolveTo({
|
||||
uploadedCount: 1,
|
||||
piggybackedOps: [piggybackedSyncImport],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
});
|
||||
|
||||
const pendingEntry: OperationLogEntry = {
|
||||
seq: 1,
|
||||
op: {
|
||||
|
|
@ -3749,14 +3917,17 @@ describe('OperationLogSyncService', () => {
|
|||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
};
|
||||
// The op is pending BEFORE the upload but marked synced DURING it (server
|
||||
// accepted it in the same round that piggybacked the import) — a live
|
||||
// post-upload read no longer sees it.
|
||||
let getUnsyncedCalls = 0;
|
||||
opLogStoreSpy.getUnsynced.and.callFake(async () => {
|
||||
getUnsyncedCalls++;
|
||||
return getUnsyncedCalls === 1 ? [pendingEntry] : [];
|
||||
uploadServiceSpy.uploadPendingOps.and.resolveTo({
|
||||
uploadedCount: 1,
|
||||
piggybackedOps: [piggybackedSyncImport],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
selectedPendingOps: [pendingEntry],
|
||||
pendingAcknowledgementSeqs: [pendingEntry.seq],
|
||||
});
|
||||
// The accepted operation is represented by the exact in-lock upload snapshot;
|
||||
// it no longer needs to remain live-unsynced for the gate to protect it.
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([]);
|
||||
|
||||
syncImportConflictDialogServiceSpy.showConflictDialog.and.resolveTo('CANCEL');
|
||||
|
||||
|
|
@ -3771,6 +3942,7 @@ describe('OperationLogSyncService', () => {
|
|||
expect(syncImportConflictDialogServiceSpy.showConflictDialog).toHaveBeenCalledWith(
|
||||
jasmine.objectContaining({ scenario: 'INCOMING_IMPORT' }),
|
||||
);
|
||||
expect(opLogStoreSpy.markSynced).not.toHaveBeenCalled();
|
||||
expect(result.kind).toBe('cancelled');
|
||||
});
|
||||
|
||||
|
|
@ -3819,7 +3991,7 @@ describe('OperationLogSyncService', () => {
|
|||
expect(result.kind).toBe('completed');
|
||||
});
|
||||
|
||||
it('should not flush again before checking piggybacked SYNC_IMPORT conflicts', async () => {
|
||||
it('should flush again before checking piggybacked SYNC_IMPORT conflicts', async () => {
|
||||
const piggybackedSyncImport: Operation = {
|
||||
id: 'import-1',
|
||||
clientId: 'client-B',
|
||||
|
|
@ -3846,7 +4018,7 @@ describe('OperationLogSyncService', () => {
|
|||
|
||||
const result = await service.uploadPendingOps(mockProvider);
|
||||
|
||||
expect(writeFlushServiceSpy.flushPendingWrites).toHaveBeenCalledTimes(1);
|
||||
expect(writeFlushServiceSpy.flushPendingWrites).toHaveBeenCalledTimes(2);
|
||||
expect(opLogStoreSpy.getUnsynced).toHaveBeenCalled();
|
||||
expect(result.kind).toBe('completed');
|
||||
});
|
||||
|
|
@ -3898,6 +4070,7 @@ describe('OperationLogSyncService', () => {
|
|||
});
|
||||
|
||||
it('should process piggybacked ops normally when no SYNC_IMPORT present', async () => {
|
||||
const events: string[] = [];
|
||||
const piggybackedOp: Operation = {
|
||||
id: 'op-1',
|
||||
clientId: 'client-B',
|
||||
|
|
@ -3916,9 +4089,23 @@ describe('OperationLogSyncService', () => {
|
|||
piggybackedOps: [piggybackedOp],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
pendingAcknowledgementSeqs: [1],
|
||||
});
|
||||
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([]);
|
||||
remoteOpsProcessingServiceSpy.processRemoteOps.and.callFake(async () => {
|
||||
events.push('processRemoteOps');
|
||||
return {
|
||||
localWinOpsCreated: 0,
|
||||
allOpsFilteredBySyncImport: false,
|
||||
filteredOpCount: 0,
|
||||
isLocalUnsyncedImport: false,
|
||||
blockedByIncompatibleOp: false,
|
||||
};
|
||||
});
|
||||
opLogStoreSpy.markSynced.and.callFake(async () => {
|
||||
events.push('markSynced');
|
||||
});
|
||||
|
||||
const mockProvider = {
|
||||
isReady: () => Promise.resolve(true),
|
||||
|
|
@ -3934,6 +4121,8 @@ describe('OperationLogSyncService', () => {
|
|||
expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([
|
||||
piggybackedOp,
|
||||
]);
|
||||
expect(opLogStoreSpy.markSynced).toHaveBeenCalledOnceWith([1]);
|
||||
expect(events).toEqual(['processRemoteOps', 'markSynced']);
|
||||
expect(result.kind).not.toBe('cancelled');
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -38,13 +38,23 @@ import {
|
|||
SyncImportConflictResolution,
|
||||
} from './dialog-sync-import-conflict/dialog-sync-import-conflict.component';
|
||||
import { SyncImportConflictGateService } from './sync-import-conflict-gate.service';
|
||||
import { CURRENT_SCHEMA_VERSION } from '../persistence/schema-migration.service';
|
||||
import {
|
||||
CURRENT_SCHEMA_VERSION,
|
||||
MIN_SUPPORTED_SCHEMA_VERSION,
|
||||
SchemaMigrationService,
|
||||
getOperationSchemaVersion,
|
||||
} from '../persistence/schema-migration.service';
|
||||
import { SyncProviderManager } from '../sync-providers/provider-manager.service';
|
||||
import { getDefaultMainModelData } from '../model/model-config';
|
||||
import { getDefaultMainModelData, MODEL_CONFIGS } from '../model/model-config';
|
||||
import { loadAllData } from '../../root-store/meta/load-all-data.action';
|
||||
import { SyncLocalStateService } from './sync-local-state.service';
|
||||
import { SyncImportConflictCoordinatorService } from './sync-import-conflict-coordinator.service';
|
||||
import { isExampleTaskCreateOp } from '../validation/is-example-task-op.util';
|
||||
import { Operation } from '../core/operation.types';
|
||||
import { LockService } from './lock.service';
|
||||
import { LOCK_NAMES } from '../core/operation-log.const';
|
||||
import { ValidateStateService } from '../validation/validate-state.service';
|
||||
import { extractEntityKeysFromState } from '../persistence/extract-entity-keys';
|
||||
|
||||
/**
|
||||
* Orchestrates synchronization of the Operation Log with remote storage.
|
||||
|
|
@ -122,6 +132,9 @@ export class OperationLogSyncService {
|
|||
private superSyncStatusService = inject(SuperSyncStatusService);
|
||||
private serverMigrationService = inject(ServerMigrationService);
|
||||
private writeFlushService = inject(OperationWriteFlushService);
|
||||
private lockService = inject(LockService);
|
||||
private schemaMigrationService = inject(SchemaMigrationService);
|
||||
private validateStateService = inject(ValidateStateService);
|
||||
|
||||
// Extracted services
|
||||
private remoteOpsProcessingService = inject(RemoteOpsProcessingService);
|
||||
|
|
@ -189,11 +202,9 @@ export class OperationLogSyncService {
|
|||
// we would upload an incomplete set. This flush waits for all queued writes.
|
||||
await this.writeFlushService.flushPendingWrites();
|
||||
|
||||
// Capture never-synced status BEFORE the upload runs: uploadService.uploadPendingOps
|
||||
// marks accepted ops synced, which flips hasSyncedOps() and would defeat the piggyback
|
||||
// conflict gate's never-synced guard if it read live state afterwards. The orchestrator
|
||||
// passes a value captured even earlier (pre-download, since download also persists
|
||||
// synced ops); fall back to a local read for standalone upload callers.
|
||||
// Capture never-synced status before the upload runs. The orchestrator passes a
|
||||
// value captured even earlier (pre-download, since download persists synced ops);
|
||||
// fall back to a local read for standalone upload callers.
|
||||
const isNeverSyncedAtSyncStart =
|
||||
options?.isNeverSynced ?? !(await this.opLogStore.hasSyncedOps());
|
||||
|
||||
|
|
@ -209,14 +220,6 @@ export class OperationLogSyncService {
|
|||
return { kind: 'blocked_fresh_client' };
|
||||
}
|
||||
|
||||
// Capture pending ops BEFORE the upload round (the flush at the top of this
|
||||
// method already drained in-flight writes). The piggyback conflict gate below
|
||||
// must judge "would this import discard local work?" against this snapshot:
|
||||
// ops accepted by the server during the upload are marked synced per chunk,
|
||||
// so a post-upload getUnsynced() read no longer sees local work that a
|
||||
// piggybacked import is about to replace.
|
||||
const pendingOpsAtUploadStart = await this.opLogStore.getUnsynced();
|
||||
|
||||
// SERVER MIGRATION CHECK: Passed as callback to execute INSIDE the upload lock.
|
||||
// This prevents race conditions where multiple tabs could both detect migration
|
||||
// and create duplicate SYNC_IMPORT operations.
|
||||
|
|
@ -227,6 +230,9 @@ export class OperationLogSyncService {
|
|||
? undefined
|
||||
: () => this.serverMigrationService.checkAndHandleMigration(syncProvider),
|
||||
skipPiggybackProcessing: options?.skipPiggybackProcessing,
|
||||
// Keep accepted operations pending until piggyback processing commits. This
|
||||
// preserves the conflict gate across cancellation and crash/retry boundaries.
|
||||
deferAcknowledgement: true,
|
||||
});
|
||||
|
||||
// STEP 1: Process piggybacked ops FIRST
|
||||
|
|
@ -258,7 +264,8 @@ export class OperationLogSyncService {
|
|||
result.piggybackedOps,
|
||||
{
|
||||
isNeverSynced: isNeverSyncedAtSyncStart,
|
||||
preCapturedPendingOps: pendingOpsAtUploadStart,
|
||||
flushPendingWrites: true,
|
||||
preCapturedPendingOps: result.selectedPendingOps ?? [],
|
||||
},
|
||||
);
|
||||
if (piggybackedConflict.fullStateOp) {
|
||||
|
|
@ -319,6 +326,10 @@ export class OperationLogSyncService {
|
|||
localWinOpsCreated = processResult.localWinOpsCreated;
|
||||
// Validation failure (if any) is on the session-validation latch.
|
||||
|
||||
if (processResult.blockedByIncompatibleOp) {
|
||||
return { kind: 'blocked_incompatible' };
|
||||
}
|
||||
|
||||
// #8304: Persist lastServerSeq ONLY now that the piggybacked ops have been applied
|
||||
// above. The upload service deferred this (see UploadResult.lastServerSeqToPersist)
|
||||
// so that a crash — or a cancelled/USE_REMOTE/USE_LOCAL SYNC_IMPORT dialog, all of
|
||||
|
|
@ -326,14 +337,16 @@ export class OperationLogSyncService {
|
|||
// that were never stored. Mirrors the download path's invariant.
|
||||
// A version/migration block keeps the cursor behind the blocked op so it is
|
||||
// re-downloaded and retried after an app update instead of skipped forever.
|
||||
if (
|
||||
result.lastServerSeqToPersist !== undefined &&
|
||||
!processResult.blockedByIncompatibleOp
|
||||
) {
|
||||
if (result.lastServerSeqToPersist !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.lastServerSeqToPersist);
|
||||
}
|
||||
}
|
||||
|
||||
const pendingAcknowledgementSeqs = result.pendingAcknowledgementSeqs ?? [];
|
||||
if (pendingAcknowledgementSeqs.length > 0) {
|
||||
await this.opLogStore.markSynced(pendingAcknowledgementSeqs);
|
||||
}
|
||||
|
||||
// STEP 2: Handle server-rejected operations
|
||||
// handleRejectedOps may create merged ops for concurrent modifications.
|
||||
// These need to be uploaded, so we add them to localWinOpsCreated.
|
||||
|
|
@ -367,6 +380,8 @@ export class OperationLogSyncService {
|
|||
case 'server_migration_handled':
|
||||
case 'cancelled':
|
||||
return { newOpsCount: 0 };
|
||||
case 'blocked_incompatible':
|
||||
throw new Error('Nested download blocked by an incompatible remote operation.');
|
||||
}
|
||||
};
|
||||
try {
|
||||
|
|
@ -878,6 +893,10 @@ export class OperationLogSyncService {
|
|||
result.newOps,
|
||||
);
|
||||
|
||||
if (processResult.blockedByIncompatibleOp) {
|
||||
return { kind: 'blocked_incompatible' };
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────
|
||||
// Handle SYNC_IMPORT conflict: all remote ops filtered by STORED local import.
|
||||
// This happens when user imports/restores data locally, and other devices
|
||||
|
|
@ -933,7 +952,7 @@ export class OperationLogSyncService {
|
|||
// This is the correct behavior - better to re-download than to skip ops.
|
||||
// A version/migration block keeps the cursor behind the blocked op so it is
|
||||
// re-downloaded and retried after an app update instead of skipped forever.
|
||||
if (result.latestServerSeq !== undefined && !processResult.blockedByIncompatibleOp) {
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
|
|
@ -1076,10 +1095,14 @@ export class OperationLogSyncService {
|
|||
result.newOps,
|
||||
);
|
||||
|
||||
if (processResult.blockedByIncompatibleOp) {
|
||||
return { kind: 'blocked_incompatible' };
|
||||
}
|
||||
|
||||
// Persist the cursor only AFTER the ops are applied, matching the normal
|
||||
// incremental path's crash-safety ordering. A version/migration block keeps
|
||||
// the cursor behind the blocked op (retried after an app update).
|
||||
if (result.latestServerSeq !== undefined && !processResult.blockedByIncompatibleOp) {
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
|
|
@ -1156,24 +1179,6 @@ export class OperationLogSyncService {
|
|||
'OperationLogSyncService: Force downloading remote state for a full rebuild.',
|
||||
);
|
||||
|
||||
// Safety net (#8107): snapshot current local state before we destroy it, so an
|
||||
// unintended "Use Server Data" — which can roll the user back to a stale server
|
||||
// snapshot — is reversible via the Undo affordance shown on success below.
|
||||
// Fail-safe: if the backup can't be written, abort rather than wipe without a
|
||||
// recovery point (mirrors BackupService._persistImportToOperationLog).
|
||||
let backupSavedAt: number;
|
||||
try {
|
||||
backupSavedAt = await this.backupService.captureImportBackup();
|
||||
} catch (e) {
|
||||
OpLog.warn(
|
||||
'OperationLogSyncService: Pre-replace safety backup failed; aborting force download.',
|
||||
{ name: (e as Error | undefined)?.name },
|
||||
);
|
||||
throw new Error(
|
||||
'Pre-replace safety backup failed; aborting to preserve local state.',
|
||||
);
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────
|
||||
// PHASE 1 — download and validate. Nothing local is mutated until the
|
||||
// complete server history is in memory: a network failure here must leave
|
||||
|
|
@ -1207,23 +1212,19 @@ export class OperationLogSyncService {
|
|||
throw new Error('USE_REMOTE aborted: remote returned no data to rebuild from.');
|
||||
}
|
||||
|
||||
// Refuse BEFORE destroying anything if the remote history contains ops this
|
||||
// client cannot interpret (newer schema) — replay would block midway and
|
||||
// leave a partial rebuild. Migration exceptions can still surface during
|
||||
// replay; those are handled after processRemoteOps below.
|
||||
const tooNewOp = result.newOps.find(
|
||||
(op) => (op.schemaVersion ?? 1) > CURRENT_SCHEMA_VERSION,
|
||||
);
|
||||
if (tooNewOp) {
|
||||
this.snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.VERSION_TOO_OLD,
|
||||
actionStr: T.PS.UPDATE_APP,
|
||||
actionFn: () => window.open('https://super-productivity.com/download', '_blank'),
|
||||
});
|
||||
throw new Error(
|
||||
'USE_REMOTE aborted: remote history contains ops from a newer schema version — update the app first.',
|
||||
);
|
||||
const migratedRemoteOps = this._preflightRemoteOperations(result.newOps);
|
||||
|
||||
let snapshotState = result.snapshotState as Record<string, unknown> | undefined;
|
||||
if (hasSnapshotState && snapshotState) {
|
||||
const validation = await this.validateStateService.validateAndRepair(snapshotState);
|
||||
if (!validation.isValid) {
|
||||
throw new Error(
|
||||
'USE_REMOTE aborted: remote snapshot is invalid and could not be repaired.',
|
||||
);
|
||||
}
|
||||
if (validation.wasRepaired && validation.repairedState) {
|
||||
snapshotState = validation.repairedState;
|
||||
}
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────
|
||||
|
|
@ -1234,9 +1235,6 @@ export class OperationLogSyncService {
|
|||
// variant made the duplicate filter skip them, so a "rebuild" replayed only
|
||||
// an unseen suffix onto a defaults reset.
|
||||
// ─────────────────────────────────────────────────────────────────────────
|
||||
await this.opLogStore.clearAllOperations();
|
||||
await syncProvider.setLastServerSeq(0);
|
||||
|
||||
// Reset the vector clock to the remote's causal knowledge (snapshot clock
|
||||
// merged with every downloaded op clock). This also drops entries from
|
||||
// rejected local ops that would otherwise pollute conflict detection.
|
||||
|
|
@ -1244,113 +1242,202 @@ export class OperationLogSyncService {
|
|||
for (const opClock of result.allOpClocks ?? []) {
|
||||
rebuiltClock = mergeVectorClocks(rebuiltClock, opClock);
|
||||
}
|
||||
await this.opLogStore.setVectorClock(rebuiltClock);
|
||||
OpLog.normal('OperationLogSyncService: Reset vector clock to rebuilt remote clock.');
|
||||
|
||||
// FILE-BASED SYNC: Handle snapshot state from force download.
|
||||
// When downloading from seq 0 on file-based providers, we may receive a
|
||||
// snapshotState instead of incremental ops. This happens when the remote
|
||||
// has a SYNC_IMPORT (full state snapshot) with empty recentOps.
|
||||
// hydrateFromRemoteSync persists its own state cache + vector clock.
|
||||
if (result.providerMode === 'fileSnapshotOps' && result.snapshotState) {
|
||||
OpLog.normal(
|
||||
'OperationLogSyncService: Force download received snapshotState. Hydrating...',
|
||||
);
|
||||
|
||||
// Hydrate from snapshot - DON'T create SYNC_IMPORT since we're
|
||||
// accepting remote state, not uploading local state.
|
||||
await this.syncHydrationService.hydrateFromRemoteSync(
|
||||
result.snapshotState as Record<string, unknown>,
|
||||
result.snapshotVectorClock,
|
||||
false, // Don't create SYNC_IMPORT
|
||||
);
|
||||
|
||||
// CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration.
|
||||
// Same rationale as downloadRemoteOps: file-based providers return ALL
|
||||
// recentOps on every download and rely on getAppliedOpIds() to filter them.
|
||||
if (result.newOps.length > 0) {
|
||||
const appendResult = await this.opLogStore.appendBatchSkipDuplicates(
|
||||
result.newOps,
|
||||
'remote',
|
||||
);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Wrote ${appendResult.writtenOps.length} snapshot ops to IndexedDB ` +
|
||||
'after force-download hydration.' +
|
||||
(appendResult.skippedCount > 0
|
||||
? ` Skipped ${appendResult.skippedCount} duplicate(s).`
|
||||
: ''),
|
||||
);
|
||||
}
|
||||
|
||||
// Update lastServerSeq after hydration
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
'OperationLogSyncService: Force download snapshot hydration complete.',
|
||||
);
|
||||
this._showRestorePreviousDataSnack(backupSavedAt);
|
||||
return;
|
||||
}
|
||||
|
||||
// Crash safety: persist a defaults baseline so an interrupted replay
|
||||
// hydrates as defaults + appended prefix on next boot (a consistent prefix
|
||||
// of server state, completed by the next sync from cursor 0) instead of
|
||||
// layering the new history onto the stale pre-replace snapshot.
|
||||
await this.opLogStore.saveStateCache({
|
||||
state: getDefaultMainModelData(),
|
||||
lastAppliedOpSeq: 0,
|
||||
vectorClock: rebuiltClock,
|
||||
compactedAt: Date.now(),
|
||||
snapshotEntityKeys: [],
|
||||
});
|
||||
|
||||
// Reset live state to defaults, then replay the COMPLETE server history on
|
||||
// top. A full-state op in the history replaces state again by its own
|
||||
// semantics; a purely incremental history rebuilds from this baseline.
|
||||
const defaultData = getDefaultMainModelData();
|
||||
this.store.dispatch(
|
||||
loadAllData({
|
||||
appDataComplete: defaultData as Parameters<
|
||||
typeof loadAllData
|
||||
>[0]['appDataComplete'],
|
||||
}),
|
||||
);
|
||||
// Brief yield to let NgRx process the state reset
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
const baselineState = snapshotState ?? defaultData;
|
||||
const archiveYoung =
|
||||
(snapshotState?.[
|
||||
'archiveYoung'
|
||||
] as typeof MODEL_CONFIGS.archiveYoung.defaultData) ??
|
||||
MODEL_CONFIGS.archiveYoung.defaultData!;
|
||||
const archiveOld =
|
||||
(snapshotState?.['archiveOld'] as typeof MODEL_CONFIGS.archiveOld.defaultData) ??
|
||||
MODEL_CONFIGS.archiveOld.defaultData!;
|
||||
|
||||
// Process all remote ops (no confirmation needed - user already chose USE_REMOTE).
|
||||
// Skip conflict detection because the NgRx store was just reset to empty state,
|
||||
// which causes all entities to appear missing and CONCURRENT ops to be discarded.
|
||||
// Validation failure is surfaced via the session-validation latch. (#7330)
|
||||
const processResult = await this.remoteOpsProcessingService.processRemoteOps(
|
||||
result.newOps,
|
||||
{ skipConflictDetection: true },
|
||||
);
|
||||
let capturedBackupSavedAt: number | undefined;
|
||||
let replacementCommitted = false;
|
||||
let backupSavedAt: number;
|
||||
try {
|
||||
backupSavedAt = await this.lockService.request(
|
||||
LOCK_NAMES.OPERATION_LOG,
|
||||
async () => {
|
||||
// Include actions dispatched while the network request and preflight were
|
||||
// in flight in the reversible safety backup.
|
||||
await this.writeFlushService.flushPendingWrites();
|
||||
let savedAt: number;
|
||||
try {
|
||||
savedAt = await this.backupService.captureImportBackup();
|
||||
capturedBackupSavedAt = savedAt;
|
||||
} catch (e) {
|
||||
OpLog.warn(
|
||||
'OperationLogSyncService: Pre-replace safety backup failed; aborting force download.',
|
||||
{ name: (e as Error | undefined)?.name },
|
||||
);
|
||||
throw new Error(
|
||||
'Pre-replace safety backup failed; aborting to preserve local state.',
|
||||
);
|
||||
}
|
||||
|
||||
if (processResult.blockedByIncompatibleOp) {
|
||||
// Version blocks were pre-checked above; only a migration exception lands
|
||||
// here. The rebuild is partial: keep the cursor at 0 so the next sync
|
||||
// retries the remainder, and surface the failure — the Undo snack still
|
||||
// offers the pre-replace backup.
|
||||
this._showRestorePreviousDataSnack(backupSavedAt);
|
||||
throw new Error(
|
||||
'USE_REMOTE incomplete: an op failed schema migration during replay.',
|
||||
// The provider cursor lives outside SUP_OPS, so it cannot join the IDB
|
||||
// transaction. Reset it first: a crash/failure before the transaction
|
||||
// merely causes a safe re-download onto intact local state, while a
|
||||
// commit can never become visible with a stale cursor that skips the
|
||||
// remote history required to rebuild the baseline.
|
||||
await syncProvider.setLastServerSeq(0);
|
||||
await this.opLogStore.runRemoteStateReplacement({
|
||||
baselineState,
|
||||
vectorClock: rebuiltClock,
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
snapshotEntityKeys: extractEntityKeysFromState(
|
||||
baselineState as Parameters<typeof extractEntityKeysFromState>[0],
|
||||
),
|
||||
archiveYoung,
|
||||
archiveOld,
|
||||
});
|
||||
replacementCommitted = true;
|
||||
|
||||
OpLog.normal(
|
||||
'OperationLogSyncService: Replaced local persistence with remote baseline.',
|
||||
);
|
||||
|
||||
// FILE-BASED SYNC: Handle snapshot state from force download.
|
||||
// When downloading from seq 0 on file-based providers, we may receive a
|
||||
// snapshotState instead of incremental ops. This happens when the remote
|
||||
// has a SYNC_IMPORT (full state snapshot) with empty recentOps.
|
||||
// hydrateFromRemoteSync persists its own state cache + vector clock.
|
||||
if (result.providerMode === 'fileSnapshotOps' && snapshotState) {
|
||||
OpLog.normal(
|
||||
'OperationLogSyncService: Force download received snapshotState. Hydrating...',
|
||||
);
|
||||
|
||||
// Hydrate from snapshot - DON'T create SYNC_IMPORT since we're
|
||||
// accepting remote state, not uploading local state.
|
||||
await this.syncHydrationService.hydrateFromRemoteSync(
|
||||
snapshotState,
|
||||
result.snapshotVectorClock,
|
||||
false, // Don't create SYNC_IMPORT
|
||||
);
|
||||
|
||||
// CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration.
|
||||
// Same rationale as downloadRemoteOps: file-based providers return ALL
|
||||
// recentOps on every download and rely on getAppliedOpIds() to filter them.
|
||||
if (migratedRemoteOps.length > 0) {
|
||||
const appendResult = await this.opLogStore.appendBatchSkipDuplicates(
|
||||
migratedRemoteOps,
|
||||
'remote',
|
||||
);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Wrote ${appendResult.writtenOps.length} snapshot ops to IndexedDB ` +
|
||||
'after force-download hydration.' +
|
||||
(appendResult.skippedCount > 0
|
||||
? ` Skipped ${appendResult.skippedCount} duplicate(s).`
|
||||
: ''),
|
||||
);
|
||||
}
|
||||
|
||||
// Update lastServerSeq after hydration
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
'OperationLogSyncService: Force download snapshot hydration complete.',
|
||||
);
|
||||
return savedAt;
|
||||
}
|
||||
|
||||
// Reset live state to defaults, then replay the COMPLETE server history on
|
||||
// top. A full-state op in the history replaces state again by its own
|
||||
// semantics; a purely incremental history rebuilds from this baseline.
|
||||
this.store.dispatch(
|
||||
loadAllData({
|
||||
appDataComplete: defaultData as Parameters<
|
||||
typeof loadAllData
|
||||
>[0]['appDataComplete'],
|
||||
}),
|
||||
);
|
||||
// Brief yield to let NgRx process the state reset
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
// Process all remote ops (no confirmation needed - user already chose USE_REMOTE).
|
||||
// Skip conflict detection because the NgRx store was just reset to empty state,
|
||||
// which causes all entities to appear missing and CONCURRENT ops to be discarded.
|
||||
// Validation failure is surfaced via the session-validation latch. (#7330)
|
||||
const processResult = await this.remoteOpsProcessingService.processRemoteOps(
|
||||
migratedRemoteOps,
|
||||
{
|
||||
skipConflictDetection: true,
|
||||
callerHoldsOperationLogLock: true,
|
||||
},
|
||||
);
|
||||
|
||||
if (processResult.blockedByIncompatibleOp) {
|
||||
// Version blocks were pre-checked above; only a migration exception lands
|
||||
// here. The rebuild is partial: keep the cursor at 0 so the next sync
|
||||
// retries the remainder, and surface the failure — the Undo snack still
|
||||
// offers the pre-replace backup.
|
||||
throw new Error(
|
||||
'USE_REMOTE incomplete: an op failed schema migration during replay.',
|
||||
);
|
||||
}
|
||||
|
||||
// Update lastServerSeq
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Force download complete. Rebuilt from ${migratedRemoteOps.length} ops.`,
|
||||
);
|
||||
return savedAt;
|
||||
},
|
||||
);
|
||||
} catch (e) {
|
||||
if (replacementCommitted && capturedBackupSavedAt !== undefined) {
|
||||
this._showRestorePreviousDataSnack(capturedBackupSavedAt);
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
|
||||
// Update lastServerSeq
|
||||
if (result.latestServerSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.latestServerSeq);
|
||||
}
|
||||
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Force download complete. Rebuilt from ${result.newOps.length} ops.`,
|
||||
);
|
||||
this._showRestorePreviousDataSnack(backupSavedAt);
|
||||
}
|
||||
|
||||
private _preflightRemoteOperations(remoteOps: Operation[]): Operation[] {
|
||||
for (const op of remoteOps) {
|
||||
let version: number;
|
||||
try {
|
||||
version = getOperationSchemaVersion(op as { schemaVersion?: unknown });
|
||||
} catch {
|
||||
throw new Error(
|
||||
'USE_REMOTE aborted: remote history has an invalid schema version.',
|
||||
);
|
||||
}
|
||||
|
||||
if (version < MIN_SUPPORTED_SCHEMA_VERSION) {
|
||||
throw new Error(
|
||||
'USE_REMOTE aborted: remote history contains an unsupported schema version.',
|
||||
);
|
||||
}
|
||||
if (version > CURRENT_SCHEMA_VERSION) {
|
||||
this.snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.VERSION_TOO_OLD,
|
||||
actionStr: T.PS.UPDATE_APP,
|
||||
actionFn: () =>
|
||||
window.open('https://super-productivity.com/download', '_blank'),
|
||||
});
|
||||
throw new Error(
|
||||
'USE_REMOTE aborted: remote history contains ops from a newer schema version — update the app first.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
return this.schemaMigrationService.migrateOperations(remoteOps);
|
||||
} catch {
|
||||
throw new Error('USE_REMOTE aborted: remote operation migration failed.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shows a non-blocking snack after a destructive "Use Server Data" replace,
|
||||
* offering to restore the local snapshot captured before the wipe — making the
|
||||
|
|
|
|||
|
|
@ -320,6 +320,30 @@ describe('OperationLogUploadService', () => {
|
|||
expect(mockOpLogStore.markSynced).toHaveBeenCalledWith([1, 2]);
|
||||
});
|
||||
|
||||
it('should defer acknowledgements and return the exact selected batch for piggyback resolution', async () => {
|
||||
const pendingOps = [
|
||||
createMockEntry(1, 'op-1', 'client-1'),
|
||||
createMockEntry(2, 'op-2', 'client-1'),
|
||||
];
|
||||
mockOpLogStore.getUnsynced.and.resolveTo(pendingOps);
|
||||
mockApiProvider.uploadOps.and.resolveTo({
|
||||
results: [
|
||||
{ opId: 'op-1', accepted: true },
|
||||
{ opId: 'op-2', accepted: true },
|
||||
],
|
||||
latestSeq: 10,
|
||||
newOps: [],
|
||||
});
|
||||
|
||||
const result = await service.uploadPendingOps(mockApiProvider, {
|
||||
deferAcknowledgement: true,
|
||||
});
|
||||
|
||||
expect(mockOpLogStore.markSynced).not.toHaveBeenCalled();
|
||||
expect(result.selectedPendingOps).toEqual(pendingOps);
|
||||
expect(result.pendingAcknowledgementSeqs).toEqual([1, 2]);
|
||||
});
|
||||
|
||||
it('should mark accepted seqs correctly when server results are out of order', async () => {
|
||||
const pendingOps = [
|
||||
createMockEntry(1, 'op-1', 'client-1'),
|
||||
|
|
|
|||
|
|
@ -86,6 +86,24 @@ export class OperationLogUploadService {
|
|||
let uploadedCount = 0;
|
||||
let rejectedCount = 0;
|
||||
let hasMorePiggyback = false;
|
||||
let selectedPendingOps: OperationLogEntry[] = [];
|
||||
const pendingAcknowledgementSeqs: number[] = [];
|
||||
const pendingAcknowledgementSeqSet = new Set<number>();
|
||||
const acknowledge = async (seqs: number[]): Promise<void> => {
|
||||
if (seqs.length === 0) {
|
||||
return;
|
||||
}
|
||||
if (!options?.deferAcknowledgement) {
|
||||
await this.opLogStore.markSynced(seqs);
|
||||
return;
|
||||
}
|
||||
for (const seq of seqs) {
|
||||
if (!pendingAcknowledgementSeqSet.has(seq)) {
|
||||
pendingAcknowledgementSeqSet.add(seq);
|
||||
pendingAcknowledgementSeqs.push(seq);
|
||||
}
|
||||
}
|
||||
};
|
||||
// Track encryption state of piggybacked operations for detecting encryption config mismatch.
|
||||
// When another client disables encryption, all piggybacked ops will be unencrypted.
|
||||
// We track this BEFORE decryption to detect the server's actual encryption state.
|
||||
|
|
@ -107,6 +125,7 @@ export class OperationLogUploadService {
|
|||
}
|
||||
|
||||
const pendingOps = await this.opLogStore.getUnsynced();
|
||||
selectedPendingOps = pendingOps;
|
||||
|
||||
if (pendingOps.length === 0) {
|
||||
OpLog.normal('OperationLogUploadService: No pending operations to upload.');
|
||||
|
|
@ -198,7 +217,7 @@ export class OperationLogUploadService {
|
|||
isCleanSlateForOp,
|
||||
);
|
||||
if (result.accepted) {
|
||||
await this.opLogStore.markSynced([entry.seq]);
|
||||
await acknowledge([entry.seq]);
|
||||
uploadedCount++;
|
||||
if (result.serverSeq !== undefined) {
|
||||
await syncProvider.setLastServerSeq(result.serverSeq);
|
||||
|
|
@ -258,7 +277,7 @@ export class OperationLogUploadService {
|
|||
|
||||
if (opsIncludedInSnapshot.length > 0) {
|
||||
const seqs = opsIncludedInSnapshot.map((entry) => entry.seq);
|
||||
await this.opLogStore.markSynced(seqs);
|
||||
await acknowledge(seqs);
|
||||
uploadedCount += seqs.length;
|
||||
OpLog.normal(
|
||||
`OperationLogUploadService: Marked ${seqs.length} regular ops as synced ` +
|
||||
|
|
@ -293,7 +312,7 @@ export class OperationLogUploadService {
|
|||
}
|
||||
}
|
||||
if (localOnlySeqs.length > 0) {
|
||||
await this.opLogStore.markSynced(localOnlySeqs);
|
||||
await acknowledge(localOnlySeqs);
|
||||
uploadedCount += localOnlySeqs.length;
|
||||
OpLog.normal(
|
||||
`OperationLogUploadService: Marked ${localOnlySeqs.length} local-only op(s) as synced without upload`,
|
||||
|
|
@ -339,7 +358,7 @@ export class OperationLogUploadService {
|
|||
.filter((seq): seq is number => seq !== undefined);
|
||||
|
||||
if (acceptedSeqs.length > 0) {
|
||||
await this.opLogStore.markSynced(acceptedSeqs);
|
||||
await acknowledge(acceptedSeqs);
|
||||
uploadedCount += acceptedSeqs.length;
|
||||
}
|
||||
|
||||
|
|
@ -468,6 +487,9 @@ export class OperationLogUploadService {
|
|||
...(piggybackHasOnlyUnencryptedData ? { piggybackHasOnlyUnencryptedData } : {}),
|
||||
...(lastServerSeqToPersist !== undefined ? { lastServerSeqToPersist } : {}),
|
||||
...(encryptionRequiredKeyMissing ? { encryptionRequiredKeyMissing: true } : {}),
|
||||
...(options?.deferAcknowledgement
|
||||
? { selectedPendingOps, pendingAcknowledgementSeqs }
|
||||
: {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -73,6 +73,7 @@ describe('RemoteOpsProcessingService', () => {
|
|||
'append',
|
||||
'appendBatchSkipDuplicates',
|
||||
'appendWithVectorClockUpdate',
|
||||
'markArchivePending',
|
||||
'markApplied',
|
||||
'markFailed',
|
||||
'mergeRemoteOpClocks',
|
||||
|
|
@ -369,6 +370,57 @@ describe('RemoteOpsProcessingService', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('should log conflict identities without logging operation payloads', async () => {
|
||||
const localOp = {
|
||||
id: 'local-op',
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
payload: { title: 'private local title' },
|
||||
} as Operation;
|
||||
const remoteOp = {
|
||||
id: 'remote-op',
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
payload: { title: 'private remote title' },
|
||||
schemaVersion: 1,
|
||||
} as Operation;
|
||||
spyOn(service, 'detectConflicts').and.resolveTo({
|
||||
nonConflicting: [],
|
||||
conflicts: [
|
||||
{
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
localOps: [localOp],
|
||||
remoteOps: [remoteOp],
|
||||
suggestedResolution: 'manual',
|
||||
},
|
||||
],
|
||||
});
|
||||
conflictResolutionServiceSpy.autoResolveConflictsLWW.and.resolveTo({
|
||||
localWinOpsCreated: 0,
|
||||
});
|
||||
vectorClockServiceSpy.getEntityFrontier.and.resolveTo(new Map());
|
||||
const warnSpy = spyOn(OpLog, 'warn');
|
||||
|
||||
await service.processRemoteOps([remoteOp]);
|
||||
|
||||
const summary = warnSpy.calls
|
||||
.allArgs()
|
||||
.find(([message]) => String(message).includes('Detected 1 conflicts'))?.[1];
|
||||
expect(summary).toEqual({
|
||||
conflicts: [
|
||||
{
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
localOpIds: ['local-op'],
|
||||
remoteOpIds: ['remote-op'],
|
||||
suggestedResolution: 'manual',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(JSON.stringify(summary)).not.toContain('private');
|
||||
});
|
||||
|
||||
it('should drop operations if migrateOperation returns null', async () => {
|
||||
const remoteOps: Operation[] = [
|
||||
{ id: 'op1', schemaVersion: 1 } as Operation,
|
||||
|
|
@ -516,6 +568,21 @@ describe('RemoteOpsProcessingService', () => {
|
|||
expect(result.blockedByIncompatibleOp).toBe(true);
|
||||
});
|
||||
|
||||
for (const invalidVersion of [null, '2', 1.5, {}, Number.NaN]) {
|
||||
it(`should block malformed schemaVersion ${String(invalidVersion)}`, async () => {
|
||||
const remoteOp = {
|
||||
id: 'malformed-version',
|
||||
schemaVersion: invalidVersion,
|
||||
} as unknown as Operation;
|
||||
|
||||
const result = await service.processRemoteOps([remoteOp]);
|
||||
|
||||
expect(result.blockedByIncompatibleOp).toBeTrue();
|
||||
expect(schemaMigrationServiceSpy.migrateOperation).not.toHaveBeenCalled();
|
||||
expect(opLogStoreSpy.appendBatchSkipDuplicates).not.toHaveBeenCalled();
|
||||
});
|
||||
}
|
||||
|
||||
it('should process the prefix before a too-new op but flag the block', async () => {
|
||||
const remoteOps: Operation[] = [
|
||||
{ id: 'op1', schemaVersion: 1 } as Operation,
|
||||
|
|
@ -1337,9 +1404,10 @@ describe('RemoteOpsProcessingService', () => {
|
|||
|
||||
opLogStoreSpy.hasOp.and.returnValue(Promise.resolve(false));
|
||||
opLogStoreSpy.append.and.returnValue(Promise.resolve(1));
|
||||
operationApplierServiceSpy.applyOperations.and.returnValue(
|
||||
Promise.resolve({ appliedOps: remoteOps }),
|
||||
);
|
||||
operationApplierServiceSpy.applyOperations.and.callFake(async (ops, options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return { appliedOps: remoteOps };
|
||||
});
|
||||
|
||||
await service.applyNonConflictingOps(remoteOps);
|
||||
|
||||
|
|
@ -1352,10 +1420,14 @@ describe('RemoteOpsProcessingService', () => {
|
|||
];
|
||||
const callOrder: string[] = [];
|
||||
|
||||
operationApplierServiceSpy.applyOperations.and.callFake(async () => {
|
||||
operationApplierServiceSpy.applyOperations.and.callFake(async (ops, options) => {
|
||||
callOrder.push('applyOperations');
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return { appliedOps: remoteOps };
|
||||
});
|
||||
opLogStoreSpy.markArchivePending.and.callFake(async () => {
|
||||
callOrder.push('markArchivePending');
|
||||
});
|
||||
opLogStoreSpy.markApplied.and.callFake(async () => {
|
||||
callOrder.push('markApplied');
|
||||
});
|
||||
|
|
@ -1368,16 +1440,21 @@ describe('RemoteOpsProcessingService', () => {
|
|||
|
||||
await service.applyNonConflictingOps(remoteOps, true);
|
||||
|
||||
expect(operationApplierServiceSpy.applyOperations).toHaveBeenCalledWith(remoteOps, {
|
||||
skipDeferredLocalActions: true,
|
||||
});
|
||||
expect(operationApplierServiceSpy.applyOperations).toHaveBeenCalledWith(
|
||||
remoteOps,
|
||||
jasmine.objectContaining({
|
||||
skipDeferredLocalActions: true,
|
||||
onReducersCommitted: jasmine.any(Function),
|
||||
}),
|
||||
);
|
||||
expect(operationLogEffectsSpy.processDeferredActions).toHaveBeenCalledWith({
|
||||
callerHoldsOperationLogLock: true,
|
||||
});
|
||||
expect(callOrder).toEqual([
|
||||
'applyOperations',
|
||||
'markApplied',
|
||||
'markArchivePending',
|
||||
'mergeRemoteOpClocks',
|
||||
'markApplied',
|
||||
'processDeferredActions',
|
||||
]);
|
||||
});
|
||||
|
|
@ -1397,7 +1474,7 @@ describe('RemoteOpsProcessingService', () => {
|
|||
expect(opLogStoreSpy.mergeRemoteOpClocks).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should mark failed ops and run validation on partial failure', async () => {
|
||||
it('should charge only the attempted archive failure and run validation', async () => {
|
||||
const remoteOps: Operation[] = [
|
||||
createFullOp({ id: 'op-1' }),
|
||||
createFullOp({ id: 'op-2' }),
|
||||
|
|
@ -1407,17 +1484,19 @@ describe('RemoteOpsProcessingService', () => {
|
|||
opLogStoreSpy.append.and.returnValue(Promise.resolve(1));
|
||||
opLogStoreSpy.markApplied.and.returnValue(Promise.resolve());
|
||||
opLogStoreSpy.markFailed.and.returnValue(Promise.resolve());
|
||||
operationApplierServiceSpy.applyOperations.and.returnValue(
|
||||
Promise.resolve({
|
||||
operationApplierServiceSpy.applyOperations.and.callFake(async (ops, options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return {
|
||||
appliedOps: [remoteOps[0]],
|
||||
failedOp: { op: remoteOps[1], error: new Error('Test error') },
|
||||
}),
|
||||
);
|
||||
};
|
||||
});
|
||||
|
||||
await expectAsync(service.applyNonConflictingOps(remoteOps)).toBeRejected();
|
||||
|
||||
// Should mark op-2 and op-3 as failed
|
||||
expect(opLogStoreSpy.markFailed).toHaveBeenCalledWith(['op-2', 'op-3']);
|
||||
expect(opLogStoreSpy.markArchivePending).toHaveBeenCalledWith([1, 2, 3]);
|
||||
expect(opLogStoreSpy.mergeRemoteOpClocks).toHaveBeenCalledWith(remoteOps);
|
||||
expect(opLogStoreSpy.markFailed).toHaveBeenCalledWith(['op-2']);
|
||||
// Should run validation after partial failure
|
||||
expect(validateStateServiceSpy.validateAndRepairCurrentState).toHaveBeenCalledWith(
|
||||
'partial-apply-failure',
|
||||
|
|
@ -1486,7 +1565,10 @@ describe('RemoteOpsProcessingService', () => {
|
|||
// Should apply only the non-duplicate op
|
||||
expect(operationApplierServiceSpy.applyOperations).toHaveBeenCalledWith(
|
||||
[remoteOps[1]],
|
||||
{ skipDeferredLocalActions: true },
|
||||
jasmine.objectContaining({
|
||||
skipDeferredLocalActions: true,
|
||||
onReducersCommitted: jasmine.any(Function),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import { VectorClockService } from './vector-clock.service';
|
|||
import {
|
||||
MIN_SUPPORTED_SCHEMA_VERSION,
|
||||
SchemaMigrationService,
|
||||
getOperationSchemaVersion,
|
||||
} from '../persistence/schema-migration.service';
|
||||
import { SnackService } from '../../core/snack/snack.service';
|
||||
import { T } from '../../t.const';
|
||||
|
|
@ -92,7 +93,10 @@ export class RemoteOpsProcessingService {
|
|||
*/
|
||||
async processRemoteOps(
|
||||
remoteOps: Operation[],
|
||||
options?: { skipConflictDetection?: boolean },
|
||||
options?: {
|
||||
skipConflictDetection?: boolean;
|
||||
callerHoldsOperationLogLock?: boolean;
|
||||
},
|
||||
): Promise<{
|
||||
localWinOpsCreated: number;
|
||||
allOpsFilteredBySyncImport: boolean;
|
||||
|
|
@ -125,12 +129,22 @@ export class RemoteOpsProcessingService {
|
|||
const currentVersion = this.schemaMigrationService.getCurrentVersion();
|
||||
const migratedOps: Operation[] = [];
|
||||
const droppedEntityIds = new Set<string>();
|
||||
let blockReason: 'VERSION_UNSUPPORTED' | 'VERSION_TOO_NEW' | 'MIGRATION_FAILED' =
|
||||
'MIGRATION_FAILED';
|
||||
let blockReason:
|
||||
| 'VERSION_UNSUPPORTED'
|
||||
| 'VERSION_TOO_NEW'
|
||||
| 'INVALID_SCHEMA_VERSION'
|
||||
| 'MIGRATION_FAILED' = 'MIGRATION_FAILED';
|
||||
let blockedOp: Operation | null = null;
|
||||
|
||||
for (const op of remoteOps) {
|
||||
const opVersion = op.schemaVersion ?? 1;
|
||||
let opVersion: number;
|
||||
try {
|
||||
opVersion = getOperationSchemaVersion(op as { schemaVersion?: unknown });
|
||||
} catch {
|
||||
blockedOp = op;
|
||||
blockReason = 'INVALID_SCHEMA_VERSION';
|
||||
break;
|
||||
}
|
||||
|
||||
// Op below minimum supported version: no migration path exists.
|
||||
if (opVersion < MIN_SUPPORTED_SCHEMA_VERSION) {
|
||||
|
|
@ -281,8 +295,11 @@ export class RemoteOpsProcessingService {
|
|||
'RemoteOpsProcessingService: Skipping conflict detection (skipConflictDetection=true). ' +
|
||||
`Applying ${validOps.length} ops directly.`,
|
||||
);
|
||||
await this.applyNonConflictingOps(validOps);
|
||||
await this.validateAfterSync();
|
||||
await this.applyNonConflictingOps(
|
||||
validOps,
|
||||
options.callerHoldsOperationLogLock ?? false,
|
||||
);
|
||||
await this.validateAfterSync(options.callerHoldsOperationLogLock ?? false);
|
||||
return {
|
||||
localWinOpsCreated: 0,
|
||||
allOpsFilteredBySyncImport: false,
|
||||
|
|
@ -322,7 +339,15 @@ export class RemoteOpsProcessingService {
|
|||
if (conflicts.length > 0) {
|
||||
OpLog.warn(
|
||||
`RemoteOpsProcessingService: Detected ${conflicts.length} conflicts. Auto-resolving with LWW.`,
|
||||
conflicts,
|
||||
{
|
||||
conflicts: conflicts.map((conflict) => ({
|
||||
entityType: conflict.entityType,
|
||||
entityId: conflict.entityId,
|
||||
localOpIds: conflict.localOps.map((op) => op.id),
|
||||
remoteOpIds: conflict.remoteOps.map((op) => op.id),
|
||||
suggestedResolution: conflict.suggestedResolution,
|
||||
})),
|
||||
},
|
||||
);
|
||||
// Auto-resolve conflicts using Last-Write-Wins strategy.
|
||||
// Piggyback non-conflicting ops so they're applied with resolved conflicts.
|
||||
|
|
@ -359,9 +384,13 @@ export class RemoteOpsProcessingService {
|
|||
* until an app update or migration fix, and every retry re-hits it).
|
||||
*/
|
||||
private _notifyBlockedOp(
|
||||
reason: 'VERSION_UNSUPPORTED' | 'VERSION_TOO_NEW' | 'MIGRATION_FAILED',
|
||||
reason:
|
||||
| 'VERSION_UNSUPPORTED'
|
||||
| 'VERSION_TOO_NEW'
|
||||
| 'INVALID_SCHEMA_VERSION'
|
||||
| 'MIGRATION_FAILED',
|
||||
): void {
|
||||
if (reason === 'MIGRATION_FAILED') {
|
||||
if (reason === 'MIGRATION_FAILED' || reason === 'INVALID_SCHEMA_VERSION') {
|
||||
if (!this._hasWarnedMigrationFailureThisSession) {
|
||||
this._hasWarnedMigrationFailureThisSession = true;
|
||||
this.snackService.open({
|
||||
|
|
@ -423,9 +452,10 @@ export class RemoteOpsProcessingService {
|
|||
ops: locallyReplayableOps,
|
||||
store: this.opLogStore,
|
||||
applier: {
|
||||
applyOperations: (opsToApply) =>
|
||||
applyOperations: (opsToApply, applyOptions) =>
|
||||
this.operationApplier.applyOperations(opsToApply, {
|
||||
skipDeferredLocalActions: true,
|
||||
onReducersCommitted: applyOptions?.onReducersCommitted,
|
||||
}),
|
||||
},
|
||||
isFullStateOperation: this._isFullStateOperation,
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import { ServerMigrationService } from './server-migration.service';
|
|||
import { OperationLogStoreService } from '../persistence/operation-log-store.service';
|
||||
import { VectorClockService } from './vector-clock.service';
|
||||
import { ValidateStateService } from '../validation/validate-state.service';
|
||||
import { StateSnapshotService } from '../backup/state-snapshot.service';
|
||||
import { AppStateSnapshot, StateSnapshotService } from '../backup/state-snapshot.service';
|
||||
import { SnackService } from '../../core/snack/snack.service';
|
||||
import { UserInputWaitStateService } from '../../imex/sync/user-input-wait-state.service';
|
||||
import {
|
||||
|
|
@ -20,6 +20,10 @@ import { SYSTEM_TAG_IDS } from '../../features/tag/tag.const';
|
|||
import { INBOX_PROJECT } from '../../features/project/project.const';
|
||||
import { loadAllData } from '../../root-store/meta/load-all-data.action';
|
||||
import { CLIENT_ID_PROVIDER, ClientIdProvider } from '../util/client-id.provider';
|
||||
import { LockService } from './lock.service';
|
||||
import { OperationWriteFlushService } from './operation-write-flush.service';
|
||||
import { LOCK_NAMES } from '../core/operation-log.const';
|
||||
import { OperationCaptureService } from '../capture/operation-capture.service';
|
||||
|
||||
describe('ServerMigrationService', () => {
|
||||
let service: ServerMigrationService;
|
||||
|
|
@ -32,6 +36,9 @@ describe('ServerMigrationService', () => {
|
|||
let clientIdProviderSpy: jasmine.SpyObj<ClientIdProvider>;
|
||||
let matDialogSpy: jasmine.SpyObj<MatDialog>;
|
||||
let userInputWaitStateSpy: jasmine.SpyObj<UserInputWaitStateService>;
|
||||
let lockServiceSpy: jasmine.SpyObj<LockService>;
|
||||
let writeFlushServiceSpy: jasmine.SpyObj<OperationWriteFlushService>;
|
||||
let operationCaptureServiceSpy: jasmine.SpyObj<OperationCaptureService>;
|
||||
let defaultProvider: OperationSyncProvider;
|
||||
|
||||
// Type for operation-sync-capable provider
|
||||
|
|
@ -86,6 +93,18 @@ describe('ServerMigrationService', () => {
|
|||
'startWaiting',
|
||||
]);
|
||||
userInputWaitStateSpy.startWaiting.and.returnValue(() => {});
|
||||
lockServiceSpy = jasmine.createSpyObj('LockService', ['request']);
|
||||
lockServiceSpy.request.and.callFake(async <T>(_name: string, fn: () => Promise<T>) =>
|
||||
fn(),
|
||||
);
|
||||
writeFlushServiceSpy = jasmine.createSpyObj('OperationWriteFlushService', [
|
||||
'flushPendingWrites',
|
||||
]);
|
||||
writeFlushServiceSpy.flushPendingWrites.and.resolveTo();
|
||||
operationCaptureServiceSpy = jasmine.createSpyObj('OperationCaptureService', [
|
||||
'getPendingCount',
|
||||
]);
|
||||
operationCaptureServiceSpy.getPendingCount.and.returnValue(0);
|
||||
|
||||
// Default mock returns
|
||||
opLogStoreSpy.hasSyncedOps.and.returnValue(Promise.resolve(true));
|
||||
|
|
@ -130,6 +149,9 @@ describe('ServerMigrationService', () => {
|
|||
{ provide: CLIENT_ID_PROVIDER, useValue: clientIdProviderSpy },
|
||||
{ provide: MatDialog, useValue: matDialogSpy },
|
||||
{ provide: UserInputWaitStateService, useValue: userInputWaitStateSpy },
|
||||
{ provide: LockService, useValue: lockServiceSpy },
|
||||
{ provide: OperationWriteFlushService, useValue: writeFlushServiceSpy },
|
||||
{ provide: OperationCaptureService, useValue: operationCaptureServiceSpy },
|
||||
],
|
||||
});
|
||||
|
||||
|
|
@ -471,6 +493,52 @@ describe('ServerMigrationService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should capture and append the full-state operation inside one operation-log barrier', async () => {
|
||||
const events: string[] = [];
|
||||
writeFlushServiceSpy.flushPendingWrites.and.callFake(async () => {
|
||||
events.push('flush');
|
||||
});
|
||||
lockServiceSpy.request.and.callFake(async <T>(name: string, fn: () => Promise<T>) => {
|
||||
events.push(`lock:${name}:start`);
|
||||
const result = await fn();
|
||||
events.push(`lock:${name}:end`);
|
||||
return result;
|
||||
});
|
||||
stateSnapshotServiceSpy.getStateSnapshotAsync.and.callFake(async () => {
|
||||
events.push('snapshot');
|
||||
return {
|
||||
task: { ids: ['task-1'], entities: { 'task-1': { id: 'task-1' } } },
|
||||
project: { ids: [], entities: {} },
|
||||
tag: { ids: [], entities: {} },
|
||||
} as unknown as AppStateSnapshot;
|
||||
});
|
||||
opLogStoreSpy.append.and.callFake(async () => {
|
||||
events.push('append');
|
||||
return 1;
|
||||
});
|
||||
|
||||
await service.handleServerMigration(defaultProvider);
|
||||
|
||||
expect(events).toEqual([
|
||||
'flush',
|
||||
`lock:${LOCK_NAMES.OPERATION_LOG}:start`,
|
||||
'snapshot',
|
||||
'append',
|
||||
`lock:${LOCK_NAMES.OPERATION_LOG}:end`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should release, flush, and retry when an action lands before snapshot capture', async () => {
|
||||
operationCaptureServiceSpy.getPendingCount.and.returnValues(1, 0);
|
||||
|
||||
await service.handleServerMigration(defaultProvider);
|
||||
|
||||
expect(writeFlushServiceSpy.flushPendingWrites).toHaveBeenCalledTimes(2);
|
||||
expect(lockServiceSpy.request).toHaveBeenCalledTimes(2);
|
||||
expect(stateSnapshotServiceSpy.getStateSnapshotAsync).toHaveBeenCalledTimes(1);
|
||||
expect(opLogStoreSpy.append).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
describe('system-tag empty-state detection (tested via handleServerMigration)', () => {
|
||||
it('should identify system tags correctly', async () => {
|
||||
for (const systemTagId of SYSTEM_TAG_IDS) {
|
||||
|
|
|
|||
|
|
@ -24,7 +24,11 @@ import { OpLog } from '../../core/log';
|
|||
import { CLIENT_ID_PROVIDER } from '../util/client-id.provider';
|
||||
import { DialogServerMigrationConfirmComponent } from './dialog-server-migration-confirm/dialog-server-migration-confirm.component';
|
||||
import { hasMeaningfulStateData } from '../validation/has-meaningful-state-data.util';
|
||||
import { MODEL_CONFIGS } from '../model/model-config';
|
||||
import { AppDataComplete, MODEL_CONFIGS } from '../model/model-config';
|
||||
import { LockService } from './lock.service';
|
||||
import { OperationWriteFlushService } from './operation-write-flush.service';
|
||||
import { LOCK_NAMES } from '../core/operation-log.const';
|
||||
import { OperationCaptureService } from '../capture/operation-capture.service';
|
||||
|
||||
const MEANINGFUL_ENTITY_STATE_KEYS = new Set(['task', 'project', 'tag', 'note']);
|
||||
|
||||
|
|
@ -86,6 +90,9 @@ export class ServerMigrationService {
|
|||
private clientIdProvider = inject(CLIENT_ID_PROVIDER);
|
||||
private _matDialog = inject(MatDialog);
|
||||
private _userInputWaitState = inject(UserInputWaitStateService);
|
||||
private lockService = inject(LockService);
|
||||
private writeFlushService = inject(OperationWriteFlushService);
|
||||
private operationCaptureService = inject(OperationCaptureService);
|
||||
|
||||
/**
|
||||
* Checks if we're connecting to a new/empty server and handles migration if needed.
|
||||
|
|
@ -198,104 +205,129 @@ export class ServerMigrationService {
|
|||
'ServerMigrationService: Server migration detected. Creating full state SYNC_IMPORT.',
|
||||
);
|
||||
|
||||
// Get current full state from NgRx store (async to include archives from IndexedDB)
|
||||
// Cast to Record for validation compatibility
|
||||
let currentState: Record<string, unknown> =
|
||||
(await this.stateSnapshotService.getStateSnapshotAsync()) as unknown as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
// Drain already-captured writes, then keep snapshot capture, validation, clock
|
||||
// construction, and append behind the same mutation barrier. This makes the
|
||||
// full-state operation's local seq an exact cutoff: every earlier op is in the
|
||||
// snapshot, and any action captured while this runs is appended afterwards.
|
||||
let retryForPendingCapture: boolean;
|
||||
do {
|
||||
retryForPendingCapture = false;
|
||||
await this.writeFlushService.flushPendingWrites();
|
||||
await this.lockService.request(LOCK_NAMES.OPERATION_LOG, async () => {
|
||||
// A reducer action can land in the tiny gap between flush() releasing
|
||||
// this lock and our acquisition. Its pending counter increments
|
||||
// synchronously, before the persistence effect waits for the lock. Do
|
||||
// not take a snapshot that includes that reducer state but precedes its
|
||||
// operation; release, drain it, and retry the cutoff instead.
|
||||
if (this.operationCaptureService.getPendingCount() > 0) {
|
||||
retryForPendingCapture = true;
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip if local state is effectively empty
|
||||
if (!hasServerMigrationStateData(currentState)) {
|
||||
OpLog.warn('ServerMigrationService: Skipping SYNC_IMPORT - local state is empty.');
|
||||
return;
|
||||
}
|
||||
// Get current full state from NgRx store (async to include archives from IndexedDB)
|
||||
// Cast to Record for validation compatibility
|
||||
let currentState: Record<string, unknown> =
|
||||
(await this.stateSnapshotService.getStateSnapshotAsync()) as unknown as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
|
||||
// Validate and repair state before creating SYNC_IMPORT
|
||||
// This prevents corrupted state (e.g., orphaned menuTree references) from
|
||||
// propagating to other clients via the full state import.
|
||||
const validationResult =
|
||||
await this.validateStateService.validateAndRepair(currentState);
|
||||
// Skip if local state is effectively empty
|
||||
if (!hasServerMigrationStateData(currentState)) {
|
||||
OpLog.warn(
|
||||
'ServerMigrationService: Skipping SYNC_IMPORT - local state is empty.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// If state is invalid and couldn't be repaired, abort - don't propagate corruption
|
||||
if (!validationResult.isValid) {
|
||||
OpLog.err(
|
||||
'ServerMigrationService: Cannot create SYNC_IMPORT - state validation failed.',
|
||||
validationResult.error || validationResult.crossModelError,
|
||||
);
|
||||
this.snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.SERVER_MIGRATION_VALIDATION_FAILED,
|
||||
// Validate and repair state before creating SYNC_IMPORT
|
||||
// This prevents corrupted state (e.g., orphaned menuTree references) from
|
||||
// propagating to other clients via the full state import.
|
||||
const validationResult =
|
||||
await this.validateStateService.validateAndRepair(currentState);
|
||||
|
||||
// If state is invalid and couldn't be repaired, abort - don't propagate corruption
|
||||
if (!validationResult.isValid) {
|
||||
OpLog.err(
|
||||
'ServerMigrationService: Cannot create SYNC_IMPORT - state validation failed.',
|
||||
validationResult.error || validationResult.crossModelError,
|
||||
);
|
||||
this.snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.F.SYNC.S.SERVER_MIGRATION_VALIDATION_FAILED,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// If state was repaired, use the repaired version
|
||||
if (validationResult.repairedState) {
|
||||
OpLog.warn(
|
||||
'ServerMigrationService: State repaired before creating SYNC_IMPORT',
|
||||
validationResult.repairSummary,
|
||||
);
|
||||
currentState = validationResult.repairedState;
|
||||
|
||||
// Also update NgRx store with repaired state so local client is consistent
|
||||
this.store.dispatch(
|
||||
loadAllData({
|
||||
appDataComplete: validationResult.repairedState as AppDataComplete,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
// Get client ID
|
||||
const clientId = await this.clientIdProvider.loadClientId();
|
||||
if (!clientId) {
|
||||
OpLog.err(
|
||||
'ServerMigrationService: Cannot create SYNC_IMPORT - no client ID available.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Build vector clock by merging ALL local operation clocks.
|
||||
// This ensures the SYNC_IMPORT's clock dominates all pre-import ops,
|
||||
// so when SyncImportFilterService compares them, all prior ops are
|
||||
// LESS_THAN (not CONCURRENT) and can be properly filtered.
|
||||
const allLocalOps = await this.opLogStore.getOpsAfterSeq(0);
|
||||
let mergedClock = await this.vectorClockService.getCurrentVectorClock();
|
||||
for (const entry of allLocalOps) {
|
||||
mergedClock = mergeVectorClocks(mergedClock, entry.op.vectorClock);
|
||||
}
|
||||
const newClock = limitVectorClockSize(
|
||||
incrementVectorClock(mergedClock, clientId),
|
||||
clientId,
|
||||
);
|
||||
|
||||
OpLog.normal(
|
||||
`ServerMigrationService: Merged ${allLocalOps.length} local op clocks into SYNC_IMPORT vector clock.`,
|
||||
);
|
||||
|
||||
// Create SYNC_IMPORT operation with full state
|
||||
// NOTE: Use raw state directly (not wrapped in appDataComplete).
|
||||
// The snapshot endpoint expects raw state, and the hydrator handles
|
||||
// both formats on extraction.
|
||||
const op: Operation = {
|
||||
id: uuidv7(),
|
||||
actionType: ActionType.LOAD_ALL_DATA,
|
||||
opType: OpType.SyncImport,
|
||||
entityType: 'ALL',
|
||||
payload: currentState,
|
||||
clientId,
|
||||
vectorClock: newClock,
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
syncImportReason: options?.syncImportReason ?? 'SERVER_MIGRATION',
|
||||
};
|
||||
|
||||
// Append to operation log - will be uploaded via snapshot endpoint
|
||||
await this.opLogStore.append(op, 'local');
|
||||
|
||||
OpLog.normal(
|
||||
'ServerMigrationService: Created SYNC_IMPORT operation for server migration. ' +
|
||||
'Will be uploaded immediately via follow-up upload.',
|
||||
);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// If state was repaired, use the repaired version
|
||||
if (validationResult.repairedState) {
|
||||
OpLog.warn(
|
||||
'ServerMigrationService: State repaired before creating SYNC_IMPORT',
|
||||
validationResult.repairSummary,
|
||||
);
|
||||
currentState = validationResult.repairedState;
|
||||
|
||||
// Also update NgRx store with repaired state so local client is consistent
|
||||
this.store.dispatch(
|
||||
loadAllData({ appDataComplete: validationResult.repairedState as any }),
|
||||
);
|
||||
}
|
||||
|
||||
// Get client ID
|
||||
const clientId = await this.clientIdProvider.loadClientId();
|
||||
if (!clientId) {
|
||||
OpLog.err(
|
||||
'ServerMigrationService: Cannot create SYNC_IMPORT - no client ID available.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Build vector clock by merging ALL local operation clocks.
|
||||
// This ensures the SYNC_IMPORT's clock dominates all pre-import ops,
|
||||
// so when SyncImportFilterService compares them, all prior ops are
|
||||
// LESS_THAN (not CONCURRENT) and can be properly filtered.
|
||||
const allLocalOps = await this.opLogStore.getOpsAfterSeq(0);
|
||||
let mergedClock = await this.vectorClockService.getCurrentVectorClock();
|
||||
for (const entry of allLocalOps) {
|
||||
mergedClock = mergeVectorClocks(mergedClock, entry.op.vectorClock);
|
||||
}
|
||||
const newClock = limitVectorClockSize(
|
||||
incrementVectorClock(mergedClock, clientId),
|
||||
clientId,
|
||||
);
|
||||
|
||||
OpLog.normal(
|
||||
`ServerMigrationService: Merged ${allLocalOps.length} local op clocks into SYNC_IMPORT vector clock.`,
|
||||
);
|
||||
|
||||
// Create SYNC_IMPORT operation with full state
|
||||
// NOTE: Use raw state directly (not wrapped in appDataComplete).
|
||||
// The snapshot endpoint expects raw state, and the hydrator handles
|
||||
// both formats on extraction.
|
||||
const op: Operation = {
|
||||
id: uuidv7(),
|
||||
actionType: ActionType.LOAD_ALL_DATA,
|
||||
opType: OpType.SyncImport,
|
||||
entityType: 'ALL',
|
||||
payload: currentState,
|
||||
clientId,
|
||||
vectorClock: newClock,
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: CURRENT_SCHEMA_VERSION,
|
||||
syncImportReason: options?.syncImportReason ?? 'SERVER_MIGRATION',
|
||||
};
|
||||
|
||||
// Append to operation log - will be uploaded via snapshot endpoint
|
||||
await this.opLogStore.append(op, 'local');
|
||||
|
||||
OpLog.normal(
|
||||
'ServerMigrationService: Created SYNC_IMPORT operation for server migration. ' +
|
||||
'Will be uploaded immediately via follow-up upload.',
|
||||
);
|
||||
} while (retryForPendingCapture);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ describe('SyncImportConflictGateService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should not produce dialog data when pending ops are config-only', async () => {
|
||||
it('should produce dialog data when pending ops contain user config changes', async () => {
|
||||
const incomingSyncImport = createOperation();
|
||||
const pendingConfigEntry = createEntry(
|
||||
createOperation({
|
||||
|
|
@ -113,8 +113,8 @@ describe('SyncImportConflictGateService', () => {
|
|||
const result = await service.checkIncomingFullStateConflict([incomingSyncImport]);
|
||||
|
||||
expect(result.fullStateOp).toBe(incomingSyncImport);
|
||||
expect(result.hasMeaningfulPending).toBeFalse();
|
||||
expect(result.dialogData).toBeUndefined();
|
||||
expect(result.hasMeaningfulPending).toBeTrue();
|
||||
expect(result.dialogData).toBeDefined();
|
||||
});
|
||||
|
||||
it('should not produce dialog data when pending task creates are startup example tasks', async () => {
|
||||
|
|
@ -272,19 +272,27 @@ describe('SyncImportConflictGateService', () => {
|
|||
expect(opLogStoreSpy.hasSyncedOps).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not consult sync history when there are no meaningful pending ops', async () => {
|
||||
it('should not consult sync history when only startup example tasks are pending', async () => {
|
||||
const incomingSyncImport = createOperation();
|
||||
const pendingConfigEntry = createEntry(
|
||||
const pendingExampleTaskEntry = createEntry(
|
||||
createOperation({
|
||||
id: 'local-config-update',
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
id: 'local-example-task-create',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: 'example-task-1',
|
||||
payload: {
|
||||
actionPayload: {
|
||||
task: { id: 'example-task-1' },
|
||||
isExampleTask: true,
|
||||
},
|
||||
entityChanges: [],
|
||||
},
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 1 },
|
||||
}),
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([pendingConfigEntry]);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([pendingExampleTaskEntry]);
|
||||
|
||||
await service.checkIncomingFullStateConflict([incomingSyncImport]);
|
||||
|
||||
|
|
@ -383,25 +391,41 @@ describe('SyncImportConflictGateService', () => {
|
|||
expect(service.hasMeaningfulPendingOps([pendingCounter])).toBeTrue();
|
||||
});
|
||||
|
||||
it('should still treat MIGRATION/RECOVERY bookkeeping ops as not meaningful', async () => {
|
||||
it('should treat MIGRATION/RECOVERY genesis batches as meaningful recovered user data', async () => {
|
||||
const pendingMigration = createEntry(
|
||||
createOperation({
|
||||
id: 'local-genesis',
|
||||
actionType: '[Migration] Genesis' as ActionType,
|
||||
opType: OpType.Create,
|
||||
opType: OpType.Batch,
|
||||
entityType: 'MIGRATION',
|
||||
entityId: 'genesis',
|
||||
payload: { task: { ids: ['recovered-task'] } },
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 1 },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(service.hasMeaningfulPendingOps([pendingMigration])).toBeFalse();
|
||||
const pendingRecovery = createEntry(
|
||||
createOperation({
|
||||
id: 'local-recovery',
|
||||
actionType: '[Recovery] Data Import' as ActionType,
|
||||
opType: OpType.Batch,
|
||||
entityType: 'RECOVERY',
|
||||
entityId: 'genesis',
|
||||
payload: { project: { ids: ['recovered-project'] } },
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 2 },
|
||||
}),
|
||||
{ seq: 2 },
|
||||
);
|
||||
|
||||
expect(service.hasMeaningfulPendingOps([pendingMigration])).toBeTrue();
|
||||
expect(service.hasMeaningfulPendingOps([pendingRecovery])).toBeTrue();
|
||||
});
|
||||
});
|
||||
|
||||
describe('preCapturedPendingOps (piggyback-upload race)', () => {
|
||||
it('should judge meaningfulness against the pre-upload snapshot, not the live pending set', async () => {
|
||||
it('should judge meaningfulness against the union of the upload snapshot and live pending set', async () => {
|
||||
// Live pending set is empty — the op was accepted and marked synced during
|
||||
// the same upload round that piggybacked the import.
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([]);
|
||||
|
|
@ -426,6 +450,54 @@ describe('SyncImportConflictGateService', () => {
|
|||
expect(result.dialogData).toBeDefined();
|
||||
});
|
||||
|
||||
it('should include meaningful work created after the upload snapshot', async () => {
|
||||
const createdDuringUpload = createEntry(
|
||||
createOperation({
|
||||
id: 'created-during-upload',
|
||||
actionType: '[SimpleCounter] Increase Counter Today' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'SIMPLE_COUNTER',
|
||||
entityId: 'counter-1',
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 2 },
|
||||
}),
|
||||
{ seq: 2 },
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([createdDuringUpload]);
|
||||
|
||||
const result = await service.checkIncomingFullStateConflict([createOperation()], {
|
||||
flushPendingWrites: true,
|
||||
preCapturedPendingOps: [],
|
||||
});
|
||||
|
||||
expect(writeFlushServiceSpy.flushPendingWrites).toHaveBeenCalled();
|
||||
expect(result.pendingOps).toEqual([createdDuringUpload]);
|
||||
expect(result.hasMeaningfulPending).toBeTrue();
|
||||
expect(result.dialogData).toBeDefined();
|
||||
});
|
||||
|
||||
it('should de-duplicate operations present in both upload and live snapshots', async () => {
|
||||
const selectedForUpload = createEntry(
|
||||
createOperation({
|
||||
id: 'same-op',
|
||||
actionType: '[Task] Update task' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: 'task-1',
|
||||
clientId: 'client-A',
|
||||
vectorClock: { clientA: 1 },
|
||||
}),
|
||||
);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([selectedForUpload]);
|
||||
|
||||
const result = await service.checkIncomingFullStateConflict([createOperation()], {
|
||||
preCapturedPendingOps: [selectedForUpload],
|
||||
});
|
||||
|
||||
expect(result.pendingOps).toEqual([selectedForUpload]);
|
||||
expect(result.dialogData?.filteredOpCount).toBe(1);
|
||||
});
|
||||
|
||||
it('should derive discardable example-task ids from the LIVE pending set', async () => {
|
||||
const exampleCreate = (id: string): OperationLogEntry =>
|
||||
createEntry(
|
||||
|
|
|
|||
|
|
@ -10,17 +10,6 @@ import { OperationWriteFlushService } from './operation-write-flush.service';
|
|||
import { SyncImportConflictData } from './dialog-sync-import-conflict/dialog-sync-import-conflict.component';
|
||||
import { isExampleTaskCreateOp } from '../validation/is-example-task-op.util';
|
||||
|
||||
/**
|
||||
* Pending ops on these entity types are startup/system noise, not user work:
|
||||
* config writes happen automatically (defaults, migrations) and sync settings
|
||||
* are intentionally local; MIGRATION/RECOVERY are bookkeeping genesis ops.
|
||||
* Everything else — including MOV/BATCH ops and entities like TIME_TRACKING,
|
||||
* SIMPLE_COUNTER, TASK_REPEAT_CFG, PLANNER, BOARD — is user work an incoming
|
||||
* full-state import would silently discard (imports drop concurrent ops by
|
||||
* design), so it must count as meaningful and trigger the conflict dialog.
|
||||
*/
|
||||
const DISCARDABLE_ENTITY_TYPES = new Set(['GLOBAL_CONFIG', 'MIGRATION', 'RECOVERY']);
|
||||
|
||||
export interface IncomingFullStateConflictGateResult {
|
||||
fullStateOp?: Operation;
|
||||
pendingOps: OperationLogEntry[];
|
||||
|
|
@ -45,10 +34,11 @@ export class SyncImportConflictGateService {
|
|||
private writeFlushService = inject(OperationWriteFlushService);
|
||||
|
||||
/**
|
||||
* Every pending op is user work unless it is on a DISCARDABLE_ENTITY_TYPES
|
||||
* entity or is an onboarding example-task create. Full-state ops are always
|
||||
* meaningful because applying a newer full-state op can invalidate their
|
||||
* local import/repair semantics.
|
||||
* Every pending op is user work unless it is an onboarding example-task create.
|
||||
* Entity-wide exemptions are unsafe: GLOBAL_CONFIG contains synced preferences,
|
||||
* while MIGRATION and RECOVERY genesis operations contain the user's full recovered
|
||||
* database. Full-state ops are always meaningful because applying a newer full-state
|
||||
* op can invalidate their local import/repair semantics.
|
||||
*/
|
||||
hasMeaningfulPendingOps(ops: OperationLogEntry[]): boolean {
|
||||
return ops.some((entry) => {
|
||||
|
|
@ -58,16 +48,15 @@ export class SyncImportConflictGateService {
|
|||
if (isExampleTaskCreateOp(entry)) {
|
||||
return false;
|
||||
}
|
||||
return !DISCARDABLE_ENTITY_TYPES.has(entry.op.entityType);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* @param options.preCapturedPendingOps - Pending ops captured BEFORE the upload
|
||||
* round started. The piggyback-upload path MUST pass this: by the time
|
||||
* its gate runs, ops accepted in the same round were already marked
|
||||
* synced, so a live getUnsynced() read would no longer see local work
|
||||
* that the piggybacked import is about to discard.
|
||||
* @param options.preCapturedPendingOps - Exact pending ops selected by the upload
|
||||
* round. The piggyback path unions this snapshot with a live read so it
|
||||
* protects both accepted work from that upload and work created while the
|
||||
* network request was in flight.
|
||||
*/
|
||||
async checkIncomingFullStateConflict(
|
||||
incomingOps: Operation[],
|
||||
|
|
@ -94,8 +83,10 @@ export class SyncImportConflictGateService {
|
|||
await this.writeFlushService.flushPendingWrites();
|
||||
}
|
||||
|
||||
const pendingOps =
|
||||
options.preCapturedPendingOps ?? (await this.opLogStore.getUnsynced());
|
||||
const livePendingOps = await this.opLogStore.getUnsynced();
|
||||
const pendingOps = options.preCapturedPendingOps
|
||||
? this._mergePendingOps(options.preCapturedPendingOps, livePendingOps)
|
||||
: livePendingOps;
|
||||
const hasMeaningfulPending = this.hasMeaningfulPendingOps(pendingOps);
|
||||
// Example-task ops that the caller may reject when it accepts the import silently.
|
||||
// When `hasMeaningfulPending` is true (real work pending alongside example tasks),
|
||||
|
|
@ -105,9 +96,6 @@ export class SyncImportConflictGateService {
|
|||
// These must come from a LIVE read: with a pre-captured snapshot, example ops
|
||||
// accepted earlier in the same upload round are already marked synced and must
|
||||
// not be re-marked rejected by the caller.
|
||||
const livePendingOps = options.preCapturedPendingOps
|
||||
? await this.opLogStore.getUnsynced()
|
||||
: pendingOps;
|
||||
const discardablePendingOpIds = livePendingOps
|
||||
.filter(isExampleTaskCreateOp)
|
||||
.map((entry) => entry.op.id);
|
||||
|
|
@ -148,4 +136,21 @@ export class SyncImportConflictGateService {
|
|||
},
|
||||
};
|
||||
}
|
||||
|
||||
private _mergePendingOps(
|
||||
uploadSnapshot: OperationLogEntry[],
|
||||
livePendingOps: OperationLogEntry[],
|
||||
): OperationLogEntry[] {
|
||||
const merged = [...uploadSnapshot];
|
||||
const seenOpIds = new Set(uploadSnapshot.map((entry) => entry.op.id));
|
||||
|
||||
for (const entry of livePendingOps) {
|
||||
if (!seenOpIds.has(entry.op.id)) {
|
||||
merged.push(entry);
|
||||
seenOpIds.add(entry.op.id);
|
||||
}
|
||||
}
|
||||
|
||||
return merged;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ describe('WsTriggeredDownloadService', () => {
|
|||
|
||||
mockProviderManager = jasmine.createSpyObj(
|
||||
'SyncProviderManager',
|
||||
['getActiveProvider'],
|
||||
['getActiveProvider', 'setSyncStatus'],
|
||||
{
|
||||
isSyncInProgress: false,
|
||||
},
|
||||
|
|
@ -244,9 +244,6 @@ describe('WsTriggeredDownloadService', () => {
|
|||
// reset clears them) or leak into the next session. The service must
|
||||
// be its own session boundary.
|
||||
it('sets sync status ERROR when the download flips the validation latch', fakeAsync(() => {
|
||||
if (mockProviderManager.setSyncStatus === undefined) {
|
||||
mockProviderManager.setSyncStatus = jasmine.createSpy('setSyncStatus');
|
||||
}
|
||||
const latch = TestBed.inject(SyncSessionValidationService);
|
||||
mockSyncService.downloadRemoteOps.and.callFake(async () => {
|
||||
latch.setFailed();
|
||||
|
|
@ -262,9 +259,6 @@ describe('WsTriggeredDownloadService', () => {
|
|||
}));
|
||||
|
||||
it('does not flag ERROR when the download leaves the latch reset', fakeAsync(() => {
|
||||
if (mockProviderManager.setSyncStatus === undefined) {
|
||||
mockProviderManager.setSyncStatus = jasmine.createSpy('setSyncStatus');
|
||||
}
|
||||
const latch = TestBed.inject(SyncSessionValidationService);
|
||||
latch._resetForTest();
|
||||
|
||||
|
|
@ -276,6 +270,19 @@ describe('WsTriggeredDownloadService', () => {
|
|||
expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalledWith('ERROR');
|
||||
}));
|
||||
|
||||
it('sets sync status ERROR when processing is blocked by an incompatible op', fakeAsync(() => {
|
||||
mockSyncService.downloadRemoteOps.and.resolveTo({
|
||||
kind: 'blocked_incompatible',
|
||||
});
|
||||
|
||||
service.start();
|
||||
notification$.next({ latestSeq: 1 });
|
||||
tick(500);
|
||||
flushMicrotasks();
|
||||
|
||||
expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR');
|
||||
}));
|
||||
|
||||
// Defense against stale latch from a prior path: the WS service opens its
|
||||
// own session, which resets the latch up front so the read at the end
|
||||
// reflects only this session's outcome.
|
||||
|
|
|
|||
|
|
@ -151,6 +151,14 @@ export class WsTriggeredDownloadService implements OnDestroy {
|
|||
|
||||
SyncLog.log(`WsTriggeredDownloadService: Download complete. kind=${result.kind}`);
|
||||
|
||||
if (result.kind === 'blocked_incompatible') {
|
||||
SyncLog.warn(
|
||||
'WsTriggeredDownloadService: Download blocked by an incompatible operation',
|
||||
);
|
||||
this._providerManager.setSyncStatus('ERROR');
|
||||
return;
|
||||
}
|
||||
|
||||
if (this._sessionValidation.hasFailed()) {
|
||||
SyncLog.err(
|
||||
'WsTriggeredDownloadService: Post-sync validation failed during WS download — reporting ERROR',
|
||||
|
|
|
|||
|
|
@ -56,7 +56,10 @@ const defineStorePortContract = (
|
|||
} => {
|
||||
const applyOperationsSpy = jasmine
|
||||
.createSpy('applyOperations')
|
||||
.and.resolveTo(result);
|
||||
.and.callFake(async (ops: Operation[], options) => {
|
||||
await options?.onReducersCommitted?.(ops);
|
||||
return result;
|
||||
});
|
||||
|
||||
return {
|
||||
applier: { applyOperations: applyOperationsSpy },
|
||||
|
|
@ -97,7 +100,10 @@ const defineStorePortContract = (
|
|||
applier,
|
||||
});
|
||||
|
||||
expect(applyOperationsSpy).toHaveBeenCalledOnceWith([newOp]);
|
||||
expect(applyOperationsSpy).toHaveBeenCalledOnceWith(
|
||||
[newOp],
|
||||
jasmine.objectContaining({ onReducersCommitted: jasmine.any(Function) }),
|
||||
);
|
||||
expect(result.appendedOps).toEqual([newOp]);
|
||||
expect(result.skippedCount).toBe(1);
|
||||
expect(result.appliedOps).toEqual([newOp]);
|
||||
|
|
@ -141,7 +147,7 @@ const defineStorePortContract = (
|
|||
});
|
||||
|
||||
expect(result.failedOp).toEqual({ op: failedOp, error });
|
||||
expect(result.failedOpIds).toEqual([failedOp.id, remainingOp.id]);
|
||||
expect(result.failedOpIds).toEqual([failedOp.id]);
|
||||
|
||||
const appliedEntry = await store.getOpById(appliedOp.id);
|
||||
const failedEntry = await store.getOpById(failedOp.id);
|
||||
|
|
@ -149,12 +155,16 @@ const defineStorePortContract = (
|
|||
expect(appliedEntry?.applicationStatus).toBe('applied');
|
||||
expect(failedEntry?.applicationStatus).toBe('failed');
|
||||
expect(failedEntry?.retryCount).toBe(1);
|
||||
expect(remainingEntry?.applicationStatus).toBe('failed');
|
||||
expect(remainingEntry?.retryCount).toBe(1);
|
||||
expect(remainingEntry?.applicationStatus).toBe('archive_pending');
|
||||
expect(remainingEntry?.retryCount).toBeUndefined();
|
||||
expect(
|
||||
(await store.getFailedRemoteOps()).map((entry) => entry.op.id).sort(),
|
||||
).toEqual([failedOp.id, remainingOp.id].sort());
|
||||
expect(await store.getVectorClock()).toEqual({ clientA: 2 });
|
||||
expect(await store.getVectorClock()).toEqual({
|
||||
clientA: 2,
|
||||
clientB: 4,
|
||||
clientC: 6,
|
||||
});
|
||||
});
|
||||
|
||||
it('should clear older full-state ops and reset the vector clock after an applied remote import', async () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue