diff --git a/packages/core/src/patch_manager/ModelWithPatches.ts b/packages/core/src/patch_manager/ModelWithPatches.ts index 2a1926490..7a8abdbf4 100644 --- a/packages/core/src/patch_manager/ModelWithPatches.ts +++ b/packages/core/src/patch_manager/ModelWithPatches.ts @@ -1,7 +1,7 @@ import { enablePatches, produceWithPatches } from 'immer'; import EditorModel from '../editor/model/Editor'; import { Model, ObjectHash, SetOptions } from '../common'; -import { serialize } from '../utils/mixins'; +import { createId, serialize } from '../utils/mixins'; import PatchManager, { PatchChangeProps, PatchPath } from './index'; enablePatches(); @@ -46,6 +46,16 @@ const syncDraftToState = (draft: any, target: any) => { }); }; +const isValidPatchUid = (uid: any): uid is string | number => { + if (typeof uid === 'string') return uid !== ''; + return typeof uid === 'number'; +}; + +const createStableUid = () => { + const randomUUID = typeof crypto !== 'undefined' && (crypto as any).randomUUID; + return typeof randomUUID === 'function' ? randomUUID.call(crypto) : createId(); +}; + export default class ModelWithPatches extends Model { em?: EditorModel; patchObjectType?: string; @@ -56,21 +66,48 @@ export default class ModelWithPatches(args); + + const existingUid = this.get('uid' as any) as string | number | undefined; + const hasExistingUid = isValidPatchUid(existingUid); + const incomingUid = (rawAttrs as any).uid; + + // UID is immutable: ignore any attempt to change/unset it via public `set` + const attrs = hasExistingUid && 'uid' in (rawAttrs as any) ? (({ uid: _uid, ...rest }) => rest)(rawAttrs as any) : rawAttrs; + const pm = this.patchManager; - const objectId = this.getPatchObjectId(); - if (!pm || !objectId) { - return (super.set as any).apply(this, args); + if (!pm) { + return super.set(attrs as any, opts as any); + } + + const uid = hasExistingUid ? existingUid : isValidPatchUid(incomingUid) ? incomingUid : pm.createId(); + + // Ensure UID exists before taking snapshots to avoid recording it inside patches + if (!hasExistingUid && isValidPatchUid(uid)) { + super.set({ uid } as any, { silent: true }); } - const { attrs, opts } = normalizeSetArgs(args); + // Never track UID mutations via patches + const attrsNoUid = 'uid' in (attrs as any) ? (({ uid: _uid, ...rest }) => rest)(attrs as any) : attrs; + + const objectId = this.getPatchObjectId(); + + if (!isValidPatchUid(objectId)) { + return super.set(attrsNoUid as any, opts as any); + } const beforeState = serialize(this.attributes || {}); - const result = super.set(attrs as any, opts as any); + const result = super.set(attrsNoUid as any, opts as any); const afterState = serialize(this.attributes || {}); const [, patches, inversePatches] = produceWithPatches(beforeState, (draft: any) => { syncDraftToState(draft, afterState); diff --git a/packages/core/src/patch_manager/index.ts b/packages/core/src/patch_manager/index.ts index 6206c2cd9..f765b3205 100644 --- a/packages/core/src/patch_manager/index.ts +++ b/packages/core/src/patch_manager/index.ts @@ -65,6 +65,10 @@ export default class PatchManager { this.applyHandler = options.applyPatch; } + createId(): string { + return createPatchId(); + } + createOrGetCurrentPatch(): PatchProps { if (!this.shouldRecord()) { return this.createVoidPatch(); @@ -226,3 +230,5 @@ export default class PatchManager { this.emitter?.trigger?.(event, payload); } } + +export { PatchObjectsRegistry, createRegistryApplyPatchHandler, type PatchUid } from './registry'; diff --git a/packages/core/src/patch_manager/registry.ts b/packages/core/src/patch_manager/registry.ts new file mode 100644 index 000000000..198d6363d --- /dev/null +++ b/packages/core/src/patch_manager/registry.ts @@ -0,0 +1,91 @@ +import { applyPatches } from 'immer'; +import { serialize } from '../utils/mixins'; +import type { PatchApplyHandler, PatchApplyOptions, PatchChangeProps, PatchPath } from './index'; + +export type PatchUid = string | number; + +export class PatchObjectsRegistry { + private byType: Record> = {}; + + register(type: string, uid: PatchUid, obj: T): void { + if (!this.byType[type]) { + this.byType[type] = new Map(); + } + + this.byType[type].set(uid, obj); + } + + unregister(type: string, uid: PatchUid): void { + this.byType[type]?.delete(uid); + } + + get(type: string, uid: PatchUid): T | undefined { + return this.byType[type]?.get(uid); + } + + clear(type?: string): void { + if (type) { + delete this.byType[type]; + return; + } + + this.byType = {}; + } +} + +type PatchGroup = { + type: string; + uid: PatchUid; + patches: PatchChangeProps[]; +}; + +const getPatchGroupKey = (type: string, uid: PatchUid) => `${type}::${uid}`; + +const stripPrefix = (path: PatchPath, prefixLen: number): PatchPath => path.slice(prefixLen); + +const normalizeForApply = (patch: PatchChangeProps): PatchChangeProps => { + const prefixLen = 3; // [type, uid, 'attributes', ...] + return { + ...patch, + path: stripPrefix(patch.path, prefixLen), + ...(patch.from ? { from: stripPrefix(patch.from, prefixLen) } : {}), + }; +}; + +const syncModelToState = (model: any, state: any, options?: PatchApplyOptions) => { + const current = model.attributes || {}; + Object.keys(current).forEach((key) => { + if (!(key in state)) { + model.unset(key, options as any); + } + }); + + model.set(state, options as any); +}; + +export const createRegistryApplyPatchHandler = (registry: PatchObjectsRegistry): PatchApplyHandler => { + return (changes: PatchChangeProps[], options?: PatchApplyOptions) => { + const groups = new Map(); + + changes.forEach((patch) => { + const [type, uid, scope] = patch.path; + if (typeof type !== 'string' || (typeof uid !== 'string' && typeof uid !== 'number')) return; + if (scope !== 'attributes') return; + + const key = getPatchGroupKey(type, uid); + const group = groups.get(key) || { type, uid, patches: [] }; + group.patches.push(patch); + groups.set(key, group); + }); + + groups.forEach(({ type, uid, patches }) => { + const model = registry.get(type, uid); + if (!model) return; + + const baseState = serialize(model.attributes || {}); + const nextState = applyPatches(baseState, patches.map(normalizeForApply) as any); + syncModelToState(model, nextState, options); + }); + }; +}; + diff --git a/packages/core/test/specs/patch_manager/model/ModelWithPatches.js b/packages/core/test/specs/patch_manager/model/ModelWithPatches.js index 1f0c8de02..63aeaea9b 100644 --- a/packages/core/test/specs/patch_manager/model/ModelWithPatches.js +++ b/packages/core/test/specs/patch_manager/model/ModelWithPatches.js @@ -10,6 +10,7 @@ describe('ModelWithPatches', () => { trigger: (event, payload) => events.push({ event, payload }), }, }); + pm.createId = () => 'uid-1'; const model = new ModelWithPatches({ id: 'model-1', foo: 'bar' }); model.em = { Patches: pm }; @@ -23,16 +24,17 @@ describe('ModelWithPatches', () => { expect(events[0].event).toBe(PatchManagerEvents.update); const patch = events[0].payload; + expect(model.get('uid')).toBe('uid-1'); expect(patch.changes).toHaveLength(1); expect(patch.reverseChanges).toHaveLength(1); expect(patch.changes[0]).toMatchObject({ op: 'replace', - path: ['model', 'model-1', 'attributes', 'foo'], + path: ['model', 'uid-1', 'attributes', 'foo'], value: 'baz', }); expect(patch.reverseChanges[0]).toMatchObject({ op: 'replace', - path: ['model', 'model-1', 'attributes', 'foo'], + path: ['model', 'uid-1', 'attributes', 'foo'], value: 'bar', }); }); @@ -71,15 +73,15 @@ describe('ModelWithPatches', () => { }, }); - model = new ModelWithPatches({ id: 'model-3', foo: 'bar' }); + model = new ModelWithPatches({ uid: 'uid-3', id: 'model-3', foo: 'bar' }); model.em = { Patches: pm }; model.patchObjectType = 'model'; pm.apply( { id: 'patch-3', - changes: [{ op: 'replace', path: ['model', 'model-3', 'attributes', 'foo'], value: 'applied' }], - reverseChanges: [{ op: 'replace', path: ['model', 'model-3', 'attributes', 'foo'], value: 'bar' }], + changes: [{ op: 'replace', path: ['model', 'uid-3', 'attributes', 'foo'], value: 'applied' }], + reverseChanges: [{ op: 'replace', path: ['model', 'uid-3', 'attributes', 'foo'], value: 'bar' }], }, { external: true }, ); @@ -89,4 +91,25 @@ describe('ModelWithPatches', () => { expect(model.get('foo')).toBe('applied'); expect(events).toHaveLength(0); }); + + test('uid is immutable once set', async () => { + const events = []; + const pm = new PatchManager({ + enabled: true, + emitter: { + trigger: (event, payload) => events.push({ event, payload }), + }, + }); + + const model = new ModelWithPatches({ uid: 'uid-4', foo: 'bar' }); + model.em = { Patches: pm }; + model.patchObjectType = 'model'; + + model.set('uid', 'uid-changed'); + + await Promise.resolve(); + + expect(model.get('uid')).toBe('uid-4'); + expect(events).toHaveLength(0); + }); }); diff --git a/packages/core/test/specs/patch_manager/registry.js b/packages/core/test/specs/patch_manager/registry.js new file mode 100644 index 000000000..5bc6d7ea5 --- /dev/null +++ b/packages/core/test/specs/patch_manager/registry.js @@ -0,0 +1,33 @@ +import PatchManager, { PatchObjectsRegistry, createRegistryApplyPatchHandler } from 'patch_manager'; +import ModelWithPatches from 'patch_manager/ModelWithPatches'; + +describe('PatchObjectsRegistry', () => { + test('apply handler resolves models by uid and applies forward/backward changes', () => { + const registry = new PatchObjectsRegistry(); + const pm = new PatchManager({ + enabled: true, + applyPatch: createRegistryApplyPatchHandler(registry), + }); + + const model = new ModelWithPatches({ uid: 'uid-1', foo: 'bar' }); + model.em = { Patches: pm }; + model.patchObjectType = 'model'; + registry.register('model', 'uid-1', model); + + const patch = { + id: 'patch-1', + changes: [{ op: 'replace', path: ['model', 'uid-1', 'attributes', 'foo'], value: 'baz' }], + reverseChanges: [{ op: 'replace', path: ['model', 'uid-1', 'attributes', 'foo'], value: 'bar' }], + }; + + pm.apply(patch); + expect(model.get('foo')).toBe('baz'); + + pm.undo(); + expect(model.get('foo')).toBe('bar'); + + pm.redo(); + expect(model.get('foo')).toBe('baz'); + }); +}); +