Browse Source

Rewritten PluginCollection using NamedCollection.

Piotrek Koszuliński 10 năm trước cách đây
mục cha
commit
adbb68a9bc

+ 5 - 5
packages/ckeditor5-engine/src/editor.js

@@ -100,23 +100,23 @@ CKEDITOR.define( [
 				return that.plugins.load( config.plugins );
 			}
 
-			function initPlugins() {
+			function initPlugins( loadedPlugins ) {
 				// Start with a resolved promise.
 				var promise = Promise.resolve();
 
 				// Chain it with promises that resolve with the init() call of every plugin.
-				for ( var i = 0; i < that.plugins.length; i++ ) {
-					promise = promise.then( callInit( i ) );
+				for ( var i = 0; i < loadedPlugins.length; i++ ) {
+					promise = promise.then( callInit( loadedPlugins[ i ] ) );
 				}
 
 				// Return the promise chain.
 				return promise;
 
-				function callInit( index ) {
+				function callInit( plugin ) {
 					return function() {
 						// Returns init(). If it is a promise, the next then() interation will be called only when it
 						// will be resolved, enabling asynchronous init().
-						return that.plugins.get( index ).init();
+						return plugin.init();
 					};
 				}
 			}

+ 14 - 45
packages/ckeditor5-engine/src/plugincollection.js

@@ -13,12 +13,12 @@
  */
 
 CKEDITOR.define( [
-	'collection',
+	'namedcollection',
 	'plugin',
 	'ckeditorerror',
 	'log'
-], function( Collection, Plugin, CKEditorError, log ) {
-	class PluginCollection extends Collection {
+], function( NamedCollection, Plugin, CKEditorError, log ) {
+	class PluginCollection extends NamedCollection {
 		/**
 		 * Creates an instance of the PluginCollection class, initializing it with a set of plugins.
 		 *
@@ -28,9 +28,6 @@ CKEDITOR.define( [
 			super();
 
 			this._editor = editor;
-
-			// The hash table used to store pointers to loaded plugins by name.
-			this._names = {};
 		}
 
 		/**
@@ -39,25 +36,31 @@ CKEDITOR.define( [
 		 * @param {String} plugins A comma separated list of plugin names to get loaded.
 		 * @returns {Promise} A promise which gets resolved once all plugins are loaded and available into the
 		 * collection.
+		 * @param {core/Plugin[]} returns.loadedPlugins The array of loaded plugins.
 		 */
 		load( plugins ) {
 			var that = this;
 
 			// The list of plugins which are being loaded (to avoid circular references issues).
 			var loading = {};
+			// Plugins added to the collection (for the purpose of returning an array of loaded plugins).
+			var loaded = [];
 
 			// It may happen that an empty list was passed – don't fail.
 			plugins = plugins ? plugins.split( ',' ) : [];
 
 			// Creates a promise for the loading of each plugin and returns a main promise that resolves when all are
 			// done.
-			return Promise.all( plugins.map( pluginPromise ) );
+			return Promise.all( plugins.map( pluginPromise ) )
+				.then( function() {
+					return loaded;
+				} );
 
 			// Returns a promise that will load the plugin and add it to the collection before resolving.
 			function pluginPromise( plugin ) {
 				return new Promise( function( resolve, reject ) {
 					// Do nothing if the plugin is already loaded (or if is being loaded right now).
-					if ( that._names[ plugin ] || loading[ plugin ] ) {
+					if ( that._models[ plugin ] || loading[ plugin ] ) {
 						return resolve();
 					}
 
@@ -100,6 +103,7 @@ CKEDITOR.define( [
 										// the collection. This guarantees that dependecies come first in the collection.
 										if ( !isPluginDep ) {
 											that.add( loadedPlugin );
+											loaded.push( loadedPlugin );
 										}
 									} )
 							);
@@ -137,41 +141,6 @@ CKEDITOR.define( [
 			}
 		}
 
-		/**
-		 * Adds a plugin to the collection.
-		 *
-		 * The `name` property must be set to the plugin object before passing it to this function. Adding plugins
-		 * with the same name has no effect and silently fails.
-		 *
-		 * @param {Plugin} plugin The plugin to be added.
-		 */
-		add( plugin ) {
-			// Do nothing if the plugin is already loaded.
-			if ( this._names[ plugin.name ] ) {
-				return;
-			}
-
-			// Save a pointer to the plugin by its name.
-			this._names[ plugin.name ] = plugin;
-
-			// Call the original implementation.
-			super.add.apply( this, arguments );
-		}
-
-		/**
-		 * Gets a plugin from the collection.
-		 *
-		 * @param {String} name The plugin name.
-		 * @returns {Plugin} The requested plugin, if available in the collection.
-		 */
-		get( name ) {
-			if ( typeof name != 'string' ) {
-				return super.get.apply( this, arguments );
-			}
-
-			return this._names[ name ];
-		}
-
 		/**
 		 * Executes the callback for each model in the collection.
 		 *
@@ -180,8 +149,8 @@ CKEDITOR.define( [
 		 * @param {String} callback.name
 		 */
 		forEach( callback ) {
-			for ( var name in this._names ) {
-				callback( this._names[ name ], name );
+			for ( var name in this._models ) {
+				callback( this._models[ name ], name );
 			}
 		}
 	}

+ 28 - 87
packages/ckeditor5-engine/tests/plugincollection/plugincollection.js

@@ -117,8 +117,8 @@ describe( 'load', function() {
 			.then( function() {
 				expect( plugins.length ).to.equal( 2 );
 
-				expect( plugins.get( 0 ) ).to.be.an.instanceof( PluginA );
-				expect( plugins.get( 1 ) ).to.be.an.instanceof( PluginB );
+				expect( plugins.get( 'A' ) ).to.be.an.instanceof( PluginA );
+				expect( plugins.get( 'B' ) ).to.be.an.instanceof( PluginB );
 			} );
 	} );
 
@@ -126,15 +126,14 @@ describe( 'load', function() {
 		var PluginCollection = modules.plugincollection;
 
 		var plugins = new PluginCollection( editor );
+		var spy = sinon.spy( plugins, 'add' );
 
 		return plugins.load( 'A,C' )
-			.then( function() {
+			.then( function( loadedPlugins ) {
 				expect( plugins.length ).to.equal( 3 );
 
-				// The order must have dependencies first.
-				expect( plugins.get( 0 ).name ).to.equal( 'A' );
-				expect( plugins.get( 1 ).name ).to.equal( 'B' );
-				expect( plugins.get( 2 ).name ).to.equal( 'C' );
+				expect( getPluginNamesFromSpy( spy ) ).to.deep.equal( [ 'A', 'B', 'C' ], 'order by plugins.add()' );
+				expect( getPluginNames( loadedPlugins ) ).to.deep.equal( [ 'A', 'B', 'C' ], 'order by returned value' );
 			} );
 	} );
 
@@ -142,15 +141,14 @@ describe( 'load', function() {
 		var PluginCollection = modules.plugincollection;
 
 		var plugins = new PluginCollection( editor );
+		var spy = sinon.spy( plugins, 'add' );
 
 		return plugins.load( 'A,B,C' )
-			.then( function() {
+			.then( function( loadedPlugins ) {
 				expect( plugins.length ).to.equal( 3 );
 
-				// The order must have dependencies first.
-				expect( plugins.get( 0 ).name ).to.equal( 'A' );
-				expect( plugins.get( 1 ).name ).to.equal( 'B' );
-				expect( plugins.get( 2 ).name ).to.equal( 'C' );
+				expect( getPluginNamesFromSpy( spy ) ).to.deep.equal( [ 'A', 'B', 'C' ], 'order by plugins.add()' );
+				expect( getPluginNames( loadedPlugins ) ).to.deep.equal( [ 'A', 'B', 'C' ], 'order by returned value' );
 			} );
 	} );
 
@@ -158,16 +156,15 @@ describe( 'load', function() {
 		var PluginCollection = modules.plugincollection;
 
 		var plugins = new PluginCollection( editor );
+		var spy = sinon.spy( plugins, 'add' );
 
 		return plugins.load( 'D' )
-			.then( function() {
+			.then( function( loadedPlugins ) {
 				expect( plugins.length ).to.equal( 4 );
 
 				// The order must have dependencies first.
-				expect( plugins.get( 0 ).name ).to.equal( 'A' );
-				expect( plugins.get( 1 ).name ).to.equal( 'B' );
-				expect( plugins.get( 2 ).name ).to.equal( 'C' );
-				expect( plugins.get( 3 ).name ).to.equal( 'D' );
+				expect( getPluginNamesFromSpy( spy ) ).to.deep.equal( [ 'A', 'B', 'C', 'D' ], 'order by plugins.add()' );
+				expect( getPluginNames( loadedPlugins ) ).to.deep.equal( [ 'A', 'B', 'C', 'D' ], 'order by returned value' );
 			} );
 	} );
 
@@ -175,15 +172,15 @@ describe( 'load', function() {
 		var PluginCollection = modules.plugincollection;
 
 		var plugins = new PluginCollection( editor );
+		var spy = sinon.spy( plugins, 'add' );
 
 		return plugins.load( 'A,E' )
-			.then( function() {
+			.then( function( loadedPlugins ) {
 				expect( plugins.length ).to.equal( 3 );
 
 				// The order must have dependencies first.
-				expect( plugins.get( 0 ).name ).to.equal( 'A' );
-				expect( plugins.get( 1 ).name ).to.equal( 'F' );
-				expect( plugins.get( 2 ).name ).to.equal( 'E' );
+				expect( getPluginNamesFromSpy( spy ) ).to.deep.equal( [ 'A', 'F', 'E' ], 'order by plugins.add()' );
+				expect( getPluginNames( loadedPlugins ) ).to.deep.equal( [ 'A', 'F', 'E' ], 'order by returned value' );
 			} );
 	} );
 
@@ -194,8 +191,8 @@ describe( 'load', function() {
 
 		return plugins.load( 'A,B' )
 			.then( function() {
-				expect( plugins.get( 0 ).editor ).to.equal( editor );
-				expect( plugins.get( 1 ).editor ).to.equal( editor );
+				expect( plugins.get( 'A' ).editor ).to.equal( editor );
+				expect( plugins.get( 'B' ).editor ).to.equal( editor );
 			} );
 	} );
 
@@ -322,70 +319,14 @@ describe( 'load', function() {
 	} );
 } );
 
-describe( 'add', function() {
-	it( 'should add plugins to the collection', function() {
-		var PluginCollection = modules.plugincollection;
-
-		var plugins = new PluginCollection( editor );
-
-		var pluginA = new PluginA();
-		var pluginB = new PluginB();
-
-		// `add()` requires the `name` property to the defined.
-		pluginA.name = 'A';
-		pluginB.name = 'B';
-
-		plugins.add( pluginA );
-		plugins.add( pluginB );
-
-		expect( plugins.length ).to.equal( 2 );
-
-		expect( plugins.get( 0 ) ).to.be.an.instanceof( PluginA );
-		expect( plugins.get( 1 ) ).to.be.an.instanceof( PluginB );
-	} );
-
-	it( 'should do nothing if the plugin is already loaded', function() {
-		var PluginCollection = modules.plugincollection;
-
-		var plugins = new PluginCollection( editor );
-
-		return plugins.load( 'A,B' )
-			.then( function() {
-				// Check length before `add()`.
-				expect( plugins.length ).to.equal( 2 );
-
-				var pluginA = new PluginA();
-				pluginA.name = 'A';
-
-				plugins.add( pluginA );
-
-				// Length should not change after `add()`.
-				expect( plugins.length ).to.equal( 2 );
-			} );
-	} );
-} );
-
-describe( 'get', function() {
-	it( 'should get a plugin by name', function() {
-		var PluginCollection = modules.plugincollection;
-
-		var plugins = new PluginCollection( editor );
-
-		return plugins.load( 'A,B' )
-			.then( function() {
-				expect( plugins.get( 'A' ) ).to.be.an.instanceof( PluginA );
-				expect( plugins.get( 'B' ) ).to.be.an.instanceof( PluginB );
-			} );
+function getPluginNamesFromSpy( addSpy ) {
+	return addSpy.args.map( function( arg ) {
+		return arg[ 0 ].name;
 	} );
+}
 
-	it( 'should return undefined for non existing plugin', function() {
-		var PluginCollection = modules.plugincollection;
-
-		var plugins = new PluginCollection( editor );
-
-		return plugins.load( 'A,B' )
-			.then( function() {
-				expect( plugins.get( 'C' ) ).to.be.an( 'undefined' );
-			} );
+function getPluginNames( plugins ) {
+	return plugins.map( function( arg ) {
+		return arg.name;
 	} );
-} );
+}