浏览代码

Added: Extracted operation validation from `_execute()` method to `_validate()` method.

Szymon Cofalik 8 年之前
父节点
当前提交
9d7b60d438

+ 6 - 2
packages/ckeditor5-engine/src/model/operation/attributeoperation.js

@@ -114,8 +114,7 @@ export default class AttributeOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
-		// Validation.
+	_validate() {
 		for ( const item of this.range.getItems() ) {
 			if ( this.oldValue !== null && !isEqual( item.getAttribute( this.key ), this.oldValue ) ) {
 				/**
@@ -147,7 +146,12 @@ export default class AttributeOperation extends Operation {
 				);
 			}
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		// If value to set is same as old value, don't do anything.
 		if ( !isEqual( this.oldValue, this.newValue ) ) {
 			// Execution.

+ 6 - 1
packages/ckeditor5-engine/src/model/operation/detachoperation.js

@@ -62,7 +62,7 @@ export default class DetachOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
+	_validate() {
 		if ( this.sourcePosition.root.document ) {
 			/**
 			 * Cannot detach document node.
@@ -72,7 +72,12 @@ export default class DetachOperation extends Operation {
 			 */
 			throw new CKEditorError( 'detach-operation-on-document-node: Cannot detach document node.' );
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		const nodes = _remove( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ) );
 
 		return { nodes };

+ 19 - 0
packages/ckeditor5-engine/src/model/operation/insertoperation.js

@@ -14,6 +14,7 @@ import RemoveOperation from './removeoperation';
 import { _insert, _normalizeNodes } from './utils';
 import Text from '../text';
 import Element from '../element';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
  * Operation to insert one or more nodes at given position in the model.
@@ -86,6 +87,24 @@ export default class InsertOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
+	_validate() {
+		const targetElement = this.position.parent;
+
+		if ( !targetElement || targetElement.maxOffset < this.position.offset ) {
+			/**
+			 * Insertion position is invalid.
+			 *
+			 * @error insert-operation-position-invalid
+			 */
+			throw new CKEditorError(
+				'insert-operation-position-invalid: Insertion position is invalid.'
+			);
+		}
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	_execute() {
 		// What happens here is that we want original nodes be passed to writer because we want original nodes
 		// to be inserted to the model. But in InsertOperation, we want to keep those nodes as they were added

+ 6 - 1
packages/ckeditor5-engine/src/model/operation/moveoperation.js

@@ -131,7 +131,7 @@ export default class MoveOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
+	_validate() {
 		const sourceElement = this.sourcePosition.parent;
 		const targetElement = this.targetPosition.parent;
 		const sourceOffset = this.sourcePosition.offset;
@@ -183,7 +183,12 @@ export default class MoveOperation extends Operation {
 				}
 			}
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		const range = _move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
 
 		return {

+ 11 - 2
packages/ckeditor5-engine/src/model/operation/operation.js

@@ -73,8 +73,7 @@ export default class Operation {
 		 */
 
 		/**
-		 * Executes the operation - modifications described by the operation attributes
-		 * will be applied to the tree model.
+		 * Executes the operation - modifications described by the operation attributes will be applied to the tree model.
 		 *
 		 * @protected
 		 * @method #_execute
@@ -84,6 +83,16 @@ export default class Operation {
 	}
 
 	/**
+	 * Checks whether the operation's parameters are correct and the operation can be correctly executed. Throws
+	 * an error if operation is not valid.
+	 *
+	 * @protected
+	 * @method #_validate
+	 */
+	_validate() {
+	}
+
+	/**
 	 * Custom toJSON method to solve child-parent circular dependencies.
 	 *
 	 * @method #toJSON

+ 8 - 1
packages/ckeditor5-engine/src/model/operation/reinsertoperation.js

@@ -70,7 +70,9 @@ export default class ReinsertOperation extends MoveOperation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
+	_validate() {
+		super._validate();
+
 		if ( !this.sourcePosition.root.document ) {
 			throw new CKEditorError( 'reinsert-operation-on-detached-item: Cannot reinsert detached item.' );
 		}
@@ -78,7 +80,12 @@ export default class ReinsertOperation extends MoveOperation {
 		if ( !this.targetPosition.root.document ) {
 			throw new CKEditorError( 'reinsert-operation-to-detached-parent: Cannot reinsert item to detached parent.' );
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		return super._execute();
 	}
 

+ 8 - 1
packages/ckeditor5-engine/src/model/operation/removeoperation.js

@@ -52,7 +52,9 @@ export default class RemoveOperation extends MoveOperation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
+	_validate() {
+		super._validate();
+
 		if ( !this.sourcePosition.root.document ) {
 			/**
 			 * Item that is going to be removed needs to be a {@link module:engine/model/document~Document document} child.
@@ -63,7 +65,12 @@ export default class RemoveOperation extends MoveOperation {
 			 */
 			throw new CKEditorError( 'remove-operation-on-detached-item: Cannot remove detached item.' );
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		return super._execute();
 	}
 

+ 9 - 7
packages/ckeditor5-engine/src/model/operation/renameoperation.js

@@ -86,8 +86,7 @@ export default class RenameOperation extends Operation {
 	/**
 	 * @inheritDoc
 	 */
-	_execute() {
-		// Validation.
+	_validate() {
 		const element = this.position.nodeAfter;
 
 		if ( !( element instanceof Element ) ) {
@@ -109,12 +108,15 @@ export default class RenameOperation extends Operation {
 				'rename-operation-wrong-name: Element to change has different name than operation\'s old name.'
 			);
 		}
+	}
 
-		// If value to set is same as old value, don't do anything.
-		if ( element.name != this.newName ) {
-			// Execution.
-			element.name = this.newName;
-		}
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
+		const element = this.position.nodeAfter;
+
+		element.name = this.newName;
 
 		return { element, oldName: this.oldName };
 	}

+ 9 - 1
packages/ckeditor5-engine/src/model/operation/rootattributeoperation.js

@@ -105,7 +105,10 @@ export default class RootAttributeOperation extends Operation {
 		return new RootAttributeOperation( this.root, this.key, this.newValue, this.oldValue, this.baseVersion + 1 );
 	}
 
-	_execute() {
+	/**
+	 * @inheritDoc
+	 */
+	_validate() {
 		if ( this.oldValue !== null && this.root.getAttribute( this.key ) !== this.oldValue ) {
 			/**
 			 * The attribute which should be removed does not exists for the given node.
@@ -135,7 +138,12 @@ export default class RootAttributeOperation extends Operation {
 				{ root: this.root, key: this.key }
 			);
 		}
+	}
 
+	/**
+	 * @inheritDoc
+	 */
+	_execute() {
 		if ( this.newValue !== null ) {
 			this.root.setAttribute( this.key, this.newValue );
 		} else {

+ 2 - 1
packages/ckeditor5-engine/tests/model/model.js

@@ -311,7 +311,8 @@ describe( 'Model', () => {
 			const returnValue = { foo: 'bar' };
 
 			const operation = {
-				_execute: sinon.stub().returns( returnValue )
+				_execute: sinon.stub().returns( returnValue ),
+				_validate: () => true
 			};
 
 			model.applyOperation( operation );

+ 77 - 56
packages/ckeditor5-engine/tests/model/operation/attributeoperation.js

@@ -173,6 +173,25 @@ describe( 'AttributeOperation', () => {
 		expect( root.getChild( 0 ).getAttribute( 'bar' ) ).to.be.true;
 	} );
 
+	it( 'should work correctly if old and new value are same', () => {
+		root.insertChildren( 0, new Text( 'bar', { foo: 'bar' } ) );
+
+		model.applyOperation( wrapInDelta(
+			new AttributeOperation(
+				new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) ),
+				'foo',
+				'bar',
+				'bar',
+				doc.version
+			)
+		) );
+
+		expect( doc.version ).to.equal( 1 );
+		expect( root.childCount ).to.equal( 1 );
+		expect( count( root.getChild( 0 ).getAttributes() ) ).to.equal( 1 );
+		expect( root.getChild( 0 ).getAttribute( 'foo' ) ).to.equal( 'bar' );
+	} );
+
 	it( 'should remove attribute', () => {
 		root.insertChildren( 0, new Text( 'x', { foo: true, x: true, bar: true } ) );
 
@@ -193,20 +212,70 @@ describe( 'AttributeOperation', () => {
 		expect( root.getChild( 0 ).hasAttribute( 'bar' ) ).to.be.true;
 	} );
 
-	it( 'should not throw for non-primitive attribute values', () => {
-		root.insertChildren( 0, new Text( 'x', { foo: [ 'bar', 'xyz' ] } ) );
+	describe( '_validate()', () => {
+		it( 'should not throw for non-primitive attribute values', () => {
+			root.insertChildren( 0, new Text( 'x', { foo: [ 'bar', 'xyz' ] } ) );
 
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new AttributeOperation(
+			expect( () => {
+				const operation = new AttributeOperation(
 					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
 					'foo',
 					[ 'bar', 'xyz' ],
 					true,
 					doc.version
-				)
-			) );
-		} ).to.not.throw( Error );
+				);
+
+				operation._validate();
+			} ).to.not.throw( Error );
+		} );
+
+		it( 'should throw an error when one try to remove and the attribute does not exists', () => {
+			root.insertChildren( 0, new Text( 'x' ) );
+
+			expect( () => {
+				const operation = new AttributeOperation(
+					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
+					'foo',
+					true,
+					null,
+					doc.version
+				);
+
+				operation._validate();
+			} ).to.throw( CKEditorError, /attribute-operation-wrong-old-value/ );
+		} );
+
+		it( 'should throw an error when one try to insert and the attribute already exists', () => {
+			root.insertChildren( 0, new Text( 'x', { x: 1 } ) );
+
+			expect( () => {
+				const operation = new AttributeOperation(
+					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
+					'x',
+					null,
+					2,
+					doc.version
+				);
+
+				operation._validate();
+			} ).to.throw( CKEditorError, /attribute-operation-attribute-exists/ );
+		} );
+
+		it( 'should not throw when attribute value is the same', () => {
+			root.insertChildren( 0, new Text( 'x', { foo: true } ) );
+
+			expect( () => {
+				const operation = new AttributeOperation(
+					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
+					'foo',
+					true,
+					true,
+					doc.version
+				);
+
+				operation._validate();
+			} ).to.not.throw();
+		} );
 	} );
 
 	it( 'should create an AttributeOperation as a reverse', () => {
@@ -327,38 +396,6 @@ describe( 'AttributeOperation', () => {
 		expect( root.getChild( 0 ).getAttribute( 'foo' ) ).to.be.true;
 	} );
 
-	it( 'should throw an error when one try to remove and the attribute does not exists', () => {
-		root.insertChildren( 0, new Text( 'x' ) );
-
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new AttributeOperation(
-					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
-					'foo',
-					true,
-					null,
-					doc.version
-				)
-			) );
-		} ).to.throw( CKEditorError, /attribute-operation-wrong-old-value/ );
-	} );
-
-	it( 'should throw an error when one try to insert and the attribute already exists', () => {
-		root.insertChildren( 0, new Text( 'x', { x: 1 } ) );
-
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new AttributeOperation(
-					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
-					'x',
-					null,
-					2,
-					doc.version
-				)
-			) );
-		} ).to.throw( CKEditorError, /attribute-operation-attribute-exists/ );
-	} );
-
 	it( 'should create an AttributeOperation with the same parameters when cloned', () => {
 		const range = new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) );
 		const baseVersion = doc.version;
@@ -399,22 +436,6 @@ describe( 'AttributeOperation', () => {
 		expect( root.getChild( 1 ).data ).to.equal( 'bcxyz' );
 	} );
 
-	it( 'should do nothing when attribute value is the same', () => {
-		root.insertChildren( 0, new Text( 'x', { foo: true } ) );
-
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new AttributeOperation(
-					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
-					'foo',
-					true,
-					true,
-					doc.version
-				)
-			) );
-		} ).to.not.throw();
-	} );
-
 	describe( 'toJSON', () => {
 		it( 'should create proper serialized object', () => {
 			const range = new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) );

+ 10 - 8
packages/ckeditor5-engine/tests/model/operation/detachoperation.js

@@ -35,17 +35,19 @@ describe( 'DetachOperation', () => {
 		expect( docFrag.childCount ).to.equal( 0 );
 	} );
 
-	it( 'should throw when is executed on element from document', () => {
-		const root = doc.createRoot();
-		const element = new Element( 'element' );
+	describe( '_validate()', () => {
+		it( 'should throw when is executed on element from document', () => {
+			const root = doc.createRoot();
+			const element = new Element( 'element' );
 
-		root.appendChildren( [ element ] );
+			root.appendChildren( [ element ] );
 
-		const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
+			const op = new DetachOperation( Position.createBefore( element ), 1, doc.version );
 
-		expect( () => {
-			op._execute();
-		} ).to.throw( CKEditorError, /^detach-operation-on-document-node/ );
+			expect( () => {
+				op._validate();
+			} ).to.throw( CKEditorError, /^detach-operation-on-document-node/ );
+		} );
 	} );
 
 	it( 'should be not a document operation', () => {

+ 10 - 0
packages/ckeditor5-engine/tests/model/operation/insertoperation.js

@@ -11,6 +11,7 @@ import InsertOperation from '../../../src/model/operation/insertoperation';
 import RemoveOperation from '../../../src/model/operation/removeoperation';
 import Position from '../../../src/model/position';
 import Text from '../../../src/model/text';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import { jsonParseStringify, wrapInDelta } from '../../../tests/model/_utils/utils';
 
 describe( 'InsertOperation', () => {
@@ -232,6 +233,15 @@ describe( 'InsertOperation', () => {
 		} );
 	} );
 
+	describe( '_validate()', () => {
+		it( 'should throw an error if target position does not exist', () => {
+			const element = new Element( 'p' );
+			const op = new InsertOperation( new Position( root, [ 4 ] ), element, doc.version );
+
+			expect( () => op._validate() ).to.throw( CKEditorError, /insert-operation-position-invalid/ );
+		} );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const position = new Position( root, [ 0 ] );

+ 68 - 78
packages/ckeditor5-engine/tests/model/operation/moveoperation.js

@@ -151,102 +151,92 @@ describe( 'MoveOperation', () => {
 		expect( p2.maxOffset ).to.equal( 0 );
 	} );
 
-	it( 'should throw an error if number of nodes to move exceeds the number of existing nodes in given element', () => {
-		root.insertChildren( 0, new Text( 'xbarx' ) );
+	describe( '_validate()', () => {
+		it( 'should throw an error if number of nodes to move exceeds the number of existing nodes in given element', () => {
+			root.insertChildren( 0, new Text( 'xbarx' ) );
 
-		const operation = new MoveOperation(
-			new Position( root, [ 3 ] ),
-			3,
-			new Position( root, [ 1 ] ),
-			doc.version
-		);
+			const operation = new MoveOperation(
+				new Position( root, [ 3 ] ),
+				3,
+				new Position( root, [ 1 ] ),
+				doc.version
+			);
 
-		expect( () => model.applyOperation( wrapInDelta( operation ) ) ).to.throw( CKEditorError, /move-operation-nodes-do-not-exist/ );
-	} );
+			expect( () => operation._validate() ).to.throw( CKEditorError, /move-operation-nodes-do-not-exist/ );
+		} );
 
-	it( 'should throw an error if target or source parent-element specified by position does not exist', () => {
-		const p = new Element( 'p' );
-		p.insertChildren( 0, new Text( 'foo' ) );
-		root.insertChildren( 0, [ new Text( 'ab' ), p ] );
+		it( 'should throw an error if target or source parent-element specified by position does not exist', () => {
+			const p = new Element( 'p' );
+			p.insertChildren( 0, new Text( 'foo' ) );
+			root.insertChildren( 0, [ new Text( 'ab' ), p ] );
 
-		const operation = new MoveOperation(
-			new Position( root, [ 2, 0 ] ),
-			3,
-			new Position( root, [ 1 ] ),
-			doc.version
-		);
+			const operation = new MoveOperation(
+				new Position( root, [ 2, 0 ] ),
+				3,
+				new Position( root, [ 1 ] ),
+				doc.version
+			);
 
-		root.removeChildren( 1 );
+			root.removeChildren( 1 );
 
-		expect( () => model.applyOperation( wrapInDelta( operation ) ) ).to.throw( CKEditorError, /move-operation-position-invalid/ );
-	} );
+			expect( () => operation._validate() ).to.throw( CKEditorError, /move-operation-position-invalid/ );
+		} );
 
-	it( 'should throw an error if operation tries to move a range between the beginning and the end of that range', () => {
-		root.insertChildren( 0, new Text( 'xbarx' ) );
+		it( 'should throw an error if operation tries to move a range between the beginning and the end of that range', () => {
+			root.insertChildren( 0, new Text( 'xbarx' ) );
 
-		const operation = new MoveOperation(
-			new Position( root, [ 1 ] ),
-			3,
-			new Position( root, [ 2 ] ),
-			doc.version
-		);
+			const operation = new MoveOperation(
+				new Position( root, [ 1 ] ),
+				3,
+				new Position( root, [ 2 ] ),
+				doc.version
+			);
 
-		expect( () => model.applyOperation( wrapInDelta( operation ) ) ).to.throw( CKEditorError, /move-operation-range-into-itself/ );
-	} );
+			expect( () => operation._validate() ).to.throw( CKEditorError, /move-operation-range-into-itself/ );
+		} );
 
-	it( 'should throw an error if operation tries to move a range into a sub-tree of a node that is in that range', () => {
-		const p = new Element( 'p', [], [ new Element( 'p' ) ] );
-		root.insertChildren( 0, [ new Text( 'ab' ), p, new Text( 'xy' ) ] );
+		it( 'should throw an error if operation tries to move a range into a sub-tree of a node that is in that range', () => {
+			const p = new Element( 'p', [], [ new Element( 'p' ) ] );
+			root.insertChildren( 0, [ new Text( 'ab' ), p, new Text( 'xy' ) ] );
 
-		const operation = new MoveOperation(
-			new Position( root, [ 1 ] ),
-			3,
-			new Position( root, [ 2, 0, 0 ] ),
-			doc.version
-		);
-
-		expect( () => model.applyOperation( wrapInDelta( operation ) ) ).to.throw( CKEditorError, /move-operation-node-into-itself/ );
-	} );
+			const operation = new MoveOperation(
+				new Position( root, [ 1 ] ),
+				3,
+				new Position( root, [ 2, 0, 0 ] ),
+				doc.version
+			);
 
-	it( 'should not throw an error if operation move a range into a sibling', () => {
-		const p = new Element( 'p' );
-		root.insertChildren( 0, [ new Text( 'ab' ), p, new Text( 'xy' ) ] );
+			expect( () => operation._validate() ).to.throw( CKEditorError, /move-operation-node-into-itself/ );
+		} );
 
-		const operation = new MoveOperation(
-			new Position( root, [ 1 ] ),
-			1,
-			new Position( root, [ 2, 0 ] ),
-			doc.version
-		);
+		it( 'should not throw an error if operation move a range into a sibling', () => {
+			const p = new Element( 'p' );
+			root.insertChildren( 0, [ new Text( 'ab' ), p, new Text( 'xy' ) ] );
 
-		expect(
-			() => {
-				model.applyOperation( wrapInDelta( operation ) );
-			}
-		).not.to.throw();
+			const operation = new MoveOperation(
+				new Position( root, [ 1 ] ),
+				1,
+				new Position( root, [ 2, 0 ] ),
+				doc.version
+			);
 
-		expect( root.maxOffset ).to.equal( 4 );
-		expect( p.maxOffset ).to.equal( 1 );
-		expect( p.getChild( 0 ).data ).to.equal( 'b' );
-	} );
+			expect( () => operation._validate() ).not.to.throw();
+		} );
 
-	it( 'should not throw when operation paths looks like incorrect but move is between different roots', () => {
-		const p = new Element( 'p' );
-		root.insertChildren( 0, [ new Text( 'a' ), p, new Text( 'b' ) ] );
-		doc.graveyard.insertChildren( 0, new Text( 'abc' ) );
+		it( 'should not throw when operation paths looks like incorrect but move is between different roots', () => {
+			const p = new Element( 'p' );
+			root.insertChildren( 0, [ new Text( 'a' ), p, new Text( 'b' ) ] );
+			doc.graveyard.insertChildren( 0, new Text( 'abc' ) );
 
-		const operation = new MoveOperation(
-			new Position( doc.graveyard, [ 0 ] ),
-			2,
-			new Position( root, [ 1, 0 ] ),
-			doc.version
-		);
+			const operation = new MoveOperation(
+				new Position( doc.graveyard, [ 0 ] ),
+				2,
+				new Position( root, [ 1, 0 ] ),
+				doc.version
+			);
 
-		expect(
-			() => {
-				model.applyOperation( wrapInDelta( operation ) );
-			}
-		).not.to.throw();
+			expect( () => operation._validate() ).not.to.throw();
+		} );
 	} );
 
 	it( 'should create MoveOperation with the same parameters when cloned', () => {

+ 29 - 25
packages/ckeditor5-engine/tests/model/operation/reinsertoperation.js

@@ -106,34 +106,38 @@ describe( 'ReinsertOperation', () => {
 		expect( operation.isDocumentOperation ).to.true;
 	} );
 
-	it( 'should throw when target position is not in the document', () => {
-		const docFrag = new DocumentFragment();
-
-		operation = new ReinsertOperation(
-			graveyardPosition,
-			1,
-			Position.createAt( docFrag ),
-			doc.version
-		);
-
-		expect( () => {
-			operation._execute();
-		} ).to.throw( CKEditorError, /^reinsert-operation-to-detached-parent/ );
-	} );
+	describe( '_validate()', () => {
+		it( 'should throw when target position is not in the document', () => {
+			const docFrag = new DocumentFragment();
+
+			graveyard.insertChildren( 0, new Text( 'xx' ) );
+
+			operation = new ReinsertOperation(
+				graveyardPosition,
+				1,
+				Position.createAt( docFrag ),
+				doc.version
+			);
+
+			expect( () => {
+				operation._validate();
+			} ).to.throw( CKEditorError, /^reinsert-operation-to-detached-parent/ );
+		} );
 
-	it( 'should throw when source position is not in the document', () => {
-		const docFrag = new DocumentFragment();
+		it( 'should throw when source position is not in the document', () => {
+			const docFrag = new DocumentFragment( new Text( 'xx' ) );
 
-		operation = new ReinsertOperation(
-			Position.createAt( docFrag ),
-			1,
-			rootPosition,
-			doc.version
-		);
+			operation = new ReinsertOperation(
+				Position.createAt( docFrag ),
+				1,
+				rootPosition,
+				doc.version
+			);
 
-		expect( () => {
-			operation._execute();
-		} ).to.throw( CKEditorError, /^reinsert-operation-on-detached-item/ );
+			expect( () => {
+				operation._validate();
+			} ).to.throw( CKEditorError, /^reinsert-operation-on-detached-item/ );
+		} );
 	} );
 
 	describe( 'toJSON', () => {

+ 15 - 13
packages/ckeditor5-engine/tests/model/operation/removeoperation.js

@@ -143,22 +143,24 @@ describe( 'RemoveOperation', () => {
 		expect( doc.graveyard.getChild( 2 ).name ).to.equal( 'y' );
 	} );
 
-	it( 'should throw when is executed on detached item', () => {
-		const docFrag = new DocumentFragment();
-		const item = new Element( 'foo' );
+	describe( '_validate()', () => {
+		it( 'should throw when is executed on detached item', () => {
+			const docFrag = new DocumentFragment();
+			const item = new Element( 'foo' );
 
-		docFrag.appendChildren( [ item ] );
+			docFrag.appendChildren( [ item ] );
 
-		const op = new RemoveOperation(
-			new Position( docFrag, [ 0 ] ),
-			1,
-			new Position( doc.graveyard, [ 0 ] ),
-			doc.version
-		);
+			const op = new RemoveOperation(
+				new Position( docFrag, [ 0 ] ),
+				1,
+				new Position( doc.graveyard, [ 0 ] ),
+				doc.version
+			);
 
-		expect( () => {
-			op._execute();
-		} ).to.throw( CKEditorError, /^remove-operation-on-detached-item/ );
+			expect( () => {
+				op._validate();
+			} ).to.throw( CKEditorError, /^remove-operation-on-detached-item/ );
+		} );
 	} );
 
 	it( 'should always be a document operation', () => {

+ 21 - 19
packages/ckeditor5-engine/tests/model/operation/renameoperation.js

@@ -64,20 +64,30 @@ describe( 'RenameOperation', () => {
 		expect( element.name ).to.equal( oldName );
 	} );
 
-	it( 'should throw an error if position is not before an element', () => {
-		const op = new RenameOperation( Position.createAt( root, 'end' ), oldName, newName, doc.version );
+	describe( '_validate()', () => {
+		it( 'should throw an error if position is not before an element', () => {
+			const op = new RenameOperation( Position.createAt( root, 'end' ), oldName, newName, doc.version );
 
-		expect( () => {
-			model.applyOperation( wrapInDelta( op ) );
-		} ).to.throw( CKEditorError, /rename-operation-wrong-position/ );
-	} );
+			expect( () => {
+				op._validate();
+			} ).to.throw( CKEditorError, /rename-operation-wrong-position/ );
+		} );
+
+		it( 'should throw an error if oldName is different than renamed element name', () => {
+			const op = new RenameOperation( position, 'foo', newName, doc.version );
 
-	it( 'should throw an error if oldName is different than renamed element name', () => {
-		const op = new RenameOperation( position, 'foo', newName, doc.version );
+			expect( () => {
+				op._validate();
+			} ).to.throw( CKEditorError, /rename-operation-wrong-name/ );
+		} );
 
-		expect( () => {
-			model.applyOperation( wrapInDelta( op ) );
-		} ).to.throw( CKEditorError, /rename-operation-wrong-name/ );
+		it( 'should not throw when new name is the same as previous', () => {
+			const op = new RenameOperation( position, oldName, oldName, doc.version );
+
+			expect( () => {
+				op._validate();
+			} ).to.not.throw();
+		} );
 	} );
 
 	it( 'should create a RenameOperation with the same parameters when cloned', () => {
@@ -94,14 +104,6 @@ describe( 'RenameOperation', () => {
 		expect( clone.newName ).to.equal( newName );
 	} );
 
-	it( 'should do nothing when new name is the same as previous', () => {
-		const op = new RenameOperation( position, oldName, oldName, doc.version );
-
-		expect( () => {
-			model.applyOperation( wrapInDelta( op ) );
-		} ).to.not.throw();
-	} );
-
 	describe( 'isDocumentOperation', () => {
 		it( 'should be true when target item is in the document', () => {
 			const op = new RenameOperation( position, oldName, newName, doc.version );

+ 19 - 17
packages/ckeditor5-engine/tests/model/operation/rootattributeoperation.js

@@ -203,34 +203,36 @@ describe( 'RootAttributeOperation', () => {
 		expect( root.getAttribute( 'foo' ) ).to.be.true;
 	} );
 
-	it( 'should throw an error when one try to remove and the attribute does not exists', () => {
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new RootAttributeOperation(
+	describe( '_validate()', () => {
+		it( 'should throw an error when one try to remove and the attribute does not exists', () => {
+			expect( () => {
+				const op = new RootAttributeOperation(
 					root,
 					'foo',
 					true,
 					null,
 					doc.version
-				)
-			) );
-		} ).to.throw( CKEditorError, /rootattribute-operation-wrong-old-value/ );
-	} );
+				);
+
+				op._validate();
+			} ).to.throw( CKEditorError, /rootattribute-operation-wrong-old-value/ );
+		} );
 
-	it( 'should throw an error when one try to insert and the attribute already exists', () => {
-		root.setAttribute( 'x', 1 );
+		it( 'should throw an error when one try to insert and the attribute already exists', () => {
+			root.setAttribute( 'x', 1 );
 
-		expect( () => {
-			model.applyOperation( wrapInDelta(
-				new RootAttributeOperation(
+			expect( () => {
+				const op = new RootAttributeOperation(
 					root,
 					'x',
 					null,
 					2,
 					doc.version
-				)
-			) );
-		} ).to.throw( CKEditorError, /rootattribute-operation-attribute-exists/ );
+				);
+
+				op._validate();
+			} ).to.throw( CKEditorError, /rootattribute-operation-attribute-exists/ );
+		} );
 	} );
 
 	it( 'should create a RootAttributeOperation with the same parameters when cloned', () => {
@@ -299,7 +301,7 @@ describe( 'RootAttributeOperation', () => {
 
 			expect( () => {
 				RootAttributeOperation.fromJSON( serialized, doc );
-			} ).to.throw( CKEditorError, /rootattribute-operation-fromjson-no-roo/ );
+			} ).to.throw( CKEditorError, /rootattribute-operation-fromjson-no-root/ );
 		} );
 	} );
 } );