Skip to content

Commit cefea74

Browse files
caio-pizzolsuperdoc-oss-port[bot]
authored andcommitted
fix(super-editor): sync custom xml writes through part-sync (IT-1229) (#392)
* fix(super-editor): sync custom xml writes through part-sync - metadata.* and customXml.parts.* writes routed through the parts mutation layer so partChanged fires and part-sync publishes - a shared sandbox-and-diff wrapper commits every package path a custom xml create/update/remove touches (item, itemProps, item rels, document rels), not just the item xml - two-client regression tests cover metadata attach and customXml create/patch/remove, including the remote client's converted xml * fix(super-editor): record custom xml tombstones on remote part-sync deletes Remote part deletions removed the part from convertedXml but never populated removedCustomXmlPaths, so a collaborator whose original docx contained the part re-emitted it on export. The consumer now records tombstones for deleted custom xml storage parts, props parts, and item rels (including rels-linked props paths), and clears stale tombstones when a part is re-created remotely. * fix(super-editor): tombstone absent custom xml parts on late-join and clear bibliography cache - late joiners prune local custom xml parts absent from an authoritative parts map (a peer deleted them before this client joined), recording export tombstones through the same mutateParts mechanism hydration uses - tombstoning a path now also invalidates converter caches, so deleting the bibliography storage part clears converter.bibliographyPart and export cannot resurrect it * fix(super-editor): record hydration tombstones only after the prune mutation commits Tombstones (and the bibliography cache clear) were applied before mutateParts ran; a mutation failure rolled back the part store but left the converter tombstoned. The prune now collects paths and records them only after the mutation succeeds. * fix(super-editor): clear stale custom xml tombstones during hydration A client that tombstoned a custom xml part (e.g. deleted while offline) and then rehydrated from an authoritative map containing the recreated part kept the stale tombstone, so export dropped the recreated part. Hydration now clears tombstones for present custom xml paths at the same post-mutation point where absent paths are tombstoned - symmetric to the live consumer path. * fix(super-editor): invalidate bibliography cache on remote custom xml writes Remote writes and hydration cleared stale tombstones but left the bibliography cache pointing at the old content, so export rebuilt the part from the stale cache over the collaborator's version. Written and present custom xml paths now invalidate converter caches, and the part path predicates agree on casing. * fix(super-editor): clear hydration tombstones only for parts that hydrated A key present in the authoritative map whose envelope failed to decode never lands in convertedXml; clearing its tombstone would let export copy the stale original zip entry through instead of dropping the part. * ci: retrigger checks on current head * fix(super-editor): scope hydration tombstone clearing to hydrated parts and make converter-state application rollback-safe - hydration only clears a custom xml tombstone when the part actually hydrated this pass (a decode+op succeeded), not merely because a stale local copy is present; a failed-to-hydrate authoritative part no longer resurrects the stale local one on export - applyConverterState is atomic and runs only after the part-store mutation durably commits, so a compound rollback cannot leave removedCustomXmlPaths / bibliographyPart mutated while the store is restored * fix: address PR review comments on #392 * fix(super-editor): validate metadata payload before mutating on attach metadata.attach serialized the payload only in the live path, inside the part-sync sandbox that runs after the anchor is inserted. A non-serializable payload (a BigInt or a cyclic object, neither of which the payload type can rule out) therefore threw while building the envelope, after wrapRangeInAnchor had already dispatched the anchor transaction, leaving an inline content control with no matching customXml payload. Build the envelope during the attach preview as well, so serialization fails before the operation touches the document. Adds a regression test asserting a cyclic payload throws without dispatching an anchor transaction. * refactor(super-editor): serialize only the new entry in the attach preview The attach preview rebuilt the whole metadata envelope to validate one payload, re-serializing every existing entry in the namespace before the live path serialized them again. Serialize just the new entry — existing entries were already validated when they were written — so the preview stays a fail-fast check for non-serializable payloads without the per-attach serialization cost. * test(super-editor): fix concurrent-seed race in collaborator export test The two-client export test created both collaborative editors on unsynced ydocs, so both bootstrapped as a new room and both seeded the baseline package into their parts maps as concurrent Yjs writes. The map-key conflict on word/_rels/document.xml.rels is resolved by clientID, and with the wrong random winner the second client's baseline seed discarded the first client's published rels update, dropping the customXml document relationship from the export (~50% failure rate; deterministically reproducible by pinning clientIDs). Real clients receive room state before bootstrapping, see the capability marker, and hydrate instead of seeding, so the conflict is an artifact of the test's join order. Create the second editor only after syncing the first editor's seeded state, matching the real join flow, and wait for the received document rels content (the baseline part always exists, so an existence check passed before the remote update landed). Ported-From-Source-Repo: superdoc/orbit Ported-From-Source-Commit: 13119b74e8785850867256c4a8fffb06b41e73c3 Ported-Public-Prefix: superdoc/public
1 parent 4ee24ac commit cefea74

12 files changed

Lines changed: 1548 additions & 51 deletions

File tree

packages/super-editor/src/editors/v1/core/super-converter/custom-xml-parts.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ export function removeCustomXmlPart(convertedXml, target, converter) {
610610
return true;
611611
}
612612

613-
function invalidateConverterCachesForPath(converter, partName) {
613+
export function invalidateConverterCachesForPath(converter, partName) {
614614
if (!converter || typeof partName !== 'string') return;
615615
const biblio = converter.bibliographyPart;
616616
if (biblio && biblio.partPath === partName) {

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/anchored-metadata-export.integration.test.ts

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
2+
import * as Y from 'yjs';
3+
import { Awareness } from 'y-protocols/awareness.js';
4+
import JSZip from 'jszip';
25
import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js';
36

47
function seedMetadataPart(
@@ -38,6 +41,58 @@ async function createEditorWithEmptyPackage() {
3841
return editor;
3942
}
4043

44+
function createProviderStub(ydoc: Y.Doc) {
45+
return {
46+
synced: true,
47+
isSynced: true,
48+
on: vi.fn(),
49+
off: vi.fn(),
50+
disconnect: vi.fn(),
51+
awareness: new Awareness(ydoc),
52+
};
53+
}
54+
55+
async function createCollaborativeEditorWithEmptyPackage(ydoc: Y.Doc) {
56+
const docData = await loadTestDataForEditorTests('blank-doc.docx');
57+
const { editor } = initTestEditor({
58+
content: docData.docx,
59+
media: docData.media,
60+
mediaFiles: docData.mediaFiles,
61+
fonts: docData.fonts,
62+
ydoc,
63+
collaborationProvider: createProviderStub(ydoc),
64+
useImmediateSetTimeout: false,
65+
isHeadless: true,
66+
user: { name: 'Test', email: 'test@example.com' },
67+
});
68+
return editor;
69+
}
70+
71+
function resolveBlockId(insertReceipt: unknown): string {
72+
const receipt = insertReceipt as {
73+
target?: { blockId?: unknown };
74+
resolution?: { target?: { blockId?: unknown } };
75+
};
76+
const direct = receipt.target?.blockId;
77+
if (typeof direct === 'string' && direct.length > 0) return direct;
78+
const resolved = receipt.resolution?.target?.blockId;
79+
if (typeof resolved === 'string' && resolved.length > 0) return resolved;
80+
throw new Error('insert receipt did not include a blockId');
81+
}
82+
83+
function convertedXml(editor: unknown): Record<string, unknown> {
84+
return (editor as { converter: { convertedXml: Record<string, unknown> } }).converter.convertedXml;
85+
}
86+
87+
function syncYDocs(source: Y.Doc, target: Y.Doc): void {
88+
Y.applyUpdate(target, Y.encodeStateAsUpdate(source));
89+
}
90+
91+
async function readZipPart(buffer: Uint8Array, path: string): Promise<string | undefined> {
92+
const zip = await JSZip.loadAsync(buffer);
93+
return zip.files[path]?.async('string');
94+
}
95+
4196
describe('anchored metadata export filtering', () => {
4297
it('removes anchored-metadata entries from customXml when exporting final doc', async () => {
4398
const editor = await createEditorWithEmptyPackage();
@@ -73,4 +128,78 @@ describe('anchored metadata export filtering', () => {
73128
editor.destroy();
74129
}
75130
});
131+
132+
it('exports metadata customXml parts received from a collaborator', async () => {
133+
const ydocA = new Y.Doc();
134+
const ydocB = new Y.Doc();
135+
const editorA = await createCollaborativeEditorWithEmptyPackage(ydocA);
136+
137+
await vi.waitFor(() => {
138+
expect((editorA as unknown as { _partPublisher?: unknown })._partPublisher).toBeDefined();
139+
});
140+
141+
// Model the real join order: A seeds the room first, then B receives that
142+
// state BEFORE bootstrapping, so B hydrates instead of seeding. Creating
143+
// both editors on unsynced ydocs makes both seed the same baseline keys as
144+
// concurrent Yjs writes, and the map-key conflict is then resolved by
145+
// clientID — with the wrong winner, B's baseline word/_rels/document.xml.rels
146+
// silently discards A's published rels update.
147+
syncYDocs(ydocA, ydocB);
148+
const editorB = await createCollaborativeEditorWithEmptyPackage(ydocB);
149+
const hostText = 'Alpha metadata target.';
150+
151+
try {
152+
await vi.waitFor(() => {
153+
expect((editorB as unknown as { _partPublisher?: unknown })._partPublisher).toBeDefined();
154+
});
155+
156+
const inserted = await Promise.resolve(editorA.doc.insert({ value: hostText }));
157+
const blockId = resolveBlockId(inserted);
158+
const start = hostText.indexOf('metadata');
159+
const end = start + 'metadata'.length;
160+
161+
const attached = editorA.doc.metadata.attach({
162+
id: 'meta-export-sync',
163+
namespace: 'urn:test:metadata-export-sync',
164+
payload: { label: 'Remote export' },
165+
target: {
166+
kind: 'selection',
167+
start: { kind: 'text', blockId, offset: start },
168+
end: { kind: 'text', blockId, offset: end },
169+
},
170+
});
171+
expect(attached.success).toBe(true);
172+
173+
syncYDocs(ydocA, ydocB);
174+
await vi.waitFor(() => {
175+
expect(convertedXml(editorB)['customXml/item1.xml']).toBeDefined();
176+
expect(convertedXml(editorB)['customXml/itemProps1.xml']).toBeDefined();
177+
expect(convertedXml(editorB)['customXml/_rels/item1.xml.rels']).toBeDefined();
178+
// The baseline package already contains document.xml.rels, so checking
179+
// mere existence would pass before the remote UPDATE lands — wait for
180+
// the received rels content to reference the custom XML part.
181+
expect(JSON.stringify(convertedXml(editorB)['word/_rels/document.xml.rels'])).toContain(
182+
'relationships/customXml',
183+
);
184+
});
185+
186+
const exported = (await editorB.exportDocx({ getUpdatedDocs: false })) as Uint8Array;
187+
const metadataXml = await readZipPart(exported, 'customXml/item1.xml');
188+
const propsXml = await readZipPart(exported, 'customXml/itemProps1.xml');
189+
const itemRelsXml = await readZipPart(exported, 'customXml/_rels/item1.xml.rels');
190+
const documentRelsXml = await readZipPart(exported, 'word/_rels/document.xml.rels');
191+
192+
expect(metadataXml).toContain('meta-export-sync');
193+
expect(metadataXml).toContain('Remote export');
194+
expect(propsXml).toContain('datastoreItem');
195+
expect(itemRelsXml).toContain('customXmlProps');
196+
expect(documentRelsXml).toContain('relationships/customXml');
197+
expect(documentRelsXml).toContain('../customXml/item1.xml');
198+
} finally {
199+
editorA.destroy();
200+
editorB.destroy();
201+
ydocA.destroy();
202+
ydocB.destroy();
203+
}
204+
});
76205
});

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/anchored-metadata-wrappers.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,26 @@ describe('anchored metadata wrappers', () => {
288288
expect(metadataListWrapper(editor).total).toBe(0);
289289
});
290290

291+
it('rejects a non-serializable payload before mutating the document', () => {
292+
const editor = makeEditor();
293+
294+
// A cyclic object cannot be represented in the JSON envelope and cannot be
295+
// ruled out by the payload type. The preview must fail serialization before
296+
// the live path inserts the SDT anchor; otherwise attach dispatches the
297+
// anchor transaction and only then throws while building the envelope,
298+
// leaving an anchor with no matching customXml payload.
299+
const cyclic: Record<string, unknown> = {};
300+
cyclic.self = cyclic;
301+
302+
expect(() =>
303+
metadataAttachWrapper(editor, { id: 'cyclic', target: TARGET, namespace: 'urn:test:metadata', payload: cyclic }),
304+
).toThrow();
305+
306+
// No anchor transaction was dispatched and no payload entry was written.
307+
expect(editor.view!.dispatch).not.toHaveBeenCalled();
308+
expect(metadataListWrapper(editor).total).toBe(0);
309+
});
310+
291311
it('attach dry-run throws REVISION_MISMATCH when expectedRevision is stale', () => {
292312
const editor = makeEditor();
293313

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/anchored-metadata-wrappers.ts

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import { findAllSdtNodes, SDT_INLINE_NAME } from '../helpers/content-controls/in
4141
import { executeOutOfBandMutation } from '../out-of-band-mutation.js';
4242
import { executeDomainCommand } from './plan-wrappers.js';
4343
import { checkRevision, getRevision } from './revision-tracker.js';
44+
import { mutateCustomXmlParts } from './custom-xml-part-mutation.js';
4445

4546
type XmlNode = {
4647
type?: string;
@@ -394,56 +395,83 @@ function writeEntry(
394395
const convertedXml = getConvertedXml(editor);
395396
const converter = getConverter(editor);
396397
const existing = findPartByNamespace(convertedXml, namespace);
397-
const entries = existing?.entries.filter((entry) => entry.id !== id) ?? [];
398398
const next: MetadataEntry = {
399399
id,
400400
namespace,
401401
partName: existing?.partName ?? predictPartName(convertedXml, converter),
402402
payload,
403403
};
404-
const xml = buildEnvelopeXml(namespace, [...entries, next]);
405404

406405
if (dryRun) {
406+
// Serialize just the new entry during the preview so a non-serializable
407+
// payload (BigInt, cyclic object) fails here, before the live path mutates
408+
// the document. metadata.attach runs this preview before wrapRangeInAnchor;
409+
// without it, the live path would insert the SDT anchor and only then throw
410+
// while building the envelope, leaving an anchor with no customXml payload.
411+
// Only the new entry needs checking: existing entries were validated when
412+
// they were written, and the live path re-serializes the full envelope.
413+
buildEnvelopeXml(namespace, [next]);
407414
return { partName: next.partName };
408415
}
409416

410-
if (existing) {
411-
patchCustomXmlPart(
412-
convertedXml,
413-
{ partName: existing.partName },
414-
{ content: xml, schemaRefs: undefined },
415-
converter ?? undefined,
417+
const partName = mutateCustomXmlParts(editor, 'metadata.writeEntry', (sandboxXml, sandboxConverter) => {
418+
const sandboxExisting = findPartByNamespace(sandboxXml, namespace);
419+
const sandboxEntries = sandboxExisting?.entries.filter((entry) => entry.id !== id) ?? [];
420+
const sandboxNext: MetadataEntry = {
421+
id,
422+
namespace,
423+
partName: sandboxExisting?.partName ?? predictPartName(sandboxXml, sandboxConverter),
424+
payload,
425+
};
426+
const sandboxEnvelope = buildEnvelopeXml(namespace, [...sandboxEntries, sandboxNext]);
427+
428+
if (sandboxExisting) {
429+
patchCustomXmlPart(
430+
sandboxXml,
431+
{ partName: sandboxExisting.partName },
432+
{ content: sandboxEnvelope, schemaRefs: undefined },
433+
sandboxConverter,
434+
);
435+
return sandboxExisting.partName;
436+
}
437+
438+
const created = createCustomXmlPart(
439+
sandboxXml,
440+
{ content: sandboxEnvelope, schemaRefs: undefined },
441+
sandboxConverter,
416442
);
417-
markConverterDirty(editor);
418-
return { partName: existing.partName };
419-
}
443+
return created.partName;
444+
});
420445

421-
const created = createCustomXmlPart(convertedXml, { content: xml, schemaRefs: undefined }, converter ?? undefined);
422-
markConverterDirty(editor);
423-
return { partName: created.partName };
446+
return { partName };
424447
}
425448

426449
function removeEntry(editor: Editor, id: string, dryRun: boolean): boolean {
427450
const convertedXml = getConvertedXml(editor);
428-
const converter = getConverter(editor);
429451
const part = listMetadataParts(convertedXml).find((candidate) => candidate.entries.some((entry) => entry.id === id));
430452
if (!part) return false;
431453

432454
if (dryRun) return true;
433455

434-
const remaining = part.entries.filter((entry) => entry.id !== id);
435-
if (remaining.length === 0) {
436-
removeCustomXmlPart(convertedXml, { partName: part.partName }, converter ?? undefined);
437-
} else {
438-
patchCustomXmlPart(
439-
convertedXml,
440-
{ partName: part.partName },
441-
{ content: buildEnvelopeXml(part.namespace, remaining), schemaRefs: undefined },
442-
converter ?? undefined,
456+
return mutateCustomXmlParts(editor, 'metadata.removeEntry', (sandboxXml, sandboxConverter) => {
457+
const sandboxPart = listMetadataParts(sandboxXml).find((candidate) =>
458+
candidate.entries.some((entry) => entry.id === id),
443459
);
444-
}
445-
markConverterDirty(editor);
446-
return true;
460+
if (!sandboxPart) return false;
461+
462+
const remaining = sandboxPart.entries.filter((entry) => entry.id !== id);
463+
if (remaining.length === 0) {
464+
removeCustomXmlPart(sandboxXml, { partName: sandboxPart.partName }, sandboxConverter);
465+
} else {
466+
patchCustomXmlPart(
467+
sandboxXml,
468+
{ partName: sandboxPart.partName },
469+
{ content: buildEnvelopeXml(sandboxPart.namespace, remaining), schemaRefs: undefined },
470+
sandboxConverter,
471+
);
472+
}
473+
return true;
474+
});
447475
}
448476

449477
function toSummary(entry: MetadataEntry, editor: Editor): AnchoredMetadataSummary {
@@ -619,7 +647,7 @@ export function metadataUpdateWrapper(
619647
if (!dryRun) {
620648
writeEntry(editor, existing.namespace, input.id, input.payload, false);
621649
}
622-
return { changed: true, payload: { success: true, id: input.id } };
650+
return { changed: false, payload: { success: true, id: input.id } };
623651
},
624652
{ dryRun: options?.dryRun ?? false, expectedRevision: options?.expectedRevision },
625653
);
@@ -648,8 +676,8 @@ export function metadataRemoveWrapper(
648676
return executeOutOfBandMutation<AnchoredMetadataMutationResult>(
649677
editor,
650678
(dryRun) => {
651-
const changed = removeEntry(editor, input.id, dryRun);
652-
return { changed, payload: { success: true, id: input.id } };
679+
removeEntry(editor, input.id, dryRun);
680+
return { changed: false, payload: { success: true, id: input.id } };
653681
},
654682
{ dryRun: false, expectedRevision: options?.expectedRevision },
655683
);

0 commit comments

Comments
 (0)