Skip to content

Commit 83a8f44

Browse files
chriszaratechriszarate
authored
Move WordPress meta key from sync package to core-data (WordPress#75846)
1 parent e998e0d commit 83a8f44

12 files changed

Lines changed: 119 additions & 182 deletions

File tree

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

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
applyPostChangesToCRDTDoc,
2020
defaultSyncConfig,
2121
getPostChangesFromCRDTDoc,
22+
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
2223
} from './utils/crdt';
2324

2425
export const DEFAULT_ENTITY_KEY = 'id';
@@ -305,11 +306,17 @@ export const prePersistPostType = (
305306
if ( persistedRecord ) {
306307
const objectType = `postType/${ name }`;
307308
const objectId = persistedRecord.id;
308-
const meta = getSyncManager()?.createMeta( objectType, objectId );
309-
newEdits.meta = {
310-
...edits.meta,
311-
...meta,
312-
};
309+
const serializedDoc = getSyncManager()?.createPersistedCRDTDoc(
310+
objectType,
311+
objectId
312+
);
313+
314+
if ( serializedDoc ) {
315+
newEdits.meta = {
316+
...edits.meta,
317+
[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: serializedDoc,
318+
};
319+
}
313320
}
314321

315322
return newEdits;
@@ -404,13 +411,14 @@ async function loadPostTypeEntities() {
404411
getPostChangesFromCRDTDoc( crdtDoc, editedRecord, postType ),
405412

406413
/**
407-
* Sync features supported by the entity.
414+
* Extract changes from a CRDT document that can be used to update the
415+
* local editor state.
408416
*
409-
* @type {Record< string, boolean >}
417+
* @param {import('@wordpress/sync').ObjectData} record
418+
* @return {Partial< import('@wordpress/sync').ObjectData >} Changes to record
410419
*/
411-
supports: {
412-
crdtPersistence: true,
413-
},
420+
getPersistedCRDTDoc: ( record ) =>
421+
record[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ] || null,
414422
};
415423

416424
return entity;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,12 @@ export const getEntityRecord =
235235
resolveSelect
236236
.getEditedEntityRecord( kind, name, key )
237237
.then( ( editedRecord ) => {
238+
// Don't trigger a save if the record is still an auto-draft.
239+
const { status } = editedRecord;
240+
if ( 'auto-draft' === status ) {
241+
return;
242+
}
243+
238244
dispatch.saveEntityRecord(
239245
kind,
240246
name,

‎packages/core-data/src/sync.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ const {
1717
CRDT_DOC_META_PERSISTENCE_KEY,
1818
CRDT_RECORD_MAP_KEY,
1919
LOCAL_EDITOR_ORIGIN,
20-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
2120
} = unlock( syncPrivateApis );
2221

2322
export {
2423
Delta,
2524
CRDT_DOC_META_PERSISTENCE_KEY,
2625
CRDT_RECORD_MAP_KEY,
2726
LOCAL_EDITOR_ORIGIN,
28-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
2927
};
3028

3129
let syncManager: SyncManager;

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

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
import apiFetch from '@wordpress/api-fetch';
55

66
jest.mock( '@wordpress/api-fetch' );
7+
jest.mock( '../sync', () => ( {
8+
...jest.requireActual( '../sync' ),
9+
getSyncManager: jest.fn(),
10+
} ) );
711

812
/**
913
* Internal dependencies
@@ -14,6 +18,8 @@ import {
1418
prePersistPostType,
1519
additionalEntityConfigLoaders,
1620
} from '../entities';
21+
import { getSyncManager } from '../sync';
22+
import { POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE } from '../utils/crdt';
1723

1824
describe( 'getMethodName', () => {
1925
it( 'should return the right method name for an entity with the root kind', () => {
@@ -53,15 +59,14 @@ describe( 'prePersistPostType', () => {
5359
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
5460
status: 'draft',
5561
title: '',
56-
meta: {},
5762
} );
5863

5964
record = {
6065
status: 'publish',
6166
};
62-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
63-
meta: {},
64-
} );
67+
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual(
68+
{}
69+
);
6570

6671
record = {
6772
status: 'auto-draft',
@@ -70,16 +75,15 @@ describe( 'prePersistPostType', () => {
7075
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
7176
status: 'draft',
7277
title: '',
73-
meta: {},
7478
} );
7579

7680
record = {
7781
status: 'publish',
7882
title: 'My Title',
7983
};
80-
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual( {
81-
meta: {},
82-
} );
84+
expect( prePersistPostType( record, edits, 'post', false ) ).toEqual(
85+
{}
86+
);
8387
} );
8488

8589
it( 'does not set the status to draft and empty the title when saving templates', () => {
@@ -88,9 +92,34 @@ describe( 'prePersistPostType', () => {
8892
title: 'Auto Draft',
8993
};
9094
const edits = {};
91-
expect( prePersistPostType( record, edits, 'post', true ) ).toEqual( {
92-
meta: {},
95+
expect( prePersistPostType( record, edits, 'post', true ) ).toEqual(
96+
{}
97+
);
98+
} );
99+
100+
it( 'adds meta with serialized CRDT doc when createPersistedCRDTDoc returns a value', () => {
101+
const mockSerializedDoc = 'serialized-crdt-doc-data';
102+
getSyncManager.mockReturnValue( {
103+
createPersistedCRDTDoc: jest
104+
.fn()
105+
.mockReturnValue( mockSerializedDoc ),
106+
} );
107+
108+
const record = { id: 123, status: 'publish' };
109+
const edits = {};
110+
const result = prePersistPostType( record, edits, 'post', false );
111+
112+
expect( result.meta ).toEqual( {
113+
[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: mockSerializedDoc,
93114
} );
115+
116+
expect( getSyncManager ).toHaveBeenCalled();
117+
expect( getSyncManager().createPersistedCRDTDoc ).toHaveBeenCalledWith(
118+
'postType/post',
119+
123
120+
);
121+
122+
getSyncManager.mockReset();
94123
} );
95124
} );
96125

‎packages/core-data/src/utils/crdt.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@ import {
2828
} from './crdt-blocks';
2929
import { type Post } from '../entity-types/post';
3030
import { type Type } from '../entity-types';
31-
import {
32-
CRDT_DOC_META_PERSISTENCE_KEY,
33-
CRDT_RECORD_MAP_KEY,
34-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
35-
} from '../sync';
31+
import { CRDT_DOC_META_PERSISTENCE_KEY, CRDT_RECORD_MAP_KEY } from '../sync';
3632
import type { WPSelection } from '../types';
3733
import {
3834
getSelectionHistory,
@@ -78,6 +74,8 @@ export interface YPostRecord extends YMapRecord {
7874
title: Y.Text;
7975
}
8076

77+
export const POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE = '_crdt_document';
78+
8179
// Properties that are allowed to be synced for a post.
8280
const allowedPostProperties = new Set< string >( [
8381
'author',
@@ -101,7 +99,7 @@ const allowedPostProperties = new Set< string >( [
10199

102100
// Post meta keys that should *not* be synced.
103101
const disallowedPostMetaKeys = new Set< string >( [
104-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
102+
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
105103
] );
106104

107105
/**

‎packages/core-data/src/utils/test/crdt.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals';
1111
/**
1212
* Internal dependencies
1313
*/
14-
import {
15-
CRDT_RECORD_MAP_KEY,
16-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
17-
} from '../../sync';
14+
import { CRDT_RECORD_MAP_KEY } from '../../sync';
1815
import {
1916
applyPostChangesToCRDTDoc,
2017
getPostChangesFromCRDTDoc,
18+
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
2119
type PostChanges,
2220
type YPostRecord,
2321
} from '../crdt';
@@ -605,10 +603,7 @@ describe( 'crdt', () => {
605603
it( 'excludes disallowed meta keys in changes', () => {
606604
const metaMap = createYMap();
607605
metaMap.set( 'public_meta', 'new value' );
608-
metaMap.set(
609-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE,
610-
'exclude me'
611-
);
606+
metaMap.set( POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE, 'exclude me' );
612607
map.set( 'meta', metaMap );
613608

614609
const editedRecord = {
@@ -627,7 +622,7 @@ describe( 'crdt', () => {
627622
public_meta: 'new value', // from CRDT
628623
} );
629624
expect( changes.meta ).not.toHaveProperty(
630-
WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE
625+
POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE
631626
);
632627
} );
633628

‎packages/sync/src/config.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,3 @@ export const LOCAL_EDITOR_ORIGIN = 'gutenberg';
4848
* Origin string for CRDT document changes originating from the sync manager.
4949
*/
5050
export const LOCAL_SYNC_MANAGER_ORIGIN = 'syncManager';
51-
52-
/**
53-
* WordPress meta key used to persist the CRDT document for an entity.
54-
*/
55-
export const WORDPRESS_META_KEY_FOR_CRDT_DOC_PERSISTENCE = '_crdt_document';

‎packages/sync/src/manager.ts‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,16 @@ import type { Awareness } from 'y-protocols/awareness';
88
* Internal dependencies
99
*/
1010
import {
11-
CRDT_RECORD_MAP_KEY as RECORD_KEY,
12-
LOCAL_SYNC_MANAGER_ORIGIN,
11+
CRDT_RECORD_MAP_KEY,
1312
CRDT_STATE_MAP_KEY,
1413
CRDT_STATE_MAP_SAVED_AT_KEY as SAVED_AT_KEY,
14+
LOCAL_SYNC_MANAGER_ORIGIN,
1515
} from './config';
1616
import {
1717
logPerformanceTiming,
1818
passThru,
1919
yieldToEventLoop,
2020
} from './performance';
21-
import { createPersistedCRDTDoc, getPersistedCrdtDoc } from './persistence';
2221
import { getProviderCreators } from './providers';
2322
import type {
2423
CollectionHandlers,
@@ -35,7 +34,12 @@ import type {
3534
SyncUndoManager,
3635
} from './types';
3736
import { createUndoManager } from './undo-manager';
38-
import { createYjsDoc, markEntityAsSaved } from './utils';
37+
import {
38+
createYjsDoc,
39+
deserializeCrdtDoc,
40+
markEntityAsSaved,
41+
serializeCrdtDoc,
42+
} from './utils';
3943

4044
interface CollectionState {
4145
awareness?: Awareness;
@@ -150,7 +154,7 @@ export function createSyncManager( debug = false ): SyncManager {
150154
};
151155

152156
const ydoc = createYjsDoc( { objectType } );
153-
const recordMap = ydoc.getMap( RECORD_KEY );
157+
const recordMap = ydoc.getMap( CRDT_RECORD_MAP_KEY );
154158
const stateMap = ydoc.getMap( CRDT_STATE_MAP_KEY );
155159
const now = Date.now();
156160

@@ -404,32 +408,22 @@ export function createSyncManager( debug = false ): SyncManager {
404408
syncConfig: {
405409
applyChangesToCRDTDoc,
406410
getChangesFromCRDTDoc,
407-
supports,
411+
getPersistedCrdtDoc,
408412
},
409413
ydoc: targetDoc,
410414
} = entityState;
411415

412-
if ( ! supports?.crdtPersistence ) {
413-
// Apply the current record as changes.
414-
targetDoc.transact( () => {
415-
applyChangesToCRDTDoc( targetDoc, record );
416-
}, LOCAL_SYNC_MANAGER_ORIGIN );
417-
return;
418-
}
419-
420416
// Get the persisted CRDT document, if it exists.
421-
const tempDoc = getPersistedCrdtDoc( record );
417+
const serialized = getPersistedCrdtDoc?.( record );
418+
const tempDoc = serialized ? deserializeCrdtDoc( serialized ) : null;
422419

423420
if ( ! tempDoc ) {
424421
// Apply the current record as changes and trigger a save, which will
425-
// persist the CRDT document. (The entity should call `createEntityMeta`
422+
// persist the CRDT document. (The entity should call `createPersistedCRDTDoc`
426423
// via its pre-persist hook.)
427424
targetDoc.transact( () => {
428425
applyChangesToCRDTDoc( targetDoc, record );
429-
430-
if ( 'auto-draft' !== record.status ) {
431-
handlers.saveRecord();
432-
}
426+
handlers.saveRecord();
433427
}, LOCAL_SYNC_MANAGER_ORIGIN );
434428
return;
435429
}
@@ -477,8 +471,8 @@ export function createSyncManager( debug = false ): SyncManager {
477471
);
478472

479473
// Apply the changes and trigger a save, which will persist the CRDT
480-
// document. (The entity should call `createEntityMeta` via its pre-persist
481-
// hook.)
474+
// document. (The entity should call `createPersistedCRDTDoc` via its
475+
// pre-persist hook.)
482476
targetDoc.transact( () => {
483477
applyChangesToCRDTDoc( targetDoc, changes );
484478
handlers.saveRecord();
@@ -580,18 +574,18 @@ export function createSyncManager( debug = false ): SyncManager {
580574
* @param {ObjectType} objectType Object type.
581575
* @param {ObjectID} objectId Object ID.
582576
*/
583-
function createEntityMeta(
577+
function createPersistedCRDTDoc(
584578
objectType: ObjectType,
585579
objectId: ObjectID
586-
): Record< string, string > {
580+
): string | null {
587581
const entityId = getEntityId( objectType, objectId );
588582
const entityState = entityStates.get( entityId );
589583

590-
if ( ! entityState?.syncConfig.supports?.crdtPersistence ) {
591-
return {};
584+
if ( ! entityState?.ydoc ) {
585+
return null;
592586
}
593587

594-
return createPersistedCRDTDoc( entityState.ydoc );
588+
return serializeCrdtDoc( entityState.ydoc );
595589
}
596590

597591
// Collect internal functions so that they can be wrapped before calling.
@@ -602,7 +596,7 @@ export function createSyncManager( debug = false ): SyncManager {
602596

603597
// Wrap and return the public API.
604598
return {
605-
createMeta: debugWrap( createEntityMeta ),
599+
createPersistedCRDTDoc: debugWrap( createPersistedCRDTDoc ),
606600
getAwareness,
607601
load: debugWrap( loadEntity ),
608602
loadCollection: debugWrap( loadCollection ),

0 commit comments

Comments
 (0)