Browse Source

Made editor throw when config.creator is not defined.

Piotrek Koszuliński 10 years ago
parent
commit
61e15643d1

+ 24 - 8
packages/ckeditor5-engine/src/editor.js

@@ -17,8 +17,9 @@ CKEDITOR.define( [
 	'editorconfig',
 	'plugincollection',
 	'creator',
-	'ckeditorerror'
-], function( Model, EditorConfig, PluginCollection, Creator, CKEditorError ) {
+	'ckeditorerror',
+	'utils'
+], function( Model, EditorConfig, PluginCollection, Creator, CKEditorError, utils ) {
 	class Editor extends Model {
 		/**
 		 * Creates a new instance of the Editor class.
@@ -48,14 +49,16 @@ CKEDITOR.define( [
 			 * global configurations available in {@link CKEDITOR.config} if configurations are not found in the
 			 * instance itself.
 			 *
-			 * @type {Config}
+			 * @readonly
+			 * @property {Config}
 			 */
 			this.config = new EditorConfig( config );
 
 			/**
 			 * The plugins loaded and in use by this editor instance.
 			 *
-			 * @type {PluginCollection}
+			 * @readonly
+			 * @property {PluginCollection}
 			 */
 			this.plugins = new PluginCollection( this );
 
@@ -69,9 +72,10 @@ CKEDITOR.define( [
 			/**
 			 * The list of detected creators.
 			 *
+			 * @property {Map}
 			 * @protected
 			 */
-			this._creators = {};
+			this._creators = new Map();
 		}
 
 		/**
@@ -116,19 +120,31 @@ CKEDITOR.define( [
 			function findCreators() {
 				that.plugins.forEach( ( plugin, name ) => {
 					if ( plugin instanceof Creator ) {
-						that._creators[ name ] = plugin;
+						that._creators.set( name, plugin );
 					}
 				} );
 			}
 
 			function fireCreator() {
 				// Take the name of the creator to use (config or any of the registered ones).
-				var creatorName = config.creator ? ( 'creator-' + config.creator ) : Object.keys( that._creators )[ 0 ];
+				var creatorName = config.creator && ( 'creator-' + config.creator );
 				var creator;
 
 				if ( creatorName ) {
 					// Take the registered class for the given creator name.
-					creator = that._creators[ creatorName ];
+					creator = that._creators.get( creatorName );
+				} else if ( that._creators.size > 1 ) {
+					/**
+					 * The `config.creator` option was not defined.
+					 *
+					 * This error is thrown when more than one creator is available and the configuration does
+					 * not specify which one to use.
+					 *
+					 * @error editor-undefined-creator
+					 */
+					throw new CKEditorError( 'editor-undefined-creator: The config.creator option was not defined.' );
+				} else {
+					creator = utils.nth( 0, that._creators.values() );
 				}
 
 				if ( !creator ) {

+ 18 - 0
packages/ckeditor5-engine/src/utils.js

@@ -91,6 +91,24 @@ CKEDITOR.define( [ 'utils-lodash', 'lib/lodash/lodash-ckeditor' ], function( lod
 				// Compared array is longer so it is a suffix of the other array.
 				return utils.compareArrays.EXTENSION;
 			}
+		},
+
+		/**
+		 * Returns `nth` (starts from `0` of course) item of an `iterable`.
+		 *
+		 * @param {Number} index
+		 * @param {Iterable.<*>} iterable
+		 * @returns {*}
+		 */
+		nth( index, iterable ) {
+			for ( let item of iterable ) {
+				if ( index === 0 ) {
+					return item;
+				}
+				index -= 1;
+			}
+
+			return null;
 		}
 	};
 

+ 12 - 9
packages/ckeditor5-engine/tests/editor/creator.js

@@ -26,8 +26,8 @@ bender.tools.createSinonSandbox();
 before( () => {
 	bender.tools.core.defineEditorCreatorMock( 'test1' );
 
-	bender.tools.core.defineEditorCreatorMock( 'test-any1' );
-	bender.tools.core.defineEditorCreatorMock( 'test-any2' );
+	bender.tools.core.defineEditorCreatorMock( 'test-throw-on-many1' );
+	bender.tools.core.defineEditorCreatorMock( 'test-throw-on-many2' );
 
 	bender.tools.core.defineEditorCreatorMock( 'test-config1' );
 	bender.tools.core.defineEditorCreatorMock( 'test-config2' );
@@ -84,15 +84,18 @@ describe( 'init', () => {
 			} );
 	} );
 
-	it( 'should instantiate any creator when more than one is available', () => {
+	it( 'should throw if more than one creator is available but config.creator is not defined', () => {
+		const CKEditorError = modules.ckeditorerror;
+
 		return initEditor( {
-				plugins: 'creator-test-any1,creator-test-any2'
+				plugins: 'creator-test-throw-on-many1,creator-test-throw-on-many2'
 			} )
 			.then( () => {
-				let creator1 = editor.plugins.get( 'creator-test-any1' );
-				let creator2 = editor.plugins.get( 'creator-test-any2' );
-
-				expect( creator1.create.called + creator2.create.called ).to.be.equal( 1, 'only one of the creators should be used' );
+				throw new Error( 'This should not be executed.' );
+			} )
+			.catch( ( err ) => {
+				expect( err ).to.be.instanceof( CKEditorError );
+				expect( err.message ).to.match( /^editor-undefined-creator:/ );
 			} );
 	} );
 
@@ -126,7 +129,7 @@ describe( 'init', () => {
 			} );
 	} );
 
-	it( 'should throw an error no creators are defined', () => {
+	it( 'should throw an error if no creators are defined', () => {
 		const CKEditorError = modules.ckeditorerror;
 
 		return initEditor( {} )

+ 80 - 52
packages/ckeditor5-engine/tests/utils/utils.js

@@ -5,30 +5,30 @@
 
 'use strict';
 
-var modules = bender.amd.require( 'utils', 'utils-lodash' );
+const modules = bender.amd.require( 'utils', 'utils-lodash' );
 
-describe( 'utils', function() {
-	var utils;
+describe( 'utils', () => {
+	let utils;
 
-	before( function() {
+	before( () => {
 		utils = modules.utils;
 	} );
 
-	describe( 'extend()', function() {
+	describe( 'extend()', () => {
 		// Properties of the subsequent objects should override properties of the preceding objects. This is critical for
 		// CKEditor so we keep this test to ensure that Lo-Dash (or whatever) implements it in the way we need it.
-		it( 'should extend by several params in the correct order', function() {
-			var target = {
+		it( 'should extend by several params in the correct order', () => {
+			let target = {
 				a: 0,
 				b: 0
 			};
 
-			var ext1 = {
+			let ext1 = {
 				b: 1,
 				c: 1
 			};
 
-			var ext2 = {
+			let ext2 = {
 				c: 2,
 				d: 2
 			};
@@ -42,16 +42,16 @@ describe( 'utils', function() {
 		} );
 	} );
 
-	describe( 'spy', function() {
-		it( 'should not have `called` after creation', function() {
-			var spy = utils.spy();
+	describe( 'spy', () => {
+		it( 'should not have `called` after creation', () => {
+			let spy = utils.spy();
 
 			expect( spy.called ).to.not.be.true();
 		} );
 
-		it( 'should register calls', function() {
-			var fn1 = utils.spy();
-			var fn2 = utils.spy();
+		it( 'should register calls', () => {
+			let fn1 = utils.spy();
+			let fn2 = utils.spy();
 
 			fn1();
 
@@ -60,11 +60,11 @@ describe( 'utils', function() {
 		} );
 	} );
 
-	describe( 'uid', function() {
-		it( 'should return different ids', function() {
-			var id1 = utils.uid();
-			var id2 = utils.uid();
-			var id3 = utils.uid();
+	describe( 'uid', () => {
+		it( 'should return different ids', () => {
+			let id1 = utils.uid();
+			let id2 = utils.uid();
+			let id3 = utils.uid();
 
 			expect( id1 ).to.be.a( 'number' );
 			expect( id2 ).to.be.a( 'number' ).to.not.equal( id1 ).to.not.equal( id3 );
@@ -72,20 +72,20 @@ describe( 'utils', function() {
 		} );
 	} );
 
-	describe( 'isIterable', function() {
-		it( 'should be true for string', function() {
-			var string = 'foo';
+	describe( 'isIterable', () => {
+		it( 'should be true for string', () => {
+			let string = 'foo';
 
 			expect( utils.isIterable( string ) ).to.be.true;
 		} );
 
-		it( 'should be true for arrays', function() {
-			var array = [ 1, 2, 3 ];
+		it( 'should be true for arrays', () => {
+			let array = [ 1, 2, 3 ];
 
 			expect( utils.isIterable( array ) ).to.be.true;
 		} );
 
-		it( 'should be true for iterable classes', function() {
+		it( 'should be true for iterable classes', () => {
 			class IterableClass {
 				constructor() {
 					this.array = [ 1, 2, 3 ];
@@ -96,67 +96,95 @@ describe( 'utils', function() {
 				}
 			}
 
-			var instance = new IterableClass();
+			let instance = new IterableClass();
 
 			expect( utils.isIterable( instance ) ).to.be.true;
 		} );
 
-		it( 'should be false for not iterable objects', function() {
-			var notIterable = { foo: 'bar' };
+		it( 'should be false for not iterable objects', () => {
+			let notIterable = { foo: 'bar' };
 
 			expect( utils.isIterable( notIterable ) ).to.be.false;
 		} );
 
-		it( 'should be false for undefined', function() {
+		it( 'should be false for undefined', () => {
 			expect( utils.isIterable() ).to.be.false;
 		} );
 	} );
 
-	describe( 'compareArrays', function() {
-		it( 'should return SAME flag, when arrays are same', function() {
-			var a = [ 'abc', 0, 3 ];
-			var b = [ 'abc', 0, 3 ];
+	describe( 'compareArrays', () => {
+		it( 'should return SAME flag, when arrays are same', () => {
+			let a = [ 'abc', 0, 3 ];
+			let b = [ 'abc', 0, 3 ];
 
-			var result = utils.compareArrays( a, b );
+			let result = utils.compareArrays( a, b );
 
 			expect( result ).to.equal( utils.compareArrays.SAME );
 		} );
 
-		it( 'should return PREFIX flag, when all n elements of first array are same as n first elements of the second array', function() {
-			var a = [ 'abc', 0 ];
-			var b = [ 'abc', 0, 3 ];
+		it( 'should return PREFIX flag, when all n elements of first array are same as n first elements of the second array', () => {
+			let a = [ 'abc', 0 ];
+			let b = [ 'abc', 0, 3 ];
 
-			var result = utils.compareArrays( a, b );
+			let result = utils.compareArrays( a, b );
 
 			expect( result ).to.equal( utils.compareArrays.PREFIX );
 		} );
 
-		it( 'should return EXTENSION flag, when n first elements of first array are same as all elements of the second array', function() {
-			var a = [ 'abc', 0, 3 ];
-			var b = [ 'abc', 0 ];
+		it( 'should return EXTENSION flag, when n first elements of first array are same as all elements of the second array', () => {
+			let a = [ 'abc', 0, 3 ];
+			let b = [ 'abc', 0 ];
 
-			var result = utils.compareArrays( a, b );
+			let result = utils.compareArrays( a, b );
 
 			expect( result ).to.equal( utils.compareArrays.EXTENSION );
 		} );
 
-		it( 'should return DIFFERENT flag, when arrays are not same', function() {
-			var a = [ 'abc', 0, 3 ];
-			var b = [ 'abc', 1, 3 ];
+		it( 'should return DIFFERENT flag, when arrays are not same', () => {
+			let a = [ 'abc', 0, 3 ];
+			let b = [ 'abc', 1, 3 ];
 
-			var result = utils.compareArrays( a, b );
+			let result = utils.compareArrays( a, b );
 
 			expect( result ).to.equal( utils.compareArrays.DIFFERENT );
 		} );
 	} );
 
-	describe( 'Lo-Dash extensions', function() {
+	describe( 'nth', () => {
+		it( 'should return 0th item', () => {
+			expect( utils.nth( 0, getIterator() ) ).to.equal( 11 );
+		} );
+
+		it( 'should return the last item', () => {
+			expect( utils.nth( 2, getIterator() ) ).to.equal( 33 );
+		} );
+
+		it( 'should return null if out of range (bottom)', () => {
+			expect( utils.nth( -1, getIterator() ) ).to.be.null;
+		} );
+
+		it( 'should return null if out of range (top)', () => {
+			expect( utils.nth( 3, getIterator() ) ).to.be.null;
+		} );
+
+		it( 'should return null if iterator is empty', () => {
+			expect( utils.nth( 0, [] ) ).to.be.null;
+		} );
+
+		function *getIterator() {
+			yield 11;
+			yield 22;
+			yield 33;
+		}
+	} );
+
+	describe( 'Lo-Dash extensions', () => {
 		// Ensures that the required Lo-Dash extensions are available in `utils`.
-		it( 'should be exposed in utils', function() {
-			var utils = modules.utils;
-			var extensions = modules[ 'utils-lodash' ];
+		it( 'should be exposed in utils', () => {
+			let utils = modules.utils;
+			let extensions = modules[ 'utils-lodash' ];
 
-			extensions.forEach( function( extension ) {
+			extensions.forEach( ( extension ) => {
 				expect( utils ).to.have.property( extension ).to.not.be.undefined();
 			} );
 		} );