Browse Source

Change Node to use AttributeList.

Szymon Cofalik 10 years ago
parent
commit
7af4e5e320

+ 24 - 62
packages/ckeditor5-engine/src/treemodel/node.js

@@ -5,7 +5,11 @@
 
 'use strict';
 
-CKEDITOR.define( [ 'treemodel/attribute', 'utils', 'ckeditorerror' ], ( Attribute, utils, CKEditorError ) => {
+CKEDITOR.define( [
+	'treemodel/attributelist',
+	'utils',
+	'ckeditorerror'
+], ( AttributeList, utils, CKEditorError ) => {
 	/**
 	 * Abstract document tree node class.
 	 *
@@ -31,14 +35,13 @@ CKEDITOR.define( [ 'treemodel/attribute', 'utils', 'ckeditorerror' ], ( Attribut
 			this.parent = null;
 
 			/**
-			 * Attributes set.
-			 *
+			 * List of attributes set on this node.
 			 * Attributes of nodes attached to the document can be changed only be the {@link treeModel.operation.AttributeOperation}.
 			 *
 			 * @private
-			 * @property {Set} _attrs
+			 * @property {treeModel.AttributeList} _attrs
 			 */
-			this._attrs = new Set( attrs );
+			this._attrs = new AttributeList( attrs );
 		}
 
 		/**
@@ -101,30 +104,17 @@ CKEDITOR.define( [ 'treemodel/attribute', 'utils', 'ckeditorerror' ], ( Attribut
 		}
 
 		/**
-		 * Finds an attribute by a key.
-		 *
-		 * @param {String} attr The attribute key.
-		 * @returns {treeModel.Attribute} The found attribute.
+		 * @see {@link treeModel.AttributeList#getAttr}
 		 */
 		getAttr( key ) {
-			for ( let attr of this._attrs ) {
-				if ( attr.key == key ) {
-					return attr.value;
-				}
-			}
-
-			return null;
+			return this._attrs.getAttr( key );
 		}
 
 		/**
-		 * Returns attribute iterator. It can be use to create a new element with the same attributes:
-		 *
-		 *		const copy = new Element( element.name, element.getAttrs() );
-		 *
-		 * @returns {Iterable.<treeModel.Attribute>} Attribute iterator.
+		 * @see {@link treeModel.AttributeList#getAttrs}
 		 */
 		getAttrs() {
-			return this._attrs[ Symbol.iterator ]();
+			return this._attrs.getAttrs();
 		}
 
 		/**
@@ -173,59 +163,31 @@ CKEDITOR.define( [ 'treemodel/attribute', 'utils', 'ckeditorerror' ], ( Attribut
 		}
 
 		/**
-		 * Returns `true` if the node contains an attribute with the same key and value as given or the same key if the
-		 * given parameter is a string.
-		 *
-		 * @param {treeModel.Attribute|String} key An attribute or a key to compare.
-		 * @returns {Boolean} True if node contains given attribute or an attribute with the given key.
+		 * @see {@link treeModel.AttributeList#hasAttr}
 		 */
 		hasAttr( key ) {
-			let attr;
-
-			// Attribute.
-			if ( key instanceof Attribute ) {
-				for ( attr of this._attrs ) {
-					if ( attr.isEqual( key ) ) {
-						return true;
-					}
-				}
-			}
-			// Key.
-			else {
-				for ( attr of this._attrs ) {
-					if ( attr.key == key ) {
-						return true;
-					}
-				}
-			}
-
-			return false;
+			return this._attrs.hasAttr( key );
 		}
 
 		/**
-		 * Removes attribute from the list of attributes.
-		 *
-		 * @param {String} key The attribute key.
+		 * @see {@link treeModel.AttributeList#removeAttr}
 		 */
 		removeAttr( key ) {
-			for ( let attr of this._attrs ) {
-				if ( attr.key == key ) {
-					this._attrs.delete( attr );
-
-					return;
-				}
-			}
+			this._attrs.removeAttr( key );
 		}
 
 		/**
-		 * Sets a given attribute. If the attribute with the same key already exists it will be removed.
-		 *
-		 * @param {treeModel.Attribute} attr Attribute to set.
+		 * @see {@link treeModel.AttributeList#setAttr}
 		 */
 		setAttr( attr ) {
-			this.removeAttr( attr.key );
+			this._attrs.setAttr( attr );
+		}
 
-			this._attrs.add( attr );
+		/**
+		 * @see {@link treeModel.AttributeList#setAttrsTo}
+		 */
+		setAttrsTo( attrs ) {
+			this._attrs.setAttrsTo( attrs );
 		}
 
 		/**

+ 116 - 114
packages/ckeditor5-engine/tests/treemodel/node.js

@@ -15,21 +15,24 @@ const modules = bender.amd.require(
 	'treemodel/element',
 	'treemodel/character',
 	'treemodel/attribute',
+	'treemodel/attributelist',
 	'treemodel/nodelist',
 	'ckeditorerror'
 );
 
 describe( 'Node', () => {
-	let Element, Character, Attribute, NodeList, CKEditorError;
+	let Element, Character, Attribute, AttributeList, NodeList, CKEditorError;
 
 	let root;
 	let one, two, three;
-	let charB, charA, charR, img;
+	let charB, charA, charR, img, attrEle;
+	let attrFooBar;
 
 	before( () => {
 		Element = modules[ 'treemodel/element' ];
 		Character = modules[ 'treemodel/character' ];
 		Attribute = modules[ 'treemodel/attribute' ];
+		AttributeList = modules[ 'treemodel/attributelist' ];
 		NodeList = modules[ 'treemodel/nodelist' ];
 		CKEditorError = modules.ckeditorerror;
 
@@ -43,6 +46,12 @@ describe( 'Node', () => {
 		three = new Element( 'three' );
 
 		root = new Element( null, null, [ one, two, three ] );
+
+		attrFooBar = new Attribute( 'foo', 'bar' );
+	} );
+
+	beforeEach( () => {
+		attrEle = new Element( 'element' );
 	} );
 
 	describe( 'should have a correct property', () => {
@@ -100,7 +109,7 @@ describe( 'Node', () => {
 	} );
 
 	describe( 'constructor', () => {
-		it( 'should copy attributes, not pass by reference', () => {
+		it( 'should copy attributes list, not pass by reference', () => {
 			let attrs = [ new Attribute( 'attr', true ) ];
 			let foo = new Element( 'foo', attrs );
 			let bar = new Element( 'bar', attrs );
@@ -112,165 +121,158 @@ describe( 'Node', () => {
 		} );
 	} );
 
-	describe( 'getAttr', () => {
-		let fooAttr, element;
-
-		beforeEach( () => {
-			fooAttr = new Attribute( 'foo', true );
-			element = new Element( 'foo', [ fooAttr ] );
-		} );
+	it( 'should create proper JSON string using toJSON method', () => {
+		let b = new Character( 'b' );
+		let foo = new Element( 'foo', [], [ b ] );
 
-		it( 'should be possible to get attribute by key', () => {
-			expect( element.getAttr( 'foo' ) ).to.equal( fooAttr.value );
-		} );
+		let parsedFoo = JSON.parse( JSON.stringify( foo ) );
+		let parsedBar = JSON.parse( JSON.stringify( b ) );
 
-		it( 'should return null if attribute was not found by key', () => {
-			expect( element.getAttr( 'bar' ) ).to.be.null;
-		} );
+		expect( parsedFoo.parent ).to.equal( null );
+		expect( parsedBar.parent ).to.equal( 'foo' );
 	} );
 
-	describe( 'setAttr', () => {
-		it( 'should insert an attribute', () => {
-			let element = new Element( 'elem' );
-			let attr = new Attribute( 'foo', 'bar' );
+	describe( 'getIndex', () => {
+		it( 'should return null if the parent is null', () => {
+			expect( root.getIndex() ).to.be.null;
+		} );
 
-			element.setAttr( attr );
+		it( 'should return index in the parent', () => {
+			expect( one.getIndex() ).to.equal( 0 );
+			expect( two.getIndex() ).to.equal( 1 );
+			expect( three.getIndex() ).to.equal( 2 );
 
-			expect( getIteratorCount( element.getAttrs() ) ).to.equal( 1 );
-			expect( element.getAttr( attr.key ) ).to.equal( attr.value );
+			expect( charB.getIndex() ).to.equal( 0 );
+			expect( charA.getIndex() ).to.equal( 1 );
+			expect( img.getIndex() ).to.equal( 2 );
+			expect( charR.getIndex() ).to.equal( 3 );
 		} );
 
-		it( 'should overwrite attribute with the same key', () => {
-			let oldAttr = new Attribute( 'foo', 'bar' );
-			let newAttr = new Attribute( 'foo', 'bar' );
-			let element = new Element( 'elem', [ oldAttr ] );
+		it( 'should throw an error if parent does not contains element', () => {
+			let f = new Character( 'f' );
+			let bar = new Element( 'bar', [], [] );
 
-			element.setAttr( newAttr );
+			f.parent = bar;
 
-			expect( getIteratorCount( element.getAttrs() ) ).to.equal( 1 );
-			expect( element.getAttr( newAttr.key ) ).to.equal( newAttr.value );
+			expect(
+				() => {
+					f.getIndex();
+				}
+			).to.throw( CKEditorError, /node-not-found-in-parent/ );
 		} );
 	} );
 
-	describe( 'removeAttr', () => {
-		it( 'should remove an attribute', () => {
-			let attrA = new Attribute( 'a', 'A' );
-			let attrB = new Attribute( 'b', 'b' );
-			let attrC = new Attribute( 'c', 'C' );
-			let element = new Element( 'elem', [ attrA, attrB, attrC ] );
+	describe( 'getPath', () => {
+		it( 'should return proper path', () => {
+			expect( root.getPath() ).to.deep.equal( [] );
 
-			element.removeAttr( attrB.key );
+			expect( one.getPath() ).to.deep.equal( [ 0 ] );
+			expect( two.getPath() ).to.deep.equal( [ 1 ] );
+			expect( three.getPath() ).to.deep.equal( [ 2 ] );
 
-			expect( getIteratorCount( element.getAttrs() ) ).to.equal( 2 );
-			expect( element.getAttr( attrA.key ) ).to.equal( attrA.value );
-			expect( element.getAttr( attrC.key ) ).to.equal( attrC.value );
-			expect( element.getAttr( attrB.key ) ).to.be.null;
+			expect( charB.getPath() ).to.deep.equal( [ 1, 0 ] );
+			expect( charA.getPath() ).to.deep.equal( [ 1, 1 ] );
+			expect( img.getPath() ).to.deep.equal( [ 1, 2 ] );
+			expect( charR.getPath() ).to.deep.equal( [ 1, 3 ] );
 		} );
 	} );
 
-	describe( 'hasAttr', () => {
-		it( 'should check attribute by key', () => {
-			let fooAttr = new Attribute( 'foo', true );
-			let element = new Element( 'foo', [ fooAttr ] );
-
-			expect( element.hasAttr( 'foo' ) ).to.be.true;
-		} );
+	// Testing integration with attributes list.
+	// Tests copied from AttributeList tests.
+	// Some cases were omitted.
 
-		it( 'should return false if attribute was not found by key', () => {
-			let fooAttr = new Attribute( 'foo', true );
-			let element = new Element( 'foo', [ fooAttr ] );
+	describe( 'setAttr', () => {
+		it( 'should insert an attribute', () => {
+			attrEle.setAttr( attrFooBar );
 
-			expect( element.hasAttr( 'bar' ) ).to.be.false;
+			expect( getIteratorCount( attrEle.getAttrs() ) ).to.equal( 1 );
+			expect( attrEle.getAttr( attrFooBar.key ) ).to.equal( attrFooBar.value );
 		} );
+	} );
 
-		it( 'should check attribute by object', () => {
-			let fooAttr = new Attribute( 'foo', true );
-			let foo2Attr = new Attribute( 'foo', true );
-			let element = new Element( 'foo', [ fooAttr ] );
+	describe( 'setAttrsTo', () => {
+		it( 'should remove all attributes and set passed ones', () => {
+			attrEle.setAttr( attrFooBar );
 
-			expect( element.hasAttr( foo2Attr ) ).to.be.true;
-		} );
+			let attrs = [ new Attribute( 'abc', true ), new Attribute( 'xyz', false ) ];
 
-		it( 'should return false if attribute was not found by object', () => {
-			let fooAttr = new Attribute( 'foo', true );
-			let element = new Element( 'foo' );
+			attrEle.setAttrsTo( attrs );
 
-			expect( element.hasAttr( fooAttr ) ).to.be.false;
+			expect( getIteratorCount( attrEle.getAttrs() ) ).to.equal( 2 );
+			expect( attrEle.getAttr( 'foo' ) ).to.be.null;
+			expect( attrEle.getAttr( 'abc' ) ).to.be.true;
+			expect( attrEle.getAttr( 'xyz' ) ).to.be.false;
 		} );
+	} );
 
-		it( 'should create proper JSON string using toJSON method', () => {
-			let b = new Character( 'b' );
-			let foo = new Element( 'foo', [], [ b ] );
+	describe( 'getAttr', () => {
+		beforeEach( () => {
+			attrEle = new Element( 'e', [ attrFooBar ] );
+		} );
 
-			let parsedFoo = JSON.parse( JSON.stringify( foo ) );
-			let parsedBar = JSON.parse( JSON.stringify( b ) );
+		it( 'should return attribute value if key of previously set attribute has been passed', () => {
+			expect( attrEle.getAttr( 'foo' ) ).to.equal( attrFooBar.value );
+		} );
 
-			expect( parsedFoo.parent ).to.equal( null );
-			expect( parsedBar.parent ).to.equal( 'foo' );
+		it( 'should return null if attribute with given key has not been found', () => {
+			expect( attrEle.getAttr( 'bar' ) ).to.be.null;
 		} );
 	} );
 
-	describe( 'getAttrs', () => {
-		it( 'should allows to get attribute count', () => {
-			let element = new Element( 'foo', [
-				new Attribute( 1, true ),
-				new Attribute( 2, true ),
-				new Attribute( 3, true )
-			] );
+	describe( 'removeAttr', () => {
+		it( 'should remove an attribute', () => {
+			let attrA = new Attribute( 'a', 'A' );
+			let attrB = new Attribute( 'b', 'B' );
+			let attrC = new Attribute( 'c', 'C' );
 
-			expect( getIteratorCount( element.getAttrs() ) ).to.equal( 3 );
-		} );
+			attrEle.setAttr( attrA );
+			attrEle.setAttr( attrB );
+			attrEle.setAttr( attrC );
 
-		it( 'should allows to copy attributes', () => {
-			let element = new Element( 'foo', [ new Attribute( 'x', true ) ] );
-			let copy = new Element( 'bar', element.getAttrs() );
+			attrEle.removeAttr( attrB.key );
 
-			expect( copy.getAttr( 'x' ) ).to.be.true;
+			expect( getIteratorCount( attrEle.getAttrs() ) ).to.equal( 2 );
+			expect( attrEle.getAttr( attrA.key ) ).to.equal( attrA.value );
+			expect( attrEle.getAttr( attrC.key ) ).to.equal( attrC.value );
+			expect( attrEle.getAttr( attrB.key ) ).to.be.null;
 		} );
 	} );
 
-	describe( 'getIndex', () => {
-		it( 'should return null if the parent is null', () => {
-			expect( root.getIndex() ).to.be.null;
+	describe( 'hasAttr', () => {
+		it( 'should check attribute by key', () => {
+			attrEle.setAttr( attrFooBar );
+			expect( attrEle.hasAttr( 'foo' ) ).to.be.true;
 		} );
 
-		it( 'should return index in the parent', () => {
-			expect( one.getIndex() ).to.equal( 0 );
-			expect( two.getIndex() ).to.equal( 1 );
-			expect( three.getIndex() ).to.equal( 2 );
-
-			expect( charB.getIndex() ).to.equal( 0 );
-			expect( charA.getIndex() ).to.equal( 1 );
-			expect( img.getIndex() ).to.equal( 2 );
-			expect( charR.getIndex() ).to.equal( 3 );
+		it( 'should return false if attribute was not found by key', () => {
+			expect( attrEle.hasAttr( 'bar' ) ).to.be.false;
 		} );
 
-		it( 'should throw an error if parent does not contains element', () => {
-			let f = new Character( 'f' );
-			let bar = new Element( 'bar', [], [] );
-
-			f.parent = bar;
+		it( 'should check attribute by object', () => {
+			attrEle.setAttr( attrFooBar );
+			expect( attrEle.hasAttr( attrFooBar ) ).to.be.true;
+		} );
 
-			expect(
-				() => {
-					f.getIndex();
-				}
-			).to.throw( CKEditorError, /node-not-found-in-parent/ );
+		it( 'should return false if attribute was not found by object', () => {
+			expect( attrEle.hasAttr( attrFooBar ) ).to.be.false;
 		} );
 	} );
 
-	describe( 'getPath', () => {
-		it( 'should return proper path', () => {
-			expect( root.getPath() ).to.deep.equal( [] );
+	describe( 'getAttrs', () => {
+		it( 'should return all set attributes', () => {
+			let attrA = new Attribute( 'a', 'A' );
+			let attrB = new Attribute( 'b', 'B' );
+			let attrC = new Attribute( 'c', 'C' );
 
-			expect( one.getPath() ).to.deep.equal( [ 0 ] );
-			expect( two.getPath() ).to.deep.equal( [ 1 ] );
-			expect( three.getPath() ).to.deep.equal( [ 2 ] );
+			attrEle.setAttrsTo( [
+				attrA,
+				attrB,
+				attrC
+			] );
 
-			expect( charB.getPath() ).to.deep.equal( [ 1, 0 ] );
-			expect( charA.getPath() ).to.deep.equal( [ 1, 1 ] );
-			expect( img.getPath() ).to.deep.equal( [ 1, 2 ] );
-			expect( charR.getPath() ).to.deep.equal( [ 1, 3 ] );
+			attrEle.removeAttr( attrB.key );
+
+			expect( [ attrA, attrC ] ).to.deep.equal( Array.from( attrEle.getAttrs() ) );
 		} );
 	} );
 } );