Переглянути джерело

Merge pull request #140 from ckeditor/t/132

Feature: Two–way data binding between Collection instances. Closes #132.
Piotr Jasiun 8 роки тому
батько
коміт
0db92f5079

+ 105 - 51
packages/ckeditor5-utils/src/collection.js

@@ -57,13 +57,36 @@ export default class Collection {
 		this._idProperty = options && options.idProperty || 'id';
 
 		/**
-		 * A helper mapping external items from bound collection ({@link #bindTo})
-		 * and actual items of the collection.
+		 * A helper mapping external items of a bound collection ({@link #bindTo})
+		 * and actual items of this collection. It provides information
+		 * necessary to properly remove items bound to another collection.
+		 *
+		 * See {@link #_bindToInternalToExternalMap}.
 		 *
 		 * @protected
-		 * @member {Map}
+		 * @member {WeakMap}
+		 */
+		this._bindToExternalToInternalMap = new WeakMap();
+
+		/**
+		 * A helper mapping items of this collection to external items of a bound collection
+		 * ({@link #bindTo}). It provides information necessary to manage the bindings, e.g.
+		 * to avoid loops in two–way bindings.
+		 *
+		 * See {@link #_bindToExternalToInternalMap}.
+		 *
+		 * @protected
+		 * @member {WeakMap}
+		 */
+		this._bindToInternalToExternalMap = new WeakMap();
+
+		/**
+		 * A collection instance this collection is bound to as a result
+		 * of calling {@link #bindTo} method.
+		 *
+		 * @protected
+		 * @member {module:utils/collection~Collection} #_bindToCollection
 		 */
-		this._boundItemsMap = new Map();
 	}
 
 	/**
@@ -226,6 +249,10 @@ export default class Collection {
 		this._items.splice( index, 1 );
 		this._itemMap.delete( id );
 
+		const externalItem = this._bindToInternalToExternalMap.get( item );
+		this._bindToInternalToExternalMap.delete( item );
+		this._bindToExternalToInternalMap.delete( externalItem );
+
 		this.fire( 'remove', item );
 
 		return item;
@@ -271,9 +298,15 @@ export default class Collection {
 	}
 
 	/**
-	 * Removes all items from the collection.
+	 * Removes all items from the collection and destroys the binding created using
+	 * {@link #bindTo}.
 	 */
 	clear() {
+		if ( this._bindToCollection ) {
+			this.stopListening( this._bindToCollection );
+			this._bindToCollection = null;
+		}
+
 		while ( this.length ) {
 			this.remove( 0 );
 		}
@@ -351,32 +384,24 @@ export default class Collection {
 	 *		console.log( target.get( 0 ).value ); // 'foo'
 	 *		console.log( target.get( 1 ).value ); // 'bar'
 	 *
+	 * **Note**: {@link #clear} can be used to break the binding.
+	 *
 	 * @param {module:utils/collection~Collection} collection A collection to be bound.
-	 * @returns {module:ui/viewcollection~ViewCollection#bindTo#using}
+	 * @returns {Object}
+	 * @returns {module:utils/collection~Collection#bindTo#as} return.as
+	 * @returns {module:utils/collection~Collection#bindTo#using} return.using
 	 */
-	bindTo( collection ) {
-		// Sets the actual binding using provided factory.
-		//
-		// @private
-		// @param {Function} factory A collection item factory returning collection items.
-		const bind = ( factory ) => {
-			// Load the initial content of the collection.
-			for ( let item of collection ) {
-				this.add( factory( item ) );
-			}
-
-			// Synchronize the with collection as new items are added.
-			this.listenTo( collection, 'add', ( evt, item, index ) => {
-				this.add( factory( item ), index );
-			} );
-
-			// Synchronize the with collection as new items are removed.
-			this.listenTo( collection, 'remove', ( evt, item ) => {
-				this.remove( this._boundItemsMap.get( item ) );
+	bindTo( externalCollection ) {
+		if ( this._bindToCollection ) {
+			/**
+			 * The collection cannot be bound more than once.
+			 *
+			 * @error collection-bind-to-rebind
+			 */
+			throw new CKEditorError( 'collection-bind-to-rebind: The collection cannot be bound more than once.' );
+		}
 
-				this._boundItemsMap.delete( item );
-			} );
-		};
+		this._bindToCollection = externalCollection;
 
 		return {
 			/**
@@ -386,13 +411,7 @@ export default class Collection {
 			 * @param {Function} Class Specifies which class factory is to be initialized.
 			 */
 			as: ( Class ) => {
-				bind( ( item ) => {
-					const instance = new Class( item );
-
-					this._boundItemsMap.set( item, instance );
-
-					return instance;
-				} );
+				this._setUpBindToBinding( item => new Class( item ) );
 			},
 
 			/**
@@ -404,29 +423,64 @@ export default class Collection {
 			 * the bound collection items.
 			 */
 			using: ( callbackOrProperty ) => {
-				let factory;
-
 				if ( typeof callbackOrProperty == 'function' ) {
-					factory = ( item ) => {
-						const instance = callbackOrProperty( item );
+					this._setUpBindToBinding( item => callbackOrProperty( item ) );
+				} else {
+					this._setUpBindToBinding( item => item[ callbackOrProperty ] );
+				}
+			}
+		};
+	}
+
+	/**
+	 * Finalizes and activates a binding initiated by {#bindTo}.
+	 *
+	 * @protected
+	 * @param {Function} factory A function which produces collection items.
+	 */
+	_setUpBindToBinding( factory ) {
+		const externalCollection = this._bindToCollection;
 
-						this._boundItemsMap.set( item, instance );
+		// Adds the item to the collection once a change has been done to the external collection.
+		//
+		// @private
+		const addItem = ( evt, externalItem, index ) => {
+			const isExternalBoundToThis = externalCollection._bindToCollection == this;
+			const externalItemBound = externalCollection._bindToInternalToExternalMap.get( externalItem );
+
+			// If an external collection is bound to this collection, which makes it a 2–way binding,
+			// and the particular external collection item is already bound, don't add it here.
+			// The external item has been created **out of this collection's item** and (re)adding it will
+			// cause a loop.
+			if ( isExternalBoundToThis && externalItemBound ) {
+				this._bindToExternalToInternalMap.set( externalItem, externalItemBound );
+				this._bindToInternalToExternalMap.set( externalItemBound, externalItem );
+			} else {
+				const item = factory( externalItem );
+
+				this._bindToExternalToInternalMap.set( externalItem, item );
+				this._bindToInternalToExternalMap.set( item, externalItem );
+
+				this.add( item, index );
+			}
+		};
 
-						return instance;
-					};
-				} else {
-					factory = ( item ) => {
-						const instance = item[ callbackOrProperty ];
+		// Load the initial content of the collection.
+		for ( let externalItem of externalCollection ) {
+			addItem( null, externalItem );
+		}
 
-						this._boundItemsMap.set( item, instance );
+		// Synchronize the with collection as new items are added.
+		this.listenTo( externalCollection, 'add', addItem );
 
-						return instance;
-					};
-				}
+		// Synchronize the with collection as new items are removed.
+		this.listenTo( externalCollection, 'remove', ( evt, externalItem ) => {
+			const item = this._bindToExternalToInternalMap.get( externalItem );
 
-				bind( factory );
+			if ( item ) {
+				this.remove( item );
 			}
-		};
+		} );
 	}
 
 	/**

+ 271 - 3
packages/ckeditor5-utils/tests/collection.js

@@ -206,7 +206,6 @@ describe( 'Collection', () => {
 		} );
 
 		it( 'should support an optional index argument', () => {
-			let collection = new Collection();
 			let item1 = getItem( 'foo' );
 			let item2 = getItem( 'bar' );
 			let item3 = getItem( 'baz' );
@@ -224,7 +223,6 @@ describe( 'Collection', () => {
 		} );
 
 		it( 'should throw when index argument is invalid', () => {
-			let collection = new Collection();
 			let item1 = getItem( 'foo' );
 			let item2 = getItem( 'bar' );
 			let item3 = getItem( 'baz' );
@@ -510,6 +508,24 @@ describe( 'Collection', () => {
 			expect( spy.callCount ).to.equal( 3 );
 			expect( collection.length ).to.equal( 0 );
 		} );
+
+		it( 'breaks the binding', () => {
+			const external = new Collection();
+			collection.bindTo( external ).using( i => i );
+
+			external.add( { foo: 'bar' } );
+			expect( collection ).to.have.length( 1 );
+
+			collection.clear();
+
+			external.add( { foo: 'baz' } );
+			expect( collection ).to.have.length( 0 );
+
+			external.remove( 0 );
+			expect( collection ).to.have.length( 0 );
+
+			expect( collection._bindToCollection ).to.be.null;
+		} );
 	} );
 
 	describe( 'bindTo()', () => {
@@ -519,6 +535,18 @@ describe( 'Collection', () => {
 			}
 		}
 
+		function assertItems( collection, expectedItems ) {
+			expect( collection.map( i => i.v ) ).to.deep.equal( expectedItems );
+		}
+
+		it( 'throws when binding more than once', () => {
+			collection.bindTo( {} );
+
+			expect( () => {
+				collection.bindTo( {} );
+			} ).to.throw( CKEditorError, /^collection-bind-to-rebind/ );
+		} );
+
 		it( 'provides "using()" and "as()" interfaces', () => {
 			const returned = collection.bindTo( {} );
 
@@ -527,6 +555,18 @@ describe( 'Collection', () => {
 			expect( returned.as ).to.be.a( 'function' );
 		} );
 
+		it( 'stores reference to bound collection', () => {
+			const collectionB = new Collection();
+
+			expect( collection._bindToCollection ).to.be.undefined;
+			expect( collectionB._bindToCollection ).to.be.undefined;
+
+			collection.bindTo( collectionB ).as( FactoryClass );
+
+			expect( collection._bindToCollection ).to.equal( collectionB );
+			expect( collectionB._bindToCollection ).to.be.undefined;
+		} );
+
 		describe( 'as()', () => {
 			let items;
 
@@ -602,7 +642,6 @@ describe( 'Collection', () => {
 
 			describe( 'callback', () => {
 				it( 'creates a binding (arrow function)', () => {
-					collection = new Collection();
 					collection.bindTo( items ).using( ( item ) => {
 						return new FactoryClass( item );
 					} );
@@ -728,6 +767,235 @@ describe( 'Collection', () => {
 				} );
 			} );
 		} );
+
+		describe( 'two–way data binding', () => {
+			it( 'works with custom factories (1)', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+
+				// A<--->B
+				collectionA.bindTo( collectionB ).using( i => ( { v: i.v * 2 } ) );
+				collectionB.bindTo( collectionA ).using( i => ( { v: i.v / 2 } ) );
+
+				assertItems( collectionA, [] );
+				assertItems( collectionB, [] );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 } );
+
+				assertItems( collectionA, [ 4, 6 ] );
+				assertItems( collectionB, [ 2, 3 ] );
+
+				collectionB.add( { v: 4 } );
+
+				assertItems( collectionA, [ 4, 6, 8 ] );
+				assertItems( collectionB, [ 2, 3, 4 ] );
+
+				sinon.assert.callCount( spyA, 3 );
+				sinon.assert.callCount( spyB, 3 );
+			} );
+
+			it( 'works with custom factories (2)', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+
+				// A<--->B
+				collectionA.bindTo( collectionB ).using( 'data' );
+				collectionB.bindTo( collectionA ).using( i => new FactoryClass( i ) );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 } );
+
+				expect( [ ...collectionB ].every( i => i instanceof FactoryClass ) ).to.be.true;
+				expect( [ ...collectionB ].map( i => i.data ) ).to.deep.equal( [ ...collectionA ] );
+				expect( collectionB.map( i => i.data.v ) ).to.deep.equal( [ 4, 6 ] );
+				expect( collectionA.map( i => i.v ) ).to.deep.equal( [ 4, 6 ] );
+
+				collectionB.add( new FactoryClass( { v: 8 } ) );
+
+				expect( [ ...collectionB ].every( i => i instanceof FactoryClass ) ).to.be.true;
+				expect( [ ...collectionB ].map( i => i.data ) ).to.deep.equal( [ ...collectionA ] );
+				expect( collectionB.map( i => i.data.v ) ).to.deep.equal( [ 4, 6, 8 ] );
+				expect( collectionA.map( i => i.v ) ).to.deep.equal( [ 4, 6, 8 ] );
+			} );
+
+			it( 'works with custom factories (custom index)', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+
+				// A<--->B
+				collectionA.bindTo( collectionB ).using( i => ( { v: i.v * 2 } ) );
+				collectionB.bindTo( collectionA ).using( i => ( { v: i.v / 2 } ) );
+
+				assertItems( collectionA, [] );
+				assertItems( collectionB, [] );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 }, 0 );
+
+				assertItems( collectionA, [ 6, 4 ] );
+				assertItems( collectionB, [ 3, 2 ] );
+
+				collectionB.add( { v: 4 }, 1 );
+
+				assertItems( collectionA, [ 6, 8, 4 ] );
+				assertItems( collectionB, [ 3, 4, 2 ] );
+
+				sinon.assert.callCount( spyA, 3 );
+				sinon.assert.callCount( spyB, 3 );
+			} );
+
+			it( 'works with 1:1 binding', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+
+				// A<--->B
+				collectionA.bindTo( collectionB ).using( i => i );
+				collectionB.bindTo( collectionA ).using( i => i );
+
+				assertItems( collectionA, [], [] );
+				assertItems( collectionB, [], [] );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 } );
+
+				assertItems( collectionA, [ 4, 6 ] );
+				assertItems( collectionB, [ 4, 6 ] );
+
+				collectionB.add( { v: 8 } );
+
+				assertItems( collectionA, [ 4, 6, 8 ] );
+				assertItems( collectionB, [ 4, 6, 8 ] );
+
+				expect( collectionA ).to.deep.equal( collectionB );
+
+				sinon.assert.callCount( spyA, 3 );
+				sinon.assert.callCount( spyB, 3 );
+			} );
+
+			it( 'works with double chaining', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+				const collectionC = new Collection();
+
+				const spyA = sinon.spy();
+				const spyB = sinon.spy();
+				const spyC = sinon.spy();
+
+				collectionA.on( 'add', spyA );
+				collectionB.on( 'add', spyB );
+				collectionC.on( 'add', spyC );
+
+				// A<--->B--->C
+				collectionA.bindTo( collectionB ).using( i => ( { v: i.v * 2 } ) );
+				collectionB.bindTo( collectionA ).using( i => ( { v: i.v / 2 } ) );
+				collectionC.bindTo( collectionB ).using( i => ( { v: -i.v } ) );
+
+				assertItems( collectionA, [] );
+				assertItems( collectionB, [] );
+				assertItems( collectionC, [] );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 } );
+
+				assertItems( collectionA, [ 4, 6 ] );
+				assertItems( collectionB, [ 2, 3 ] );
+				assertItems( collectionC, [ -2, -3 ] );
+
+				collectionB.add( { v: 4 } );
+
+				assertItems( collectionA, [ 4, 6, 8 ] );
+				assertItems( collectionB, [ 2, 3, 4 ] );
+				assertItems( collectionC, [ -2, -3, -4 ] );
+
+				collectionC.add( { v: -1000 } );
+
+				assertItems( collectionA, [ 4, 6, 8 ] );
+				assertItems( collectionB, [ 2, 3, 4 ] );
+				assertItems( collectionC, [ -2, -3, -4, -1000 ] );
+
+				sinon.assert.callCount( spyA, 3 );
+				sinon.assert.callCount( spyB, 3 );
+				sinon.assert.callCount( spyC, 4 );
+			} );
+
+			it( 'removes items correctly', () => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+
+				const spyAddA = sinon.spy();
+				const spyAddB = sinon.spy();
+				const spyRemoveA = sinon.spy();
+				const spyRemoveB = sinon.spy();
+
+				collectionA.on( 'add', spyAddA );
+				collectionB.on( 'add', spyAddB );
+				collectionA.on( 'remove', spyRemoveA );
+				collectionB.on( 'remove', spyRemoveB );
+
+				// A<--->B
+				collectionA.bindTo( collectionB ).using( i => ( { v: i.v * 2 } ) );
+				collectionB.bindTo( collectionA ).using( i => ( { v: i.v / 2 } ) );
+
+				assertItems( collectionA, [], [] );
+				assertItems( collectionB, [], [] );
+
+				collectionA.add( { v: 4 } );
+				collectionA.add( { v: 6 } );
+
+				assertItems( collectionA, [ 4, 6 ] );
+				assertItems( collectionB, [ 2, 3 ] );
+
+				collectionB.add( { v: 4 } );
+
+				assertItems( collectionA, [ 4, 6, 8 ] );
+				assertItems( collectionB, [ 2, 3, 4 ] );
+
+				collectionB.remove( 0 );
+
+				assertItems( collectionA, [ 6, 8 ] );
+				assertItems( collectionB, [ 3, 4 ] );
+
+				sinon.assert.callCount( spyAddA, 3 );
+				sinon.assert.callCount( spyAddB, 3 );
+				sinon.assert.callCount( spyRemoveA, 1 );
+				sinon.assert.callCount( spyRemoveB, 1 );
+
+				collectionA.remove( 1 );
+
+				assertItems( collectionA, [ 6 ] );
+				assertItems( collectionB, [ 3 ] );
+
+				sinon.assert.callCount( spyAddA, 3 );
+				sinon.assert.callCount( spyAddB, 3 );
+				sinon.assert.callCount( spyRemoveA, 2 );
+				sinon.assert.callCount( spyRemoveB, 2 );
+			} );
+		} );
 	} );
 
 	describe( 'iterator', () => {