mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-01-23 10:45:57 +00:00
fix(sync): multi-tab vector clock staleness + shared code extraction
Bug fix: - Fix vector clock cache staleness in multi-tab scenarios by clearing cache when acquiring operation write lock. Each browser tab has its own in-memory cache, so Tab B's cache could be stale if Tab A wrote while Tab B was waiting for the lock. Shared code extraction (client/server consistency): - Extract vector clock comparison to @sp/shared-schema - Client wraps shared impl with null handling - Server imports directly from shared - Extract entity types to @sp/shared-schema - Single source of truth for ENTITY_TYPES array - Removes duplicated "must match" comments Files: - packages/shared-schema/src/vector-clock.ts (new) - packages/shared-schema/src/entity-types.ts (new) - src/app/op-log/store/operation-log-store.service.ts (cache clear) - src/app/op-log/capture/operation-log.effects.ts (call cache clear)
This commit is contained in:
parent
9ddced5648
commit
cbeecfd39a
10 changed files with 239 additions and 145 deletions
42
packages/shared-schema/src/entity-types.ts
Normal file
42
packages/shared-schema/src/entity-types.ts
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
/**
|
||||
* Entity Types for distributed synchronization.
|
||||
*
|
||||
* These are the valid entity types for operations in the sync system.
|
||||
* Shared between client and server to ensure consistency.
|
||||
*
|
||||
* IMPORTANT: This module is shared between client and server.
|
||||
* Any changes must be compatible with both environments.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Valid entity types for operations.
|
||||
* Operations with unknown entity types will be rejected by the server.
|
||||
*/
|
||||
export const ENTITY_TYPES = [
|
||||
'TASK',
|
||||
'PROJECT',
|
||||
'TAG',
|
||||
'NOTE',
|
||||
'GLOBAL_CONFIG',
|
||||
'SIMPLE_COUNTER',
|
||||
'WORK_CONTEXT',
|
||||
'TIME_TRACKING',
|
||||
'TASK_REPEAT_CFG',
|
||||
'ISSUE_PROVIDER',
|
||||
'PLANNER',
|
||||
'MENU_TREE',
|
||||
'METRIC',
|
||||
'BOARD',
|
||||
'REMINDER',
|
||||
'PLUGIN_USER_DATA',
|
||||
'PLUGIN_METADATA',
|
||||
'MIGRATION',
|
||||
'RECOVERY', // For disaster recovery imports
|
||||
'ALL', // For full state imports (sync, backup)
|
||||
] as const;
|
||||
|
||||
/**
|
||||
* Entity type - identifies the kind of data entity being operated on.
|
||||
* Derived from ENTITY_TYPES array for single source of truth.
|
||||
*/
|
||||
export type EntityType = (typeof ENTITY_TYPES)[number];
|
||||
|
|
@ -26,3 +26,11 @@ export {
|
|||
|
||||
// Migration registry (for inspection/debugging)
|
||||
export { MIGRATIONS } from './migrations/index.js';
|
||||
|
||||
// Vector clock types and comparison (shared between client and server)
|
||||
export type { VectorClock, VectorClockComparison } from './vector-clock.js';
|
||||
export { compareVectorClocks, mergeVectorClocks } from './vector-clock.js';
|
||||
|
||||
// Entity types (shared between client and server)
|
||||
export type { EntityType } from './entity-types.js';
|
||||
export { ENTITY_TYPES } from './entity-types.js';
|
||||
|
|
|
|||
80
packages/shared-schema/src/vector-clock.ts
Normal file
80
packages/shared-schema/src/vector-clock.ts
Normal file
|
|
@ -0,0 +1,80 @@
|
|||
/**
|
||||
* Vector Clock types and comparison functions for distributed synchronization.
|
||||
*
|
||||
* A vector clock is a data structure used to determine the partial ordering of events
|
||||
* in a distributed system and detect causality violations.
|
||||
*
|
||||
* Each process/device maintains its own component in the vector, incrementing it
|
||||
* on local updates. This allows us to determine if two states are:
|
||||
* - EQUAL: Same vector values
|
||||
* - LESS_THAN: A happened before B
|
||||
* - GREATER_THAN: B happened before A
|
||||
* - CONCURRENT: Neither happened before the other (true conflict)
|
||||
*
|
||||
* IMPORTANT: This module is shared between client and server.
|
||||
* Any changes must be compatible with both environments.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Vector clock data structure.
|
||||
* Maps client IDs to their respective clock values.
|
||||
*/
|
||||
export interface VectorClock {
|
||||
[clientId: string]: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of comparing two vector clocks.
|
||||
*/
|
||||
export type VectorClockComparison = 'EQUAL' | 'LESS_THAN' | 'GREATER_THAN' | 'CONCURRENT';
|
||||
|
||||
/**
|
||||
* Compare two vector clocks to determine their relationship.
|
||||
*
|
||||
* CRITICAL: This algorithm must produce identical results on client and server.
|
||||
* Both implementations import from this shared module to ensure consistency.
|
||||
*
|
||||
* @param a First vector clock
|
||||
* @param b Second vector clock
|
||||
* @returns The comparison result
|
||||
*/
|
||||
export const compareVectorClocks = (
|
||||
a: VectorClock,
|
||||
b: VectorClock,
|
||||
): VectorClockComparison => {
|
||||
const allKeys = new Set([...Object.keys(a), ...Object.keys(b)]);
|
||||
|
||||
let aGreater = false;
|
||||
let bGreater = false;
|
||||
|
||||
for (const key of allKeys) {
|
||||
const aVal = a[key] ?? 0;
|
||||
const bVal = b[key] ?? 0;
|
||||
|
||||
if (aVal > bVal) aGreater = true;
|
||||
if (bVal > aVal) bGreater = true;
|
||||
}
|
||||
|
||||
if (aGreater && bGreater) return 'CONCURRENT';
|
||||
if (aGreater) return 'GREATER_THAN';
|
||||
if (bGreater) return 'LESS_THAN';
|
||||
return 'EQUAL';
|
||||
};
|
||||
|
||||
/**
|
||||
* Merge two vector clocks, taking the maximum value for each client.
|
||||
* Creates a new clock that dominates both inputs.
|
||||
*
|
||||
* @param a First vector clock
|
||||
* @param b Second vector clock
|
||||
* @returns A new merged vector clock
|
||||
*/
|
||||
export const mergeVectorClocks = (a: VectorClock, b: VectorClock): VectorClock => {
|
||||
const merged: VectorClock = { ...a };
|
||||
|
||||
for (const [key, value] of Object.entries(b)) {
|
||||
merged[key] = Math.max(merged[key] ?? 0, value);
|
||||
}
|
||||
|
||||
return merged;
|
||||
};
|
||||
|
|
@ -13,34 +13,15 @@ import {
|
|||
sanitizeVectorClock,
|
||||
validatePayload,
|
||||
} from '../sync.types';
|
||||
import { ENTITY_TYPES } from '@sp/shared-schema';
|
||||
|
||||
/**
|
||||
* Valid entity types for operations.
|
||||
* Must match the EntityType union in the client's operation.types.ts.
|
||||
* Uses shared ENTITY_TYPES from @sp/shared-schema to ensure client/server consistency.
|
||||
* Operations with unknown entity types will be rejected.
|
||||
* Typed as Set<string> since we're validating unknown input strings.
|
||||
*/
|
||||
export const ALLOWED_ENTITY_TYPES = new Set([
|
||||
'TASK',
|
||||
'PROJECT',
|
||||
'TAG',
|
||||
'NOTE',
|
||||
'GLOBAL_CONFIG',
|
||||
'TIME_TRACKING',
|
||||
'SIMPLE_COUNTER',
|
||||
'WORK_CONTEXT',
|
||||
'TASK_REPEAT_CFG',
|
||||
'ISSUE_PROVIDER',
|
||||
'PLANNER',
|
||||
'MENU_TREE',
|
||||
'METRIC',
|
||||
'BOARD',
|
||||
'REMINDER',
|
||||
'MIGRATION',
|
||||
'RECOVERY',
|
||||
'ALL',
|
||||
'PLUGIN_USER_DATA',
|
||||
'PLUGIN_METADATA',
|
||||
]);
|
||||
export const ALLOWED_ENTITY_TYPES: Set<string> = new Set(ENTITY_TYPES);
|
||||
|
||||
export interface ValidationResult {
|
||||
valid: boolean;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,12 @@
|
|||
import { Logger } from '../logger';
|
||||
import {
|
||||
VectorClock,
|
||||
VectorClockComparison,
|
||||
compareVectorClocks,
|
||||
} from '@sp/shared-schema';
|
||||
|
||||
// Re-export for consumers of this module
|
||||
export { VectorClock, VectorClockComparison, compareVectorClocks };
|
||||
|
||||
// Structured error codes for client handling
|
||||
export const SYNC_ERROR_CODES = {
|
||||
|
|
@ -50,21 +58,8 @@ export const OP_TYPES = [
|
|||
|
||||
export type OpType = (typeof OP_TYPES)[number];
|
||||
|
||||
export type VectorClock = Record<string, number>;
|
||||
|
||||
/**
|
||||
* Compare two vector clocks.
|
||||
*
|
||||
* SYNC NOTE: This algorithm must match the client implementation at:
|
||||
* src/app/core/util/vector-clock.ts - compareVectorClocks()
|
||||
*
|
||||
* Returns:
|
||||
* - 'LESS_THAN': a happened before b
|
||||
* - 'GREATER_THAN': b happened before a
|
||||
* - 'EQUAL': clocks are identical
|
||||
* - 'CONCURRENT': neither happened before the other (conflict!)
|
||||
*/
|
||||
export type VectorClockComparison = 'LESS_THAN' | 'GREATER_THAN' | 'EQUAL' | 'CONCURRENT';
|
||||
// VectorClock, VectorClockComparison, and compareVectorClocks are imported from @sp/shared-schema
|
||||
// and re-exported above. This ensures client and server use identical implementations.
|
||||
|
||||
/**
|
||||
* Validates and sanitizes a vector clock.
|
||||
|
|
@ -122,32 +117,7 @@ export const sanitizeVectorClock = (
|
|||
return { valid: true, clock: sanitized };
|
||||
};
|
||||
|
||||
/**
|
||||
* SYNC NOTE: This algorithm must match the client implementation at:
|
||||
* src/app/core/util/vector-clock.ts - compareVectorClocks()
|
||||
*/
|
||||
export const compareVectorClocks = (
|
||||
a: VectorClock,
|
||||
b: VectorClock,
|
||||
): VectorClockComparison => {
|
||||
const allKeys = new Set([...Object.keys(a), ...Object.keys(b)]);
|
||||
|
||||
let aGreater = false;
|
||||
let bGreater = false;
|
||||
|
||||
for (const key of allKeys) {
|
||||
const aVal = a[key] ?? 0;
|
||||
const bVal = b[key] ?? 0;
|
||||
|
||||
if (aVal > bVal) aGreater = true;
|
||||
if (bVal > aVal) bGreater = true;
|
||||
}
|
||||
|
||||
if (aGreater && bGreater) return 'CONCURRENT';
|
||||
if (aGreater) return 'GREATER_THAN';
|
||||
if (bGreater) return 'LESS_THAN';
|
||||
return 'EQUAL';
|
||||
};
|
||||
// compareVectorClocks is imported from @sp/shared-schema (see imports at top of file)
|
||||
|
||||
export interface Operation {
|
||||
id: string;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,9 @@
|
|||
import { PFLog } from '../log';
|
||||
import {
|
||||
VectorClock as SharedVectorClock,
|
||||
compareVectorClocks as sharedCompareVectorClocks,
|
||||
mergeVectorClocks as sharedMergeVectorClocks,
|
||||
} from '@sp/shared-schema';
|
||||
|
||||
/**
|
||||
* Vector Clock implementation for distributed synchronization
|
||||
|
|
@ -12,18 +17,20 @@ import { PFLog } from '../log';
|
|||
* - LESS_THAN: A happened before B
|
||||
* - GREATER_THAN: B happened before A
|
||||
* - CONCURRENT: Neither happened before the other (true conflict)
|
||||
*
|
||||
* IMPORTANT: Core comparison logic is shared with the server via @sp/shared-schema.
|
||||
* This file wraps the shared logic with null handling for client-side use.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Vector clock data structure
|
||||
* Maps client IDs to their respective clock values
|
||||
*/
|
||||
export interface VectorClock {
|
||||
[clientId: string]: number;
|
||||
}
|
||||
export type VectorClock = SharedVectorClock;
|
||||
|
||||
/**
|
||||
* Result of comparing two vector clocks
|
||||
* Result of comparing two vector clocks.
|
||||
* Uses enum for client-side ergonomics, values match shared string literals.
|
||||
*/
|
||||
export enum VectorClockComparison {
|
||||
EQUAL = 'EQUAL',
|
||||
|
|
@ -112,10 +119,11 @@ export const sanitizeVectorClock = (clock: any): VectorClock => {
|
|||
};
|
||||
|
||||
/**
|
||||
* Compare two vector clocks to determine their relationship
|
||||
* Compare two vector clocks to determine their relationship.
|
||||
*
|
||||
* SYNC NOTE: This algorithm must match the server implementation at:
|
||||
* packages/super-sync-server/src/sync/sync.types.ts - compareVectorClocks()
|
||||
* Uses the shared implementation from @sp/shared-schema to ensure
|
||||
* client and server produce identical results. This wrapper adds
|
||||
* null/undefined handling for client-side convenience.
|
||||
*
|
||||
* @param a First vector clock
|
||||
* @param b Second vector clock
|
||||
|
|
@ -125,56 +133,20 @@ export const compareVectorClocks = (
|
|||
a: VectorClock | null | undefined,
|
||||
b: VectorClock | null | undefined,
|
||||
): VectorClockComparison => {
|
||||
// Handle null/undefined cases
|
||||
// Handle null/undefined cases (shared implementation requires non-null)
|
||||
if (isVectorClockEmpty(a) && isVectorClockEmpty(b)) {
|
||||
// Both empty = equal (both have no history)
|
||||
return VectorClockComparison.EQUAL;
|
||||
}
|
||||
if (isVectorClockEmpty(a)) {
|
||||
// a is empty, b has history = a happened before b (a < b)
|
||||
return VectorClockComparison.LESS_THAN;
|
||||
}
|
||||
if (isVectorClockEmpty(b)) {
|
||||
// b is empty, a has history = a happened after b (a > b)
|
||||
return VectorClockComparison.GREATER_THAN;
|
||||
}
|
||||
|
||||
// Safe type assertion after null checks
|
||||
const clockA = a!;
|
||||
const clockB = b!;
|
||||
|
||||
// Get all client IDs from both clocks
|
||||
const allClientIds = new Set([...Object.keys(clockA), ...Object.keys(clockB)]);
|
||||
|
||||
let aHasGreater = false;
|
||||
let bHasGreater = false;
|
||||
|
||||
// Compare each component
|
||||
for (const clientId of allClientIds) {
|
||||
const aVal = clockA[clientId] || 0;
|
||||
const bVal = clockB[clientId] || 0;
|
||||
|
||||
if (aVal > bVal) {
|
||||
aHasGreater = true;
|
||||
}
|
||||
if (bVal > aVal) {
|
||||
bHasGreater = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Determine relationship
|
||||
if (!aHasGreater && !bHasGreater) {
|
||||
return VectorClockComparison.EQUAL;
|
||||
} else if (!aHasGreater) {
|
||||
// B has some greater components, A has none -> A < B
|
||||
return VectorClockComparison.LESS_THAN;
|
||||
} else if (!bHasGreater) {
|
||||
// A has some greater components, B has none -> A > B
|
||||
return VectorClockComparison.GREATER_THAN;
|
||||
} else {
|
||||
// Both have some greater components -> concurrent
|
||||
return VectorClockComparison.CONCURRENT;
|
||||
}
|
||||
// Delegate to shared implementation and convert string result to enum
|
||||
const result = sharedCompareVectorClocks(a!, b!);
|
||||
return VectorClockComparison[result];
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -229,8 +201,11 @@ export const incrementVectorClock = (
|
|||
};
|
||||
|
||||
/**
|
||||
* Merge two vector clocks by taking the maximum value for each component
|
||||
* This is used when receiving updates to ensure we have the most recent view
|
||||
* Merge two vector clocks by taking the maximum value for each component.
|
||||
*
|
||||
* Uses the shared implementation from @sp/shared-schema to ensure
|
||||
* client and server produce identical results. This wrapper adds
|
||||
* null/undefined handling for client-side convenience.
|
||||
*
|
||||
* @param a First vector clock
|
||||
* @param b Second vector clock
|
||||
|
|
@ -240,19 +215,12 @@ export const mergeVectorClocks = (
|
|||
a: VectorClock | null | undefined,
|
||||
b: VectorClock | null | undefined,
|
||||
): VectorClock => {
|
||||
// Handle null/undefined cases (shared implementation requires non-null)
|
||||
if (isVectorClockEmpty(a)) return { ...(b || {}) };
|
||||
if (isVectorClockEmpty(b)) return { ...(a || {}) };
|
||||
|
||||
const merged: VectorClock = {};
|
||||
const allClientIds = new Set([...Object.keys(a!), ...Object.keys(b!)]);
|
||||
|
||||
for (const clientId of allClientIds) {
|
||||
const aVal = a![clientId] || 0;
|
||||
const bVal = b![clientId] || 0;
|
||||
merged[clientId] = Math.max(aVal, bVal);
|
||||
}
|
||||
|
||||
return merged;
|
||||
// Delegate to shared implementation
|
||||
return sharedMergeVectorClocks(a!, b!);
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -152,6 +152,11 @@ export class OperationLogEffects {
|
|||
//
|
||||
// By acquiring the lock first, flushPendingWrites() must wait for us to finish writing.
|
||||
await this.lockService.request(LOCK_NAMES.OPERATION_LOG, async () => {
|
||||
// MULTI-TAB SAFETY: Clear vector clock cache to ensure fresh read after other tabs
|
||||
// may have written while we were waiting for the lock. Each tab has its own in-memory
|
||||
// cache, so Tab B's cache could be stale if Tab A wrote while Tab B was waiting.
|
||||
this.opLogStore.clearVectorClockCache();
|
||||
|
||||
// Get entity changes from the FIFO queue (for TIME_TRACKING and TASK time sync)
|
||||
// For most actions, this returns empty array since action payloads are sufficient.
|
||||
// IMPORTANT: This MUST happen inside the lock to prevent race with flushPendingWrites.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { VectorClock } from '../../core/util/vector-clock';
|
||||
import { ActionType } from './action-types.enum';
|
||||
export { VectorClock, ActionType };
|
||||
import { EntityType as SharedEntityType, ENTITY_TYPES } from '@sp/shared-schema';
|
||||
export { VectorClock, ActionType, ENTITY_TYPES };
|
||||
|
||||
export enum OpType {
|
||||
Create = 'CRT',
|
||||
|
|
@ -13,27 +14,11 @@ export enum OpType {
|
|||
Repair = 'REPAIR', // Auto-repair operation with full repaired state
|
||||
}
|
||||
|
||||
export type EntityType =
|
||||
| 'TASK'
|
||||
| 'PROJECT'
|
||||
| 'TAG'
|
||||
| 'NOTE'
|
||||
| 'GLOBAL_CONFIG'
|
||||
| 'SIMPLE_COUNTER'
|
||||
| 'WORK_CONTEXT'
|
||||
| 'TIME_TRACKING'
|
||||
| 'TASK_REPEAT_CFG'
|
||||
| 'ISSUE_PROVIDER'
|
||||
| 'PLANNER'
|
||||
| 'MENU_TREE'
|
||||
| 'METRIC'
|
||||
| 'BOARD'
|
||||
| 'REMINDER'
|
||||
| 'PLUGIN_USER_DATA'
|
||||
| 'PLUGIN_METADATA'
|
||||
| 'MIGRATION'
|
||||
| 'RECOVERY' // For disaster recovery imports
|
||||
| 'ALL'; // For full state imports (sync, backup)
|
||||
/**
|
||||
* Entity type - identifies the kind of data entity being operated on.
|
||||
* Re-exported from @sp/shared-schema to ensure client/server consistency.
|
||||
*/
|
||||
export type EntityType = SharedEntityType;
|
||||
|
||||
export interface Operation {
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1757,6 +1757,40 @@ describe('OperationLogStoreService', () => {
|
|||
const clock3 = await service.getVectorClock();
|
||||
expect(clock3).toEqual({ client: 5 });
|
||||
});
|
||||
|
||||
it('should clear cache when clearVectorClockCache is called', async () => {
|
||||
// Set vector clock and read it (populates cache)
|
||||
await service.setVectorClock({ tabA: 10, tabB: 5 });
|
||||
const cachedClock = await service.getVectorClock();
|
||||
expect(cachedClock).toEqual({ tabA: 10, tabB: 5 });
|
||||
|
||||
// Clear the cache
|
||||
service.clearVectorClockCache();
|
||||
|
||||
// Next read should fetch from IndexedDB (which still has the value)
|
||||
const freshClock = await service.getVectorClock();
|
||||
expect(freshClock).toEqual({ tabA: 10, tabB: 5 });
|
||||
});
|
||||
|
||||
it('should force fresh read from IndexedDB after clearVectorClockCache', async () => {
|
||||
// This test simulates the multi-tab scenario where another tab updates IndexedDB
|
||||
// while this tab has a stale cache.
|
||||
|
||||
// Set initial clock and populate cache
|
||||
await service.setVectorClock({ originalClient: 1 });
|
||||
await service.getVectorClock();
|
||||
|
||||
// Simulate another tab writing directly to IndexedDB (bypassing our cache)
|
||||
// We do this by using setVectorClock which updates both DB and cache
|
||||
await service.setVectorClock({ originalClient: 1, anotherTabClient: 99 });
|
||||
|
||||
// Now clear cache to simulate what happens after acquiring a lock
|
||||
service.clearVectorClockCache();
|
||||
|
||||
// Should get the updated value from IndexedDB
|
||||
const clock = await service.getVectorClock();
|
||||
expect(clock).toEqual({ originalClient: 1, anotherTabClient: 99 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergeRemoteOpClocks', () => {
|
||||
|
|
|
|||
|
|
@ -898,6 +898,27 @@ export class OperationLogStoreService {
|
|||
this._vectorClockCache = clock;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears the in-memory vector clock cache, forcing next read to fetch from IndexedDB.
|
||||
*
|
||||
* MULTI-TAB SAFETY: Each browser tab maintains its own in-memory cache. When Tab A
|
||||
* writes a new operation and updates its cache, Tab B's cache remains stale.
|
||||
* Call this before reading the vector clock inside a Web Lock to ensure freshness
|
||||
* after other tabs may have written.
|
||||
*
|
||||
* The typical pattern is:
|
||||
* ```
|
||||
* await lockService.request(OPERATION_LOG, async () => {
|
||||
* opLogStore.clearVectorClockCache(); // Force fresh read
|
||||
* const clock = await vectorClockService.getCurrentVectorClock();
|
||||
* // ... create operation with correct clock
|
||||
* });
|
||||
* ```
|
||||
*/
|
||||
clearVectorClockCache(): void {
|
||||
this._vectorClockCache = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges remote operations' vector clocks into the local vector clock.
|
||||
*
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue