Răsfoiți Sursa

Changed: Delta is added to a batch before applying it's operations.

Szymon Cofalik 9 ani în urmă
părinte
comite
0db15e373a

+ 3 - 3
packages/ckeditor5-engine/src/treemodel/batch.js

@@ -84,6 +84,9 @@ export default class Batch {
  *			// You can use a class inherit from Delta if that class should handle OT in the special way.
  *			const delta = new Delta();
  *
+ *			// Add delta to the Batch instance. It is important to add delta to batch before applying any operation.
+ *			this.addDelta( delta );
+ *
  *			// Create operations which should be components of this delta.
  *			const operation = new InsertOperation( position, nodes, this.doc.version );
  *
@@ -93,9 +96,6 @@ export default class Batch {
  *			// Remember to apply every operation, no magic, you need to do it manually.
  *			this.doc.applyOperation( operation );
  *
- *			// Add delta to the Batch instance.
- *			this.addDelta( delta );
- *
  *			// Make this method chainable.
  *			return this;
  *		} );

+ 6 - 13
packages/ckeditor5-engine/src/treemodel/delta/attributedelta.js

@@ -119,22 +119,19 @@ register( 'removeAttr', function( key, nodeOrRange ) {
 } );
 
 function attribute( batch, key, value, nodeOrRange ) {
-	let delta;
-
 	if ( nodeOrRange instanceof Range ) {
-		delta = changeRange( batch.doc, key, value, nodeOrRange );
+		changeRange( batch, batch.doc, key, value, nodeOrRange );
 	} else {
-		delta = changeNode( batch.doc, key, value, nodeOrRange );
+		changeNode( batch, batch.doc, key, value, nodeOrRange );
 	}
-
-	batch.addDelta( delta );
 }
 
-function changeNode( doc, key, value, node ) {
+function changeNode( batch, doc, key, value, node ) {
 	const previousValue = node.getAttribute( key );
 	let range, operation;
 
 	const delta = node instanceof RootElement ? new RootAttributeDelta() : new AttributeDelta();
+	batch.addDelta( delta );
 
 	if ( previousValue != value ) {
 		if ( node instanceof RootElement ) {
@@ -157,15 +154,13 @@ function changeNode( doc, key, value, node ) {
 		delta.addOperation( operation );
 		doc.applyOperation( operation );
 	}
-
-	// It is expected that this method returns a delta.
-	return delta;
 }
 
 // Because attribute operation needs to have the same attribute value on the whole range, this function split the range
 // into smaller parts.
-function changeRange( doc, attributeKey, attributeValue, range ) {
+function changeRange( batch, doc, attributeKey, attributeValue, range ) {
 	const delta = new AttributeDelta();
+	batch.addDelta( delta );
 
 	// Position of the last split, the beginning of the new range.
 	let lastSplitPosition = range.start;
@@ -209,6 +204,4 @@ function changeRange( doc, attributeKey, attributeValue, range ) {
 		delta.addOperation( operation );
 		doc.applyOperation( operation );
 	}
-
-	return delta;
 }

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/insertdelta.js

@@ -73,10 +73,9 @@ register( 'insert', function( position, nodes ) {
 	const delta = new InsertDelta();
 	const insert = new InsertOperation( position, nodes, this.doc.version );
 
+	this.addDelta( delta );
 	delta.addOperation( insert );
 	this.doc.applyOperation( insert );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 2 - 2
packages/ckeditor5-engine/src/treemodel/delta/mergedelta.js

@@ -66,6 +66,8 @@ export default class MergeDelta extends Delta {
  */
 register( 'merge', function( position ) {
 	const delta = new MergeDelta();
+	this.addDelta( delta );
+
 	const nodeBefore = position.nodeBefore;
 	const nodeAfter = position.nodeAfter;
 
@@ -101,7 +103,5 @@ register( 'merge', function( position ) {
 	delta.addOperation( remove );
 	this.doc.applyOperation( remove );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/movedelta.js

@@ -91,6 +91,7 @@ function addMoveOperation( batch, delta, sourcePosition, howMany, targetPosition
  */
 register( 'move', function( nodeOrRange, targetPosition ) {
 	const delta = new MoveDelta();
+	this.addDelta( delta );
 
 	if ( nodeOrRange instanceof Range ) {
 		if ( !nodeOrRange.isFlat ) {
@@ -107,7 +108,5 @@ register( 'move', function( nodeOrRange, targetPosition ) {
 		addMoveOperation( this, delta, Position.createBefore( nodeOrRange ), 1, targetPosition );
 	}
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/removedelta.js

@@ -35,6 +35,7 @@ function addRemoveOperation( batch, delta, position, howMany ) {
  */
 register( 'remove', function( nodeOrRange ) {
 	const delta = new RemoveDelta();
+	this.addDelta( delta );
 
 	if ( nodeOrRange instanceof Range ) {
 		// The array is reversed, so the ranges are correct and do not have to be updated.
@@ -47,7 +48,5 @@ register( 'remove', function( nodeOrRange ) {
 		addRemoveOperation( this, delta, Position.createBefore( nodeOrRange ), 1 );
 	}
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/splitdelta.js

@@ -83,6 +83,7 @@ export default class SplitDelta extends Delta {
  */
 register( 'split', function( position ) {
 	const delta = new SplitDelta();
+	this.addDelta( delta );
 
 	const splitElement = position.parent;
 
@@ -113,7 +114,5 @@ register( 'split', function( position ) {
 	delta.addOperation( move );
 	this.doc.applyOperation( move );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/unwrapdelta.js

@@ -73,6 +73,7 @@ register( 'unwrap', function( element ) {
 	}
 
 	const delta = new UnwrapDelta();
+	this.addDelta( delta );
 
 	let sourcePosition = Position.createFromParentAndOffset( element, 0 );
 
@@ -87,7 +88,5 @@ register( 'unwrap', function( element ) {
 	delta.addOperation( remove );
 	this.doc.applyOperation( remove );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/weakinsertdelta.js

@@ -37,6 +37,7 @@ export default class WeakInsertDelta extends InsertDelta {}
  */
 register( 'weakInsert', function( position, nodes ) {
 	const delta = new WeakInsertDelta();
+	this.addDelta( delta );
 
 	nodes = new NodeList( nodes );
 
@@ -48,7 +49,5 @@ register( 'weakInsert', function( position, nodes ) {
 	delta.addOperation( operation );
 	this.doc.applyOperation( operation );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 1 - 2
packages/ckeditor5-engine/src/treemodel/delta/wrapdelta.js

@@ -119,6 +119,7 @@ register( 'wrap', function( range, elementOrString ) {
 	}
 
 	const delta = new WrapDelta();
+	this.addDelta( delta );
 
 	let insert = new InsertOperation( range.end, element, this.doc.version );
 	delta.addOperation( insert );
@@ -129,7 +130,5 @@ register( 'wrap', function( range, elementOrString ) {
 	delta.addOperation( move );
 	this.doc.applyOperation( move );
 
-	this.addDelta( delta );
-
 	return this;
 } );

+ 32 - 0
packages/ckeditor5-engine/tests/treemodel/delta/attributedelta.js

@@ -27,6 +27,10 @@ beforeEach( () => {
 describe( 'Batch', () => {
 	let batch;
 
+	const correctDeltaMatcher = sinon.match( ( operation ) => {
+		return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+	} );
+
 	beforeEach( () => {
 		batch = doc.batch();
 	} );
@@ -88,6 +92,13 @@ describe( 'Batch', () => {
 				const chain = batch.setAttr( 'b', 2, node );
 				expect( chain ).to.equal( batch );
 			} );
+
+			it( 'should add delta to batch and operation to delta before applying operation', () => {
+				sinon.spy( doc, 'applyOperation' );
+				batch.setAttr( 'b', 2, node );
+
+				expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+			} );
 		} );
 
 		describe( 'removeAttr', () => {
@@ -112,6 +123,13 @@ describe( 'Batch', () => {
 				const chain = batch.removeAttr( 'a', node );
 				expect( chain ).to.equal( batch );
 			} );
+
+			it( 'should add delta to batch and operation to delta before applying operation', () => {
+				sinon.spy( doc, 'applyOperation' );
+				batch.removeAttr( 'a', node );
+
+				expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+			} );
 		} );
 	} );
 
@@ -251,6 +269,13 @@ describe( 'Batch', () => {
 				const chain = batch.setAttr( 'a', 3, getRange( 3, 6 ) );
 				expect( chain ).to.equal( batch );
 			} );
+
+			it( 'should add delta to batch and operation to delta before applying operation', () => {
+				sinon.spy( doc, 'applyOperation' );
+				batch.setAttr( 'a', 3, getRange( 3, 6 ) );
+
+				expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+			} );
 		} );
 
 		describe( 'removeAttr', () => {
@@ -331,6 +356,13 @@ describe( 'Batch', () => {
 				const chain = batch.removeAttr( 'a', getRange( 0, 2 ) );
 				expect( chain ).to.equal( batch );
 			} );
+
+			it( 'should add delta to batch and operation to delta before applying operation', () => {
+				sinon.spy( doc, 'applyOperation' );
+				batch.removeAttr( 'a', getRange( 0, 2 ) );
+
+				expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+			} );
 		} );
 	} );
 } );

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/insertdelta.js

@@ -43,6 +43,17 @@ describe( 'Batch', () => {
 		it( 'should be chainable', () => {
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			batch.insert( new Position( root, [ 2 ] ), [ p, ul ] );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );
 

+ 13 - 2
packages/ckeditor5-engine/tests/treemodel/delta/mergedelta.js

@@ -20,7 +20,7 @@ import RemoveOperation from '/ckeditor5/core/treemodel/operation/removeoperation
 import ReinsertOperation from '/ckeditor5/core/treemodel/operation/reinsertoperation.js';
 
 describe( 'Batch', () => {
-	let doc, root, p1, p2;
+	let doc, root, p1, p2, batch;
 
 	beforeEach( () => {
 		doc = new Document();
@@ -62,11 +62,22 @@ describe( 'Batch', () => {
 		} );
 
 		it( 'should be chainable', () => {
-			const batch = doc.batch();
+			batch = doc.batch();
 
 			const chain = batch.merge( new Position( root, [ 1 ] ) );
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			batch = doc.batch().merge( new Position( root, [ 1 ] ) );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );
 

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/movedelta.js

@@ -67,6 +67,17 @@ describe( 'Batch', () => {
 
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			batch.move( div, new Position( root, [ 2 ] ) );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );
 

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/removedelta.js

@@ -64,5 +64,16 @@ describe( 'Batch', () => {
 
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			batch.remove( div );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/splitdelta.js

@@ -88,6 +88,17 @@ describe( 'Batch', () => {
 			const chain = batch.split( new Position( root, [ 0, 3 ] ) );
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			const batch = doc.batch().split( new Position( root, [ 0, 3 ] ) );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );
 

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/unwrapdelta.js

@@ -56,6 +56,17 @@ describe( 'Batch', () => {
 			const chain = batch.unwrap( p );
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			const batch = doc.batch().unwrap( p );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );
 

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/weakinsertdelta.js

@@ -45,5 +45,16 @@ describe( 'Batch', () => {
 		it( 'should be chainable', () => {
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			batch.weakInsert( new Position( root, [ 2 ] ), 'xyz' );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );

+ 11 - 0
packages/ckeditor5-engine/tests/treemodel/delta/wrapdelta.js

@@ -92,6 +92,17 @@ describe( 'Batch', () => {
 			const chain = batch.wrap( range, 'p' );
 			expect( chain ).to.equal( batch );
 		} );
+
+		it( 'should add delta to batch and operation to delta before applying operation', () => {
+			sinon.spy( doc, 'applyOperation' );
+			const batch = doc.batch().wrap( range, 'p' );
+
+			const correctDeltaMatcher = sinon.match( ( operation ) => {
+				return operation.delta && operation.delta.batch && operation.delta.batch == batch;
+			} );
+
+			expect( doc.applyOperation.calledWith( correctDeltaMatcher ) ).to.be.true;
+		} );
 	} );
 } );