-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Real-time collaboration: Fix persisted document meta and add tests #73985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import * as Y from 'yjs'; | ||
| import * as buffer from 'lib0/buffer'; | ||
| import { describe, expect, it, beforeEach } from '@jest/globals'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { createYjsDoc, serializeCrdtDoc, deserializeCrdtDoc } from '../utils'; | ||
| import { | ||
| CRDT_DOC_META_PERSISTENCE_KEY, | ||
| CRDT_DOC_VERSION, | ||
| CRDT_STATE_MAP_KEY, | ||
| CRDT_STATE_VERSION_KEY, | ||
| } from '../config'; | ||
|
|
||
| describe( 'utils', () => { | ||
| describe( 'createYjsDoc', () => { | ||
| it( 'creates a Y.Doc with metadata', () => { | ||
| const documentMeta = { | ||
| userId: '123', | ||
| entityType: 'post', | ||
| }; | ||
|
|
||
| const ydoc = createYjsDoc( documentMeta ); | ||
|
|
||
| expect( ydoc ).toBeInstanceOf( Y.Doc ); | ||
| expect( ydoc.meta ).toBeDefined(); | ||
| expect( ydoc.meta?.get( 'userId' ) ).toBe( '123' ); | ||
| expect( ydoc.meta?.get( 'entityType' ) ).toBe( 'post' ); | ||
| } ); | ||
|
|
||
| it( 'creates a Y.Doc with empty metadata', () => { | ||
| const ydoc = createYjsDoc(); | ||
|
|
||
| expect( ydoc ).toBeInstanceOf( Y.Doc ); | ||
| expect( ydoc.meta ).toBeDefined(); | ||
| expect( ydoc.meta?.size ).toBe( 0 ); | ||
| } ); | ||
|
|
||
| it( 'sets the CRDT document version in the state map', () => { | ||
| const ydoc = createYjsDoc( {} ); | ||
| const stateMap = ydoc.getMap( CRDT_STATE_MAP_KEY ); | ||
|
|
||
| expect( stateMap.get( CRDT_STATE_VERSION_KEY ) ).toBe( | ||
| CRDT_DOC_VERSION | ||
| ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'serializeCrdtDoc', () => { | ||
| let testDoc: Y.Doc; | ||
|
|
||
| beforeEach( () => { | ||
| testDoc = createYjsDoc(); | ||
| } ); | ||
|
|
||
| it( 'serializes a CRDT doc with data', () => { | ||
| const ymap = testDoc.getMap( 'testMap' ); | ||
| ymap.set( 'title', 'Test Title' ); | ||
| ymap.set( 'content', 'Test Content' ); | ||
|
|
||
| const serialized = serializeCrdtDoc( testDoc ); | ||
| const parsed = JSON.parse( serialized ); | ||
|
|
||
| expect( parsed ).toHaveProperty( 'document' ); | ||
| expect( typeof parsed.document ).toBe( 'string' ); | ||
| expect( parsed.document.length ).toBeGreaterThan( 0 ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'deserializeCrdtDoc', () => { | ||
| let originalDoc: Y.Doc; | ||
| let serialized: string; | ||
|
|
||
| beforeEach( () => { | ||
| originalDoc = createYjsDoc(); | ||
| const ymap = originalDoc.getMap( 'testMap' ); | ||
| ymap.set( 'title', 'Test Title' ); | ||
| ymap.set( 'count', 42 ); | ||
| serialized = serializeCrdtDoc( originalDoc ); | ||
| } ); | ||
|
|
||
| it( 'restores the data from the serialized doc', () => { | ||
| const deserialized = deserializeCrdtDoc( serialized ); | ||
|
|
||
| expect( deserialized ).toBeInstanceOf( Y.Doc ); | ||
|
|
||
| const ymap = deserialized!.getMap( 'testMap' ); | ||
| expect( ymap.get( 'title' ) ).toBe( 'Test Title' ); | ||
| expect( ymap.get( 'count' ) ).toBe( 42 ); | ||
| } ); | ||
|
|
||
| it( 'marks the document as from persistence', () => { | ||
| const deserialized = deserializeCrdtDoc( serialized ); | ||
|
|
||
| expect( deserialized ).toBeInstanceOf( Y.Doc ); | ||
| expect( deserialized!.meta ).toBeDefined(); | ||
| expect( | ||
| deserialized!.meta?.get( CRDT_DOC_META_PERSISTENCE_KEY ) | ||
| ).toBe( true ); | ||
| } ); | ||
|
|
||
| it( 'assigns a random client ID to the deserialized document', () => { | ||
| const deserialized = deserializeCrdtDoc( serialized ); | ||
|
|
||
| expect( deserialized ).toBeInstanceOf( Y.Doc ); | ||
|
|
||
| // Client ID should not match the original. | ||
| expect( deserialized!.clientID ).not.toBe( originalDoc.clientID ); | ||
| } ); | ||
|
|
||
| it( 'returns null for invalid JSON', () => { | ||
| const result = deserializeCrdtDoc( 'invalid json {' ); | ||
|
|
||
| expect( result ).toBeNull(); | ||
| } ); | ||
|
|
||
| it( 'returns null for JSON missing document property', () => { | ||
| const invalidSerialized = JSON.stringify( { data: 'test' } ); | ||
| const result = deserializeCrdtDoc( invalidSerialized ); | ||
|
|
||
| expect( result ).toBeNull(); | ||
| } ); | ||
|
|
||
| it( 'returns null for corrupted CRDT data', () => { | ||
| const corruptedSerialized = JSON.stringify( { | ||
| document: buffer.toBase64( | ||
| new Uint8Array( [ 1, 2, 3, 4, 5 ] ) | ||
| ), | ||
| } ); | ||
| const result = deserializeCrdtDoc( corruptedSerialized ); | ||
|
|
||
| expect( result ).toBeNull(); | ||
| } ); | ||
|
|
||
| it( 'preserves the CRDT state version', () => { | ||
| const deserialized = deserializeCrdtDoc( serialized ); | ||
|
|
||
| expect( deserialized ).toBeInstanceOf( Y.Doc ); | ||
|
|
||
| const stateMap = deserialized!.getMap( CRDT_STATE_MAP_KEY ); | ||
| expect( stateMap.get( CRDT_STATE_VERSION_KEY ) ).toBe( | ||
| CRDT_DOC_VERSION | ||
| ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'serialization round-trip', () => { | ||
| it( 'maintains data integrity through serialize/deserialize cycle', () => { | ||
| const originalDoc = createYjsDoc( {} ); | ||
| const ymap = originalDoc.getMap( 'data' ); | ||
| ymap.set( 'string', 'value' ); | ||
| ymap.set( 'number', 123 ); | ||
| ymap.set( 'boolean', true ); | ||
|
|
||
| const serialized = serializeCrdtDoc( originalDoc ); | ||
| const deserialized = deserializeCrdtDoc( serialized ); | ||
|
|
||
| expect( deserialized ).not.toBeNull(); | ||
|
|
||
| const deserializedMap = deserialized!.getMap( 'data' ); | ||
| expect( deserializedMap.get( 'string' ) ).toBe( 'value' ); | ||
| expect( deserializedMap.get( 'number' ) ).toBe( 123 ); | ||
| expect( deserializedMap.get( 'boolean' ) ).toBe( true ); | ||
| } ); | ||
|
|
||
| it( 'handles multiple serialize/deserialize cycles', () => { | ||
| const doc = createYjsDoc(); | ||
| doc.getMap( 'test' ).set( 'value', 'original' ); | ||
|
|
||
| // Cycle 1 | ||
| let serialized = serializeCrdtDoc( doc ); | ||
| let deserialized = deserializeCrdtDoc( serialized ); | ||
| expect( deserialized ).not.toBeNull(); | ||
|
|
||
| // Cycle 2 | ||
| serialized = serializeCrdtDoc( deserialized! ); | ||
| deserialized = deserializeCrdtDoc( serialized ); | ||
| expect( deserialized ).not.toBeNull(); | ||
|
|
||
| // Verify data is still intact | ||
| expect( deserialized!.getMap( 'test' ).get( 'value' ) ).toBe( | ||
| 'original' | ||
| ); | ||
| } ); | ||
| } ); | ||
| } ); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,7 +15,11 @@ import { | |||||||||||||||||||||||
| } from './config'; | ||||||||||||||||||||||||
| import type { CRDTDoc } from './types'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export function createYjsDoc( documentMeta: Record< string, unknown > ): Y.Doc { | ||||||||||||||||||||||||
| interface DocumentMeta { | ||||||||||||||||||||||||
| [ key: string ]: boolean | number | string; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export function createYjsDoc( documentMeta: DocumentMeta = {} ): Y.Doc { | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we messing with these "meta"? Is it because we need to persist the doc in the database or something, so we need some serialized information.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main use case for CRDT document metadata is local state that is colocated with the document. "Local" because we don't want this metadata to be synced to peers or persisted—it applies only to this document instance, in-memory. We have one use case currently: We use CRDT document metadata to store a marker that the CRDT document was loaded from persistence. This allows us to have code in gutenberg/packages/core-data/src/utils/crdt.ts Lines 282 to 292 in dbcbe3d
tl;dr: They are in-memory descriptors for a CRDT document. |
||||||||||||||||||||||||
| // Meta is not synced and does not get persisted with the document. | ||||||||||||||||||||||||
| const metaMap = new Map< string, unknown >( | ||||||||||||||||||||||||
| Object.entries( documentMeta ) | ||||||||||||||||||||||||
|
|
@@ -42,11 +46,12 @@ export function deserializeCrdtDoc( | |||||||||||||||||||||||
| const { document } = JSON.parse( serializedCrdtDoc ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Mark this document as from persistence. | ||||||||||||||||||||||||
| const docMetaMap = new Map< string, boolean >(); | ||||||||||||||||||||||||
| docMetaMap.set( CRDT_DOC_META_PERSISTENCE_KEY, true ); | ||||||||||||||||||||||||
| const docMeta: DocumentMeta = { | ||||||||||||||||||||||||
| [ CRDT_DOC_META_PERSISTENCE_KEY ]: true, | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Apply the document as an update against a new (temporary) Y.Doc. | ||||||||||||||||||||||||
| const ydoc = createYjsDoc( { meta: docMetaMap } ); | ||||||||||||||||||||||||
| const ydoc = createYjsDoc( docMeta ); | ||||||||||||||||||||||||
| const yupdate = buffer.fromBase64( document ); | ||||||||||||||||||||||||
| Y.applyUpdateV2( ydoc, yupdate ); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of hesitant here, are these the only meta attributes that third-party blocks can have, I wonder if it's possible to have arrays and nested objects too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@youknowriad CRDT document metadata are in-memory metadata of the CRDT document instance (example: a boolean indicating whether the document was loaded from persistence / post meta). The sync package fully controls this metadata and it is not persisted or synced. I've added a comment in 2beb2f4 that hopefully makes this more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I thought this was about post meta