Parcourir la source

Cleared and rewritten engine.model.History.

Szymon Cofalik il y a 9 ans
Parent
commit
4df8fced7d

+ 65 - 77
packages/ckeditor5-engine/src/model/history.js

@@ -5,18 +5,14 @@
 
 'use strict';
 
-// Load all basic deltas and transformations, they register themselves, but they need to be imported somewhere.
-import deltas from './delta/basic-deltas.js'; // jshint ignore:line
-import transformations from './delta/basic-transformations.js'; // jshint ignore:line
-
-import transform from './delta/transform.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 
 /**
- * History keeps the track of all the deltas applied to the {@link engine.model.Document document} and provides
- * utility tools to operate on the history. Most of times history is needed to transform a delta that has wrong
- * {@link engine.model.delta.Delta#baseVersion} to a state where it can be applied to the document.
+ * `History` keeps the track of all the deltas applied to the {@link engine.model.Document document}. `History` can be
+ * seen as "add-only" structure. You can read and add deltas, but can't modify them. Use this version of history to
+ * retrieve applied deltas as they were, in the original form.
  *
+ * @see engine.model.CompressedHistory
  * @memberOf engine.model
  */
 export default class History {
@@ -27,7 +23,7 @@ export default class History {
 		/**
 		 * Deltas added to the history.
 		 *
-		 * @private
+		 * @protected
 		 * @member {Array.<engine.model.delta.Delta>} engine.model.History#_deltas
 		 */
 		this._deltas = [];
@@ -36,38 +32,19 @@ export default class History {
 		 * Helper structure that maps added delta's base version to the index in {@link engine.model.History#_deltas}
 		 * at which the delta was added.
 		 *
-		 * @private
+		 * @protected
 		 * @member {Map} engine.model.History#_historyPoints
 		 */
 		this._historyPoints = new Map();
 	}
 
 	/**
-	 * Gets the number of base version which an up-to-date operation should have.
+	 * Adds delta to the history.
 	 *
-	 * @private
-	 * @type {Number}
+	 * @param {engine.model.delta.Delta} delta Delta to add.
 	 */
-	get _nextHistoryPoint() {
-		const lastDelta = this._deltas[ this._deltas.length - 1 ];
-
-		return lastDelta.baseVersion + lastDelta.operations.length;
-	}
-
-	/**
-	 * Adds an operation to the history.
-	 *
-	 * @param {engine.model.operation.Operation} operation Operation to add.
-	 */
-	addOperation( operation ) {
-		const delta = operation.delta;
-
-		// History cares about deltas not singular operations.
-		// Operations from a delta are added one by one, from first to last.
-		// Operations from one delta cannot be mixed with operations from other deltas.
-		// This all leads us to the conclusion that we could just save deltas history.
-		// What is more, we need to check only the last position in history to check if delta is already in the history.
-		if ( delta && this._deltas[ this._deltas.length - 1 ] !== delta ) {
+	addDelta( delta ) {
+		if ( delta.operations.length > 0 && !this._historyPoints.has( delta.baseVersion ) ) {
 			const index = this._deltas.length;
 
 			this._deltas[ index ] = delta;
@@ -76,69 +53,80 @@ export default class History {
 	}
 
 	/**
-	 * Transforms out-dated delta by all deltas that were added to the history since the given delta's base version. In other
-	 * words, it makes the delta up-to-date with the history. The transformed delta(s) is (are) ready to be applied
-	 * to the {@link engine.model.Document document}.
+	 * Returns deltas added to the history.
 	 *
-	 * @param {engine.model.delta.Delta} delta Delta to update.
-	 * @returns {Array.<engine.model.delta.Delta>} Result of transformation which is an array containing one or more deltas.
+	 * @param {Number} from Base version from which deltas should be returned (inclusive). Defaults to `0` which means
+	 * that deltas from the first one will be returned.
+	 * @param {Number} to Base version up to which deltas should be returned (exclusive). Defaults to `Number.POSITIVE_INFINITY`
+	 * which means that deltas up to the last one will be returned.
+	 * @returns {Iterator.<engine.model.delta.Delta>} Deltas added to the history.
 	 */
-	getTransformedDelta( delta ) {
-		if ( delta.baseVersion === this._nextHistoryPoint ) {
-			return [ delta ];
+	*getDeltas( from = 0, to = Number.POSITIVE_INFINITY ) {
+		// No deltas added, nothing to yield.
+		if ( this._deltas.length === 0 ) {
+			return;
 		}
 
-		let transformed = [ delta ];
-
-		for ( let historyDelta of this.getDeltas( delta.baseVersion ) ) {
-			let allResults = [];
-
-			for ( let deltaToTransform of transformed ) {
-				const transformedDelta = History._transform( deltaToTransform, historyDelta );
-				allResults = allResults.concat( transformedDelta );
-			}
+		// Will throw if base version is incorrect.
+		let fromIndex = this._getIndex( from );
 
-			transformed = allResults;
+		// Base version is too low or too high and is not found in history.
+		if ( fromIndex == -1 ) {
+			return;
 		}
 
-		// Fix base versions.
-		let baseVersion = transformed[ 0 ].operations[ 0 ].baseVersion;
+		// We have correct `fromIndex` so let's iterate starting from it.
+		while ( fromIndex < this._deltas.length ) {
+			const delta = this._deltas[ fromIndex++ ];
 
-		for ( let i = 0; i < transformed.length; i++ ) {
-			transformed[ i ].baseVersion = baseVersion;
-			baseVersion += transformed[ i ].operations.length;
-		}
+			if ( delta.baseVersion >= to ) {
+				break;
+			}
 
-		return transformed;
+			yield delta;
+		}
 	}
 
 	/**
-	 * Returns all deltas from history, starting from given history point (if passed).
+	 * Returns a delta from the history that has given {@link engine.model.delta.Delta#baseVersion}.
 	 *
-	 * @param {Number} from History point.
-	 * @returns {Iterator.<engine.model.delta.Delta>} Deltas from given history point to the end of history.
+	 * @param {Number} baseVersion Base version of the delta to retrieve.
+	 * @returns {engine.model.delta.Delta|null} Delta with given base version or null if no such delta is in history.
 	 */
-	*getDeltas( from = 0 ) {
-		let i = this._historyPoints.get( from );
+	getDelta( baseVersion ) {
+		let index = this._historyPoints.get( baseVersion );
 
-		if ( i === undefined ) {
-			throw new CKEditorError( 'history-wrong-version: Cannot retrieve given point in the history.' );
-		}
-
-		for ( ; i < this._deltas.length; i++ ) {
-			yield this._deltas[ i ];
-		}
+		return this._deltas[ index ] || null;
 	}
 
 	/**
-	 * Transforms given delta by another given delta. Exposed for testing purposes.
+	 * Gets an index in {@link engine.model.History#_deltas} where delta with given `baseVersion` is added.
 	 *
-	 * @protected
-	 * @param {engine.model.delta.Delta} toTransform Delta to be transformed.
-	 * @param {engine.model.delta.Delta} transformBy Delta to transform by.
-	 * @returns {Array.<engine.model.delta.Delta>} Result of the transformation.
+	 * @param {Number} baseVersion Base version of delta.
+	 * @private
 	 */
-	static _transform( toTransform, transformBy ) {
-		return transform( toTransform, transformBy, true );
+	_getIndex( baseVersion ) {
+		let index = this._historyPoints.get( baseVersion );
+
+		// Base version not found - it is either too high or too low, or is in the middle of delta.
+		if ( index === undefined ) {
+			const lastDelta = this._deltas[ this._deltas.length - 1 ];
+			const nextBaseVersion = lastDelta.baseVersion + lastDelta.operations.length;
+
+			if ( baseVersion < 0 || baseVersion >= nextBaseVersion ) {
+				// Base version is too high or too low - it's acceptable situation.
+				// Return -1 because `baseVersion` was correct.
+				return -1;
+			}
+
+			/**
+			 * Given base version points to the middle of a delta.
+			 *
+			 * @error history-wrong-version
+			 */
+			throw new CKEditorError( 'history-wrong-version: Given base version points to the middle of a delta.' );
+		}
+
+		return index;
 	}
 }

+ 81 - 122
packages/ckeditor5-engine/tests/model/history.js

@@ -7,7 +7,8 @@
 
 import History from '/ckeditor5/engine/model/history.js';
 import Delta from '/ckeditor5/engine/model/delta/delta.js';
-import NoOperation from '/ckeditor5/engine/model/operation/nooperation.js';
+import Operation from '/ckeditor5/engine/model/operation/operation.js';
+
 import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 
 describe( 'History', () => {
@@ -19,166 +20,124 @@ describe( 'History', () => {
 
 	describe( 'constructor', () => {
 		it( 'should create an empty History instance', () => {
-			expect( history._deltas.length ).to.equal( 0 );
-			expect( history._historyPoints.size ).to.equal( 0 );
+			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
 		} );
 	} );
 
-	describe( 'addOperation', () => {
-		it( 'should save delta containing passed operation in the history', () => {
+	describe( 'addDelta', () => {
+		it( 'should save delta in the history', () => {
 			let delta = new Delta();
-			let operation = new NoOperation( 0 );
+			delta.addOperation( new Operation( 0 ) );
 
-			delta.addOperation( operation );
-			history.addOperation( operation );
+			history.addDelta( delta );
 
-			expect( history._deltas.length ).to.equal( 1 );
-			expect( history._deltas[ 0 ] ).to.equal( delta );
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 1 );
+			expect( deltas[ 0 ] ).to.equal( delta );
 		} );
 
 		it( 'should save each delta only once', () => {
 			let delta = new Delta();
+			delta.addOperation( new Operation( 0 ) );
 
-			delta.addOperation( new NoOperation( 0 ) );
-			delta.addOperation( new NoOperation( 1 ) );
-			delta.addOperation( new NoOperation( 2 ) );
-
-			for ( let operation of delta.operations ) {
-				history.addOperation( operation );
-			}
+			history.addDelta( delta );
+			history.addDelta( delta );
 
-			expect( history._deltas.length ).to.equal( 1 );
-			expect( history._deltas[ 0 ] ).to.equal( delta );
+			const deltas = Array.from( history.getDeltas() );
+			expect( deltas.length ).to.equal( 1 );
+			expect( deltas[ 0 ] ).to.equal( delta );
 		} );
 
 		it( 'should save multiple deltas and keep their order', () => {
-			let deltaA = new Delta();
-			let deltaB = new Delta();
-			let deltaC = new Delta();
-
-			let deltas = [ deltaA, deltaB, deltaC ];
-
-			let i = 0;
+			let deltas = getDeltaSet();
 
 			for ( let delta of deltas ) {
-				delta.addOperation( new NoOperation( i++ ) );
-				delta.addOperation( new NoOperation( i++ ) );
+				history.addDelta( delta );
 			}
 
-			for ( let delta of deltas ) {
-				for ( let operation of delta.operations ) {
-					history.addOperation( operation );
-				}
-			}
-
-			expect( history._deltas.length ).to.equal( 3 );
-			expect( history._deltas[ 0 ] ).to.equal( deltaA );
-			expect( history._deltas[ 1 ] ).to.equal( deltaB );
-			expect( history._deltas[ 2 ] ).to.equal( deltaC );
+			const historyDeltas = Array.from( history.getDeltas() );
+			expect( historyDeltas ).to.deep.equal( deltas );
 		} );
-	} );
-
-	describe( 'getTransformedDelta', () => {
-		it( 'should transform given delta by deltas from history which were applied since the baseVersion of given delta', () => {
-			sinon.spy( History, '_transform' );
 
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
-
-			let deltaB = new Delta();
-			deltaB.addOperation( new NoOperation( 1 ) );
-
-			let deltaC = new Delta();
-			deltaC.addOperation( new NoOperation( 2 ) );
-
-			let deltaD = new Delta();
-			deltaD.addOperation( new NoOperation( 3 ) );
-
-			let deltaX = new Delta();
-			deltaX.addOperation( new NoOperation( 1 ) );
-
-			history.addOperation( deltaA.operations[ 0 ] );
-			history.addOperation( deltaB.operations[ 0 ] );
-			history.addOperation( deltaC.operations[ 0 ] );
-			history.addOperation( deltaD.operations[ 0 ] );
-
-			// `deltaX` bases on the same history point as `deltaB` -- so it already acknowledges `deltaA` existence.
-			// It should be transformed by `deltaB` and all following deltas (`deltaC` and `deltaD`).
-			history.getTransformedDelta( deltaX );
+		it( 'should skip deltas that does not have operations', () => {
+			let delta = new Delta();
 
-			// `deltaX` was not transformed by `deltaA`.
-			expect( History._transform.calledWithExactly( deltaX, deltaA ) ).to.be.false;
+			history.addDelta( delta );
 
-			expect( History._transform.calledWithExactly( deltaX, deltaB ) ).to.be.true;
-			// We can't do exact call matching because after first transformation, what we are further transforming
-			// is no longer `deltaX` but a result of transforming `deltaX` and `deltaB`.
-			expect( History._transform.calledWithExactly( sinon.match.instanceOf( Delta ), deltaC ) ).to.be.true;
-			expect( History._transform.calledWithExactly( sinon.match.instanceOf( Delta ), deltaD ) ).to.be.true;
+			expect( Array.from( history.getDeltas() ).length ).to.equal( 0 );
 		} );
+	} );
 
-		it( 'should correctly set base versions if multiple deltas are result of transformation', () => {
-			// Let's stub History._transform so it will always return two deltas with two operations each.
-			History._transform = function() {
-				let resultA = new Delta();
-				resultA.addOperation( new NoOperation( 1 ) );
-				resultA.addOperation( new NoOperation( 1 ) );
+	describe( 'getDeltas', () => {
+		let deltas;
 
-				let resultB = new Delta();
-				resultB.addOperation( new NoOperation( 1 ) );
-				resultB.addOperation( new NoOperation( 1 ) );
+		beforeEach( () => {
+			deltas = getDeltaSet();
 
-				return [ resultA, resultB ];
-			};
+			for ( let delta of deltas ) {
+				history.addDelta( delta );
+			}
+		} );
 
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
+		it( 'should return only history deltas from given base version', () => {
+			const historyDeltas = Array.from( history.getDeltas( 3 ) );
+			expect( historyDeltas ).to.deep.equal( deltas.slice( 1 ) );
+		} );
 
-			let deltaX = new Delta();
-			deltaX.addOperation( new NoOperation( 0 ) );
+		it( 'should return only history deltas to given base version', () => {
+			const historyDeltas = Array.from( history.getDeltas( 3, 6 ) );
+			expect( historyDeltas ).to.deep.equal( deltas.slice( 1, 2 ) );
+		} );
 
-			history.addOperation( deltaA.operations[ 0 ] );
+		it( 'should return empty (finished) iterator if given history point is too high or negative', () => {
+			expect( Array.from( history.getDeltas( 20 ) ).length ).to.equal( 0 );
+			expect( Array.from( history.getDeltas( -1 ) ).length ).to.equal( 0 );
+		} );
 
-			let result = history.getTransformedDelta( deltaX );
+		it( 'should throw if given history point is "inside" delta', () => {
+			expect( () => {
+				Array.from( history.getDeltas( 2 ) );
+			} ).to.throw( CKEditorError, /history-wrong-version/ );
+		} );
+	} );
 
-			expect( result[ 0 ].operations[ 0 ].baseVersion ).to.equal( 1 );
-			expect( result[ 0 ].operations[ 1 ].baseVersion ).to.equal( 2 );
-			expect( result[ 1 ].operations[ 0 ].baseVersion ).to.equal( 3 );
-			expect( result[ 1 ].operations[ 1 ].baseVersion ).to.equal( 4 );
+	describe( 'getDelta', () => {
+		beforeEach( () => {
+			for ( let delta of getDeltaSet() ) {
+				history.addDelta( delta );
+			}
 		} );
 
-		it( 'should not transform given delta if it bases on current version of history', () => {
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
+		it( 'should return delta from history that has given base version', () => {
+			let delta = history.getDelta( 3 );
 
-			let deltaB = new Delta();
-			let opB = new NoOperation( 1 );
-			deltaB.addOperation( opB );
+			expect( delta.baseVersion ).to.equal( 3 );
+		} );
 
-			history.addOperation( deltaA.operations[ 0 ] );
+		it( 'should return null if delta has not been found in history', () => {
+			expect( history.getDelta( -1 ) ).to.be.null;
+			expect( history.getDelta( 2 ) ).to.be.null;
+			expect( history.getDelta( 20 ) ).to.be.null;
+		} );
+	} );
+} );
 
-			let result = history.getTransformedDelta( deltaB );
+function getDeltaSet() {
+	const deltas = [];
 
-			expect( result.length ).to.equal( 1 );
-			expect( result[ 0 ] ).to.equal( deltaB );
-			expect( result[ 0 ].operations[ 0 ] ).to.equal( opB );
-		} );
+	deltas.push( getDelta( 0 ) );
+	deltas.push( getDelta( 3 ) );
+	deltas.push( getDelta( 6 ) );
 
-		it( 'should throw if given delta bases on an incorrect version of history', () => {
-			let deltaA = new Delta();
-			deltaA.addOperation( new NoOperation( 0 ) );
-			deltaA.addOperation( new NoOperation( 1 ) );
+	return deltas;
+}
 
-			history.addOperation( deltaA.operations[ 0 ] );
-			history.addOperation( deltaA.operations[ 1 ] );
+function getDelta( baseVersion ) {
+	const delta = new Delta();
 
-			let deltaB = new Delta();
-			// Wrong base version - should be either 0 or 2, operation can't be based on an operation that is
-			// in the middle of other delta, because deltas are atomic, not dividable structures.
-			deltaB.addOperation( new NoOperation( 1 ) );
+	for ( let i = 0; i < 3; i++ ) {
+		delta.addOperation( new Operation( i + baseVersion ) );
+	}
 
-			expect( () => {
-				history.getTransformedDelta( deltaB );
-			} ).to.throw( CKEditorError, /history-wrong-version/ );
-		} );
-	} );
-} );
+	return delta;
+}