mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-08-04 13:32:33 +00:00
fix(supersync): widen conflict lookups to aliased and unioned entity ids
Two conflict-detection blind spots let concurrent edits slip through without a conflict: - An op carrying both a scalar entityId and an entityIds array only checked the array; the scalar and array sets are now unioned for detection while getStoredEntityIds keeps the historical storage normalization (scalar in entity_id, arrays in entity_ids). - Histories written before schema v2 keep migrated task settings under GLOBAL_CONFIG:misc. Incoming GLOBAL_CONFIG:tasks writes now consult the legacy misc row as an alias (newest of the two wins, compared by serverSeq), and legacy misc ops check the tasks key per-entity. Works for encrypted payloads since only row metadata is consulted.
This commit is contained in:
parent
982f2916ca
commit
e019ef0b71
3 changed files with 116 additions and 15 deletions
|
|
@ -1,4 +1,5 @@
|
|||
import { Prisma } from '@prisma/client';
|
||||
import { CURRENT_SCHEMA_VERSION } from '@sp/shared-schema';
|
||||
import { Logger } from '../logger';
|
||||
import {
|
||||
BatchUploadCandidate,
|
||||
|
|
@ -34,11 +35,7 @@ export const detectConflict = async (
|
|||
|
||||
// Build list of entity IDs to check for conflicts.
|
||||
// Operations may have either entityId (singular) or entityIds (batch operations).
|
||||
const rawEntityIdsToCheck = op.entityIds?.length
|
||||
? op.entityIds
|
||||
: op.entityId
|
||||
? [op.entityId]
|
||||
: [];
|
||||
const rawEntityIdsToCheck = getConflictEntityIds(op);
|
||||
|
||||
// Skip if no entity IDs (can't have entity-level conflicts)
|
||||
if (rawEntityIdsToCheck.length === 0) {
|
||||
|
|
@ -49,6 +46,18 @@ export const detectConflict = async (
|
|||
return detectConflictForEntity(userId, op, rawEntityIdsToCheck[0], tx);
|
||||
}
|
||||
|
||||
// v1 GLOBAL_CONFIG:misc contains fields that became GLOBAL_CONFIG:tasks in
|
||||
// v2. This compatibility path is intentionally per-key: it is rare, keeps
|
||||
// the normal multi-entity SQL unchanged, and works for encrypted legacy
|
||||
// payloads whose contents the server cannot inspect.
|
||||
if (isLegacyMiscConfigOperation(op)) {
|
||||
for (const entityId of rawEntityIdsToCheck) {
|
||||
const result = await detectConflictForEntity(userId, op, entityId, tx);
|
||||
if (result.hasConflict) return result;
|
||||
}
|
||||
return { hasConflict: false };
|
||||
}
|
||||
|
||||
const entityIdsToCheck = Array.from(new Set(rawEntityIdsToCheck));
|
||||
return detectConflictForEntities(userId, op, entityIdsToCheck, tx);
|
||||
};
|
||||
|
|
@ -210,16 +219,40 @@ export const detectConflictForEntity = async (
|
|||
entityType: op.entityType,
|
||||
OR: [{ entityId }, { entityIds: { has: entityId } }],
|
||||
},
|
||||
select: { clientId: true, vectorClock: true },
|
||||
select: { clientId: true, vectorClock: true, serverSeq: true },
|
||||
orderBy: { serverSeq: 'desc' },
|
||||
});
|
||||
|
||||
// Histories written before schema v2 persist migrated task settings under
|
||||
// the raw `GLOBAL_CONFIG:misc` key. Consult that key as an alias when the
|
||||
// incoming write targets `tasks`; no backfill (and no payload decryption) is
|
||||
// required. Pick the newer of the canonical and legacy rows.
|
||||
const legacyMiscOp =
|
||||
op.entityType === 'GLOBAL_CONFIG' && entityId === 'tasks'
|
||||
? await tx.operation.findFirst({
|
||||
where: {
|
||||
userId,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'misc',
|
||||
schemaVersion: { lt: CURRENT_SCHEMA_VERSION },
|
||||
},
|
||||
select: { clientId: true, vectorClock: true, serverSeq: true },
|
||||
orderBy: { serverSeq: 'desc' },
|
||||
})
|
||||
: null;
|
||||
|
||||
const latestExistingOp =
|
||||
legacyMiscOp &&
|
||||
(!existingOp || (legacyMiscOp.serverSeq ?? -1) > (existingOp.serverSeq ?? -1))
|
||||
? legacyMiscOp
|
||||
: existingOp;
|
||||
|
||||
// No existing operation = no conflict
|
||||
if (!existingOp) {
|
||||
if (!latestExistingOp) {
|
||||
return { hasConflict: false };
|
||||
}
|
||||
|
||||
return resolveConflictForExistingOp(op, entityId, existingOp);
|
||||
return resolveConflictForExistingOp(op, entityId, latestExistingOp);
|
||||
};
|
||||
|
||||
export const isSameDuplicateOperation = (
|
||||
|
|
@ -350,14 +383,21 @@ export const toStableJsonValue = (value: unknown): unknown => {
|
|||
* `entity_ids` column, which uses {@link getStoredEntityIds} (multi-entity only).
|
||||
*/
|
||||
export const getConflictEntityIds = (op: Operation): string[] => {
|
||||
const rawEntityIds = op.entityIds?.length
|
||||
? op.entityIds
|
||||
: op.entityId
|
||||
? [op.entityId]
|
||||
: [];
|
||||
const rawEntityIds = [
|
||||
...(op.entityId ? [op.entityId] : []),
|
||||
...(op.entityIds?.length ? op.entityIds : []),
|
||||
];
|
||||
if (isLegacyMiscConfigOperation(op)) {
|
||||
rawEntityIds.push('tasks');
|
||||
}
|
||||
return Array.from(new Set(rawEntityIds));
|
||||
};
|
||||
|
||||
const isLegacyMiscConfigOperation = (op: Operation): boolean =>
|
||||
op.schemaVersion < CURRENT_SCHEMA_VERSION &&
|
||||
op.entityType === 'GLOBAL_CONFIG' &&
|
||||
op.entityId === 'misc';
|
||||
|
||||
/**
|
||||
* The entity_ids array to persist with an op. Ops whose touched-entity set is
|
||||
* already covered by the scalar entity_id store an empty array, so single-entity
|
||||
|
|
@ -371,7 +411,12 @@ export const getConflictEntityIds = (op: Operation): string[] => {
|
|||
* would be invisible to conflict lookups (#8334).
|
||||
*/
|
||||
export const getStoredEntityIds = (op: Operation): string[] => {
|
||||
const ids = getConflictEntityIds(op);
|
||||
// Preserve the historical storage normalization: the scalar is persisted in
|
||||
// entity_id and only the declared multi-entity set belongs in entity_ids.
|
||||
// Conflict detection unions both fields via getConflictEntityIds above.
|
||||
const ids = Array.from(
|
||||
new Set(op.entityIds?.length ? op.entityIds : op.entityId ? [op.entityId] : []),
|
||||
);
|
||||
if (ids.length <= 1 && ids[0] === op.entityId) {
|
||||
return [];
|
||||
}
|
||||
|
|
@ -436,7 +481,8 @@ export const prefetchLatestEntityOpsForBatch = async (
|
|||
o.entity_type AS "entityType",
|
||||
eid AS "entityId",
|
||||
o.client_id AS "clientId",
|
||||
o.vector_clock AS "vectorClock"
|
||||
o.vector_clock AS "vectorClock",
|
||||
o.server_seq AS "serverSeq"
|
||||
FROM operations o
|
||||
CROSS JOIN LATERAL unnest(
|
||||
o.entity_ids || CASE WHEN o.entity_id IS NULL THEN '{}'::text[] ELSE ARRAY[o.entity_id] END
|
||||
|
|
@ -457,6 +503,37 @@ export const prefetchLatestEntityOpsForBatch = async (
|
|||
}
|
||||
}
|
||||
|
||||
if (
|
||||
entityPairs.some(
|
||||
({ entityType, entityId }) =>
|
||||
entityType === 'GLOBAL_CONFIG' && entityId === 'tasks',
|
||||
)
|
||||
) {
|
||||
const legacyMiscOp = await tx.operation.findFirst({
|
||||
where: {
|
||||
userId,
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'misc',
|
||||
schemaVersion: { lt: CURRENT_SCHEMA_VERSION },
|
||||
},
|
||||
select: { clientId: true, vectorClock: true, serverSeq: true },
|
||||
orderBy: { serverSeq: 'desc' },
|
||||
});
|
||||
const tasksKey = getEntityConflictKey('GLOBAL_CONFIG', 'tasks');
|
||||
const currentTasksOp = latestByEntity.get(tasksKey);
|
||||
if (
|
||||
legacyMiscOp &&
|
||||
(!currentTasksOp ||
|
||||
(legacyMiscOp.serverSeq ?? -1) > (currentTasksOp.serverSeq ?? -1))
|
||||
) {
|
||||
latestByEntity.set(tasksKey, {
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'tasks',
|
||||
...legacyMiscOp,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return latestByEntity;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -222,6 +222,7 @@ export interface LatestEntityOperationRow {
|
|||
entityId: string;
|
||||
clientId: string;
|
||||
vectorClock: unknown;
|
||||
serverSeq?: number;
|
||||
}
|
||||
|
||||
export interface LatestBatchEntityOperationRow extends LatestEntityOperationRow {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
getConflictEntityIds,
|
||||
isSameDuplicateOperation,
|
||||
isSameDuplicateTimestamp,
|
||||
pruneVectorClockForStorage,
|
||||
|
|
@ -48,6 +49,28 @@ const duplicateCandidate = (
|
|||
});
|
||||
|
||||
describe('conflict helpers', () => {
|
||||
it('includes a divergent scalar entityId in the incoming conflict set', () => {
|
||||
expect(
|
||||
getConflictEntityIds(op({ entityId: 'task-scalar', entityIds: ['task-array'] })),
|
||||
).toEqual(['task-scalar', 'task-array']);
|
||||
});
|
||||
|
||||
it.each([false, true])(
|
||||
'aliases legacy misc config writes to tasks when encrypted=%s',
|
||||
(isPayloadEncrypted) => {
|
||||
expect(
|
||||
getConflictEntityIds(
|
||||
op({
|
||||
entityType: 'GLOBAL_CONFIG',
|
||||
entityId: 'misc',
|
||||
schemaVersion: 1,
|
||||
isPayloadEncrypted,
|
||||
}),
|
||||
),
|
||||
).toEqual(['misc', 'tasks']);
|
||||
},
|
||||
);
|
||||
|
||||
it('accepts matching duplicate operations regardless of JSON key order', () => {
|
||||
const incoming = op({
|
||||
payload: { title: 'A', nested: { b: 2, a: 1 } },
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue