DraftEntity ID Changes

Summary: # Facebook CoEditing Prototype relies on syncing the state of the editor between different users so that we can propagate changes between them and hence have the document state synced between users. **How DraftEntity Works** `DraftEntity`, when creating a new entity uses incremental numbers as the key ([source](https://github.com/facebook/draft-js/blob/master/src/model/entity/DraftEntity.js#L211)) which can be a problem as two users can simultaneously create entity with the same id, say `6`, with different links thereby causing the document sync to go out of sync. **Change** - This diff changes how the keys are generated for new entities. Instead of incremental integers, we create a unique ID (using `uuid()`) and use that as the ID. - Also exposes `getAll` and `loadWith..` functions that are used in subsequent diffs to get the map of entities and share them with other users and when a new updated map is received, load `DraftEntity` with that. (these changes have been tested already end to end at the top of the stack). Reviewed By: mrkev Differential Revision: D20495349 fbshipit-source-id: b65ba740b2c570318b6a471f47bdbf571acf1d5d
facebookarchive · Apr 6, 2020 · 13989e3 · 13989e3
1 parent 7d3d3c8
commit 13989e3
Show file tree

Hide file tree

Showing 23 changed files with 464 additions and 403 deletions.
diff --git a/src/component/handlers/edit/__tests__/__snapshots__/editOnBeforeInput-test.js.snap b/src/component/handlers/edit/__tests__/__snapshots__/editOnBeforeInput-test.js.snap
@@ -6,7 +6,9 @@ Immutable.Record {
     "__add": [Function],
     "__create": [Function],
     "__get": [Function],
+    "__getAll": [Function],
     "__getLastCreatedEntityKey": [Function],
+    "__loadWithEntities": [Function],
     "__mergeData": [Function],
     "__replaceData": [Function],
     "add": [Function],

diff --git a/src/model/encoding/__tests__/__snapshots__/convertFromDraftStateToRaw-test.js.snap b/src/model/encoding/__tests__/__snapshots__/convertFromDraftStateToRaw-test.js.snap
@@ -163,12 +163,12 @@ Object {
           "offset": 0,
         },
         Object {
-          "key": 0,
+          "key": 1,
           "length": 5,
           "offset": 6,
         },
         Object {
-          "key": 1,
+          "key": 2,
           "length": 5,
           "offset": 12,
         },
@@ -207,28 +207,28 @@ Object {
   "entityMap": Object {
     "0": Object {
       "data": Object {
-        "url": "www.2.com",
+        "url": "www.3.com",
       },
       "mutability": "IMMUTABLE",
       "type": "LINK",
     },
     "1": Object {
       "data": Object {
-        "url": "www.3.com",
+        "url": "www.4.com",
       },
       "mutability": "IMMUTABLE",
       "type": "LINK",
     },
     "2": Object {
       "data": Object {
-        "url": "www.4.com",
+        "url": "www.5.com",
       },
       "mutability": "IMMUTABLE",
       "type": "LINK",
     },
     "3": Object {
       "data": Object {
-        "url": "www.5.com",
+        "url": "www.6.com",
       },
       "mutability": "IMMUTABLE",
       "type": "LINK",

diff --git a/src/model/encoding/__tests__/__snapshots__/convertFromHTMLToContentBlocks-test.js.snap b/src/model/encoding/__tests__/__snapshots__/convertFromHTMLToContentBlocks-test.js.snap
@@ -808,81 +808,81 @@ Array [
   Object {
     "characterList": Array [
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [],
       },
     ],

diff --git a/src/model/encoding/__tests__/__snapshots__/convertFromRawToDraftState-test.js.snap b/src/model/encoding/__tests__/__snapshots__/convertFromRawToDraftState-test.js.snap
@@ -715,31 +715,31 @@ Object {
   "b": Object {
     "characterList": Array [
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],
       },
       Object {
-        "entity": "1",
+        "entity": "2",
         "style": Array [
           "BOLD",
         ],

diff --git a/src/model/encoding/__tests__/convertFromDraftStateToRaw-test.js b/src/model/encoding/__tests__/convertFromDraftStateToRaw-test.js
@@ -11,6 +11,9 @@
 
 'use strict';
 
+const mockUUID = require('mockUUID');
+jest.mock('uuid', () => jest.fn(mockUUID));
+
 const BlockMapBuilder = require('BlockMapBuilder');
 const CharacterMetadata = require('CharacterMetadata');
 const ContentBlock = require('ContentBlock');
@@ -69,38 +72,38 @@ const getLink = entityKey =>
       url: `www.${entityKey}.com`,
     },
   });
-// We start numbering our entities with '2' because getSampleStateForTesting
-// already created an entity with key '1'.
+// We start numbering our entities with '3' because getSampleStateForTesting
+// already created an entity with key '2'.
 const contentStateWithNonContiguousEntities = ContentState.createFromBlockArray(
   [
     new ContentBlock({
       key: 'a',
       type: 'unstyled',
       text: 'link2 link2 link3',
-      characterList: getMetadata('2')
+      characterList: getMetadata('3')
         .toList()
         .push(CharacterMetadata.EMPTY)
-        .concat(getMetadata('2'))
+        .concat(getMetadata('4'))
         .push(CharacterMetadata.EMPTY)
-        .concat(getMetadata('3')),
+        .concat(getMetadata('5')),
     }),
     new ContentBlock({
       key: 'b',
       type: 'unstyled',
       text: 'link4 link2 link5',
-      characterList: getMetadata('4')
+      characterList: getMetadata('5')
         .toList()
         .push(CharacterMetadata.EMPTY)
-        .concat(getMetadata('2'))
+        .concat(getMetadata('3'))
         .push(CharacterMetadata.EMPTY)
-        .concat(getMetadata('5')),
+        .concat(getMetadata('6')),
     }),
   ],
 )
-  .addEntity(getLink('2'))
   .addEntity(getLink('3'))
   .addEntity(getLink('4'))
-  .addEntity(getLink('5'));
+  .addEntity(getLink('5'))
+  .addEntity(getLink('6'));
 
 const assertConvertFromDraftStateToRaw = content => {
   expect(convertFromDraftStateToRaw(content)).toMatchSnapshot();

diff --git a/src/model/encoding/__tests__/convertFromHTMLToContentBlocks-test.js b/src/model/encoding/__tests__/convertFromHTMLToContentBlocks-test.js
@@ -20,6 +20,7 @@ const DefaultDraftBlockRenderMap = require('DefaultDraftBlockRenderMap');
 const convertFromHTMLToContentBlocks = require('convertFromHTMLToContentBlocks');
 const cx = require('cx');
 const getSafeBodyFromHTML = require('getSafeBodyFromHTML');
+const mockUUID = require('mockUUID');
 
 const DEFAULT_CONFIG = {
   DOMBuilder: getSafeBodyFromHTML,
@@ -71,6 +72,7 @@ const toggleExperimentalTreeDataSupport = enabled => {
 
 beforeEach(() => {
   jest.resetModules();
+  jest.mock('uuid', () => mockUUID);
 });
 
 const convertFromHTML = (html_string, config) => {

diff --git a/src/model/encoding/__tests__/convertFromRawToDraftState-test.js b/src/model/encoding/__tests__/convertFromRawToDraftState-test.js
@@ -13,6 +13,7 @@
 jest.mock('generateRandomKey');
 
 const convertFromRawToDraftState = require('convertFromRawToDraftState');
+const mockUUID = require('mockUUID');
 
 const toggleExperimentalTreeDataSupport = enabled => {
   jest.doMock('gkx', () => name => {
@@ -30,6 +31,7 @@ const assertDraftState = rawState => {
 
 beforeEach(() => {
   jest.resetModules();
+  jest.mock('uuid', () => mockUUID);
 });
 
 test('must map falsey block types to default value of unstyled', () => {

diff --git a/src/model/entity/DraftEntity.js b/src/model/entity/DraftEntity.js
@@ -16,11 +16,12 @@ const DraftEntityInstance = require('DraftEntityInstance');
 
 const Immutable = require('immutable');
 const invariant = require('invariant');
+const uuid = require('uuid');
 
 const {Map} = Immutable;
 
 let instances: Map<string, DraftEntityInstance> = Map();
-let instanceKey = 0;
+let instanceKey: string = uuid();
 
 /**
  * Temporary utility for generating the warnings
@@ -52,6 +53,8 @@ export type DraftEntityMapObject = {
     key: string,
     newData: {[key: string]: any, ...},
   ) => DraftEntityInstance,
+  __loadWithEntities: (entities: Map<string, DraftEntityInstance>) => void,
+  __getAll: () => Map<string, DraftEntityInstance>,
   __getLastCreatedEntityKey: () => string,
   __create: (
     type: DraftEntityType,
@@ -143,6 +146,21 @@ const DraftEntity: DraftEntityMapObject = {
     return DraftEntity.__get(key);
   },
 
+  /**
+   * Get all the entities in the content state.
+   */
+  __getAll(): Map<string, DraftEntityInstance> {
+    return instances;
+  },
+
+  /**
+   * Load the entity map with the given set of entities.
+   */
+  __loadWithEntities(entities: Map<string, DraftEntityInstance>): void {
+    instances = entities;
+    instanceKey = uuid();
+  },
+
   /**
    * WARNING: This method will be deprecated soon!
    * Please use 'contentState.mergeEntityData' instead.
@@ -182,8 +200,8 @@ const DraftEntity: DraftEntityMapObject = {
    * We need this to support the new API, as part of transitioning to put Entity
    * storage in contentState.
    */
-  __getLastCreatedEntityKey: function(): string {
-    return '' + instanceKey;
+  __getLastCreatedEntityKey(): string {
+    return instanceKey;
   },
 
   /**
@@ -207,10 +225,10 @@ const DraftEntity: DraftEntityMapObject = {
    * Add an existing DraftEntityInstance to the DraftEntity map. This is
    * useful when restoring instances from the server.
    */
-  __add: function(instance: DraftEntityInstance): string {
-    const key = '' + ++instanceKey;
-    instances = instances.set(key, instance);
-    return key;
+  __add(instance: DraftEntityInstance): string {
+    instanceKey = uuid();
+    instances = instances.set(instanceKey, instance);
+    return instanceKey;
   },
 
   /**

diff --git a/src/model/entity/__tests__/__snapshots__/getEntityKeyForSelection-test.js.snap b/src/model/entity/__tests__/__snapshots__/getEntityKeyForSelection-test.js.snap
@@ -10,9 +10,9 @@ exports[`must not return key if segmented 1`] = `null`;
 
 exports[`must not return key if segmented with collapsed selection 1`] = `null`;
 
-exports[`must return key if mutable 1`] = `"1"`;
+exports[`must return key if mutable 1`] = `"2"`;
 
-exports[`must return key if mutable with collapsed selection 1`] = `"1"`;
+exports[`must return key if mutable with collapsed selection 1`] = `"2"`;
 
 exports[`must return null at start of block with collapsed selection 1`] = `null`;
 

diff --git a/src/model/immutable/ContentState.js b/src/model/immutable/ContentState.js
@@ -16,6 +16,7 @@ import type {BlockNodeRecord} from 'BlockNodeRecord';
 import type DraftEntityInstance from 'DraftEntityInstance';
 import type {DraftEntityMutability} from 'DraftEntityMutability';
 import type {DraftEntityType} from 'DraftEntityType';
+import type {Map} from 'immutable';
 
 const BlockMapBuilder = require('BlockMapBuilder');
 const CharacterMetadata = require('CharacterMetadata');
@@ -174,6 +175,14 @@ class ContentState extends ContentStateRecord {
     return DraftEntity.__get(key);
   }
 
+  getAllEntities(): Map<string, DraftEntityInstance> {
+    return DraftEntity.__getAll();
+  }
+
+  loadWithEntities(entities: Map<string, DraftEntityInstance>): void {
+    return DraftEntity.__loadWithEntities(entities);
+  }
+
   static createFromBlockArray(
     // TODO: update flow type when we completely deprecate the old entity API
     blocks: