Skip to content

Commit 8051e14

Browse files
shekharnwaghshekharnwaghchriszarate
authored
RTC: Fix stale CRDT document persisted on save (#75975)
* RTC: Extract shared helpers in collaboration persistence E2E tests * RTC: Flush deferred Y.Doc updates before CRDT serialization on save When sync is enabled, Y.Doc updates are deferred via setTimeout(0). saveEntityRecord was calling __unstablePrePersist without yielding first, so the serialized _crdt_document was stale. On reload this caused unnecessary reconciliation saves. Yield to the event loop before __unstablePrePersist when syncConfig is present so pending Y.Doc updates are flushed before serialization. * Await promises internal to sync manager * Update tests to await --------- Co-authored-by: shekharnwagh <shekharnwagh@git.wordpress.org> Co-authored-by: chriszarate <czarate@git.wordpress.org>
1 parent 5044b36 commit 8051e14

7 files changed

Lines changed: 213 additions & 117 deletions

File tree

‎packages/core-data/src/actions.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,10 +773,10 @@ export const saveEntityRecord =
773773
if ( entityConfig.__unstablePrePersist ) {
774774
edits = {
775775
...edits,
776-
...entityConfig.__unstablePrePersist(
776+
...( await entityConfig.__unstablePrePersist(
777777
persistedRecord,
778778
edits
779-
),
779+
) ),
780780
};
781781
}
782782
updatedRecord = await __unstableFetch( {

‎packages/core-data/src/entities.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ export const additionalEntityConfigLoaders = [
275275
* @param {Object} edits Edits.
276276
* @param {string} name Post type name.
277277
* @param {boolean} isTemplate Whether the post type is a template.
278-
* @return {Object} Updated edits.
278+
* @return {Promise< Object >} Updated edits.
279279
*/
280-
export const prePersistPostType = (
280+
export const prePersistPostType = async (
281281
persistedRecord,
282282
edits,
283283
name,
@@ -306,7 +306,7 @@ export const prePersistPostType = (
306306
if ( persistedRecord ) {
307307
const objectType = `postType/${ name }`;
308308
const objectId = persistedRecord.id;
309-
const serializedDoc = getSyncManager()?.createPersistedCRDTDoc(
309+
const serializedDoc = await getSyncManager()?.createPersistedCRDTDoc(
310310
objectType,
311311
objectId
312312
);

‎packages/core-data/src/test/entities.js‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,32 @@ describe( 'getMethodName', () => {
5151
} );
5252

5353
describe( 'prePersistPostType', () => {
54-
it( 'set the status to draft and empty the title when saving auto-draft posts', () => {
54+
it( 'set the status to draft and empty the title when saving auto-draft posts', async () => {
5555
let record = {
5656
status: 'auto-draft',
5757
};
5858
const edits = {};
59-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
59+
expect(
60+
await prePersistPostType( record, edits, 'post', false )
61+
).toEqual( {
6062
status: 'draft',
6163
title: '',
6264
} );
6365

6466
record = {
6567
status: 'publish',
6668
};
67-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual(
68-
{}
69-
);
69+
expect(
70+
await prePersistPostType( record, edits, 'post', false )
71+
).toEqual( {} );
7072

7173
record = {
7274
status: 'auto-draft',
7375
title: 'Auto Draft',
7476
};
75-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
77+
expect(
78+
await prePersistPostType( record, edits, 'post', false )
79+
).toEqual( {
7680
status: 'draft',
7781
title: '',
7882
} );
@@ -81,23 +85,23 @@ describe( 'prePersistPostType', () => {
8185
status: 'publish',
8286
title: 'My Title',
8387
};
84-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual(
85-
{}
86-
);
88+
expect(
89+
await prePersistPostType( record, edits, 'post', false )
90+
).toEqual( {} );
8791
} );
8892

89-
it( 'does not set the status to draft and empty the title when saving templates', () => {
93+
it( 'does not set the status to draft and empty the title when saving templates', async () => {
9094
const record = {
9195
status: 'auto-draft',
9296
title: 'Auto Draft',
9397
};
9498
const edits = {};
95-
expect( prePersistPostType( record, edits, 'post', true ) ).toEqual(
96-
{}
97-
);
99+
expect(
100+
await prePersistPostType( record, edits, 'post', true )
101+
).toEqual( {} );
98102
} );
99103

100-
it( 'adds meta with serialized CRDT doc when createPersistedCRDTDoc returns a value', () => {
104+
it( 'adds meta with serialized CRDT doc when createPersistedCRDTDoc returns a value', async () => {
101105
const mockSerializedDoc = 'serialized-crdt-doc-data';
102106
getSyncManager.mockReturnValue( {
103107
createPersistedCRDTDoc: jest
@@ -107,7 +111,7 @@ describe( 'prePersistPostType', () => {
107111

108112
const record = { id: 123, status: 'publish' };
109113
const edits = {};
110-
const result = prePersistPostType( record, edits, 'post', false );
114+
const result = await prePersistPostType( record, edits, 'post', false );
111115

112116
expect( result.meta ).toEqual( {
113117
[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: mockSerializedDoc,

‎packages/sync/src/manager.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,17 +633,22 @@ export function createSyncManager( debug = false ): SyncManager {
633633
* @param {ObjectType} objectType Object type.
634634
* @param {ObjectID} objectId Object ID.
635635
*/
636-
function createPersistedCRDTDoc(
636+
async function createPersistedCRDTDoc(
637637
objectType: ObjectType,
638638
objectId: ObjectID
639-
): string | null {
639+
): Promise< string | null > {
640640
const entityId = getEntityId( objectType, objectId );
641641
const entityState = entityStates.get( entityId );
642642

643643
if ( ! entityState?.ydoc ) {
644644
return null;
645645
}
646646

647+
// Y.Doc updates are deferred via yieldToEventLoop. Await a promise that
648+
// resolves on the next tick of the event loop so pending updates are flushed
649+
// before we serialize the document.
650+
await new Promise( ( resolve ) => setTimeout( resolve, 0 ) );
651+
647652
return serializeCrdtDoc( entityState.ydoc );
648653
}
649654

‎packages/sync/src/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export interface SyncManager {
147147
createPersistedCRDTDoc: (
148148
objectType: ObjectType,
149149
objectId: ObjectID
150-
) => string | null;
150+
) => Promise< string | null >;
151151
getAwareness: < State extends Awareness >(
152152
objectType: ObjectType,
153153
objectId: ObjectID

‎test/e2e/specs/editor/collaboration/collaboration-persistence.spec.ts‎

Lines changed: 73 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ test.describe( 'Collaboration - CRDT persistence', () => {
1111
page,
1212
requestUtils,
1313
} ) => {
14-
await collaborationUtils.setCollaboration( true );
15-
1614
// Create a draft post — it will not have _crdt_document meta.
1715
const post = await requestUtils.createPost( {
1816
title: 'Persistence Test - Draft',
@@ -30,73 +28,23 @@ test.describe( 'Collaboration - CRDT persistence', () => {
3028
fullscreenMode: false,
3129
} );
3230

33-
// Wait for the entity record resolver to finish.
34-
await page.waitForFunction(
35-
() => {
36-
const postId = ( window as any ).wp.data
37-
.select( 'core/editor' )
38-
.getCurrentPostId();
39-
if ( ! postId ) {
40-
return false;
41-
}
42-
return ( window as any ).wp.data
43-
.select( 'core' )
44-
.hasFinishedResolution( 'getEntityRecord', [
45-
'postType',
46-
'post',
47-
postId,
48-
] );
49-
},
50-
{ timeout: 5000 }
51-
);
52-
53-
const persistedCrdtDoc = await page.evaluate( () => {
54-
return window.wp.data
55-
.select( 'core' )
56-
.getEntityRecord(
57-
'postType',
58-
'post',
59-
window.wp.data.select( 'core/editor' ).getCurrentPostId()
60-
).meta._crdt_document;
61-
} );
31+
// Wait for collaboration runtime and entity record to be ready.
32+
await collaborationUtils.waitForEntityReady( page );
6233

34+
const persistedCrdtDoc =
35+
await collaborationUtils.getCrdtDocument( page );
6336
expect( persistedCrdtDoc ).toBeTruthy();
6437

65-
// Refresh the page and verify the CRDT document is loaded from the persisted meta.
38+
// Refresh the page and verify the CRDT document is loaded from the
39+
// persisted meta and no reconciliation save is triggered.
6640
await page.reload();
41+
await collaborationUtils.waitForEntityReady( page );
6742

68-
// Wait for the entity record resolver to finish.
69-
await page.waitForFunction(
70-
() => {
71-
const postId = ( window as any ).wp.data
72-
.select( 'core/editor' )
73-
.getCurrentPostId();
74-
if ( ! postId ) {
75-
return false;
76-
}
77-
return ( window as any ).wp.data
78-
.select( 'core' )
79-
.hasFinishedResolution( 'getEntityRecord', [
80-
'postType',
81-
'post',
82-
postId,
83-
] );
84-
},
85-
{ timeout: 5000 }
86-
);
87-
88-
const reloadedCrdtDoc = await page.evaluate( () => {
89-
return window.wp.data
90-
.select( 'core' )
91-
.getEntityRecord(
92-
'postType',
93-
'post',
94-
window.wp.data.select( 'core/editor' ).getCurrentPostId()
95-
).meta._crdt_document;
96-
} );
43+
const reloadedCrdtDoc =
44+
await collaborationUtils.getCrdtDocument( page );
9745

98-
// No invalidations should occur on reload, so the CRDT document should be
99-
// the same as before.
46+
// No invalidations should occur on reload, so the CRDT document should
47+
// be identical to the one persisted before reload.
10048
expect( reloadedCrdtDoc ).toBe( persistedCrdtDoc );
10149
} );
10250

@@ -106,51 +54,82 @@ test.describe( 'Collaboration - CRDT persistence', () => {
10654
editor,
10755
page,
10856
} ) => {
109-
await collaborationUtils.setCollaboration( true );
110-
11157
// Navigate to create a new post (auto-draft).
11258
await admin.visitAdminPage( 'post-new.php' );
11359
await editor.setPreferences( 'core/edit-post', {
11460
welcomeGuide: false,
11561
fullscreenMode: false,
11662
} );
11763

118-
// Wait for collaboration runtime to initialize.
64+
// Wait for collaboration runtime to initialize separately first, then
65+
// wait for the entity record resolver to finish.
11966
await page.waitForFunction(
12067
() => ( window as any )._wpCollaborationEnabled === true,
12168
{ timeout: 5000 }
12269
);
70+
await collaborationUtils.waitForEntityReady( page, {
71+
requireCollaboration: false,
72+
} );
12373

124-
// Wait for the entity record resolver to finish.
125-
await page.waitForFunction(
126-
() => {
127-
const postId = ( window as any ).wp.data
128-
.select( 'core/editor' )
129-
.getCurrentPostId();
130-
if ( ! postId ) {
131-
return false;
132-
}
133-
return ( window as any ).wp.data
134-
.select( 'core' )
135-
.hasFinishedResolution( 'getEntityRecord', [
136-
'postType',
137-
'post',
138-
postId,
139-
] );
140-
},
141-
{ timeout: 5000 }
142-
);
74+
const persistedCrdtDoc =
75+
await collaborationUtils.getCrdtDocument( page );
76+
expect( persistedCrdtDoc ).toBeFalsy();
77+
} );
14378

144-
const persistedCrdtDoc = await page.evaluate( () => {
145-
return window.wp.data
146-
.select( 'core' )
147-
.getEntityRecord(
148-
'postType',
149-
'post',
150-
window.wp.data.select( 'core/editor' ).getCurrentPostId()
151-
).meta._crdt_document;
79+
test( 'persisted CRDT document matches content after save and reload', async ( {
80+
admin,
81+
collaborationUtils,
82+
editor,
83+
page,
84+
requestUtils,
85+
} ) => {
86+
// Create a draft post with initial content.
87+
const post = await requestUtils.createPost( {
88+
title: 'Persistence Test - Save Reload',
89+
content:
90+
'<!-- wp:paragraph -->\n<p>Initial content</p>\n<!-- /wp:paragraph -->',
91+
status: 'draft',
92+
date_gmt: new Date().toISOString(),
15293
} );
15394

154-
expect( persistedCrdtDoc ).toBeFalsy();
95+
// Open the post in the editor.
96+
await admin.visitAdminPage(
97+
'post.php',
98+
`post=${ post.id }&action=edit`
99+
);
100+
await editor.setPreferences( 'core/edit-post', {
101+
welcomeGuide: false,
102+
fullscreenMode: false,
103+
} );
104+
105+
// Wait for collaboration runtime and entity record to be ready.
106+
await collaborationUtils.waitForEntityReady( page );
107+
108+
// Edit the paragraph to ensure Y.Doc reflects meaningful changes.
109+
await editor.canvas
110+
.getByRole( 'document', { name: 'Block: Paragraph' } )
111+
.click();
112+
await page.keyboard.press( 'Meta+a' );
113+
await page.keyboard.type( 'Updated content' );
114+
115+
// saveDraft() resolves only after the "Draft saved" notice appears,
116+
// so isSavingPost() is already false when it returns.
117+
await editor.saveDraft();
118+
119+
const crdtDocAfterSave =
120+
await collaborationUtils.getCrdtDocument( page );
121+
expect( crdtDocAfterSave ).toBeTruthy();
122+
123+
// Reload and wait for entity resolution and any in-flight save to
124+
// settle before reading store values.
125+
await page.reload();
126+
await collaborationUtils.waitForEntityReadyAndSaveSettled( page );
127+
128+
// The CRDT document must be unchanged after reload. A difference means
129+
// the _crdt_document persisted during save was stale (e.g. deferred
130+
// Y.Doc updates not yet flushed at serialization time).
131+
const crdtDocAfterReload =
132+
await collaborationUtils.getCrdtDocument( page );
133+
expect( crdtDocAfterReload ).toBe( crdtDocAfterSave );
155134
} );
156135
} );

0 commit comments

Comments
 (0)