From 0cc78076b2baac9db559189954c08e9d703cc3f8 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 4 Sep 2018 15:43:00 +0200 Subject: [PATCH 1/2] Changed: Introduced `ContextFactory` in OT algorithms. Cleaned up mess with "context" names. --- src/model/operation/transform.js | 421 +++++++++++------------ tests/model/operation/transform/utils.js | 1 + 2 files changed, 207 insertions(+), 215 deletions(-) diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index 93c9d44c7..9b63011e8 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -302,7 +302,9 @@ export function transformSets( operationsA, operationsB, options ) { originalOperationsBCount: operationsB.length }; - const context = initializeContext( operationsA, operationsB, options ); + const contextFactory = new ContextFactory( options.document, options.useContext ); + contextFactory.setOriginalOperations( operationsA ); + contextFactory.setOriginalOperations( operationsB ); // Index of currently transformed operation `a`. let i = 0; @@ -323,35 +325,16 @@ export function transformSets( operationsA, operationsB, options ) { const opB = operationsB[ indexB ]; - // Evaluate `TransformationContext` objects for operation transformation. - const contextAB = { - aIsStrong: true, - aWasUndone: context.wasUndone( opA ), - bWasUndone: context.wasUndone( opB ), - abRelation: context.getRelation( opA, opB ), - baRelation: context.getRelation( opB, opA ) - }; - - const contextBA = { - aIsStrong: false, - aWasUndone: context.wasUndone( opB ), - bWasUndone: context.wasUndone( opA ), - abRelation: context.getRelation( opB, opA ), - baRelation: context.getRelation( opA, opB ) - }; - // Transform `a` by `b` and `b` by `a`. - const newOpsA = transform( opA, opB, contextAB ); - const newOpsB = transform( opB, opA, contextBA ); + const newOpsA = transform( opA, opB, contextFactory.getContext( opA, opB, true ) ); + const newOpsB = transform( opB, opA, contextFactory.getContext( opB, opA, false ) ); // As a result we get one or more `newOpsA` and one or more `newOpsB` operations. // Update contextual information about operations. - if ( options.useContext ) { - updateRelations( opA, opB, context ); + contextFactory.updateRelation( opA, opB ); - updateOriginalOperation( opA, newOpsA, context ); - updateOriginalOperation( opB, newOpsB, context ); - } + contextFactory.setOriginalOperations( newOpsA, opA ); + contextFactory.setOriginalOperations( newOpsB, opB ); // For new `a` operations, update their index of the next operation `b` to transform them by. // @@ -391,92 +374,177 @@ export function transformSets( operationsA, operationsB, options ) { return { operationsA, operationsB }; } -/** - * An utility function that updates {@link module:engine/model/operation/operation~Operation#baseVersion base versions} - * of passed operations. - * - * The function simply sets `baseVersion` as a base version of the first passed operation and then increments it for - * each following operation in `operations`. - * - * @private - * @param {Array.} operations Operations to update. - * @param {Number} baseVersion Base version to set for the first operation in `operations`. - */ -function updateBaseVersions( operations, baseVersion ) { - for ( const operation of operations ) { - operation.baseVersion = baseVersion++; +// Gathers additional data about operations processed during transformation. Can be used to obtain contextual information +// about two operations that are about to be transformed. This contextual information can be used for better conflict resolution. +class ContextFactory { + // Creates `ContextFactory` instance. + // + // @param {module:engine/model/document~Document} document Document which the operations change. + // @param {Boolean} useContext Whether during transformation additional context information should be gathered and used. + constructor( document, useContext ) { + // `model.History` instance which information about undone operations will be taken from. + this._history = document.history; + + // Whether additional context should be used. + this._useContext = useContext; + + // For each operation that is created during transformation process, we keep a reference to the original operation + // which it comes from. The original operation works as a kind of "identifier". Every contextual information + // gathered during transformation that we want to save for given operation, is actually saved for the original operation. + // This way no matter if operation `a` is cloned, then transformed, even breaks, we still have access to the previously + // gathered data through original operation reference. + this._originalOperations = new Map(); + + // Relations is a double-map structure (maps in map) where for two operations we store how those operations were related + // to each other. Those relations are evaluated during transformation process. For every transformated pair of operations + // we keep relations between them. + this._relations = new Map(); } -} -/** - * Adds `howMany` instances of {@link module:engine/model/operation/nooperation~NoOperation} to `operations` set. - * - * @private - * @param {Array.} operations - * @param {Number} howMany - */ -function padWithNoOps( operations, howMany ) { - for ( let i = 0; i < howMany; i++ ) { - operations.push( new NoOperation( 0 ) ); + // Rewrites information about original operation to the new operations. + // + // Used when `newOps` are generated from `oldOp` (during transformation). It takes `oldOp`'s original operation and + // sets it as `newOps` original operation. + // + // It also means that if an operation is broken into multiple during transformation, all those broken "pieces" are pointing + // to the same operation as their original operation. + // + // @param {Array.} operations + // @param {module:engine/model/operation/operation~Operation|null} [takeFrom=null] + setOriginalOperations( operations, takeFrom = null ) { + const originalOperation = takeFrom ? this._originalOperations.get( takeFrom ) : null; + + for ( const operation of operations ) { + this._originalOperations.set( operation, originalOperation || operation ); + } } -} -/** - * Initializes transformation process data used to evaluate context between two transformed operations. - * - * @param {Array.} operationsA - * @param {Array.} operationsB - * @param {Object} options Additional transformation options. See {@link module:engine/model/operation/transform~transformSets} - * @returns {Object} Transformation process data object. - */ -function initializeContext( operationsA, operationsB, options ) { - const context = {}; + // Saves a relation between operations `opA` and `opB`. + // + // Relations are then later used to help solve conflicts when operations are transformed. + // + // @param {module:engine/model/operation/operation~Operation} opA + // @param {module:engine/model/operation/operation~Operation} opB + updateRelation( opA, opB ) { + // The use of relations is described in a bigger detail in transformation functions. + // + // In brief, this function, for specified pairs of operation types, checks how positions defined in those operations relate. + // Then those relations are saved. For example, for two move operations, it is saved if one of those operations target + // position is before the other operation source position. This kind of information gives contextual information when + // transformation is used during undo. Similar checks are done for other pairs of operations. + // + switch ( opA.constructor ) { + case MoveOperation: { + switch ( opB.constructor ) { + case MergeOperation: { + if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.targetPosition ) ) { + this._setRelation( opA, opB, 'insertAtSource' ); + } else if ( opA.targetPosition.isEqual( opB.deletionPosition ) ) { + this._setRelation( opA, opB, 'insertBetween' ); + } + + break; + } + + case MoveOperation: { + if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opA.targetPosition.isBefore( opB.sourcePosition ) ) { + this._setRelation( opA, opB, 'insertBefore' ); + } else { + this._setRelation( opA, opB, 'insertAfter' ); + } + + break; + } + + case UnwrapOperation: { + const isInside = opA.targetPosition.hasSameParentAs( opB.position ); + + if ( isInside ) { + this._setRelation( opA, opB, 'insertInside' ); + } + + break; + } + } + + break; + } - // For each operation that is created during transformation process, we keep a reference to the original operation - // which it comes from. The original operation works as a kind of "identifier". Every contextual information - // gathered during transformation that we want to save for given operation, is actually saved for the original operation. - // This way no matter if operation `a` is cloned, then transformed, even breaks, we still have access to the previously - // gathered data through original operation reference. - context.originalOperations = new Map(); + case SplitOperation: { + switch ( opB.constructor ) { + case MergeOperation: { + if ( opA.position.isBefore( opB.sourcePosition ) ) { + this._setRelation( opA, opB, 'splitBefore' ); + } - // At the beginning for each operation, set the original operation reference to itself. - for ( const op of operationsA.concat( operationsB ) ) { - context.originalOperations.set( op, op ); + break; + } + + case MoveOperation: { + if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) { + this._setRelation( opA, opB, 'splitBefore' ); + } + + break; + } + } + + break; + } + } } - context.document = options.document; + // Evaluates and returns contextual information about two given operations `opA` and `opB` which are about to be transformed. + // + // @param {module:engine/model/operation/operation~Operation} opA + // @param {module:engine/model/operation/operation~Operation} opB + // @returns {module:engine/model/operation/transform~TransformationContext} + getContext( opA, opB, aIsStrong ) { + if ( !this._useContext ) { + // Additional contextual data is `false` or `null` if additional context is not used. - // Relations is a double-map structure (maps in map) where for two operations we store how those operations were related - // to each other. Those relations are evaluated during transformation process. For every transformated pair of operations - // we keep relations between them. - context.relations = new Map(); + return { + aIsStrong, + aWasUndone: false, + bWasUndone: false, + abRelation: null, + baRelation: null + }; + } + + return { + aIsStrong, + aWasUndone: this._wasUndone( opA ), + bWasUndone: this._wasUndone( opB ), + abRelation: this._getRelation( opA, opB ), + baRelation: this._getRelation( opB, opA ) + }; + } // Returns whether given operation `op` has already been undone. // // This is only used when additional context mode is on (options.useContext == true). // // Information whether an operation was undone gives more context when making a decision when two operations are in conflict. - context.wasUndone = function( op ) { - // If additional context is not used, just return false. - if ( !options.useContext ) { - return false; - } - + // + // @param {module:engine/model/operation/operation~Operation} op + // @returns {Boolean} + _wasUndone( op ) { // For `op`, get its original operation. After all, if `op` is a clone (or even transformed clone) of another // operation, literally `op` couldn't be undone. It was just generated. If anything, it was the operation it origins // from which was undone. So get that original operation. - const originalOp = this.originalOperations.get( op ); + const originalOp = this._originalOperations.get( op ); // And check with the document if the original operation was undone. - return this.document.history.isUndoneOperation( originalOp ); - }; + return this._history.isUndoneOperation( originalOp ); + } // Returns a relation between `opA` and an operation which is undone by `opB`. This can be `String` value if a relation // was set earlier or `null` if there was no relation between those operations. // // This is only used when additional context mode is on (options.useContext == true). // - // This is a little tricky to understand, so let's compare it to `wasUndone()`. + // This is a little tricky to understand, so let's compare it to `ContextFactory#_wasUndone`. // // When `wasUndone( opB )` is used, we check if the `opB` has already been undone. It is obvious, that the // undoing operation must happen after the undone operation. So, essentially, we have `opB`, we take document history, @@ -485,7 +553,7 @@ function initializeContext( operationsA, operationsB, options ) { // Relations is a backward process to `wasUndone()`. // // Long story short - using relations is asking what happened in the past. Looking back. This time we have an undoing - // operation `opB` which has undone some other operation. When there is a transformation `opA` * `opB` and there is + // operation `opB` which has undone some other operation. When there is a transformation `opA` x `opB` and there is // a conflict to solve and `opB` is an undoing operation, we can look back in the history and see what was a relation // between `opA` and the operation which `opB` undone. Basing on that relation from the past, we can now make // a better decision when resolving a conflict between two operations, because we know more about the context of @@ -493,23 +561,22 @@ function initializeContext( operationsA, operationsB, options ) { // // This is why this function does not return a relation directly between `opA` and `opB` because we need to look // back to search for a meaningful contextual information. - context.getRelation = function( opA, opB ) { - // If additional context is not used, there is no relation. - if ( !options.useContext ) { - return null; - } - + // + // @param {module:engine/model/operation/operation~Operation} opA + // @param {module:engine/model/operation/operation~Operation} opB + // @returns {String|null} + _getRelation( opA, opB ) { // Get the original operation. Similarly as in `wasUndone()` it is used as an universal identifier for stored data. - const origB = this.originalOperations.get( opB ); - const undoneB = this.document.history.getUndoneOperation( origB ); + const origB = this._originalOperations.get( opB ); + const undoneB = this._history.getUndoneOperation( origB ); // If `opB` is not undoing any operation, there is no relation. if ( !undoneB ) { return null; } - const origA = this.originalOperations.get( opA ); - const relationsA = this.relations.get( origA ); + const origA = this._originalOperations.get( opA ); + const relationsA = this._relations.get( origA ); // Get all relations for `opA`, and check if there is a relation with `opB`-undone-counterpart. If so, return it. if ( relationsA ) { @@ -517,148 +584,72 @@ function initializeContext( operationsA, operationsB, options ) { } return null; - }; - - return context; -} - -/** - * Saves a relation between operations `opA` and `opB`. - * - * Relations are then later used to help solve conflicts when operations are transformed. - * - * @private - * @param {module:engine/model/operation/operation~Operation} opA - * @param {module:engine/model/operation/operation~Operation} opB - * @param {Object} context - */ -function updateRelations( opA, opB, context ) { - // The use of relations is described in a bigger detail in transformation functions. - // - // In brief, this function, for specified pairs of operation types, checks how positions defined in those operations relate. - // Then those relations are saved. For example, for two move operations, it is saved if one of those operations target - // position is before the other operation source position. This kind of information gives contextual information when - // transformation is used during undo. Similar checks are done for other pairs of operations. - // - switch ( opA.constructor ) { - case MoveOperation: { - switch ( opB.constructor ) { - case MergeOperation: { - if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.targetPosition ) ) { - setRelation( context, opA, opB, 'insertAtSource' ); - } else if ( opA.targetPosition.isEqual( opB.deletionPosition ) ) { - setRelation( context, opA, opB, 'insertBetween' ); - } - - break; - } - - case MoveOperation: { - if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opA.targetPosition.isBefore( opB.sourcePosition ) ) { - setRelation( context, opA, opB, 'insertBefore' ); - } else { - setRelation( context, opA, opB, 'insertAfter' ); - } - - break; - } - - case UnwrapOperation: { - const isInside = opA.targetPosition.hasSameParentAs( opB.position ); + } - if ( isInside ) { - setRelation( context, opA, opB, 'insertInside' ); - } + // Helper function for `ContextFactory#updateRelations`. + // + // @private + // @param {module:engine/model/operation/operation~Operation} opA + // @param {module:engine/model/operation/operation~Operation} opB + // @param {String} relation + _setRelation( opA, opB, relation ) { + // As always, setting is for original operations, not the clones/transformed operations. + const origA = this._originalOperations.get( opA ); + const origB = this._originalOperations.get( opB ); - break; - } - } + let relationsA = this._relations.get( origA ); - break; + if ( !relationsA ) { + relationsA = new Map(); + this._relations.set( origA, relationsA ); } - case SplitOperation: { - switch ( opB.constructor ) { - case MergeOperation: { - if ( opA.position.isBefore( opB.sourcePosition ) ) { - setRelation( context, opA, opB, 'splitBefore' ); - } - - break; - } - - case MoveOperation: { - if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) { - setRelation( context, opA, opB, 'splitBefore' ); - } - - break; - } - } - - break; - } + relationsA.set( origB, relation ); } } /** - * Helper function for {@link module:engine/model/operation/transform~updateRelations}. + * Holds additional contextual information about a transformed pair of operations (`a` and `b`). Those information + * can be used for better conflict resolving. * - * @private - * @param {Object} context - * @param {module:engine/model/operation/operation~Operation} opA - * @param {module:engine/model/operation/operation~Operation} opB - * @param {String} relation + * @typedef {Object} module:engine/model/operation/transform~TransformationContext + * + * @property {Boolean} aIsStrong Whether `a` is strong operation in this transformation, or weak. + * @property {Boolean} aWasUndone Whether `a` operation was undone. + * @property {Boolean} bWasUndone Whether `b` operation was undone. + * @property {String|null} abRelation The relation between `a` operation and an operation undone by `b` operation. + * @property {String|null} baRelation The relation between `b` operation and an operation undone by `a` operation. */ -function setRelation( context, opA, opB, relation ) { - // As always, setting is for original operations, not the clones/transformed operations. - const origA = context.originalOperations.get( opA ); - const origB = context.originalOperations.get( opB ); - - let relationsA = context.relations.get( origA ); - - if ( !relationsA ) { - relationsA = new Map(); - context.relations.set( origA, relationsA ); - } - - relationsA.set( origB, relation ); -} /** - * Rewrites information about original operation to the new operations. - * - * Used when `newOps` are generated from `oldOp` (during transformation). It takes `oldOp`'s original operation and - * sets it as `newOps` original operation. + * An utility function that updates {@link module:engine/model/operation/operation~Operation#baseVersion base versions} + * of passed operations. * - * It also means that if an operation is broken into multiple during transformation, all those broken "pieces" are pointing - * to the same operation as their original operation. + * The function simply sets `baseVersion` as a base version of the first passed operation and then increments it for + * each following operation in `operations`. * * @private - * @param {module:engine/model/operation/operation~Operation} oldOp - * @param {Array.} newOps - * @param {Object} context + * @param {Array.} operations Operations to update. + * @param {Number} baseVersion Base version to set for the first operation in `operations`. */ -function updateOriginalOperation( oldOp, newOps, context ) { - const originalOp = context.originalOperations.get( oldOp ); - - for ( const op of newOps ) { - context.originalOperations.set( op, originalOp ); +function updateBaseVersions( operations, baseVersion ) { + for ( const operation of operations ) { + operation.baseVersion = baseVersion++; } } /** - * Holds additional contextual information about a transformed pair of operations (`a` and `b`). Those information - * can be used for better conflict resolving. - * - * @typedef {Object} module:engine/model/operation/transform~TransformationContext + * Adds `howMany` instances of {@link module:engine/model/operation/nooperation~NoOperation} to `operations` set. * - * @property {Boolean} aIsStrong Whether `a` is strong operation in this transformation, or weak. - * @property {Boolean} aWasUndone Whether `a` operation was undone. - * @property {Boolean} bWasUndone Whether `b` operation was undone. - * @property {String|null} abRelation The relation between `a` operation and an operation undone by `b` operation. - * @property {String|null} baRelation The relation between `b` operation and an operation undone by `a` operation. + * @private + * @param {Array.} operations + * @param {Number} howMany */ +function padWithNoOps( operations, howMany ) { + for ( let i = 0; i < howMany; i++ ) { + operations.push( new NoOperation( 0 ) ); + } +} // ----------------------- diff --git a/tests/model/operation/transform/utils.js b/tests/model/operation/transform/utils.js index bf3e21f0a..2edd9f914 100644 --- a/tests/model/operation/transform/utils.js +++ b/tests/model/operation/transform/utils.js @@ -285,6 +285,7 @@ export function syncClients() { let remoteOperationsTransformed = null; const options = { + document: localClient.document, useContext: false, padWithNoOps: true }; From 575ace54c46b7267308115b058d0269b42496759 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 4 Sep 2018 17:47:11 +0200 Subject: [PATCH 2/2] Docs: Improved ContextFactory docs. --- src/model/operation/transform.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index 9b63011e8..f1852c206 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -401,13 +401,21 @@ class ContextFactory { this._relations = new Map(); } - // Rewrites information about original operation to the new operations. + // Sets "original operation" for given operations. // - // Used when `newOps` are generated from `oldOp` (during transformation). It takes `oldOp`'s original operation and - // sets it as `newOps` original operation. + // During transformation process, operations are cloned, then changed, then processed again, sometimes broken into two + // or multiple operations. When gathering additional data it is important that all operations can be somehow linked + // so a cloned and transformed "version" still kept track of the data assigned earlier to it. // - // It also means that if an operation is broken into multiple during transformation, all those broken "pieces" are pointing - // to the same operation as their original operation. + // The original operation object will be used as such an universal linking id. Throughout the transformation process + // all cloned operations will refer to "the original operation" when storing and reading additional data. + // + // If `takeFrom` is not set, each operation from `operations` array will be assigned itself as "the original operation". + // This should be used as an initialization step. + // + // If `takeFrom` is set, each operation from `operations` will be assigned the same original operation as assigned + // for `takeFrom` operation. This should be used to update original operations. It should be used in a way that + // `operations` are the result of `takeFrom` transformation to ensure proper "original operation propagation". // // @param {Array.} operations // @param {module:engine/model/operation/operation~Operation|null} [takeFrom=null] @@ -501,8 +509,6 @@ class ContextFactory { // @returns {module:engine/model/operation/transform~TransformationContext} getContext( opA, opB, aIsStrong ) { if ( !this._useContext ) { - // Additional contextual data is `false` or `null` if additional context is not used. - return { aIsStrong, aWasUndone: false,