diff --git a/packages/core-data/src/entities.js b/packages/core-data/src/entities.js index 83e1ccd1c79e5f..34dcff5586882a 100644 --- a/packages/core-data/src/entities.js +++ b/packages/core-data/src/entities.js @@ -9,6 +9,7 @@ import { capitalCase, pascalCase } from 'change-case'; import apiFetch from '@wordpress/api-fetch'; import { __unstableSerializeAndClean, parse } from '@wordpress/blocks'; import { __ } from '@wordpress/i18n'; +import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies @@ -25,6 +26,145 @@ import { export const DEFAULT_ENTITY_KEY = 'id'; const POST_RAW_ATTRIBUTES = [ 'title', 'excerpt', 'content' ]; +const POST_TYPES_WITH_STALE_SAVE_PROTECTION = new Set( [ 'post', 'page' ] ); + +function getRawPostValue( value ) { + return value && typeof value === 'object' && 'raw' in value + ? value.raw + : value; +} + +function getSerializedBlockValue( block ) { + return __unstableSerializeAndClean( [ block ] ).trim(); +} + +function getSerializedCRDTBlockContent( crdtRecord ) { + return Array.isArray( crdtRecord?.blocks ) + ? __unstableSerializeAndClean( crdtRecord.blocks ).trim() + : undefined; +} + +function getCRDTRawPostValue( crdtRecord, key ) { + if ( key === 'content' ) { + return ( + getSerializedCRDTBlockContent( crdtRecord ) ?? + getRawPostValue( crdtRecord?.content ) + ); + } + + return getRawPostValue( crdtRecord?.[ key ] ); +} + +function areSerializedBlocksEqualAt( blocksA, blocksB, index ) { + return ( + blocksA[ index ]?.name === blocksB[ index ]?.name && + getSerializedBlockValue( blocksA[ index ] ) === + getSerializedBlockValue( blocksB[ index ] ) + ); +} + +function mergeStaleSerializedBlockContent( + baseContent, + latestContent, + localContent +) { + if ( + typeof baseContent !== 'string' || + typeof latestContent !== 'string' || + typeof localContent !== 'string' + ) { + return; + } + + const baseBlocks = parse( baseContent ); + const latestBlocks = parse( latestContent ); + const localBlocks = parse( localContent ); + + if ( + ! baseBlocks.length || + ! latestBlocks.length || + ! localBlocks.length + ) { + return; + } + + if ( + latestBlocks.length > localBlocks.length && + baseBlocks.length === latestBlocks.length + ) { + for ( let index = 0; index < localBlocks.length; index++ ) { + if ( localBlocks[ index ].name !== latestBlocks[ index ].name ) { + return; + } + } + + return __unstableSerializeAndClean( [ + ...localBlocks, + ...latestBlocks.slice( localBlocks.length ), + ] ); + } + + if ( + baseBlocks.length < latestBlocks.length && + baseBlocks.length < localBlocks.length + ) { + for ( let index = 0; index < baseBlocks.length; index++ ) { + if ( + ! areSerializedBlocksEqualAt( + baseBlocks, + latestBlocks, + index + ) || + ! areSerializedBlocksEqualAt( baseBlocks, localBlocks, index ) + ) { + return; + } + } + + return __unstableSerializeAndClean( [ + ...localBlocks, + ...latestBlocks.slice( baseBlocks.length ), + ] ); + } + + if ( + baseBlocks.length !== latestBlocks.length || + baseBlocks.length !== localBlocks.length + ) { + return; + } + + const mergedBlocks = []; + + for ( let index = 0; index < baseBlocks.length; index++ ) { + const baseBlock = baseBlocks[ index ]; + const latestBlock = latestBlocks[ index ]; + const localBlock = localBlocks[ index ]; + + if ( + baseBlock.name !== latestBlock.name || + baseBlock.name !== localBlock.name + ) { + return; + } + + const baseValue = getSerializedBlockValue( baseBlock ); + const latestValue = getSerializedBlockValue( latestBlock ); + const localValue = getSerializedBlockValue( localBlock ); + + if ( localValue === latestValue ) { + mergedBlocks.push( localBlock ); + } else if ( localValue === baseValue ) { + mergedBlocks.push( latestBlock ); + } else if ( latestValue === baseValue ) { + mergedBlocks.push( localBlock ); + } else { + return; + } + } + + return __unstableSerializeAndClean( mergedBlocks ); +} const blocksTransientEdits = { blocks: { @@ -279,15 +419,31 @@ export const additionalEntityConfigLoaders = [ * @param {Object} edits Edits. * @param {string} name Post type name. * @param {boolean} isTemplate Whether the post type is a template. + * @param {string} baseURL REST base URL for the post type. * @return {Promise< Object >} Updated edits. */ export const prePersistPostType = async ( persistedRecord, edits, name, - isTemplate + isTemplate, + baseURL ) => { const newEdits = {}; + const objectType = `postType/${ name }`; + const objectId = persistedRecord?.id; + let syncManager; + let serializedDoc; + let hasSerializedDoc = false; + const editedSavedFields = POST_RAW_ATTRIBUTES.filter( + ( key ) => key in edits + ); + const locallyChangedSavedFields = editedSavedFields.filter( + ( key ) => + getRawPostValue( edits[ key ] ) !== + getRawPostValue( persistedRecord?.[ key ] ) + ); + const locallyChangedSavedFieldSet = new Set( locallyChangedSavedFields ); if ( ! isTemplate && persistedRecord?.status === 'auto-draft' ) { // Saving an auto-draft should create a draft by default. @@ -306,14 +462,125 @@ export const prePersistPostType = async ( } } + if ( + window._wpCollaborationEnabled && + POST_TYPES_WITH_STALE_SAVE_PROTECTION.has( name ) && + baseURL && + objectId && + editedSavedFields.length + ) { + try { + syncManager = getSyncManager(); + serializedDoc = await syncManager?.createPersistedCRDTDoc( + objectType, + objectId + ); + hasSerializedDoc = !! serializedDoc; + const latestRecord = await apiFetch( { + path: addQueryArgs( `${ baseURL }/${ objectId }`, { + context: 'edit', + } ), + } ); + const serverChangedSavedFields = editedSavedFields.filter( + ( key ) => + getRawPostValue( latestRecord?.[ key ] ) !== + getRawPostValue( persistedRecord?.[ key ] ) + ); + for ( const key of serverChangedSavedFields ) { + if ( + ! locallyChangedSavedFieldSet.has( key ) && + key in ( latestRecord ?? {} ) + ) { + newEdits[ key ] = getRawPostValue( latestRecord[ key ] ); + } + } + + const hasLatestPersistedCRDTDoc = Boolean( + latestRecord?.meta?.[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ] + ); + const shouldApplyLatestCRDTDoc = + hasLatestPersistedCRDTDoc || locallyChangedSavedFields.length; + const didApplyLatestCRDTDoc = shouldApplyLatestCRDTDoc + ? ( await syncManager?.applyPersistedCRDTDoc?.( + objectType, + objectId, + latestRecord + ) ) ?? false + : false; + + if ( + didApplyLatestCRDTDoc || + ( hasLatestPersistedCRDTDoc && serverChangedSavedFields.length ) + ) { + serializedDoc = await syncManager?.createPersistedCRDTDoc( + objectType, + objectId + ); + hasSerializedDoc = !! serializedDoc; + + if ( + hasLatestPersistedCRDTDoc && + locallyChangedSavedFields.length + ) { + const crdtRecord = syncManager?.getCRDTRecordData?.( + objectType, + objectId + ); + + for ( const key of locallyChangedSavedFields ) { + const hasCRDTValue = + key === 'content' + ? key in ( crdtRecord ?? {} ) || + Array.isArray( crdtRecord?.blocks ) + : key in ( crdtRecord ?? {} ); + + if ( hasCRDTValue ) { + const crdtValue = getCRDTRawPostValue( + crdtRecord, + key + ); + + if ( + crdtValue !== + getRawPostValue( latestRecord?.[ key ] ) + ) { + newEdits[ key ] = crdtValue; + } + } + } + } + } + + if ( + locallyChangedSavedFieldSet.has( 'content' ) && + ! ( 'content' in newEdits ) + ) { + const mergedContent = mergeStaleSerializedBlockContent( + getRawPostValue( persistedRecord?.content ), + getRawPostValue( latestRecord?.content ), + getRawPostValue( edits.content ) + ); + + if ( + mergedContent !== undefined && + mergedContent !== getRawPostValue( edits.content ) + ) { + newEdits.content = mergedContent; + } + } + } catch { + // A failed freshness check should not block saving. The request itself + // will still surface any real save errors to the editor. + } + } + // Add meta for persisted CRDT document. if ( persistedRecord ) { - const objectType = `postType/${ name }`; - const objectId = persistedRecord.id; - const serializedDoc = await getSyncManager()?.createPersistedCRDTDoc( - objectType, - objectId - ); + if ( ! hasSerializedDoc ) { + serializedDoc = await ( + syncManager ?? getSyncManager() + )?.createPersistedCRDTDoc( objectType, objectId ); + } if ( serializedDoc ) { newEdits.meta = { @@ -387,7 +654,13 @@ async function loadPostTypeEntities() { ? capitalCase( record.slug ?? '' ) : String( record.id ) ), __unstablePrePersist: ( persistedRecord, edits ) => - prePersistPostType( persistedRecord, edits, name, isTemplate ), + prePersistPostType( + persistedRecord, + edits, + name, + isTemplate, + `/${ namespace }/${ postType.rest_base }` + ), __unstable_rest_base: postType.rest_base, supportsPagination: true, getRevisionsUrl: ( parentId, revisionId ) => diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index dfef02c0968e12..b010d97580317a 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -250,11 +250,13 @@ export const getEntityRecord = // Trigger a save to persist the CRDT document. The entity's // pre-persist hooks will create the persisted CRDT document // and apply it to the record's meta. - dispatch.saveEntityRecord( - kind, - name, - editedRecord - ); + const entityIdKey = + entityConfig.key || DEFAULT_ENTITY_KEY; + dispatch.saveEntityRecord( kind, name, { + [ entityIdKey ]: + editedRecord[ entityIdKey ] ?? key, + meta, + } ); } ); }, addUndoMeta: ( ydoc, meta ) => { diff --git a/packages/core-data/src/test/entities.js b/packages/core-data/src/test/entities.js index 5f45462655f5eb..fd4e4174b0b390 100644 --- a/packages/core-data/src/test/entities.js +++ b/packages/core-data/src/test/entities.js @@ -2,6 +2,11 @@ * WordPress dependencies */ import apiFetch from '@wordpress/api-fetch'; +import { + parse, + registerBlockType, + unregisterBlockType, +} from '@wordpress/blocks'; jest.mock( '@wordpress/api-fetch' ); jest.mock( '../sync', () => ( { @@ -28,6 +33,18 @@ import { POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE, } from '../utils/crdt'; +const TEST_BLOCK_NAME = 'test/stale-content-block'; + +function paragraphMarkup( content ) { + return ``; +} + +function pageContent( contents ) { + return contents.map( paragraphMarkup ).join( '\n\n' ); +} + describe( 'getMethodName', () => { it( 'should return the right method name for an entity with the root kind', () => { const methodName = getMethodName( 'root', 'postType' ); @@ -58,6 +75,36 @@ describe( 'getMethodName', () => { } ); describe( 'prePersistPostType', () => { + let originalCollaborationEnabled; + + beforeAll( () => { + registerBlockType( TEST_BLOCK_NAME, { + apiVersion: 3, + title: 'Stale content test block', + category: 'text', + attributes: { + content: { + type: 'string', + }, + }, + save: () => null, + } ); + } ); + + afterAll( () => { + unregisterBlockType( TEST_BLOCK_NAME ); + } ); + + beforeEach( () => { + apiFetch.mockReset(); + getSyncManager.mockReset(); + originalCollaborationEnabled = window._wpCollaborationEnabled; + } ); + + afterEach( () => { + window._wpCollaborationEnabled = originalCollaborationEnabled; + } ); + it( 'set the status to draft and empty the title when saving auto-draft posts', async () => { let record = { status: 'auto-draft', @@ -132,6 +179,451 @@ describe( 'prePersistPostType', () => { getSyncManager.mockReset(); } ); + + it( 'preserves latest saved content when a full-record save only changes other fields', async () => { + const baseContent = pageContent( [ 'Alpha', 'Beta' ] ); + const latestContent = pageContent( [ 'Alpha', 'current content' ] ); + const latestRecord = { + id: 123, + content: { raw: latestContent }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'local-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: 'older local crdt content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: baseContent }, + meta: { + foo: 'base', + }, + }, + { + content: baseContent, + meta: { + foo: 'changed', + }, + }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( apiFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/pages/123?context=edit', + } ); + expect( syncManager.applyPersistedCRDTDoc ).toHaveBeenCalledWith( + 'postType/page', + 123, + latestRecord + ); + expect( syncManager.getCRDTRecordData ).not.toHaveBeenCalled(); + expect( result ).toEqual( { + content: latestContent, + meta: { + foo: 'changed', + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'local-doc', + }, + } ); + } ); + + it( 'merges the latest persisted CRDT record before saving stale post content', async () => { + const latestRecord = { + id: 123, + content: { raw: 'current content' }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn(), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: 'merged content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: 'base content' }, + }, + { content: 'stale local content' }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( apiFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/pages/123?context=edit', + } ); + expect( syncManager.applyPersistedCRDTDoc ).toHaveBeenCalledWith( + 'postType/page', + 123, + latestRecord + ); + expect( syncManager.getCRDTRecordData ).toHaveBeenCalledWith( + 'postType/page', + 123 + ); + expect( result ).toEqual( { + content: 'merged content', + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + }, + } ); + } ); + + it( 'uses the CRDT record when applying the latest persisted document changes local state', async () => { + const latestRecord = { + id: 123, + content: { raw: 'current content' }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( true ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: 'merged content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: 'current content' }, + }, + { content: 'stale local content' }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( syncManager.applyPersistedCRDTDoc ).toHaveBeenCalledWith( + 'postType/page', + 123, + latestRecord + ); + expect( syncManager.getCRDTRecordData ).toHaveBeenCalledWith( + 'postType/page', + 123 + ); + expect( result ).toEqual( { + content: 'merged content', + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + }, + } ); + } ); + + it( 'derives stale saved content from CRDT blocks instead of serialized CRDT content', async () => { + const mergedContent = pageContent( [ + 'Alpha', + 'stale local content', + 'current content', + ] ); + const latestRecord = { + id: 123, + content: { raw: pageContent( [ 'Alpha', 'current content' ] ) }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( true ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + blocks: parse( mergedContent ), + content: 'mangled serialized CRDT content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: pageContent( [ 'Alpha' ] ) }, + }, + { content: pageContent( [ 'Alpha', 'stale local content' ] ) }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( result.content ).toBe( mergedContent ); + expect( result.content ).not.toContain( 'mangled' ); + expect( result.meta ).toEqual( { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + } ); + } ); + + it( 'merges non-conflicting stale serialized content edits with the latest saved content', async () => { + const latestRecord = { + id: 123, + content: { raw: pageContent( [ 'Alpha', 'current content' ] ) }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: latestRecord.content.raw, + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: pageContent( [ 'Alpha', 'Beta' ] ) }, + }, + { content: pageContent( [ 'stale local content', 'Beta' ] ) }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( result.content ).toContain( 'stale local content' ); + expect( result.content ).toContain( 'current content' ); + expect( result.meta ).toEqual( { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + } ); + } ); + + it( 'preserves latest trailing serialized blocks when a stale content edit submits an older shorter body', async () => { + const latestContent = pageContent( [ + 'Alpha', + 'Beta', + 'current content', + ] ); + const latestRecord = { + id: 123, + content: { raw: latestContent }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: latestContent, + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: latestContent }, + }, + { content: pageContent( [ 'stale local content', 'Beta' ] ) }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( result.content ).toContain( 'stale local content' ); + expect( result.content ).toContain( 'current content' ); + expect( result.meta ).toEqual( { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + } ); + } ); + + it( 'merges sibling serialized blocks appended from a shared stale base', async () => { + const latestRecord = { + id: 123, + content: { raw: pageContent( [ 'Alpha', 'current content' ] ) }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: latestRecord.content.raw, + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: pageContent( [ 'Alpha' ] ) }, + }, + { content: pageContent( [ 'Alpha', 'stale local content' ] ) }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( result.content ).toContain( 'stale local content' ); + expect( result.content ).toContain( 'current content' ); + expect( result.meta ).toEqual( { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + } ); + } ); + + it( 'does not merge stale serialized content edits when the same block changed locally and remotely', async () => { + const latestRecord = { + id: 123, + content: { raw: pageContent( [ 'current content', 'Beta' ] ) }, + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'latest-doc', + }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'merged-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: latestRecord.content.raw, + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: pageContent( [ 'Alpha', 'Beta' ] ) }, + }, + { content: pageContent( [ 'stale local content', 'Beta' ] ) }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( result ).toEqual( { + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'merged-doc', + }, + } ); + } ); + + it( 'does not replace edited content from CRDT when the latest record has no persisted CRDT document', async () => { + const latestRecord = { + id: 123, + content: { raw: 'base content' }, + meta: {}, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( true ), + createPersistedCRDTDoc: jest + .fn() + .mockResolvedValueOnce( 'before-apply-doc' ) + .mockResolvedValueOnce( 'after-apply-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: 'partially flushed local crdt content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: 'base content' }, + }, + { content: 'new local content' }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( syncManager.applyPersistedCRDTDoc ).toHaveBeenCalledWith( + 'postType/page', + 123, + latestRecord + ); + expect( syncManager.getCRDTRecordData ).not.toHaveBeenCalled(); + expect( result ).toEqual( { + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'after-apply-doc', + }, + } ); + } ); + + it( 'does not replace edited content when the latest saved post has not changed', async () => { + const latestRecord = { + id: 123, + content: { raw: 'base content' }, + }; + const syncManager = { + applyPersistedCRDTDoc: jest.fn().mockResolvedValue( false ), + createPersistedCRDTDoc: jest.fn().mockResolvedValue( 'local-doc' ), + getCRDTRecordData: jest.fn( () => ( { + content: 'older local crdt content', + } ) ), + }; + apiFetch.mockResolvedValue( latestRecord ); + getSyncManager.mockReturnValue( syncManager ); + window._wpCollaborationEnabled = true; + + const result = await prePersistPostType( + { + id: 123, + status: 'publish', + content: { raw: 'base content' }, + }, + { content: 'new local content' }, + 'page', + false, + '/wp/v2/pages' + ); + + expect( syncManager.applyPersistedCRDTDoc ).toHaveBeenCalledWith( + 'postType/page', + 123, + latestRecord + ); + expect( syncManager.getCRDTRecordData ).not.toHaveBeenCalled(); + expect( result ).toEqual( { + meta: { + [ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ]: 'local-doc', + }, + } ); + } ); } ); describe( 'loadPostTypeEntities', () => { diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 84bff2d9cf1cbf..116fa3d466e5ae 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -232,7 +232,7 @@ describe( 'getEntityRecord', () => { expect( dispatch.saveEntityRecord ).not.toHaveBeenCalled(); } ); - it( 'persistCRDTDoc fetches edited record and saves full entity record', async () => { + it( 'persistCRDTDoc fetches edited record and saves only entity meta', async () => { const POST_RECORD = { id: 1, title: 'Test Post', meta: {} }; const EDITED_RECORD = { id: 1, title: 'Edited Post', meta: {} }; const POST_RESPONSE = { @@ -281,11 +281,12 @@ describe( 'getEntityRecord', () => { resolveSelectWithSync.getEditedEntityRecord ).toHaveBeenCalledWith( 'postType', 'post', 1 ); - // Should have called saveEntityRecord (not saveEditedEntityRecord). + // Should have called saveEntityRecord (not saveEditedEntityRecord) + // without resaving the full edited body. expect( dispatch.saveEntityRecord ).toHaveBeenCalledWith( 'postType', 'post', - EDITED_RECORD + { id: 1, meta: {} } ); } ); @@ -332,11 +333,11 @@ describe( 'getEntityRecord', () => { handlers.persistCRDTDoc(); await resolveSelectWithSync.getEditedEntityRecord(); - // Should save the record even with no edits (the whole point of the fix). + // Should save the meta even with no edits (the whole point of the fix). expect( dispatch.saveEntityRecord ).toHaveBeenCalledWith( 'postType', 'post', - POST_RECORD + { id: 1, meta: {} } ); } ); diff --git a/packages/core-data/src/utils/crdt-blocks.ts b/packages/core-data/src/utils/crdt-blocks.ts index 0c5e56ff33c7a2..1157ab4fc0eacb 100644 --- a/packages/core-data/src/utils/crdt-blocks.ts +++ b/packages/core-data/src/utils/crdt-blocks.ts @@ -78,6 +78,7 @@ export type YBlockAttributes = Y.Map< Y.Text | unknown >; export type MergeCursorPosition = WPBlockSelection | null; const serializableBlocksCache = new WeakMap< WeakKey, Block[] >(); +const previousBlocksByYArray = new WeakMap< YBlocks, Block[] >(); /** * Recursively walk an attribute value and convert any RichTextData instances @@ -157,6 +158,313 @@ function makeBlocksSerializable( blocks: Block[] ): Block[] { } ); } +function getBlockClientId( block: Block ): string | undefined { + return 'string' === typeof block.clientId && block.clientId + ? block.clientId + : undefined; +} + +function getClientIdsIfEveryBlockHasUniqueId( + blocks: Block[] +): string[] | null { + const ids: string[] = []; + const seenIds = new Set< string >(); + + for ( const block of blocks ) { + const clientId = getBlockClientId( block ); + + if ( ! clientId || seenIds.has( clientId ) ) { + return null; + } + + ids.push( clientId ); + seenIds.add( clientId ); + } + + return ids; +} + +function getBlocksByClientIdIfEveryBlockHasUniqueId( + blocks: Block[] +): Map< string, Block > | null { + const clientIds = getClientIdsIfEveryBlockHasUniqueId( blocks ); + + if ( ! clientIds ) { + return null; + } + + return new Map( + clientIds.map( ( clientId, index ) => [ clientId, blocks[ index ] ] ) + ); +} + +function findBlockIndexByClientId( blocks: Block[], clientId: string ): number { + return blocks.findIndex( + ( block ) => getBlockClientId( block ) === clientId + ); +} + +function getRemoteBlockInsertIndex( + currentBlocks: Block[], + currentIndex: number, + blocksToSync: Block[], + blockIdsToSync: Set< string > +): number { + for ( let index = currentIndex - 1; index >= 0; index-- ) { + const previousClientId = getBlockClientId( currentBlocks[ index ] ); + + if ( previousClientId && blockIdsToSync.has( previousClientId ) ) { + const previousIndex = findBlockIndexByClientId( + blocksToSync, + previousClientId + ); + + return -1 === previousIndex + ? blocksToSync.length + : previousIndex + 1; + } + } + + for ( + let index = currentIndex + 1; + index < currentBlocks.length; + index++ + ) { + const nextClientId = getBlockClientId( currentBlocks[ index ] ); + + if ( nextClientId && blockIdsToSync.has( nextClientId ) ) { + const nextIndex = findBlockIndexByClientId( + blocksToSync, + nextClientId + ); + + return -1 === nextIndex ? blocksToSync.length : nextIndex; + } + } + + return blocksToSync.length; +} + +function reconcileStaleLocalBlockAttributes( + localAttributes: BlockAttributes, + previousAttributes: BlockAttributes, + currentAttributes: BlockAttributes +): BlockAttributes { + let reconciledAttributes: BlockAttributes | undefined; + const attributeNames = new Set( [ + ...Object.keys( localAttributes ), + ...Object.keys( previousAttributes ), + ...Object.keys( currentAttributes ), + ] ); + + attributeNames.forEach( ( attributeName ) => { + const hasLocalAttribute = Object.hasOwn( + localAttributes, + attributeName + ); + const hadPreviousAttribute = Object.hasOwn( + previousAttributes, + attributeName + ); + const localAttribute = localAttributes[ attributeName ]; + const previousAttribute = previousAttributes[ attributeName ]; + const localAttributeIsUnchanged = + hasLocalAttribute === hadPreviousAttribute && + ( ! hasLocalAttribute || + fastDeepEqual( localAttribute, previousAttribute ) ); + + if ( ! localAttributeIsUnchanged ) { + return; + } + + if ( ! reconciledAttributes ) { + reconciledAttributes = { ...localAttributes }; + } + + const hasCurrentAttribute = Object.hasOwn( + currentAttributes, + attributeName + ); + const currentAttribute = currentAttributes[ attributeName ]; + + if ( hasCurrentAttribute ) { + reconciledAttributes[ attributeName ] = currentAttribute; + } else { + delete reconciledAttributes[ attributeName ]; + } + } ); + + return reconciledAttributes ?? localAttributes; +} + +function reconcileStaleLocalBlock( + localBlock: Block, + previousBlock: Block, + currentBlock: Block +): Block { + if ( fastDeepEqual( localBlock, previousBlock ) ) { + return currentBlock; + } + + let reconciledBlock: Block | undefined; + const getReconciledBlock = () => { + if ( ! reconciledBlock ) { + reconciledBlock = { ...localBlock }; + } + return reconciledBlock; + }; + + if ( + localBlock.name === previousBlock.name && + localBlock.name === currentBlock.name + ) { + const reconciledAttributes = reconcileStaleLocalBlockAttributes( + localBlock.attributes, + previousBlock.attributes, + currentBlock.attributes + ); + + if ( reconciledAttributes !== localBlock.attributes ) { + getReconciledBlock().attributes = reconciledAttributes; + } + } + + const reconciledInnerBlocks = reconcileStaleLocalBlockValues( + localBlock.innerBlocks ?? [], + previousBlock.innerBlocks ?? [], + currentBlock.innerBlocks ?? [] + ); + + if ( reconciledInnerBlocks !== localBlock.innerBlocks ) { + getReconciledBlock().innerBlocks = reconciledInnerBlocks; + } + + return reconciledBlock ?? localBlock; +} + +function reconcileStaleLocalBlockValues( + localBlocks: Block[], + previousBlocks: Block[], + currentBlocks: Block[] +): Block[] { + const previousBlocksByClientId = + getBlocksByClientIdIfEveryBlockHasUniqueId( previousBlocks ); + const currentBlocksByClientId = + getBlocksByClientIdIfEveryBlockHasUniqueId( currentBlocks ); + + if ( ! previousBlocksByClientId || ! currentBlocksByClientId ) { + return localBlocks; + } + + let reconciledBlocks: Block[] | undefined; + + localBlocks.forEach( ( localBlock, index ) => { + const clientId = getBlockClientId( localBlock ); + + if ( ! clientId ) { + return; + } + + const previousBlock = previousBlocksByClientId.get( clientId ); + const currentBlock = currentBlocksByClientId.get( clientId ); + + if ( ! previousBlock || ! currentBlock ) { + return; + } + + const reconciledBlock = reconcileStaleLocalBlock( + localBlock, + previousBlock, + currentBlock + ); + + if ( reconciledBlock === localBlock ) { + return; + } + + if ( ! reconciledBlocks ) { + reconciledBlocks = [ ...localBlocks ]; + } + + reconciledBlocks[ index ] = reconciledBlock; + } ); + + return reconciledBlocks ?? localBlocks; +} + +function reconcileStaleLocalBlocks( + yblocks: YBlocks, + localBlocksToSync: Block[] +): Block[] { + const previousBlocks = previousBlocksByYArray.get( yblocks ); + + if ( ! previousBlocks ) { + return localBlocksToSync; + } + + const localClientIds = + getClientIdsIfEveryBlockHasUniqueId( localBlocksToSync ); + const previousClientIds = + getClientIdsIfEveryBlockHasUniqueId( previousBlocks ); + const currentBlocks = yblocks.toJSON() as Block[]; + const currentClientIds = + getClientIdsIfEveryBlockHasUniqueId( currentBlocks ); + + if ( ! localClientIds || ! previousClientIds || ! currentClientIds ) { + return localBlocksToSync; + } + + const reconciledLocalBlocks = reconcileStaleLocalBlockValues( + localBlocksToSync, + previousBlocks, + currentBlocks + ); + const localClientIdSet = new Set( localClientIds ); + const previousClientIdSet = new Set( previousClientIds ); + const currentClientIdSet = new Set( currentClientIds ); + // The local editor sends full block snapshots. Reconcile those snapshots + // against the last local base before running the full-array merge so remote + // top-level inserts/deletes are not inferred as local structural edits. + const remotelyDeletedClientIds = new Set( + previousClientIds.filter( + ( clientId ) => + localClientIdSet.has( clientId ) && + ! currentClientIdSet.has( clientId ) + ) + ); + const blocksToSync = reconciledLocalBlocks.filter( ( block ) => { + const clientId = getBlockClientId( block ); + return ! clientId || ! remotelyDeletedClientIds.has( clientId ); + } ); + const blockIdsToSync = new Set( + blocksToSync.map( ( block ) => getBlockClientId( block ) as string ) + ); + + currentBlocks.forEach( ( currentBlock, currentIndex ) => { + const clientId = getBlockClientId( currentBlock ); + + if ( + ! clientId || + localClientIdSet.has( clientId ) || + previousClientIdSet.has( clientId ) || + blockIdsToSync.has( clientId ) + ) { + return; + } + + const insertIndex = getRemoteBlockInsertIndex( + currentBlocks, + currentIndex, + blocksToSync, + blockIdsToSync + ); + blocksToSync.splice( insertIndex, 0, currentBlock ); + blockIdsToSync.add( clientId ); + } ); + + return blocksToSync; +} + /** * Recursively walk an attribute value and convert any strings that correspond * to rich-text schema nodes into RichTextData instances. This is the inverse @@ -433,9 +741,12 @@ export function mergeCrdtBlocks( makeBlocksSerializable( incomingBlocks ) ); } - - const incomingBlocksToSync = + const localBlocksToSync = serializableBlocksCache.get( incomingBlocks ) ?? []; + const incomingBlocksToSync = reconcileStaleLocalBlocks( + yblocks, + localBlocksToSync + ); // This is a rudimentary diff implementation similar to the y-prosemirror diffing // approach. @@ -648,6 +959,8 @@ export function mergeCrdtBlocks( } knownClientIds.add( clientId ); } + + previousBlocksByYArray.set( yblocks, localBlocksToSync ); } /** diff --git a/packages/core-data/src/utils/crdt.ts b/packages/core-data/src/utils/crdt.ts index a8d7b4bc2f378c..aec6f89ad6dc00 100644 --- a/packages/core-data/src/utils/crdt.ts +++ b/packages/core-data/src/utils/crdt.ts @@ -47,7 +47,10 @@ import { } from './crdt-utils'; // Changes that can be applied to a post entity record. -export type PostChanges = Partial< Post > & { +export type PostChanges = Omit< + Partial< Post >, + 'blocks' | 'content' | 'excerpt' | 'selection' | 'title' +> & { blocks?: Block[]; content?: Post[ 'content' ] | string; excerpt?: Post[ 'excerpt' ] | string; @@ -130,6 +133,8 @@ export function applyPostChangesToCRDTDoc( syncedProperties: Set< string > ): void { const ymap = getRootMap< YPostRecord >( ydoc, CRDT_RECORD_MAP_KEY ); + const shouldDeriveContentFromBlocks = + syncedProperties.has( 'content' ) && Array.isArray( changes.blocks ); Object.keys( changes ).forEach( ( key ) => { if ( ! syncedProperties.has( key ) ) { @@ -176,6 +181,10 @@ export function applyPostChangesToCRDTDoc( case 'content': case 'excerpt': case 'title': { + if ( key === 'content' && shouldDeriveContentFromBlocks ) { + break; + } + const currentValue = ymap.get( key ); let rawValue = getRawValue( newValue ); @@ -249,6 +258,23 @@ export function applyPostChangesToCRDTDoc( } } ); + if ( shouldDeriveContentFromBlocks ) { + const currentBlocks = ymap.get( 'blocks' ); + + if ( currentBlocks instanceof Y.Array ) { + const currentValue = ymap.get( 'content' ); + const rawValue = __unstableSerializeAndClean( + currentBlocks.toJSON() + ).trim(); + + if ( currentValue instanceof Y.Text ) { + mergeRichTextUpdate( currentValue, rawValue ); + } else { + ymap.set( 'content', new Y.Text( rawValue ) ); + } + } + } + // Process changes that we don't want to persist to the CRDT document. if ( changes.selection ) { const selection = changes.selection; diff --git a/packages/core-data/src/utils/test/crdt-stale-top-level-blocks.test.ts b/packages/core-data/src/utils/test/crdt-stale-top-level-blocks.test.ts new file mode 100644 index 00000000000000..ddca71be9ab1c3 --- /dev/null +++ b/packages/core-data/src/utils/test/crdt-stale-top-level-blocks.test.ts @@ -0,0 +1,327 @@ +/** + * WordPress dependencies + */ +import { Y } from '@wordpress/sync'; + +/** + * External dependencies + */ +import { + afterEach, + beforeEach, + describe, + expect, + it, + jest, +} from '@jest/globals'; + +/** + * Mock getBlockTypes so CRDT merging can identify rich-text attributes. + */ +jest.mock( '@wordpress/blocks', () => { + const actual = jest.requireActual( '@wordpress/blocks' ) as Record< + string, + unknown + >; + return { + ...actual, + __unstableSerializeAndClean: ( + blocks: { attributes: { content?: string } }[] + ) => + blocks + .map( ( block ) => `

${ block.attributes.content }

` ) + .join( '\n\n' ), + getBlockTypes: () => [ + { + name: 'core/paragraph', + attributes: { content: { type: 'rich-text' } }, + }, + ], + }; +} ); + +/** + * Internal dependencies + */ +import { CRDT_RECORD_MAP_KEY } from '../../sync'; +import { applyPostChangesToCRDTDoc, type YPostRecord } from '../crdt'; +import { + mergeCrdtBlocks, + type Block, + type YBlock, + type YBlocks, +} from '../crdt-blocks'; +import { getRootMap } from '../crdt-utils'; + +const SYNCED_BLOCK_PROPERTIES = new Set( [ 'blocks' ] ); +const SYNCED_POST_PROPERTIES = new Set( [ 'blocks', 'content' ] ); + +function paragraph( clientId: string, content: string ): Block { + return { + name: 'core/paragraph', + clientId, + attributes: { content }, + innerBlocks: [], + }; +} + +function contentsOf( yblocks: YBlocks ): string[] { + return ( yblocks.toJSON() as Block[] ).map( + ( block ) => block.attributes.content as string + ); +} + +function postBlocks( doc: Y.Doc ): YBlocks { + return getRootMap< YPostRecord >( doc, CRDT_RECORD_MAP_KEY ).get( + 'blocks' + ) as YBlocks; +} + +function postContent( doc: Y.Doc ): string { + return ( + getRootMap< YPostRecord >( doc, CRDT_RECORD_MAP_KEY ) + .get( 'content' ) + ?.toString() ?? '' + ); +} + +function serializeBlocks( blocks: Block[] ): string { + return blocks + .map( ( block ) => `

${ block.attributes.content }

` ) + .join( '\n\n' ); +} + +describe( 'stale top-level block snapshots', () => { + let doc: Y.Doc; + let yblocks: Y.Array< YBlock >; + + beforeEach( () => { + doc = new Y.Doc(); + yblocks = doc.getArray< YBlock >(); + } ); + + afterEach( () => { + doc.destroy(); + } ); + + it( 'preserves a remote top-level append when a stale local edit touches a different block', () => { + const initialBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'unchanged', 'Beta' ), + ]; + mergeCrdtBlocks( yblocks, initialBlocks, null ); + + const remoteDoc = new Y.Doc(); + const remoteBlocks = remoteDoc.getArray< YBlock >(); + Y.applyUpdate( remoteDoc, Y.encodeStateAsUpdate( doc ) ); + + mergeCrdtBlocks( + remoteBlocks, + [ ...initialBlocks, paragraph( 'remote-appended', 'Gamma' ) ], + null + ); + + Y.applyUpdate( doc, Y.encodeStateAsUpdate( remoteDoc ) ); + expect( contentsOf( yblocks ) ).toEqual( [ 'Alpha', 'Beta', 'Gamma' ] ); + + const staleLocalBlocks = [ + paragraph( 'local-edited', 'Alpha local edit' ), + paragraph( 'unchanged', 'Beta' ), + ]; + mergeCrdtBlocks( yblocks, staleLocalBlocks, null ); + + expect( contentsOf( yblocks ) ).toEqual( [ + 'Alpha local edit', + 'Beta', + 'Gamma', + ] ); + + remoteDoc.destroy(); + } ); + + it( 'preserves a remote top-level delete when a stale local edit touches a different block', () => { + const initialBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'unchanged', 'Beta' ), + paragraph( 'remote-deleted', 'Gamma' ), + ]; + mergeCrdtBlocks( yblocks, initialBlocks, null ); + + const remoteDoc = new Y.Doc(); + const remoteBlocks = remoteDoc.getArray< YBlock >(); + Y.applyUpdate( remoteDoc, Y.encodeStateAsUpdate( doc ) ); + + mergeCrdtBlocks( + remoteBlocks, + [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'unchanged', 'Beta' ), + ], + null + ); + + Y.applyUpdate( doc, Y.encodeStateAsUpdate( remoteDoc ) ); + expect( contentsOf( yblocks ) ).toEqual( [ 'Alpha', 'Beta' ] ); + + const staleLocalBlocks = [ + paragraph( 'local-edited', 'Alpha local edit' ), + paragraph( 'unchanged', 'Beta' ), + paragraph( 'remote-deleted', 'Gamma' ), + ]; + mergeCrdtBlocks( yblocks, staleLocalBlocks, null ); + + expect( contentsOf( yblocks ) ).toEqual( [ + 'Alpha local edit', + 'Beta', + ] ); + + remoteDoc.destroy(); + } ); + + it( 'preserves a remote rich-text edit when a stale local edit touches a different block', () => { + const initialBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'remote-edited', 'Beta' ), + ]; + mergeCrdtBlocks( yblocks, initialBlocks, null ); + + const remoteDoc = new Y.Doc(); + const remoteBlocks = remoteDoc.getArray< YBlock >(); + Y.applyUpdate( remoteDoc, Y.encodeStateAsUpdate( doc ) ); + + mergeCrdtBlocks( + remoteBlocks, + [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'remote-edited', 'Beta remote edit' ), + ], + null + ); + + Y.applyUpdate( doc, Y.encodeStateAsUpdate( remoteDoc ) ); + expect( contentsOf( yblocks ) ).toEqual( [ + 'Alpha', + 'Beta remote edit', + ] ); + + const staleLocalBlocks = [ + paragraph( 'local-edited', 'Alpha stale edit' ), + paragraph( 'remote-edited', 'Beta' ), + ]; + mergeCrdtBlocks( yblocks, staleLocalBlocks, null ); + + expect( contentsOf( yblocks ) ).toEqual( [ + 'Alpha stale edit', + 'Beta remote edit', + ] ); + + remoteDoc.destroy(); + } ); + + it( 'derives post content from merged blocks instead of stale serialized content', () => { + const initialBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'remote-edited', 'Beta' ), + ]; + applyPostChangesToCRDTDoc( + doc, + { + blocks: initialBlocks, + content: serializeBlocks( initialBlocks ), + }, + SYNCED_POST_PROPERTIES + ); + + const remoteDoc = new Y.Doc(); + Y.applyUpdate( remoteDoc, Y.encodeStateAsUpdate( doc ) ); + + const remoteBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'remote-edited', 'Beta remote edit' ), + ]; + applyPostChangesToCRDTDoc( + remoteDoc, + { + blocks: remoteBlocks, + content: serializeBlocks( remoteBlocks ), + }, + SYNCED_POST_PROPERTIES + ); + + Y.applyUpdate( doc, Y.encodeStateAsUpdate( remoteDoc ) ); + expect( postContent( doc ) ).toContain( 'Beta remote edit' ); + + const staleLocalBlocks = [ + paragraph( 'local-edited', 'Alpha stale edit' ), + paragraph( 'remote-edited', 'Beta' ), + ]; + applyPostChangesToCRDTDoc( + doc, + { + blocks: staleLocalBlocks, + content: serializeBlocks( staleLocalBlocks ), + }, + SYNCED_POST_PROPERTIES + ); + + expect( contentsOf( postBlocks( doc ) ) ).toEqual( [ + 'Alpha stale edit', + 'Beta remote edit', + ] ); + expect( postContent( doc ) ).toContain( 'Alpha stale edit' ); + expect( postContent( doc ) ).toContain( 'Beta remote edit' ); + + remoteDoc.destroy(); + } ); + + it( 'preserves a remote top-level append through the post CRDT adapter', () => { + const initialBlocks = [ + paragraph( 'local-edited', 'Alpha' ), + paragraph( 'unchanged', 'Beta' ), + ]; + applyPostChangesToCRDTDoc( + doc, + { blocks: initialBlocks }, + SYNCED_BLOCK_PROPERTIES + ); + + const remoteDoc = new Y.Doc(); + Y.applyUpdate( remoteDoc, Y.encodeStateAsUpdate( doc ) ); + applyPostChangesToCRDTDoc( + remoteDoc, + { + blocks: [ + ...initialBlocks, + paragraph( 'remote-appended', 'Gamma' ), + ], + }, + SYNCED_BLOCK_PROPERTIES + ); + + Y.applyUpdate( doc, Y.encodeStateAsUpdate( remoteDoc ) ); + expect( contentsOf( postBlocks( doc ) ) ).toEqual( [ + 'Alpha', + 'Beta', + 'Gamma', + ] ); + + applyPostChangesToCRDTDoc( + doc, + { + blocks: [ + paragraph( 'local-edited', 'Alpha local edit' ), + paragraph( 'unchanged', 'Beta' ), + ], + }, + SYNCED_BLOCK_PROPERTIES + ); + + expect( contentsOf( postBlocks( doc ) ) ).toEqual( [ + 'Alpha local edit', + 'Beta', + 'Gamma', + ] ); + + remoteDoc.destroy(); + } ); +} ); diff --git a/packages/sync/src/manager.ts b/packages/sync/src/manager.ts index 7444f1c4b9b318..bafdd44bc615c3 100644 --- a/packages/sync/src/manager.ts +++ b/packages/sync/src/manager.ts @@ -60,6 +60,14 @@ interface EntityState { ydoc: CRDTDoc; } +function areUint8ArraysEqual( a: Uint8Array, b: Uint8Array ): boolean { + if ( a.length !== b.length ) { + return false; + } + + return a.every( ( value, index ) => value === b[ index ] ); +} + /** * Get the entity ID for the given object type and object ID. * @@ -666,6 +674,46 @@ export function createSyncManager( debug = false ): SyncManager { return serializeCrdtDoc( entityState.ydoc ); } + async function applyPersistedCRDTDoc( + objectType: ObjectType, + objectId: ObjectID, + record: ObjectData + ): Promise< boolean > { + const entityId = getEntityId( objectType, objectId ); + const entityState = entityStates.get( entityId ); + const previousStateVector = entityState?.ydoc + ? Y.encodeStateVector( entityState.ydoc ) + : null; + + internal.applyPersistedCrdtDoc( objectType, objectId, record ); + + // Applying a persisted document can schedule local store updates. Yield so + // callers that immediately inspect the document see the completed merge. + await new Promise( ( resolve ) => setTimeout( resolve, 0 ) ); + + const nextStateVector = entityState?.ydoc + ? Y.encodeStateVector( entityState.ydoc ) + : null; + + return !! ( + previousStateVector && + nextStateVector && + ! areUint8ArraysEqual( previousStateVector, nextStateVector ) + ); + } + + function getCRDTRecordData( + objectType: ObjectType, + objectId: ObjectID + ): ObjectData | undefined { + const entityId = getEntityId( objectType, objectId ); + const entityState = entityStates.get( entityId ); + + return entityState?.ydoc.getMap( CRDT_RECORD_MAP_KEY ).toJSON() as + | ObjectData + | undefined; + } + // Collect internal functions so that they can be wrapped before calling. const internal = { applyPersistedCrdtDoc: debugWrap( _applyPersistedCrdtDoc ), @@ -674,7 +722,9 @@ export function createSyncManager( debug = false ): SyncManager { // Wrap and return the public API. return { + applyPersistedCRDTDoc: debugWrap( applyPersistedCRDTDoc ), createPersistedCRDTDoc: debugWrap( createPersistedCRDTDoc ), + getCRDTRecordData: debugWrap( getCRDTRecordData ), getAwareness, load: debugWrap( loadEntity ), loadCollection: debugWrap( loadCollection ), diff --git a/packages/sync/src/types.ts b/packages/sync/src/types.ts index e9a3e57bfeae09..b74d3b867679e4 100644 --- a/packages/sync/src/types.ts +++ b/packages/sync/src/types.ts @@ -157,10 +157,19 @@ export interface SyncConfig { } export interface SyncManager { + applyPersistedCRDTDoc: ( + objectType: ObjectType, + objectId: ObjectID, + record: ObjectData + ) => Promise< boolean >; createPersistedCRDTDoc: ( objectType: ObjectType, objectId: ObjectID ) => Promise< string | null >; + getCRDTRecordData: ( + objectType: ObjectType, + objectId: ObjectID + ) => ObjectData | undefined; getAwareness: < State extends Awareness >( objectType: ObjectType, objectId: ObjectID diff --git a/test/e2e/specs/editor/collaboration/collaboration-same-user-stale-content-overwrite.spec.ts b/test/e2e/specs/editor/collaboration/collaboration-same-user-stale-content-overwrite.spec.ts new file mode 100644 index 00000000000000..fe260e79f06e63 --- /dev/null +++ b/test/e2e/specs/editor/collaboration/collaboration-same-user-stale-content-overwrite.spec.ts @@ -0,0 +1,656 @@ +/** + * External dependencies + */ +import type { BrowserContext, Page, Request } from '@playwright/test'; + +/** + * WordPress dependencies + */ +import type { + Admin, + Editor, + RequestUtils, +} from '@wordpress/e2e-test-utils-playwright'; + +/** + * Internal dependencies + */ +import { test, expect } from './fixtures'; + +const BASE_URL = process.env.WP_BASE_URL || 'http://localhost:8889'; +const ADMIN_USER = process.env.WP_USERNAME || 'admin'; +const ADMIN_PASSWORD = process.env.WP_PASSWORD || 'password'; +const DELAYED_CONTROL_MS = 12000; + +type RestField = { raw?: string; rendered?: string } | string; +type RestPost = { + content?: RestField; + id: number; +}; + +type SyncEvidence = { + count: number; + rooms: Set< string >; +}; + +type SaveTraceEntry = { + label: string; + method: string; + requestPostData: string; + requestAt: string; + responseContentRaw?: string; + responseAt?: string; + responseStatus?: number; + url: string; +}; + +type SaveSummary = { + label: string; + requestHasA: boolean; + requestHasB: boolean; + responseHasA: boolean; + responseHasB: boolean; + responseStatus?: number; +}; + +type ScenarioResult = { + attempt: number; + beforeBSaveHadA: boolean; + bSave?: SaveSummary; + finalContent: string; + finalHasA: boolean; + finalHasB: boolean; + markerA: string; + markerB: string; + postId: number; + room: string; + rtc: { + collaborationEnabled: boolean; + scripts: string[]; + }; + stalePreconditionExercised: boolean; + syncA: { + count: number; + rooms: string[]; + }; + syncB: { + count: number; + rooms: string[]; + }; +}; + +function paragraphMarkup( content: string ) { + return `

${ content }

`; +} + +function rawField( field?: RestField ): string { + if ( ! field ) { + return ''; + } + return typeof field === 'string' + ? field + : field.raw ?? field.rendered ?? ''; +} + +function sleep( ms: number ) { + return new Promise( ( resolve ) => setTimeout( resolve, ms ) ); +} + +function syncObserver( page: Page ): SyncEvidence { + const state: SyncEvidence = { count: 0, rooms: new Set() }; + + page.on( 'response', async ( response ) => { + if ( + ! response.url().includes( 'wp-sync' ) || + response.status() !== 200 + ) { + return; + } + state.count += 1; + try { + const payload = await response.json(); + for ( const room of payload?.rooms ?? [] ) { + if ( room?.room ) { + state.rooms.add( room.room ); + } + } + } catch {} + } ); + + return state; +} + +function isPostSaveRequest( request: Request, postId: number ) { + const url = request.url(); + const method = request.method(); + return ( + ( method === 'POST' || method === 'PUT' ) && + ( url.includes( `/wp/v2/posts/${ postId }` ) || + url.includes( `rest_route=%2Fwp%2Fv2%2Fposts%2F${ postId }` ) ) + ); +} + +function attachSaveTrace( page: Page, label: string, postId: number ) { + const entries: SaveTraceEntry[] = []; + + page.on( 'request', ( request ) => { + if ( ! isPostSaveRequest( request, postId ) ) { + return; + } + entries.push( { + label, + method: request.method(), + requestPostData: request.postData() ?? '', + requestAt: new Date().toISOString(), + url: request.url(), + } ); + } ); + + page.on( 'response', async ( response ) => { + const request = response.request(); + if ( ! isPostSaveRequest( request, postId ) ) { + return; + } + const entry = entries + .slice() + .reverse() + .find( + ( item ) => + item.url === request.url() && + item.method === request.method() && + item.responseStatus === undefined + ); + if ( ! entry ) { + return; + } + entry.responseStatus = response.status(); + entry.responseAt = new Date().toISOString(); + try { + const body = await response.json(); + entry.responseContentRaw = body?.content?.raw ?? ''; + } catch { + entry.responseContentRaw = ''; + } + } ); + + return entries; +} + +function requestContent( entry: SaveTraceEntry ) { + try { + return String( JSON.parse( entry.requestPostData )?.content ?? '' ); + } catch {} + return new URLSearchParams( entry.requestPostData ).get( 'content' ) ?? ''; +} + +function summarizeSave( + entries: SaveTraceEntry[], + markerA: string, + markerB: string, + label: string +): SaveSummary | undefined { + const candidates = entries + .filter( ( entry ) => entry.label === label ) + .map( ( entry ) => { + const requestBody = requestContent( entry ); + const responseBody = String( entry.responseContentRaw ?? '' ); + return { + label, + requestHasA: requestBody.includes( markerA ), + requestHasB: requestBody.includes( markerB ), + responseHasA: responseBody.includes( markerA ), + responseHasB: responseBody.includes( markerB ), + responseStatus: entry.responseStatus, + }; + } ); + + return ( + candidates + .slice() + .reverse() + .find( ( entry ) => entry.requestHasA || entry.requestHasB ) ?? + candidates.at( -1 ) + ); +} + +async function waitForEditorReady( page: Page, postId: number ) { + await page.waitForFunction( + ( id ) => + ( window as any )._wpCollaborationEnabled === true && + ( window as any ).wp?.data && + ( window as any ).wp?.blocks && + ( window as any ).wp.data + .select( 'core/editor' ) + .getCurrentPostId() === Number( id ) && + ( window as any ).wp.data + .select( 'core' ) + .hasFinishedResolution( 'getEntityRecord', [ + 'postType', + 'post', + Number( id ), + ] ) && + ! ( window as any ).wp.data.select( 'core/editor' ).isSavingPost(), + postId, + { timeout: 30000 } + ); + await page.waitForFunction( + () => document.querySelector( 'iframe[name="editor-canvas"]' ), + undefined, + { timeout: 30000 } + ); +} + +async function openPrimaryEditor( + admin: Admin, + editor: Editor, + page: Page, + postId: number +) { + await admin.visitAdminPage( 'post.php', `post=${ postId }&action=edit` ); + await editor.setPreferences( 'core/edit-post', { + welcomeGuide: false, + fullscreenMode: false, + } ); + await waitForEditorReady( page, postId ); +} + +async function openSameAdminEditor( admin: Admin, postId: number ) { + const context = await admin.browser.newContext( { + baseURL: BASE_URL, + } ); + const page = await context.newPage(); + + try { + await page.goto( '/wp-login.php' ); + await page.locator( '#user_login' ).fill( ADMIN_USER ); + await page.locator( '#user_pass' ).fill( ADMIN_PASSWORD ); + await page.getByRole( 'button', { name: 'Log In' } ).click(); + await page.waitForURL( '**/wp-admin/**' ); + + await page.goto( `/wp-admin/post.php?post=${ postId }&action=edit` ); + await page.evaluate( () => { + ( window as any ).wp.data + .dispatch( 'core/preferences' ) + .set( 'core/edit-post', 'welcomeGuide', false ); + ( window as any ).wp.data + .dispatch( 'core/preferences' ) + .set( 'core/edit-post', 'fullscreenMode', false ); + } ); + await waitForEditorReady( page, postId ); + return { context, page }; + } catch ( error ) { + await context.close(); + throw error; + } +} + +async function waitForMutualDiscovery( pageA: Page, pageB: Page ) { + await Promise.all( + [ pageA, pageB ].map( ( page ) => + page + .getByRole( 'button', { name: /Collaborators list/ } ) + .waitFor( { timeout: 30000 } ) + ) + ); +} + +async function waitForSyncRoom( + state: SyncEvidence, + room: string, + label: string +) { + await expect + .poll( + () => ( { + count: state.count, + hasRoom: state.rooms.has( room ), + rooms: Array.from( state.rooms ), + } ), + { + message: `${ label } should poll ${ room } via /wp-sync`, + timeout: 35000, + } + ) + .toMatchObject( { + hasRoom: true, + } ); +} + +async function collectRtcEvidence( page: Page ) { + return page.evaluate( () => ( { + collaborationEnabled: + ( window as any )._wpCollaborationEnabled === true, + scripts: Array.from( document.scripts ) + .map( ( script ) => script.src ) + .filter( + ( src ) => + src.includes( '/build/scripts/sync/' ) || + src.includes( '/build/scripts/core-data/' ) || + src.includes( '/build/scripts/editor/' ) || + src.includes( '/build/scripts/edit-post/' ) + ), + } ) ); +} + +function editorFrame( page: Page ) { + const frame = page.frame( { name: 'editor-canvas' } ); + if ( ! frame ) { + throw new Error( 'Editor iframe is not available.' ); + } + return frame; +} + +async function appendParagraphWithKeyboard( page: Page, marker: string ) { + const frame = editorFrame( page ); + const editable = frame + .locator( '[data-type="core/paragraph"][contenteditable="true"]' ) + .first(); + await editable.waitFor( { state: 'visible', timeout: 30000 } ); + + await editable.click(); + await page.keyboard.press( 'End' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( marker ); + await page.waitForFunction( + ( expected ) => + ( window as any ).wp.data + .select( 'core/block-editor' ) + .getBlocks() + .some( ( block: { attributes?: { content?: string } } ) => + String( block.attributes?.content ?? '' ).includes( + expected + ) + ), + marker, + { timeout: 7000 } + ); + await page.waitForFunction( + ( expected ) => + String( + ( window as any ).wp.data + .select( 'core/editor' ) + .getEditedPostContent() + ).includes( expected ), + marker, + { timeout: 7000 } + ); +} + +async function saveDraftWithToolbar( page: Page ) { + const button = page + .getByRole( 'region', { name: 'Editor top bar' } ) + .getByRole( 'button', { name: /^Save draft$/ } ); + await button.waitFor( { state: 'visible', timeout: 30000 } ); + await page.waitForFunction( + () => + ( window as any ).wp.data + .select( 'core/editor' ) + .isEditedPostDirty(), + undefined, + { timeout: 30000 } + ); + await button.click(); + await page.waitForFunction( + () => + ! ( window as any ).wp.data.select( 'core/editor' ).isSavingPost(), + undefined, + { timeout: 30000 } + ); +} + +async function editorHasText( page: Page, marker: string ) { + return page.evaluate( + ( expected ) => + ( window as any ).wp.data + .select( 'core/block-editor' ) + .getBlocks() + .some( ( block: { attributes?: { content?: string } } ) => + String( block.attributes?.content ?? '' ).includes( + expected + ) + ), + marker + ); +} + +async function waitForServerText( + requestUtils: RequestUtils, + postId: number, + marker: string +) { + await expect + .poll( + async () => + rawField( + ( + await requestUtils.rest< RestPost >( { + path: `/wp/v2/posts/${ postId }?context=edit`, + } ) + ).content + ), + { timeout: 30000 } + ) + .toContain( marker ); +} + +async function getPersistedContent( + requestUtils: RequestUtils, + postId: number +) { + return rawField( + ( + await requestUtils.rest< RestPost >( { + path: `/wp/v2/posts/${ postId }?context=edit`, + } ) + ).content + ); +} + +async function runSameAccountScenario( { + admin, + attempt, + delayBeforeBSaveMs, + editor, + page, + requestUtils, + requireStaleBeforeBSave, +}: { + admin: Admin; + attempt: number; + delayBeforeBSaveMs: number; + editor: Editor; + page: Page; + requestUtils: RequestUtils; + requireStaleBeforeBSave: boolean; +} ): Promise< ScenarioResult > { + const markerA = `rtc-a-${ Date.now() }-${ attempt }`; + const markerB = `rtc-b-${ Date.now() }-${ attempt }`; + const post = await requestUtils.createPost( { + title: `Same-account stale save ${ Date.now() }`, + status: 'draft', + date_gmt: new Date().toISOString(), + content: paragraphMarkup( 'Initial body.' ), + } ); + const postId = post.id; + const room = `postType/post:${ postId }`; + let secondaryContext: BrowserContext | undefined; + + try { + await openPrimaryEditor( admin, editor, page, postId ); + const joined = await openSameAdminEditor( admin, postId ); + secondaryContext = joined.context; + const pageB = joined.page; + + const syncA = syncObserver( page ); + const syncB = syncObserver( pageB ); + const saveTraceA = attachSaveTrace( page, 'A', postId ); + const saveTraceB = attachSaveTrace( pageB, 'B', postId ); + + await waitForMutualDiscovery( page, pageB ); + await Promise.all( [ + waitForSyncRoom( syncA, room, 'Window A' ), + waitForSyncRoom( syncB, room, 'Window B' ), + ] ); + const rtc = await collectRtcEvidence( page ); + + await appendParagraphWithKeyboard( page, markerA ); + await saveDraftWithToolbar( page ); + await waitForServerText( requestUtils, postId, markerA ); + + if ( delayBeforeBSaveMs > 0 ) { + await sleep( delayBeforeBSaveMs ); + } + + const beforeBSaveHadA = await editorHasText( pageB, markerA ); + if ( requireStaleBeforeBSave && beforeBSaveHadA ) { + return { + attempt, + beforeBSaveHadA, + finalContent: '', + finalHasA: false, + finalHasB: false, + markerA, + markerB, + postId, + room, + rtc, + stalePreconditionExercised: false, + syncA: { + count: syncA.count, + rooms: Array.from( syncA.rooms ), + }, + syncB: { + count: syncB.count, + rooms: Array.from( syncB.rooms ), + }, + }; + } + + await appendParagraphWithKeyboard( pageB, markerB ); + await saveDraftWithToolbar( pageB ); + await waitForServerText( requestUtils, postId, markerB ); + + const finalContent = await getPersistedContent( requestUtils, postId ); + const combinedTrace = [ ...saveTraceA, ...saveTraceB ]; + return { + attempt, + beforeBSaveHadA, + bSave: summarizeSave( combinedTrace, markerA, markerB, 'B' ), + finalContent, + finalHasA: finalContent.includes( markerA ), + finalHasB: finalContent.includes( markerB ), + markerA, + markerB, + postId, + room, + rtc, + stalePreconditionExercised: true, + syncA: { + count: syncA.count, + rooms: Array.from( syncA.rooms ), + }, + syncB: { + count: syncB.count, + rooms: Array.from( syncB.rooms ), + }, + }; + } finally { + await secondaryContext?.close(); + } +} + +async function runUntilStalePrecondition( + options: Omit< + Parameters< typeof runSameAccountScenario >[ 0 ], + 'attempt' + >, + maxAttempts: number +) { + let lastResult: ScenarioResult | undefined; + for ( let attempt = 1; attempt <= maxAttempts; attempt++ ) { + lastResult = await runSameAccountScenario( { + ...options, + attempt, + } ); + if ( lastResult.stalePreconditionExercised ) { + return lastResult; + } + } + throw new Error( + `Could not exercise a stale same-account editor window after ${ maxAttempts } attempts. Last result: ${ JSON.stringify( + lastResult + ) }` + ); +} + +test.describe( 'Collaboration - same-user stale content overwrite', () => { + test( 'preserves content saved by another same-account window before polling catches up', async ( { + admin, + collaborationUtils, + editor, + page, + requestUtils, + }, testInfo ) => { + test.setTimeout( 180000 ); + void collaborationUtils; + + const result = await runUntilStalePrecondition( + { + admin, + delayBeforeBSaveMs: 0, + editor, + page, + requestUtils, + requireStaleBeforeBSave: true, + }, + 6 + ); + + await testInfo.attach( 'same-account-stale-save-trace', { + body: JSON.stringify( result, null, 2 ), + contentType: 'application/json', + } ); + + expect( result.rtc.collaborationEnabled ).toBe( true ); + expect( result.syncA.rooms ).toContain( result.room ); + expect( result.syncB.rooms ).toContain( result.room ); + expect( result.beforeBSaveHadA ).toBe( false ); + expect( result.bSave?.requestHasB ).toBe( true ); + expect( result.finalHasA ).toBe( true ); + expect( result.finalHasB ).toBe( true ); + } ); + + test( 'preserves both edits after the stale window receives polling updates', async ( { + admin, + collaborationUtils, + editor, + page, + requestUtils, + }, testInfo ) => { + test.setTimeout( 150000 ); + void collaborationUtils; + + const result = await runSameAccountScenario( { + admin, + attempt: 1, + delayBeforeBSaveMs: DELAYED_CONTROL_MS, + editor, + page, + requestUtils, + requireStaleBeforeBSave: false, + } ); + + await testInfo.attach( 'same-account-delayed-control-trace', { + body: JSON.stringify( result, null, 2 ), + contentType: 'application/json', + } ); + + expect( result.rtc.collaborationEnabled ).toBe( true ); + expect( result.syncA.rooms ).toContain( result.room ); + expect( result.syncB.rooms ).toContain( result.room ); + expect( result.beforeBSaveHadA ).toBe( true ); + expect( result.bSave?.requestHasB ).toBe( true ); + expect( result.finalHasA ).toBe( true ); + expect( result.finalHasB ).toBe( true ); + } ); +} );