Merge pull request #163 from ckeditor/t/162

Feature: Introduced `ObservableMixin#decorate()` and support for setting `EmitterMixin#fire()`'s return value by listeners. Closes #162.

Author: szymonkups

src/emittermixin.js

@@ -22,14 +22,23 @@ const _emitterId = Symbol( 'emitterId' );
  */
 const EmitterMixin = {
 	/**
-	 * Registers a callback function to be executed when an event is fired. Events can be grouped in namespaces using `:`.
+	 * Registers a callback function to be executed when an event is fired.
+	 *
+	 * Events can be grouped in namespaces using `:`.
 	 * When namespaced event is fired, it additionaly fires all callbacks for that namespace.
 	 *
 	 *		myEmitter.on( 'myGroup', genericCallback );
 	 *		myEmitter.on( 'myGroup:myEvent', specificCallback );
-	 *		myEmitter.fire( 'myGroup' ); // genericCallback is fired.
-	 *		myEmitter.fire( 'myGroup:myEvent' ); // both genericCallback and specificCallback are fired.
-	 *		myEmitter.fire( 'myGroup:foo' ); // genericCallback is fired even though there are no callbacks for "foo".
+	 *
+	 *		// genericCallback is fired.
+	 *		myEmitter.fire( 'myGroup' );
+	 *		// both genericCallback and specificCallback are fired.
+	 *		myEmitter.fire( 'myGroup:myEvent' );
+	 *		// genericCallback is fired even though there are no callbacks for "foo".
+	 *		myEmitter.fire( 'myGroup:foo' );
+	 *
+	 * An event callback can {@link module:utils/eventinfo~EventInfo#stop stop the event} and
+	 * set the {@link module:utils/eventinfo~EventInfo#return return value} of the {@link #fire} method.
 	 *
 	 * @method #on
 	 * @param {String} event The name of the event.
@@ -244,6 +253,9 @@ const EmitterMixin = {
 	 * @method #fire
 	 * @param {String|module:utils/eventinfo~EventInfo} eventOrInfo The name of the event or `EventInfo` object if event is delegated.
 	 * @param {...*} [args] Additional arguments to be passed to the callbacks.
+	 * @returns {*} By default the method returns `undefined`. However, the return value can be changed by listeners
+	 * through modification of the {@link module:utils/eventinfo~EventInfo#return}'s value (the event info
+	 * is the first param of every callback).
 	 */
 	fire( eventOrInfo, ...args ) {
 		const eventInfo = eventOrInfo instanceof EventInfo ? eventOrInfo : new EventInfo( this, eventOrInfo );
@@ -296,6 +308,8 @@ const EmitterMixin = {
 				fireDelegatedEvents( passAllDestinations, eventInfo, args );
 			}
 		}
+
+		return eventInfo.return;
 	},
 
 	/**

src/eventinfo.js

@@ -58,5 +58,22 @@ export default class EventInfo {
 		 * @method #off
 		 */
 		this.off = spy();
+
+		/**
+		 * The value which will be returned by {@link module:utils/emittermixin~EmitterMixin#fire}.
+		 *
+		 * It's `undefined` by default and can be changed by an event listener:
+		 *
+		 *		dataController.fire( 'getSelectedContent', ( evt ) => {
+		 *			// This listener will make `dataController.fire( 'getSelectedContent' )`
+		 *			// always return an empty DocumentFragment.
+		 *			evt.return = new DocumentFragment();
+		 *
+		 *			// Make sure no other listeners are executed.
+		 *			evt.stop();
+		 *		} );
+		 *
+		 * @member #return
+		 */
 	}
 }

src/observablemixin.js

@@ -247,6 +247,88 @@ const ObservableMixin = {
 			boundObservables.clear();
 			boundAttributes.clear();
 		}
+	},
+
+	/**
+	 * Turns the given methods of this object into event-based ones. This means that the new method will fire an event
+	 * (named after the method) and the original action will be plugged as a listener to that event.
+	 *
+	 * This is a very simplified method decoration. Itself it doesn't change the behavior of a method (expect adding the event),
+	 * but it allows to modify it later on by listening to the method's event.
+	 *
+	 * For example, in order to cancel the method execution one can stop the event:
+	 *
+	 *		class Foo {
+	 *			constructor() {
+	 *				this.decorate( 'method' );
+	 *			}
+	 *
+	 *			method() {
+	 *				console.log( 'called!' );
+	 *			}
+	 *		}
+	 *
+	 *		const foo = new Foo();
+	 *		foo.on( 'method', ( evt ) => {
+	 *			evt.stop();
+	 *		}, { priority: 'high' } );
+	 *
+	 *		foo.method(); // Nothing is logged.
+	 *
+	 *
+	 * Note: we used a high priority listener here to execute this callback before the one which
+	 * calls the orignal method (which used the default priority).
+	 *
+	 * It's also possible to change the return value:
+	 *
+	 *		foo.on( 'method', ( evt ) => {
+	 *			evt.return = 'Foo!';
+	 *		} );
+	 *
+	 *		foo.method(); // -> 'Foo'
+	 *
+	 * Finally, it's possible to access and modify the parameters:
+	 *
+	 *		method( a, b ) {
+	 *			console.log( `${ a }, ${ b }`  );
+	 *		}
+	 *
+	 *		// ...
+	 *
+	 *		foo.on( 'method', ( evt, args ) => {
+	 *			args[ 0 ] = 3;
+	 *
+	 *			console.log( args[ 1 ] ); // -> 2
+	 *		}, { priority: 'high' } );
+	 *
+	 *		foo.method( 1, 2 ); // -> '3, 2'
+	 *
+	 * @param {String} methodName Name of the method to decorate.
+	 */
+	decorate( methodName ) {
+		const originalMethod = this[ methodName ];
+
+		if ( !originalMethod ) {
+			/**
+			 * Cannot decorate an undefined method.
+			 *
+			 * @error observablemixin-cannot-decorate-undefined
+			 * @param {Object} object The object which method should be decorated.
+			 * @param {String} methodName Name of the method which does not exist.
+			 */
+			throw new CKEditorError(
+				'observablemixin-cannot-decorate-undefined: Cannot decorate an undefined method.',
+				{ object: this, methodName }
+			);
+		}
+
+		this.on( methodName, ( evt, args ) => {
+			evt.return = originalMethod.apply( this, args );
+		} );
+
+		this[ methodName ] = function( ...args ) {
+			return this.fire( methodName, args );
+		};
 	}
 
 	/**

tests/emittermixin.js

@@ -172,6 +172,72 @@ describe( 'EmitterMixin', () => {
 			sinon.assert.calledThrice( spyFoo );
 			sinon.assert.calledThrice( spyFoo2 );
 		} );
+
+		describe( 'return value', () => {
+			it( 'is undefined by default', () => {
+				expect( emitter.fire( 'foo' ) ).to.be.undefined;
+			} );
+
+			it( 'is undefined if none of the listeners modified EventInfo#return', () => {
+				emitter.on( 'foo', () => {} );
+
+				expect( emitter.fire( 'foo' ) ).to.be.undefined;
+			} );
+
+			it( 'equals EventInfo#return\'s value', () => {
+				emitter.on( 'foo', evt => {
+					evt.return = 1;
+				} );
+
+				expect( emitter.fire( 'foo' ) ).to.equal( 1 );
+			} );
+
+			it( 'equals EventInfo#return\'s value even if the event was stopped', () => {
+				emitter.on( 'foo', evt => {
+					evt.return = 1;
+				} );
+				emitter.on( 'foo', evt => {
+					evt.stop();
+				} );
+
+				expect( emitter.fire( 'foo' ) ).to.equal( 1 );
+			} );
+
+			it( 'equals EventInfo#return\'s value when it was set in a namespaced event', () => {
+				emitter.on( 'foo', evt => {
+					evt.return = 1;
+				} );
+
+				expect( emitter.fire( 'foo:bar' ) ).to.equal( 1 );
+			} );
+
+			// Rationale – delegation keeps the listeners of the two objects separate.
+			// E.g. the emitterB's listeners will always be executed before emitterA's ones.
+			// Hence, values should not be shared either.
+			it( 'is not affected by listeners executed on emitter to which the event was delegated', () => {
+				const emitterA = getEmitterInstance();
+				const emitterB = getEmitterInstance();
+
+				emitterB.delegate( 'foo' ).to( emitterA );
+
+				emitterA.on( 'foo', evt => {
+					evt.return = 1;
+				} );
+
+				expect( emitterB.fire( 'foo' ) ).to.be.undefined;
+			} );
+
+			it( 'equals the value set by the last callback', () => {
+				emitter.on( 'foo', evt => {
+					evt.return = 1;
+				} );
+				emitter.on( 'foo', evt => {
+					evt.return = 2;
+				}, { priority: 'high' } );
+
+				expect( emitter.fire( 'foo' ) ).to.equal( 1 );
+			} );
+		} );
 	} );
 
 	describe( 'on', () => {

tests/observablemixin.js

@@ -61,7 +61,7 @@ describe( 'Observable', () => {
 		expect( car.color ).to.equal( 'blue' );
 	} );
 
-	describe( 'set', () => {
+	describe( 'set()', () => {
 		it( 'should work when passing an object', () => {
 			car.set( {
 				color: 'blue',	// Override
@@ -185,7 +185,7 @@ describe( 'Observable', () => {
 		} );
 	} );
 
-	describe( 'bind', () => {
+	describe( 'bind()', () => {
 		it( 'should chain for a single attribute', () => {
 			expect( car.bind( 'color' ) ).to.contain.keys( 'to' );
 		} );
@@ -225,7 +225,7 @@ describe( 'Observable', () => {
 			} ).to.throw( CKEditorError, /observable-bind-rebind/ );
 		} );
 
-		describe( 'to', () => {
+		describe( 'to()', () => {
 			it( 'should not chain', () => {
 				expect(
 					car.bind( 'color' ).to( new Observable( { color: 'red' } ) )
@@ -732,7 +732,7 @@ describe( 'Observable', () => {
 		} );
 	} );
 
-	describe( 'unbind', () => {
+	describe( 'unbind()', () => {
 		it( 'should not fail when unbinding a fresh observable', () => {
 			const observable = new Observable();
 
@@ -811,4 +811,112 @@ describe( 'Observable', () => {
 			);
 		} );
 	} );
+
+	describe( 'decorate()', () => {
+		it( 'makes the method fire an event', () => {
+			const spy = sinon.spy();
+
+			class Foo extends Observable {
+				method() {}
+			}
+
+			const foo = new Foo();
+
+			foo.decorate( 'method' );
+
+			foo.on( 'method', spy );
+
+			foo.method( 1, 2 );
+
+			expect( spy.calledOnce ).to.be.true;
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( [ 1, 2 ] );
+		} );
+
+		it( 'executes the original method in a listener with the default priority', () => {
+			const calls = [];
+
+			class Foo extends Observable {
+				method() {
+					calls.push( 'original' );
+				}
+			}
+
+			const foo = new Foo();
+
+			foo.decorate( 'method' );
+
+			foo.on( 'method', () => calls.push( 'high' ), { priority: 'high' } );
+			foo.on( 'method', () => calls.push( 'low' ), { priority: 'low' } );
+
+			foo.method();
+
+			expect( calls ).to.deep.equal( [ 'high', 'original', 'low' ] );
+		} );
+
+		it( 'supports overriding return values', () => {
+			class Foo extends Observable {
+				method() {
+					return 1;
+				}
+			}
+
+			const foo = new Foo();
+
+			foo.decorate( 'method' );
+
+			foo.on( 'method', evt => {
+				expect( evt.return ).to.equal( 1 );
+
+				evt.return = 2;
+			} );
+
+			expect( foo.method() ).to.equal( 2 );
+		} );
+
+		it( 'supports overriding arguments', () => {
+			class Foo extends Observable {
+				method( a ) {
+					expect( a ).to.equal( 2 );
+				}
+			}
+
+			const foo = new Foo();
+
+			foo.decorate( 'method' );
+
+			foo.on( 'method', ( evt, args ) => {
+				args[ 0 ] = 2;
+			}, { priority: 'high' } );
+
+			foo.method( 1 );
+		} );
+
+		it( 'supports stopping the event (which prevents execution of the orignal method', () => {
+			class Foo extends Observable {
+				method() {
+					throw new Error( 'this should not be executed' );
+				}
+			}
+
+			const foo = new Foo();
+
+			foo.decorate( 'method' );
+
+			foo.on( 'method', evt => {
+				evt.stop();
+			}, { priority: 'high' } );
+
+			foo.method();
+		} );
+
+		it( 'throws when trying to decorate non existing method', () => {
+			class Foo extends Observable {}
+
+			const foo = new Foo();
+
+			expect( () => {
+				foo.decorate( 'method' );
+			} ).to.throw( CKEditorError, /^observablemixin-cannot-decorate-undefined:/ );
+		} );
+	} );
 } );