mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
* fix(sync): skip file-based conflict dialog for example-only stores (#7985) A fresh file-based client (Dropbox/WebDAV) that seeded onboarding example tasks before its first sync hit a LocalDataConflictError dialog when adopting a populated remote, because hasMeaningfulStateData() counts any task and the isExampleTask marker lives only on the op-log op, not the NgRx Task entity. Derive the pending example-task ids (via the now-shared isExampleTaskCreateOp predicate) and let the file-based store-meaning check ignore them, so an example-only store adopts remote silently — the same smoothing #7976/#7980 gave the SuperSync path (plan §7 "Option 2" for the file-based path). A real task / non-INBOX project / non-system tag / note still reads as meaningful and shows the dialog. hasMeaningfulStateData() gains an optional ignoreTaskIds arg that only narrows the result; omitting it preserves behavior for the #7892 empty-overwrite guard, snapshot/compaction, and first-time-sync callers. Note: the SuperSync upload-pollution residual (Fix C in the #7980 plan) is NOT addressed here — a status-based upload guard is ineffective (hasSyncedOps flips during the pre-upload download) and the correct fix is identity-based cleanup, left as a scoped follow-up. * fix(sync): discard example-task ops on first adoption of populated remote (#7985) The deferred "Fix C" from #7980: a never-synced client that adopts a populated, non-encrypted SuperSync account (seeded by normal upload, so no SYNC_IMPORT fires the incoming-import gate's discard) would upload its 4 onboarding example-task ops onto that remote, propagating throwaway scaffolding to every device. In OperationLogSyncService.uploadPendingOps, when the pre-download-captured isNeverSynced is true AND hasSyncedOps() is now true (this cycle's download just adopted remote ops), reject the pending example-create ops so the upload never sends them. The pre-download capture is what makes this reliable — a live hasSyncedOps() read is useless here because the download flips it mid-cycle (#7980 §10). Stranding safety: discard only from a PRISTINE post-boot batch — example creates plus default GLOBAL_CONFIG writes, nothing else. hasMeaningfulPendingOps() is not sufficient: it ignores Move/Batch (and planner/board/section) ops, which can reference an example task id without being "meaningful". Unlike the incoming- SYNC_IMPORT discard — where SyncImportFilterService drops any concurrent dependent op against the import — this adoption path has no import to filter them, so a surviving reorder Move would strand a dangling reference once its create is rejected. Requiring an all-examples+config batch also preserves "an edited example syncs as real data": any user action (edit, reorder, real task) adds a non-config op and skips the cleanup, so everything uploads together. Scope: stops cross-device propagation (the documented harm). On SuperSync the example tasks remain in this device's local NgRx state (op-based adoption merges rather than replaces); removing them from local state is a separate increment. The file-based path already clears them via snapshot hydration. * test(sync): real-store integration proof for example-task adoption cleanup (#7985) The sync-service unit specs mock opLogStore.hasSyncedOps(), so they only prove the Fix C hook's truth-table — not the load-bearing #7980 §10 claim that the pre-upload download phase persists adopted remote ops with syncedAt (flipping hasSyncedOps() to true) while the local example creates stay unsynced. This integration test exercises the REAL OperationLogStoreService against real IndexedDB: appending a remote-sourced op genuinely flips hasSyncedOps() to true while getUnsynced() still returns the local example creates, the production "pristine post-boot batch" predicate evaluates correctly on real ops, and the real markRejected() removes only the example creates (config survives). Also covers the reorder-Move stranding guard and the empty-server-seeding no-fire case. No docker / SuperSync server needed; complements the (still-recommended) e2e.
This commit is contained in:
Johannes Millan
GitHub
parent
31a45d0f76
commit
3e58b6d8d2
9 changed files with 651 additions and 34 deletions
|
|
@ -260,6 +260,202 @@ describe('OperationLogSyncService', () => {
|
|||
.and.returnValue(Promise.resolve(1)); // Not fresh (has seq)
|
||||
});
|
||||
|
||||
describe('example-task cleanup on first adoption of a populated remote (#7985)', () => {
|
||||
const exampleCreate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 1,
|
||||
op: {
|
||||
id: `ex-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {
|
||||
actionPayload: { task: { id: taskId }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const realTaskCreate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 2,
|
||||
op: {
|
||||
id: `real-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Task] Add Task' as ActionType,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const configOp = (): OperationLogEntry => ({
|
||||
seq: 3,
|
||||
op: {
|
||||
id: 'config-op-1',
|
||||
clientId: 'client-A',
|
||||
actionType: '[Global Config] Update Global Config Section' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
payload: { sectionKey: 'sync' },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
// A reorder of an example task: a Move op (entityType TASK, entityId = the example id)
|
||||
// that hasMeaningfulPendingOps() does NOT flag — would strand a dangling reference if
|
||||
// the cleanup rejected the create but let this Move upload (cross-validated by review).
|
||||
const exampleMove = (taskId: string): OperationLogEntry => ({
|
||||
seq: 4,
|
||||
op: {
|
||||
id: `mov-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Project] Move Task In Backlog/Today' as ActionType,
|
||||
opType: OpType.Move,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const exampleUpdate = (taskId: string): OperationLogEntry => ({
|
||||
seq: 5,
|
||||
op: {
|
||||
id: `upd-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: '[Task] Update Task' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: { actionPayload: { task: { id: taskId } }, entityChanges: [] },
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const mockProvider = { isReady: () => Promise.resolve(true) } as any;
|
||||
|
||||
beforeEach(() => {
|
||||
uploadServiceSpy.uploadPendingOps.and.resolveTo({
|
||||
uploadedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedCount: 0,
|
||||
rejectedOps: [],
|
||||
});
|
||||
opLogStoreSpy.markRejected.and.resolveTo();
|
||||
});
|
||||
|
||||
it('discards untouched example-task ops when a never-synced client adopted a populated remote', async () => {
|
||||
// isNeverSynced captured pre-download = true; hasSyncedOps now true = adopted this cycle.
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleCreate('ex-2'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'ex-op-ex-1',
|
||||
'ex-op-ex-2',
|
||||
]);
|
||||
});
|
||||
|
||||
it('does NOT discard when the client had already synced before this cycle (isNeverSynced false)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: false });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when nothing was adopted this cycle (empty-server seeding, hasSyncedOps false)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(false);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([exampleCreate('ex-1')]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when meaningful user work is also pending (a real task) — keeps everything', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
realTaskCreate('real-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still discards when only default config ops coexist with the example tasks', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
configOp(),
|
||||
exampleCreate('ex-2'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith([
|
||||
'ex-op-ex-1',
|
||||
'ex-op-ex-2',
|
||||
]);
|
||||
});
|
||||
|
||||
it('does NOT discard when a reorder Move of an example task is pending (would otherwise strand a dangling ref)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleMove('ex-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT discard when an example task was edited (its Update keeps the whole batch)', async () => {
|
||||
opLogStoreSpy.hasSyncedOps.and.resolveTo(true);
|
||||
opLogStoreSpy.getUnsynced.and.resolveTo([
|
||||
exampleCreate('ex-1'),
|
||||
exampleUpdate('ex-1'),
|
||||
]);
|
||||
|
||||
await service.uploadPendingOps(mockProvider, { isNeverSynced: true });
|
||||
|
||||
expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('uploadPendingOps', () => {
|
||||
it('should return localWinOpsCreated: 0 when no piggybacked ops', async () => {
|
||||
opLogStoreSpy.getUnsynced.and.returnValue(Promise.resolve([]));
|
||||
|
|
@ -1109,6 +1305,100 @@ describe('OperationLogSyncService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
// #7985: a never-synced file-based client whose store + pending ops contain only
|
||||
// onboarding example tasks must not see the spurious conflict dialog.
|
||||
const exampleCreateEntry = (taskId: string): OperationLogEntry => ({
|
||||
seq: 1,
|
||||
op: {
|
||||
id: `ex-op-${taskId}`,
|
||||
clientId: 'client-A',
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {
|
||||
actionPayload: { task: { id: taskId }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
vectorClock: { clientA: 1 },
|
||||
timestamp: Date.now(),
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: Date.now(),
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const fileSnapshotDownloadResult = {
|
||||
newOps: [],
|
||||
hasMore: false,
|
||||
latestSeq: 0,
|
||||
needsFullStateUpload: false,
|
||||
success: true,
|
||||
providerMode: 'fileSnapshotOps',
|
||||
failedFileCount: 0,
|
||||
snapshotState: { tasks: [{ id: 'remote-task' }] },
|
||||
snapshotVectorClock: { clientB: 5 },
|
||||
latestServerSeq: 1,
|
||||
};
|
||||
|
||||
it('does NOT throw LocalDataConflictError when store + pending ops contain only example tasks (#7985)', async () => {
|
||||
opLogStoreSpy.getUnsynced.and.returnValue(
|
||||
Promise.resolve([exampleCreateEntry('ex-task-1')]),
|
||||
);
|
||||
// Store holds ONLY that example task (the marker lives on the op, not the entity).
|
||||
stateSnapshotServiceSpy.getStateSnapshot.and.returnValue({
|
||||
task: { ids: ['ex-task-1'] },
|
||||
project: { ids: [INBOX_PROJECT.id] },
|
||||
tag: { ids: [TODAY_TAG.id] },
|
||||
note: { ids: [] },
|
||||
} as any);
|
||||
|
||||
const syncHydrationServiceSpy = TestBed.inject(
|
||||
SyncHydrationService,
|
||||
) as jasmine.SpyObj<SyncHydrationService>;
|
||||
syncHydrationServiceSpy.hydrateFromRemoteSync.and.resolveTo();
|
||||
|
||||
downloadServiceSpy.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve(fileSnapshotDownloadResult as any),
|
||||
);
|
||||
|
||||
const mockProvider = {
|
||||
isReady: () => Promise.resolve(true),
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
|
||||
await expectAsync(service.downloadRemoteOps(mockProvider)).toBeResolved();
|
||||
expect(syncHydrationServiceSpy.hydrateFromRemoteSync).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still throws LocalDataConflictError when a real task exists alongside example tasks (#7985)', async () => {
|
||||
opLogStoreSpy.getUnsynced.and.returnValue(
|
||||
Promise.resolve([exampleCreateEntry('ex-task-1')]),
|
||||
);
|
||||
// Store has the example task AND a real one — the real task must still be protected.
|
||||
stateSnapshotServiceSpy.getStateSnapshot.and.returnValue({
|
||||
task: { ids: ['ex-task-1', 'real-task-1'] },
|
||||
project: { ids: [INBOX_PROJECT.id] },
|
||||
tag: { ids: [TODAY_TAG.id] },
|
||||
note: { ids: [] },
|
||||
} as any);
|
||||
|
||||
downloadServiceSpy.downloadRemoteOps.and.returnValue(
|
||||
Promise.resolve(fileSnapshotDownloadResult as any),
|
||||
);
|
||||
|
||||
const mockProvider = {
|
||||
isReady: () => Promise.resolve(true),
|
||||
supportsOperationSync: true,
|
||||
setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(),
|
||||
} as any;
|
||||
|
||||
await expectAsync(service.downloadRemoteOps(mockProvider)).toBeRejectedWith(
|
||||
jasmine.any(LocalDataConflictError),
|
||||
);
|
||||
});
|
||||
|
||||
it('should include correct context in LocalDataConflictError', async () => {
|
||||
const unsyncedEntries: OperationLogEntry[] = [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ import { getDefaultMainModelData } 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';
|
||||
|
||||
/**
|
||||
* Orchestrates synchronization of the Operation Log with remote storage.
|
||||
|
|
@ -197,6 +198,47 @@ export class OperationLogSyncService {
|
|||
return { kind: 'blocked_fresh_client' };
|
||||
}
|
||||
|
||||
// #7985: identity-based example-task cleanup (the deferred "Fix C" of #7980).
|
||||
// `isNeverSyncedAtSyncStart` is captured PRE-download; if hasSyncedOps() is now true,
|
||||
// this cycle's download just ADOPTED a populated remote. A genuinely-fresh client's
|
||||
// onboarding example tasks are throwaway scaffolding every install regenerates;
|
||||
// uploading them onto the adopted remote pollutes it and propagates to every device
|
||||
// (the residual #7980 documented for non-encrypted accounts seeded by normal upload,
|
||||
// where no SYNC_IMPORT fires the incoming-import gate's discard). Reject the example
|
||||
// create ops so the upload never sends them.
|
||||
//
|
||||
// A status read alone is unsafe (download flips hasSyncedOps mid-cycle — #7980 §10);
|
||||
// the PRE-download `isNeverSynced` is what makes this reliable.
|
||||
//
|
||||
// Stranding safety: we discard only from a PRISTINE post-boot batch — example creates
|
||||
// plus the default GLOBAL_CONFIG writes, nothing else. hasMeaningfulPendingOps() is NOT
|
||||
// enough here: it ignores Move/Batch (and planner/board/section) ops, which can reference
|
||||
// an example task id without being "meaningful". Unlike the incoming-SYNC_IMPORT discard
|
||||
// — where SyncImportFilterService drops any concurrent dependent op against the import —
|
||||
// this adoption path has no import to filter them, so a surviving Move would strand a
|
||||
// dangling reference once its create is rejected. Requiring an all-examples+config batch
|
||||
// also preserves the "edited example syncs as real data" property: any user action (edit,
|
||||
// reorder, real task) adds a non-config op and skips the cleanup, so everything uploads.
|
||||
if (isNeverSyncedAtSyncStart && (await this.opLogStore.hasSyncedOps())) {
|
||||
const pendingOps = await this.opLogStore.getUnsynced();
|
||||
const isPristinePostBootBatch = pendingOps.every(
|
||||
(entry) =>
|
||||
isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG',
|
||||
);
|
||||
if (isPristinePostBootBatch) {
|
||||
const exampleOpIds = pendingOps
|
||||
.filter(isExampleTaskCreateOp)
|
||||
.map((entry) => entry.op.id);
|
||||
if (exampleOpIds.length > 0) {
|
||||
await this._discardExampleTaskOps(exampleOpIds);
|
||||
OpLog.normal(
|
||||
`OperationLogSyncService: Discarded ${exampleOpIds.length} untouched example-task ` +
|
||||
'op(s) — never-synced client adopted a populated remote this sync (#7985).',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
|
@ -474,14 +516,24 @@ export class OperationLogSyncService {
|
|||
// SuperSync→Dropbox, only has a config-change op (not "meaningful"), but the
|
||||
// store is full of real data that would be overwritten by old Dropbox state.
|
||||
//
|
||||
// Known limitation (#7985): hasMeaningfulStoreData() counts ANY task, including onboarding
|
||||
// example tasks (they carry the isExampleTask marker only on their op-log ops, not
|
||||
// in NgRx state). So a fresh file-based client (Dropbox/WebDAV) that created example
|
||||
// tasks while sync was disabled — or before a slow first sync completed — can still
|
||||
// hit this dialog. afterInitialSyncDoneStrict$ shrinks but does not close that window.
|
||||
// #7985: hasMeaningfulStoreData() counts ANY task, including onboarding example
|
||||
// tasks (they carry the isExampleTask marker only on their op-log ops, not in NgRx
|
||||
// state). Derive the example task ids from the pending example-create ops and let
|
||||
// the store check ignore them, so a fresh file-based client (Dropbox/WebDAV) that
|
||||
// only has example tasks adopts remote silently instead of hitting the spurious
|
||||
// conflict dialog #7976/#7980 removed for the SuperSync path. Scope: this fires only
|
||||
// while the example create ops are still pending (a never-synced file client) —
|
||||
// exactly the reachable scenario. A real (non-example) task / non-INBOX project /
|
||||
// non-system tag / note still reads as meaningful and shows the dialog.
|
||||
const exampleTaskIds = new Set(
|
||||
unsyncedOps
|
||||
.filter(isExampleTaskCreateOp)
|
||||
.map((entry) => entry.op.entityId)
|
||||
.filter((id): id is string => id !== undefined),
|
||||
);
|
||||
const hasMeaningfulUserData =
|
||||
this.syncImportConflictGateService.hasMeaningfulPendingOps(unsyncedOps) ||
|
||||
this.syncLocalStateService.hasMeaningfulStoreData();
|
||||
this.syncLocalStateService.hasMeaningfulStoreData(exampleTaskIds);
|
||||
|
||||
if (hasMeaningfulUserData) {
|
||||
// Client has meaningful user data - show conflict dialog
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { OperationLogStoreService } from '../persistence/operation-log-store.service';
|
||||
import {
|
||||
ActionType,
|
||||
extractActionPayload,
|
||||
FULL_STATE_OP_TYPES,
|
||||
Operation,
|
||||
OperationLogEntry,
|
||||
|
|
@ -10,30 +8,10 @@ import {
|
|||
} from '../core/operation.types';
|
||||
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';
|
||||
|
||||
const USER_ENTITY_TYPES = new Set(['TASK', 'PROJECT', 'TAG', 'NOTE']);
|
||||
|
||||
/**
|
||||
* Startup example/onboarding tasks are generated locally on first run (see
|
||||
* ExampleTasksService) and must not count as "meaningful local work" that would block
|
||||
* an incoming SYNC_IMPORT. This only ever runs against local pending ops from
|
||||
* getUnsynced() — never against incoming remote ops — so a remote-supplied
|
||||
* `isExampleTask` flag cannot be used to bypass the conflict dialog.
|
||||
*/
|
||||
const isExampleTaskCreateOp = (entry: OperationLogEntry): boolean => {
|
||||
const { op } = entry;
|
||||
if (
|
||||
op.actionType !== ActionType.TASK_SHARED_ADD ||
|
||||
op.opType !== OpType.Create ||
|
||||
op.entityType !== 'TASK'
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const actionPayload = extractActionPayload(op.payload);
|
||||
return actionPayload['isExampleTask'] === true;
|
||||
};
|
||||
|
||||
export interface IncomingFullStateConflictGateResult {
|
||||
fullStateOp?: Operation;
|
||||
pendingOps: OperationLogEntry[];
|
||||
|
|
|
|||
|
|
@ -22,7 +22,13 @@ export class SyncLocalStateService {
|
|||
return !snapshot && lastSeq === 0;
|
||||
}
|
||||
|
||||
hasMeaningfulStoreData(): boolean {
|
||||
/**
|
||||
* @param ignoreTaskIds Optional task ids to exclude from the "has a task?" check.
|
||||
* The file-based conflict gate passes the ids of pending onboarding example tasks so
|
||||
* an example-only store is not treated as meaningful (#7985). Omitting it preserves
|
||||
* the original behavior for every other caller.
|
||||
*/
|
||||
hasMeaningfulStoreData(ignoreTaskIds?: ReadonlySet<string>): boolean {
|
||||
const snapshot = this.stateSnapshotService.getStateSnapshot();
|
||||
|
||||
if (!snapshot) {
|
||||
|
|
@ -32,7 +38,7 @@ export class SyncLocalStateService {
|
|||
return false;
|
||||
}
|
||||
|
||||
return hasMeaningfulStateData(snapshot);
|
||||
return hasMeaningfulStateData(snapshot, ignoreTaskIds);
|
||||
}
|
||||
|
||||
confirmFreshClientSync(opCount: number): boolean {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,150 @@
|
|||
/* eslint-disable @typescript-eslint/naming-convention */
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { OperationLogStoreService } from '../../persistence/operation-log-store.service';
|
||||
import { ActionType, OperationLogEntry, OpType } from '../../core/operation.types';
|
||||
import { isExampleTaskCreateOp } from '../../validation/is-example-task-op.util';
|
||||
import { resetTestUuidCounter, TestClient } from './helpers/test-client.helper';
|
||||
|
||||
/**
|
||||
* Integration proof for the #7985 "Fix C" cleanup (the hook in
|
||||
* OperationLogSyncService.uploadPendingOps that rejects example-task ops when a
|
||||
* never-synced client adopts a populated remote).
|
||||
*
|
||||
* The sync-service unit specs MOCK opLogStore.hasSyncedOps(), so they can only prove the
|
||||
* hook's truth-table — not the load-bearing #7980 §10 claim that the (pre-upload) download
|
||||
* phase persists adopted remote ops with `syncedAt`, flipping hasSyncedOps() to true BEFORE
|
||||
* the upload runs while the local example creates stay unsynced. These tests exercise the
|
||||
* REAL store so that exact sequencing — and the real getUnsynced()/markRejected() the hook
|
||||
* composes, plus the production "pristine post-boot batch" predicate against real ops — is
|
||||
* verified end-to-end (no docker / SuperSync server needed).
|
||||
*/
|
||||
describe('Example-task adoption cleanup mechanic (integration)', () => {
|
||||
let store: OperationLogStoreService;
|
||||
const local = new TestClient('client-local');
|
||||
const remote = new TestClient('client-remote');
|
||||
|
||||
// Mirrors the production gate in operation-log-sync.service.ts uploadPendingOps.
|
||||
const isPristinePostBootBatch = (ops: OperationLogEntry[]): boolean =>
|
||||
ops.every(
|
||||
(entry) => isExampleTaskCreateOp(entry) || entry.op.entityType === 'GLOBAL_CONFIG',
|
||||
);
|
||||
|
||||
const exampleCreate = (taskId: string): ReturnType<TestClient['createOperation']> =>
|
||||
local.createOperation({
|
||||
actionType: ActionType.TASK_SHARED_ADD,
|
||||
opType: OpType.Create,
|
||||
entityType: 'TASK',
|
||||
entityId: taskId,
|
||||
payload: {
|
||||
actionPayload: { task: { id: taskId }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
});
|
||||
|
||||
const configOp = (): ReturnType<TestClient['createOperation']> =>
|
||||
local.createOperation({
|
||||
actionType: ActionType.GLOBAL_CONFIG_UPDATE_SECTION,
|
||||
opType: OpType.Update,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'sync',
|
||||
payload: { sectionKey: 'sync' },
|
||||
});
|
||||
|
||||
// A real op adopted FROM a populated remote (e.g. a task from the account being joined).
|
||||
const adoptedRemoteOp = (): ReturnType<TestClient['createOperation']> =>
|
||||
remote.createOperation({
|
||||
actionType: '[Task] Update Task' as ActionType,
|
||||
opType: OpType.Update,
|
||||
entityType: 'TASK',
|
||||
entityId: 'remote-task-1',
|
||||
payload: { actionPayload: { task: { id: 'remote-task-1' } }, entityChanges: [] },
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
TestBed.configureTestingModule({ providers: [OperationLogStoreService] });
|
||||
store = TestBed.inject(OperationLogStoreService);
|
||||
await store.init();
|
||||
await store._clearAllDataForTesting();
|
||||
resetTestUuidCounter();
|
||||
});
|
||||
|
||||
it('a never-synced client: hasSyncedOps() is false with only local example+config ops pending', async () => {
|
||||
await store.append(exampleCreate('ex-1'), 'local');
|
||||
await store.append(exampleCreate('ex-2'), 'local');
|
||||
await store.append(configOp(), 'local');
|
||||
|
||||
// Pre-download: nothing synced. (This is what sync-wrapper captures as isNeverSynced.)
|
||||
expect(await store.hasSyncedOps()).toBe(false);
|
||||
});
|
||||
|
||||
it('adopting a populated remote really flips hasSyncedOps() while example creates stay unsynced, and the cleanup rejects only the examples', async () => {
|
||||
const ex1 = exampleCreate('ex-1');
|
||||
const ex2 = exampleCreate('ex-2');
|
||||
const cfg = configOp();
|
||||
await store.append(ex1, 'local');
|
||||
await store.append(ex2, 'local');
|
||||
await store.append(cfg, 'local');
|
||||
|
||||
// Simulate the download phase adopting the populated remote: a remote op is persisted
|
||||
// with syncedAt set. THIS is the #7980 §10 sequencing the unit tests mock.
|
||||
await store.append(adoptedRemoteOp(), 'remote');
|
||||
|
||||
// The live read the hook makes at upload time is now true — purely from the download.
|
||||
expect(await store.hasSyncedOps()).toBe(true);
|
||||
|
||||
// The example creates are still pending (local, unsynced); the remote op is not pending.
|
||||
const pending = await store.getUnsynced();
|
||||
const pendingIds = pending.map((e) => e.op.id);
|
||||
expect(pendingIds).toContain(ex1.id);
|
||||
expect(pendingIds).toContain(ex2.id);
|
||||
expect(pendingIds).toContain(cfg.id);
|
||||
expect(pendingIds.length).toBe(3);
|
||||
|
||||
// Production gate evaluates true against the real pending batch.
|
||||
expect(isPristinePostBootBatch(pending)).toBe(true);
|
||||
|
||||
// The hook's discard (real markRejected) removes ONLY the example creates.
|
||||
const exampleOpIds = pending.filter(isExampleTaskCreateOp).map((e) => e.op.id);
|
||||
await store.markRejected(exampleOpIds);
|
||||
|
||||
const after = await store.getUnsynced();
|
||||
const afterIds = after.map((e) => e.op.id);
|
||||
expect(afterIds).not.toContain(ex1.id);
|
||||
expect(afterIds).not.toContain(ex2.id);
|
||||
// The config op survives and would still upload (not throwaway scaffolding).
|
||||
expect(afterIds).toContain(cfg.id);
|
||||
});
|
||||
|
||||
it('a pending reorder Move of an example task breaks the pristine-batch gate (cleanup correctly skips → no stranding)', async () => {
|
||||
await store.append(exampleCreate('ex-1'), 'local');
|
||||
// A reorder Move (entityType TASK, OpType.Move) — NOT flagged by hasMeaningfulPendingOps,
|
||||
// but it references the example task id, so rejecting the create would strand it.
|
||||
await store.append(
|
||||
local.createOperation({
|
||||
actionType: '[Project] Move Task in Today' as ActionType,
|
||||
opType: OpType.Move,
|
||||
entityType: 'TASK',
|
||||
entityId: 'ex-1',
|
||||
payload: {},
|
||||
}),
|
||||
'local',
|
||||
);
|
||||
await store.append(adoptedRemoteOp(), 'remote');
|
||||
|
||||
expect(await store.hasSyncedOps()).toBe(true);
|
||||
const pending = await store.getUnsynced();
|
||||
// The gate is false → the production hook skips the discard, so nothing is rejected and
|
||||
// the example create uploads alongside its Move (no dangling reference on the server).
|
||||
expect(isPristinePostBootBatch(pending)).toBe(false);
|
||||
});
|
||||
|
||||
it('empty-server seeding: with nothing adopted, hasSyncedOps() stays false (cleanup does not fire)', async () => {
|
||||
await store.append(exampleCreate('ex-1'), 'local');
|
||||
await store.append(configOp(), 'local');
|
||||
|
||||
// No remote op appended (download returned no_new_ops on an empty server).
|
||||
expect(await store.hasSyncedOps()).toBe(false);
|
||||
// isNeverSynced && hasSyncedOps() === true is the hook's trigger; the second term is
|
||||
// false here, so a first device seeding an empty server keeps its example ops.
|
||||
});
|
||||
});
|
||||
|
|
@ -66,4 +66,35 @@ describe('hasMeaningfulStateData', () => {
|
|||
false,
|
||||
);
|
||||
});
|
||||
|
||||
describe('with ignoreTaskIds (#7985)', () => {
|
||||
it('returns false when the only tasks are in the ignore set (example-only store)', () => {
|
||||
const s = initialState();
|
||||
s.task = { ids: ['ex1', 'ex2'], entities: {} };
|
||||
expect(hasMeaningfulStateData(s, new Set(['ex1', 'ex2']))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when an unignored real task remains', () => {
|
||||
const s = initialState();
|
||||
s.task = { ids: ['ex1', 'real1'], entities: {} };
|
||||
expect(hasMeaningfulStateData(s, new Set(['ex1']))).toBe(true);
|
||||
});
|
||||
|
||||
it('still returns true for a non-INBOX project even if all tasks are ignored', () => {
|
||||
const s = initialState();
|
||||
s.task = { ids: ['ex1'], entities: {} };
|
||||
s.project = { ids: [INBOX_PROJECT.id, 'p1'], entities: {} };
|
||||
expect(hasMeaningfulStateData(s, new Set(['ex1']))).toBe(true);
|
||||
});
|
||||
|
||||
// Locks the #7892 empty-overwrite guard / snapshot / compaction callers: passing no
|
||||
// ignore set (or an empty one) must behave exactly as before.
|
||||
it('behaves identically to the no-arg call when ignoreTaskIds is undefined or empty', () => {
|
||||
const s = initialState();
|
||||
s.task = { ids: ['t1'], entities: {} };
|
||||
expect(hasMeaningfulStateData(s)).toBe(true);
|
||||
expect(hasMeaningfulStateData(s, undefined)).toBe(true);
|
||||
expect(hasMeaningfulStateData(s, new Set())).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,15 +27,29 @@ const isEntityState = (obj: unknown): obj is { ids: string[] } =>
|
|||
*
|
||||
* Accepts an arbitrary object so callers can pass an NgRx snapshot, a loaded
|
||||
* state cache, or a remote payload without type juggling.
|
||||
*
|
||||
* `ignoreTaskIds` (optional) lets a caller exclude specific task ids from the "has a
|
||||
* task?" check — used by the file-based sync conflict gate to treat a store containing
|
||||
* only onboarding example tasks as non-meaningful (#7985). It only ever NARROWS the
|
||||
* result; omitting it preserves the original behavior for every other caller (the
|
||||
* #7892 empty-overwrite guard, snapshot/compaction, first-time-sync detection).
|
||||
*/
|
||||
export const hasMeaningfulStateData = (state: unknown): boolean => {
|
||||
export const hasMeaningfulStateData = (
|
||||
state: unknown,
|
||||
ignoreTaskIds?: ReadonlySet<string>,
|
||||
): boolean => {
|
||||
if (!state || typeof state !== 'object') {
|
||||
return false;
|
||||
}
|
||||
const s = state as Record<string, unknown>;
|
||||
|
||||
if (isEntityState(s.task) && s.task.ids.length > 0) {
|
||||
return true;
|
||||
if (isEntityState(s.task)) {
|
||||
const meaningfulTaskIds = ignoreTaskIds
|
||||
? s.task.ids.filter((id) => !ignoreTaskIds.has(id))
|
||||
: s.task.ids;
|
||||
if (meaningfulTaskIds.length > 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
if (isEntityState(s.project) && s.project.ids.some((id) => id !== INBOX_PROJECT.id)) {
|
||||
|
|
|
|||
65
src/app/op-log/validation/is-example-task-op.util.spec.ts
Normal file
65
src/app/op-log/validation/is-example-task-op.util.spec.ts
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
import { isExampleTaskCreateOp } from './is-example-task-op.util';
|
||||
import { ActionType, OperationLogEntry, OpType } from '../core/operation.types';
|
||||
|
||||
const entry = (over: {
|
||||
actionType?: ActionType | string;
|
||||
opType?: OpType;
|
||||
entityType?: string;
|
||||
payload?: unknown;
|
||||
}): OperationLogEntry =>
|
||||
({
|
||||
seq: 1,
|
||||
op: {
|
||||
id: 'op-1',
|
||||
clientId: 'client-1',
|
||||
actionType: (over.actionType ?? ActionType.TASK_SHARED_ADD) as ActionType,
|
||||
opType: over.opType ?? OpType.Create,
|
||||
entityType: over.entityType ?? 'TASK',
|
||||
entityId: 'task-1',
|
||||
payload: over.payload ?? {
|
||||
actionPayload: { task: { id: 'task-1' }, isExampleTask: true },
|
||||
entityChanges: [],
|
||||
},
|
||||
vectorClock: { client1: 1 },
|
||||
timestamp: 1,
|
||||
schemaVersion: 1,
|
||||
},
|
||||
appliedAt: 1,
|
||||
source: 'local',
|
||||
}) as OperationLogEntry;
|
||||
|
||||
describe('isExampleTaskCreateOp', () => {
|
||||
it('returns true for a TASK_SHARED_ADD Create op carrying the isExampleTask marker', () => {
|
||||
expect(isExampleTaskCreateOp(entry({}))).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for a task create without the marker', () => {
|
||||
expect(
|
||||
isExampleTaskCreateOp(
|
||||
entry({
|
||||
payload: { actionPayload: { task: { id: 'task-1' } }, entityChanges: [] },
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for a non-TASK_SHARED_ADD action even with the marker', () => {
|
||||
expect(isExampleTaskCreateOp(entry({ actionType: '[Task] Add Task' }))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for an Update op even with the marker', () => {
|
||||
expect(isExampleTaskCreateOp(entry({ opType: OpType.Update }))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for a Delete op even with the marker', () => {
|
||||
expect(isExampleTaskCreateOp(entry({ opType: OpType.Delete }))).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for a non-TASK entity type', () => {
|
||||
expect(isExampleTaskCreateOp(entry({ entityType: 'PROJECT' }))).toBe(false);
|
||||
});
|
||||
|
||||
it('does not throw on an empty payload (treats it as non-example)', () => {
|
||||
expect(isExampleTaskCreateOp(entry({ payload: {} }))).toBe(false);
|
||||
});
|
||||
});
|
||||
31
src/app/op-log/validation/is-example-task-op.util.ts
Normal file
31
src/app/op-log/validation/is-example-task-op.util.ts
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
import {
|
||||
ActionType,
|
||||
extractActionPayload,
|
||||
OperationLogEntry,
|
||||
OpType,
|
||||
} from '../core/operation.types';
|
||||
|
||||
/**
|
||||
* Startup example/onboarding tasks are generated locally on first run (see
|
||||
* ExampleTasksService) and carry an `isExampleTask: true` marker on their op-log op
|
||||
* payload only — never on the NgRx Task entity. They must not count as "meaningful
|
||||
* local work" that would block an incoming SYNC_IMPORT or a file-based snapshot adopt
|
||||
* (see #7985 / #7976 / #7980).
|
||||
*
|
||||
* This predicate is only ever evaluated against LOCAL pending ops from getUnsynced() —
|
||||
* never against incoming remote ops — so a remote-supplied `isExampleTask` flag can
|
||||
* never be used to bypass a conflict dialog.
|
||||
*/
|
||||
export const isExampleTaskCreateOp = (entry: OperationLogEntry): boolean => {
|
||||
const { op } = entry;
|
||||
if (
|
||||
op.actionType !== ActionType.TASK_SHARED_ADD ||
|
||||
op.opType !== OpType.Create ||
|
||||
op.entityType !== 'TASK'
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const actionPayload = extractActionPayload(op.payload);
|
||||
return actionPayload['isExampleTask'] === true;
|
||||
};
|
||||
Loading…
Add table
Add a link
Reference in a new issue