Explorar o código

Merge pull request #36 from ckeditor/t/33

Refactored configuration class.
Piotrek Koszuliński %!s(int64=9) %!d(string=hai) anos
pai
achega
655f4fcba4
Modificáronse 2 ficheiros con 285 adicións e 198 borrados
  1. 108 88
      packages/ckeditor5-utils/src/config.js
  2. 177 110
      packages/ckeditor5-utils/tests/config.js

+ 108 - 88
packages/ckeditor5-utils/src/config.js

@@ -5,16 +5,12 @@
 
 'use strict';
 
-import ObservableMixin from './observablemixin.js';
-import isObject from './lib/lodash/isObject.js';
 import isPlainObject from './lib/lodash/isPlainObject.js';
-import mix from './mix.js';
 
 /**
  * Handles a configuration dictionary.
  *
  * @memberOf utils
- * @mixes utils.ObservableMixin
  */
 export default class Config {
 	/**
@@ -23,8 +19,17 @@ export default class Config {
 	 * @param {Object} [configurations] The initial configurations to be set.
 	 */
 	constructor( configurations ) {
+		/**
+		 * Store for the whole configuration.
+		 *
+		 * @private
+		 * @member {Object} utils.config#_config
+		 */
+		this._config = {};
+
+		// Set initial configuration.
 		if ( configurations ) {
-			this.set( configurations );
+			this._setObjectToTarget( this._config, configurations );
 		}
 	}
 
@@ -58,134 +63,149 @@ export default class Config {
 	 *			color: 'red',
 	 *		} );
 	 *
-	 *		config.toolbar.collapsed; // true
-	 *		config.toolbar.color; // 'red'
+	 *		config.get( 'toolbar.collapsed' ); // true
+	 *		config.get( 'toolbar.color' ); // 'red'
 	 *
 	 * @param {String|Object} name The configuration name or an object from which take properties as
 	 * configuration entries. Configuration names are case-insensitive.
-	 * @param {*} [value=null] The configuration value. Used if a name is passed to nameOrConfigurations.
+	 * @param {*} value The configuration value. Used if a name is passed.
 	 */
 	set( name, value ) {
-		// Just pass the call to the original set() in case of an object. It'll deal with recursing through the
-		// object and calling set( name, value ) again for each property.
-		if ( isObject( name ) ) {
-			ObservableMixin.set.apply( this, arguments );
+		this._setToTarget( this._config, name, value );
+	}
+
+	/**
+	 * Does exactly the same as {@link #set} with one exception – passed configuration extends
+	 * existing one, but does not overwrite already defined values.
+	 *
+	 * This method is supposed to be called by plugin developers to setup plugin's configurations. It would be
+	 * rarely used for other needs.
+	 *
+	 * @param {String|Object} name The configuration name or an object from which take properties as
+	 * configuration entries. Configuration names are case-insensitive.
+	 * @param {*} value The configuration value. Used if a name is passed.
+	 */
+	define( name, value ) {
+		const isDefine = true;
+
+		this._setToTarget( this._config, name, value, isDefine );
+	}
+
+	/**
+	 * Gets the value for a configuration entry.
+	 *
+	 *		config.get( 'name' );
+	 *
+	 * Deep configurations can be retrieved by separating each part with a dot.
+	 *
+	 *		config.get( 'toolbar.collapsed' );
+	 *
+	 * @param {String} name The configuration name. Configuration names are case-insensitive.
+	 * @returns {*} The configuration value or `undefined` if the configuration entry was not found.
+	 */
+	get( name ) {
+		return this._getFromSource( this._config, name );
+	}
+
+	/**
+	 * Saves passed configuration to the specified target (nested object).
+	 *
+	 * @private
+	 * @param {Object} target Nested config object.
+	 * @param {String|Object} name The configuration name or an object from which take properties as
+	 * configuration entries. Configuration names are case-insensitive.
+	 * @param {*} value The configuration value. Used if a name is passed.
+	 * @param {Boolean} [isDefine=false] Define if passed configuration should overwrite existing one.
+	 */
+	_setToTarget( target, name, value, isDefine = false ) {
+		// In case of an object, iterate through it and call `_setToTarget` again for each property.
+		if ( isPlainObject( name ) ) {
+			this._setObjectToTarget( target, name, isDefine );
 
 			return;
 		}
 
-		// The target for this configuration is, for now, this object.
-		let target = this;
-
-		// The configuration name should be split into parts if it has dots. E.g: `resize.width`.
+		// The configuration name should be split into parts if it has dots. E.g. `resize.width` -> [`resize`, `width`].
 		const parts = name.toLowerCase().split( '.' );
 
-		// Take the name of the configuration out of the parts. E.g. `resize.width` -> `width`
+		// Take the name of the configuration out of the parts. E.g. `resize.width` -> `width`.
 		name = parts.pop();
 
-		// Retrieves the final target for this configuration recursively.
-		for ( let i = 0; i < parts.length; i++ ) {
-			// The target will always be an instance of Config.
-			if ( !( target[ parts[ i ] ] instanceof Config ) ) {
-				target.set( parts[ i ], new Config() );
+		// Iterate over parts to check if currently stored configuration has proper structure.
+		for ( let part of parts ) {
+			// If there is no object for specified part then create one.
+			if ( !isPlainObject( target[ part ] ) ) {
+				target[ part ] = {};
 			}
 
-			target = target[ parts[ i ] ];
+			// Nested object becomes a target.
+			target = target[ part ];
 		}
 
-		// Values set as pure objects will be treated as Config subsets.
+		// In case of value is an object.
 		if ( isPlainObject( value ) ) {
-			// If the target is an instance of Config (a deep config subset).
-			if ( target[ name ] instanceof Config ) {
-				// Amend the target with the value, instead of replacing it.
-				target[ name ].set( value );
-
-				return;
+			// We take care of proper config structure.
+			if ( !isPlainObject( target[ name ] ) ) {
+				target[ name ] = {};
 			}
 
-			value = new Config( value );
+			target = target[ name ];
+
+			// And iterate through this object calling `_setToTarget` again for each property.
+			this._setObjectToTarget( target, value, isDefine );
+
+			return;
 		}
 
-		// Values will never be undefined.
-		if ( typeof value == 'undefined' ) {
-			value = null;
+		// Do nothing if we are defining configuration for non empty name.
+		if ( isDefine && typeof target[ name ] != 'undefined' ) {
+			return;
 		}
 
-		// Call the original set() on the target.
-		ObservableMixin.set.call( target, name, value );
+		target[ name ] = value;
 	}
 
 	/**
-	 * Gets the value for a configuration entry.
-	 *
-	 *		config.get( 'name' );
-	 *
-	 * Deep configurations can be retrieved by separating each part with a dot.
-	 *
-	 *		config.get( 'toolbar.collapsed' );
+	 * Get specified configuration from specified source (nested object).
 	 *
+	 * @private
+	 * @param {Object} source level of nested object.
 	 * @param {String} name The configuration name. Configuration names are case-insensitive.
 	 * @returns {*} The configuration value or `undefined` if the configuration entry was not found.
 	 */
-	get( name ) {
-		// The target for this configuration is, for now, this object.
-		let source = this;
-
-		// The configuration name should be split into parts if it has dots. E.g. `resize.width` -> [`resize`, `width`]
+	_getFromSource( source, name ) {
+		// The configuration name should be split into parts if it has dots. E.g. `resize.width` -> [`resize`, `width`].
 		const parts = name.toLowerCase().split( '.' );
 
-		// Take the name of the configuration from the parts. E.g. `resize.width` -> `width`
+		// Take the name of the configuration out of the parts. E.g. `resize.width` -> `width`.
 		name = parts.pop();
 
-		// Retrieves the source for this configuration recursively.
-		for ( let i = 0; i < parts.length; i++ ) {
-			// The target will always be an instance of Config.
-			if ( !( source[ parts[ i ] ] instanceof Config ) ) {
+		// Iterate over parts to check if currently stored configuration has proper structure.
+		for ( let part of parts ) {
+			if ( !isPlainObject( source[ part ] ) ) {
 				source = null;
 				break;
 			}
 
-			source = source[ parts[ i ] ];
-		}
-
-		// Try to retrieve it from the source object.
-		if ( source && ( typeof source[ name ] != 'undefined' ) ) {
-			return source[ name ];
+			// Nested object becomes a source.
+			source = source[ part ];
 		}
 
-		// If not found, take it from the definition.
-		if ( this.definition ) {
-			return this.definition[ name ];
-		}
+		// Always returns undefined for non existing configuration
+		return source ? source[ name ] : undefined;
 	}
 
 	/**
-	 * Defines the name and default value for configurations. It accepts the same parameters as the
-	 * {@link Config#set set()} method.
-	 *
-	 * On first call, the {@link Config#definition definition} property is created to hold all defined
-	 * configurations.
+	 * Iterate through passed object and call {@link #_setToTarget} method with object key and value for each property.
 	 *
-	 * This method is supposed to be called by plugin developers to setup plugin's configurations. It would be
-	 * rarely used for other needs.
-	 *
-	 * @param {String|Object} name The configuration name or an object from which take properties as
-	 * configuration entries.
-	 * @param {*} [value] The configuration value. Used if a name is passed to nameOrConfigurations. If undefined,
-	 * the configuration is set to `null`.
+	 * @private
+	 * @param {Object} target Nested config object.
+	 * @param {Object} configuration Configuration data set
+	 * @param {Boolean} [isDefine] Defines if passed configuration is default configuration or not.
 	 */
-	define( name, value ) {
-		if ( !this.definition ) {
-			/**
-			 * TODO
-			 *
-			 * @type {Config}
-			 */
-			this.definition = new Config();
-		}
-
-		this.definition.set( name, value );
+	_setObjectToTarget( target, configuration, isDefine ) {
+		Object.keys( configuration ).forEach( key => {
+			this._setToTarget( target, key, configuration[ key ], isDefine );
+		} );
 	}
 }
-
-mix( Config, ObservableMixin );

+ 177 - 110
packages/ckeditor5-utils/tests/config.js

@@ -26,13 +26,16 @@ beforeEach( () => {
 
 describe( 'constructor', () => {
 	it( 'should set configurations', () => {
-		expect( config ).to.have.property( 'creator' ).to.equal( 'inline' );
-		expect( config ).to.have.property( 'language' ).to.equal( 'pl' );
-		expect( config ).to.have.property( 'resize' ).to.have.property( 'minheight' ).to.equal( 300 );
-		expect( config ).to.have.property( 'resize' ).to.have.property( 'maxheight' ).to.equal( 800 );
-		expect( config ).to.have.property( 'resize' ).to.have.property( 'icon' )
-			.to.have.property( 'path' ).to.equal( 'xyz' );
-		expect( config ).to.have.property( 'toolbar' ).to.equal( 'top' );
+		expect( config.get( 'creator' ) ).to.equal( 'inline' );
+		expect( config.get( 'language' ) ).to.equal( 'pl' );
+		expect( config.get( 'resize' ) ).to.deep.equal( {
+			minheight: 300,
+			maxheight: 800,
+			icon: {
+				path: 'xyz'
+			}
+		} );
+		expect( config.get( 'toolbar' ) ).to.equal( 'top' );
 	} );
 
 	it( 'should work with no parameters', () => {
@@ -42,11 +45,6 @@ describe( 'constructor', () => {
 } );
 
 describe( 'set', () => {
-	it( 'should create Config instances for objects', () => {
-		expect( config.resize ).to.be.an.instanceof( Config );
-		expect( config.resize.icon ).to.be.an.instanceof( Config );
-	} );
-
 	it( 'should set configurations when passing objects', () => {
 		config.set( {
 			option1: 1,
@@ -55,32 +53,53 @@ describe( 'set', () => {
 			}
 		} );
 
-		expect( config )
-			.to.have.property( 'option1' ).to.equal( 1 );
-
-		expect( config )
-			.to.have.property( 'option2' )
-			.to.have.property( 'suboption21' ).to.equal( 21 );
+		expect( config.get( 'option1' ) ).to.equal( 1 );
+		expect( config.get( 'option2.suboption21' ) ).to.equal( 21 );
 	} );
 
 	it( 'should set configurations when passing name and value', () => {
 		config.set( 'something', 'anything' );
 
-		expect( config ).to.have.property( 'something' ).to.equal( 'anything' );
+		expect( config.get( 'something' ) ).to.equal( 'anything' );
 	} );
 
 	it( 'should set configurations when passing name.with.deep and value', () => {
 		config.set( 'color.red', 'f00' );
 		config.set( 'background.color.blue', '00f' );
 
-		expect( config )
-			.to.have.property( 'color' )
-			.to.have.property( 'red' ).to.equal( 'f00' );
+		expect( config.get( 'color.red' ) ).to.equal( 'f00' );
+		expect( config.get( 'background.color.blue' ) ).to.equal( '00f' );
+	} );
+
+	it( 'should replace a simple entry with an object', () => {
+		config.set( 'test', 1 );
+		config.set( 'test', {
+			prop: 1
+		} );
+
+		expect( config.get( 'test' ) ).to.be.an( 'object' );
+		expect( config.get( 'test.prop' ) ).to.equal( 1 );
+	} );
+
+	it( 'should replace a simple entry with an object when passing only object', () => {
+		config.set( 'test', 1 );
+		config.set( {
+			test: {
+				prop: 1
+			}
+		} );
 
-		expect( config )
-			.to.have.property( 'background' )
-			.to.have.property( 'color' )
-			.to.have.property( 'blue' ).to.equal( '00f' );
+		expect( config.get( 'test' ) ).to.be.an( 'object' );
+		expect( config.get( 'test.prop' ) ).to.equal( 1 );
+	} );
+
+	it( 'should replace a simple entry with an object when passing a name.with.deep', () => {
+		config.set( 'test.prop', 1 );
+		config.set( 'test.prop.value', 1 );
+
+		expect( config.get( 'test' ) ).to.be.an( 'object' );
+		expect( config.get( 'test.prop' ) ).to.be.an( 'object' );
+		expect( config.get( 'test.prop.value' ) ).to.equal( 1 );
 	} );
 
 	it( 'should override and expand deep configurations', () => {
@@ -95,64 +114,142 @@ describe( 'set', () => {
 			}
 		} );
 
-		expect( config ).to.have.property( 'resize' );
-		expect( config.resize ).to.have.property( 'minheight' ).to.equal( 400 );
-		expect( config.resize ).to.have.property( 'maxheight' ).to.equal( 800 );	// Not touched
-		expect( config.resize ).to.have.property( 'hidden' ).to.equal( true );
+		expect( config.get( 'resize' ) ).to.be.deep.equal( {
+			minheight: 400,		// Overridden
+			maxheight: 800,		// The same
+			hidden: true,		// Expanded
+			icon: {
+				path: 'abc',	// Overridden
+				url: true		// Expanded
+			}
+		} );
+	} );
+
+	it( 'should override and expand object when passing an object', () => {
+		config.set( 'resize', {
+			minHeight: 400,		// Override
+			hidden: true,		// Expand
+			icon: {
+				path: 'abc',	// Override
+				url: true		// Expand
+			}
+		} );
 
-		expect( config.resize ).to.have.property( 'icon' );
-		expect( config.resize.icon ).to.have.property( 'path' ).to.equal( 'abc' );
-		expect( config.resize.icon ).to.have.property( 'url' ).to.equal( true );
+		expect( config.get( 'resize' ) ).to.deep.equal( {
+			minheight: 400,		// Overridden
+			maxheight: 800,		// The same
+			hidden: true,		// Expanded
+			icon: {
+				path: 'abc',	// Overridden
+				url: true		// Expanded
+			}
+		} );
 	} );
 
-	it( 'should replace a simple entry with a Config instance', () => {
-		config.set( 'test', 1 );
-		config.set( 'test', {
-			prop: 1
+	it( 'should not create object for non-pure objects', () => {
+		function SomeClass() {}
+
+		config.set( 'date', new Date() );
+		config.set( {
+			instance: new SomeClass()
 		} );
 
-		expect( config.test ).to.be.an.instanceof( Config );
-		expect( config.test.prop ).to.equal( 1 );
+		expect( config.get( 'date' ) ).to.be.an.instanceof( Date );
+		expect( config.get( 'instance' ) ).to.be.an.instanceof( SomeClass );
 	} );
+} );
 
-	it( 'should replace a simple entry with a Config instance when passing an object', () => {
-		config.set( 'test', 1 );
+describe( 'define', () => {
+	it( 'should set configurations when passing objects', () => {
 		config.set( {
-			test: {
-				prop: 1
+			option1: 1,
+			option2: {
+				subOption21: 21
 			}
 		} );
 
-		expect( config.test ).to.be.an.instanceof( Config );
-		expect( config.test.prop ).to.equal( 1 );
+		expect( config.get( 'option1' ) ).to.equal( 1 );
+		expect( config.get( 'option2.suboption21' ) ).to.equal( 21 );
 	} );
 
-	it( 'should replace a simple entry with a Config instance when passing a name.with.deep', () => {
-		config.set( 'test.prop', 1 );
-		config.set( 'test.prop.value', 1 );
+	it( 'should set configurations when passing name and value', () => {
+		config.set( 'something', 'anything' );
 
-		expect( config.test ).to.be.an.instanceof( Config );
-		expect( config.test.prop ).to.be.an.instanceof( Config );
-		expect( config.test.prop.value ).to.equal( 1 );
+		expect( config.get( 'something' ) ).to.equal( 'anything' );
 	} );
 
-	it( 'should not create Config instances for non-pure objects', () => {
-		function SomeClass() {}
+	it( 'should set configurations when passing name.with.deep and value', () => {
+		config.set( 'color.red', 'f00' );
+		config.set( 'background.color.blue', '00f' );
 
-		config.set( 'date', new Date() );
-		config.set( {
-			instance: new SomeClass()
+		expect( config.get( 'color.red' ) ).to.equal( 'f00' );
+		expect( config.get( 'background.color.blue' ) ).to.equal( '00f' );
+	} );
+
+	it( 'should not replace already defined values', () => {
+		config.define( 'language', 'en' );
+		config.define( 'resize.minHeight', 400 );
+		config.define( 'resize.icon', 'some value' );
+
+		expect( config.get( 'language' ) ).to.equal( 'pl' );
+		expect( config.get( 'resize.icon' ) ).to.be.an( 'object' );
+		expect( config.get( 'resize.minheight' ) ).to.equal( 300 );
+	} );
+
+	it( 'should expand but not override deep configurations', () => {
+		config.define( {
+			resize: {
+				minHeight: 400,		// No override
+				hidden: true,		// Expand
+				icon: {
+					path: 'abc',	// No override
+					url: true		// Expand
+				}
+			}
 		} );
 
-		expect( config.date ).to.be.an.instanceof( Date );
-		expect( config.instance ).to.be.an.instanceof( SomeClass );
+		expect( config.get( 'resize' ) ).to.be.deep.equal( {
+			minheight: 300,		// The same
+			maxheight: 800,		// The same
+			hidden: true,		// Expanded
+			icon: {
+				path: 'xyz',	// The same
+				url: true		// Expanded
+			}
+		} );
+	} );
+
+	it( 'should expand but not override when passing an object', () => {
+		config.define( 'resize', {
+			minHeight: 400,		// No override
+			hidden: true,		// Expand
+			icon: {
+				path: 'abc',	// No override
+				url: true		// Expand
+			}
+		} );
+
+		expect( config.get( 'resize' ) ).to.be.deep.equal( {
+			minheight: 300,		// The same
+			maxheight: 800,		// The same
+			hidden: true,		// Expanded
+			icon: {
+				path: 'xyz',	// The same
+				url: true		// Expanded
+			}
+		} );
 	} );
 
-	it( 'should set `null` for undefined value', () => {
-		config.set( 'test' );
+	it( 'should not create an object for non-pure objects', () => {
+		function SomeClass() {}
+
+		config.define( 'date', new Date() );
+		config.define( {
+			instance: new SomeClass()
+		} );
 
-		expect( config.test ).to.be.null;
-		expect( config.get( 'test' ) ).to.be.null;
+		expect( config.get( 'date' ) ).to.be.an.instanceof( Date );
+		expect( config.get( 'instance' ) ).to.be.an.instanceof( SomeClass );
 	} );
 } );
 
@@ -166,16 +263,15 @@ describe( 'get', () => {
 		expect( config.get( 'resize.icon.path' ) ).to.equal( 'xyz' );
 	} );
 
-	it( 'should retrieve a subset of the configuration', () => {
-		let resizeConfig = config.get( 'resize' );
-
-		expect( resizeConfig ).to.have.property( 'minheight' ).to.equal( 300 );
-		expect( resizeConfig ).to.have.property( 'maxheight' ).to.equal( 800 );
-		expect( resizeConfig ).to.have.property( 'icon' ).to.have.property( 'path' ).to.equal( 'xyz' );
+	it( 'should retrieve a object of the configuration', () => {
+		let resize = config.get( 'resize' );
 
-		let iconConfig = resizeConfig.get( 'icon' );
+		expect( resize ).to.be.an( 'object' );
+		expect( resize.minheight ).equal( 300 );
+		expect( resize.maxheight ).to.equal( 800 );
+		expect( resize.icon ).to.be.an( 'object' );
 
-		expect( iconConfig ).to.have.property( 'path' ).to.equal( 'xyz' );
+		expect( resize.icon ).to.be.an( 'object' );
 	} );
 
 	it( 'should retrieve values case-insensitively', () => {
@@ -189,50 +285,21 @@ describe( 'get', () => {
 		expect( config.get( 'invalid' ) ).to.be.undefined;
 	} );
 
-	it( 'should return undefined for non existing deep configuration', () => {
-		expect( config.get( 'resize.invalid.value' ) ).to.be.undefined;
-	} );
-} );
-
-describe( 'define', () => {
-	it( 'should create the definition property', () => {
-		expect( config ).to.not.have.property( 'definition' );
-
-		config.define( 'test', 1 );
-
-		expect( config ).to.have.property( 'definition' );
-	} );
-
-	it( 'should set configurations in the definition property', () => {
-		config.define( 'test1', 1 );
-
-		// This is for Code Coverage to ensure that it works when `definition` is already defined.
-		config.define( 'test2', 2 );
-
-		expect( config.definition ).to.have.property( 'test1' ).to.equal( 1 );
-		expect( config.definition ).to.have.property( 'test2' ).to.equal( 2 );
-	} );
-
-	it( 'should set configurations passed as object in the definition property', () => {
-		config.define( {
-			test: 1
-		} );
+	it( 'should return undefined for empty configuration', () => {
+		config = new Config();
 
-		expect( config.definition ).to.have.property( 'test' ).to.equal( 1 );
+		expect( config.get( 'invalid' ) ).to.be.undefined;
+		expect( config.get( 'deep.invalid' ) ).to.be.undefined;
 	} );
 
-	it( 'should not define main config properties but still be retrieved with get()', () => {
-		config.define( 'test', 1 );
-
-		expect( config ).to.not.have.property( 'test' );
-		expect( config.get( 'test' ) ).to.equal( 1 );
+	it( 'should return undefined for non existing deep configuration', () => {
+		expect( config.get( 'resize.invalid.value' ) ).to.be.undefined;
 	} );
 
-	it( 'should be overridden by set()', () => {
-		config.define( 'test', 1 );
-		config.set( 'test', 2 );
-
-		expect( config ).to.have.property( 'test' ).to.equal( 2 );
-		expect( config.get( 'test' ) ).to.equal( 2 );
+	it( 'should not be possible to retrieve value directly from config object', () => {
+		expect( config.creator ).to.be.undefined;
+		expect( () => {
+			config.resize.maxheight;
+		} ).to.throw();
 	} );
 } );