diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index 3eaa117..b5f9d28 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -42,20 +42,26 @@ var util = require( __dirname + '/util' ), * * @param {Function} wrap_method method wrapper * @param {Function} wrap_override method override wrapper + * @param {Function} wrap_proxy method proxy wrapper * @param {MemberBuilderValidator} validate member validator * * @constructor */ -module.exports = function MemberBuilder( wrap_method, wrap_override, validate ) +module.exports = function MemberBuilder( + wrap_method, wrap_override, wrap_proxy, validate +) { // permit omitting 'new' keyword if ( !( this instanceof module.exports ) ) { - return new module.exports( wrap_method, wrap_override, validate ); + return new module.exports( + wrap_method, wrap_override, wrap_proxy, validate + ); } this._wrapMethod = wrap_method; this._wrapOverride = wrap_override; + this._wrapProxy = wrap_proxy; this._validate = validate; }; @@ -125,7 +131,11 @@ exports.buildMethod = function( ); // we might be overriding an existing method - if ( prev ) + if ( keywords[ 'proxy' ] ) + { + dest[ name ] = this._createProxy( value, instCallback, cid, name ); + } + else if ( prev ) { if ( keywords[ 'override' ] || prev_keywords[ 'abstract' ] ) @@ -355,6 +365,27 @@ function hideMethod( super_method, new_method, instCallback, cid ) } +/** + * Create a method that proxies to the method of another object + * + * @param {string} proxy_to name of property (of instance) to proxy to + * + * @param {Function} instCallback function to call in order to retrieve + * object to bind 'this' keyword to + * + * @param {number} cid class id + * @param {string} mname name of method to invoke on destination object + * + * @return {Function} proxy method + */ +exports._createProxy = function( proxy_to, instCallback, cid, mname ) +{ + return this._wrapProxy.wrapMethod( + proxy_to, null, cid, instCallback, mname + ); +}; + + /** * Generates a method override function * diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index bb5799e..5b53759 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -94,6 +94,25 @@ exports.prototype.validateMethod = function( ); } + if ( keywords[ 'proxy' ] ) + { + // proxies are expected to provide the name of the destination object + if ( typeof value !== 'string' ) + { + throw TypeError( + "Cannot declare proxy method '" + name + "'; string value " + + "expected" + ); + } + else if ( keywords[ 'abstract' ] ) + { + // proxies are always concrete + throw TypeError( + "Proxy method '" + name + "' cannot be abstract" + ); + } + } + // search for any previous instances of this member if ( prev ) { diff --git a/lib/MethodWrapperFactory.js b/lib/MethodWrapperFactory.js index 2c29c41..c1c6a80 100644 --- a/lib/MethodWrapperFactory.js +++ b/lib/MethodWrapperFactory.js @@ -55,9 +55,12 @@ module.exports = exports = function MethodWrapperFactory( factory ) * @param {number} cid class id that method is associated with * @param {function()} getInst function to determine instance and return * associated visibility object + * @param {string=} name name of method */ -exports.prototype.wrapMethod = function( method, super_method, cid, getInst ) +exports.prototype.wrapMethod = function( + method, super_method, cid, getInst, name +) { - return this._factory( method, super_method, cid, getInst ); + return this._factory( method, super_method, cid, getInst, name ); }; diff --git a/lib/MethodWrappers.js b/lib/MethodWrappers.js index bd2734d..4d3ecd1 100644 --- a/lib/MethodWrappers.js +++ b/lib/MethodWrappers.js @@ -81,5 +81,40 @@ exports.standard = { return retval; }; }, + + + wrapProxy: function( proxy_to, _, cid, getInst, name ) + { + return function() + { + var context = getInst( this, cid ) || this, + retval = undefined, + dest = context[ proxy_to ] + ; + + // rather than allowing a cryptic error to be thrown, attempt to + // detect when the proxy call will fail and provide a useful error + // message + if ( ( typeof dest !== 'object' ) + || ( typeof dest[ name ] !== 'function' ) + ) + { + throw TypeError( + "Unable to proxy " + name + "() call to '" + proxy_to + + "'; '" + proxy_to + "' is undefined or '" + name + + "' is not a function." + ); + } + + retval = dest[ name ].apply( dest, arguments ); + + // if the object we are proxying to returns itself, then instead + // return a reference to *ourself* (so as not to break encapsulation + // and to provide a more consistent and sensible API) + return ( retval === dest ) + ? this + : retval; + }; + }, }; diff --git a/lib/class.js b/lib/class.js index d5bd987..31701e1 100644 --- a/lib/class.js +++ b/lib/class.js @@ -34,6 +34,7 @@ var util = require( __dirname + '/util' ), require( __dirname + '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), MethodWrapperFactory( wrappers.wrapOverride ), + MethodWrapperFactory( wrappers.wrapProxy ), require( __dirname + '/MemberBuilderValidator' )( function( warning ) { diff --git a/lib/interface.js b/lib/interface.js index 40738c8..f918a37 100644 --- a/lib/interface.js +++ b/lib/interface.js @@ -29,6 +29,7 @@ var util = require( __dirname + '/util' ), member_builder = require( __dirname + '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), MethodWrapperFactory( wrappers.wrapOverride ), + MethodWrapperFactory( wrappers.wrapProxy ), require( __dirname + '/MemberBuilderValidator' )() ), diff --git a/lib/prop_parser.js b/lib/prop_parser.js index 88be1d0..9a27845 100644 --- a/lib/prop_parser.js +++ b/lib/prop_parser.js @@ -34,6 +34,7 @@ var _keywords = { 'const': true, 'virtual': true, 'override': true, + 'proxy': true, }; diff --git a/lib/util.js b/lib/util.js index 3f1b6ba..34c0fb6 100644 --- a/lib/util.js +++ b/lib/util.js @@ -333,7 +333,7 @@ exports.propParse = function( data, options ) ); } // method - else if ( typeof value === 'function' ) + else if ( ( typeof value === 'function' ) || ( keywords[ 'proxy' ] ) ) { callbackMethod.call( callbackMethod, diff --git a/test/MemberBuilder/GetterSetterTest.js b/test/MemberBuilder/GetterSetterTest.js index b9941dc..8dc5217 100644 --- a/test/MemberBuilder/GetterSetterTest.js +++ b/test/MemberBuilder/GetterSetterTest.js @@ -68,7 +68,7 @@ require( 'common' ).testCase( ); this.sut = this.require( 'MemberBuilder' )( - stubFactory, stubFactory, + stubFactory, stubFactory, stubFactory, this.mockValidate = this.getMock( 'MemberBuilderValidator' ) ); diff --git a/test/MemberBuilder/MethodTest.js b/test/MemberBuilder/MethodTest.js index aa1fe77..ecaee7f 100644 --- a/test/MemberBuilder/MethodTest.js +++ b/test/MemberBuilder/MethodTest.js @@ -59,13 +59,27 @@ require( 'common' ).testCase( setUp: function() { + var _self = this; + // stub factories used for testing var stubFactory = this.require( 'MethodWrapperFactory' )( function( func ) { return func; } ); + // used for testing proxies explicitly + var stubProxyFactory = this.require( 'MethodWrapperFactory' )( + function() + { + _self.proxyFactoryCall = arguments; + return _self.proxyReturnValue; + } + ); + + this.proxyFactoryCall = null; + this.proxyReturnValue = function() {}; + this.sut = this.require( 'MemberBuilder' )( - stubFactory, stubFactory, + stubFactory, stubFactory, stubProxyFactory, this.mockValidate = this.getMock( 'MemberBuilderValidator' ) ); @@ -127,4 +141,46 @@ require( 'common' ).testCase( this.assertEqual( true, called, 'validateMethod() was not called' ); }, + + + /** + * The `proxy' keyword should result in a method that proxies to the given + * object's method (both identified by name). + */ + "Creates proxy when `proxy' keyword is given": function() + { + var _self = this, + called = false, + + cid = 1, + name = 'foo', + value = 'bar', + keywords = { 'proxy': true }, + + instCallback = function() {} + ; + + // build the proxy + this.sut.buildMethod( + this.members, {}, name, value, keywords, instCallback, cid, {} + ); + + this.assertNotEqual( null, this.proxyFactoryCall, + "Proxy factory should be used when `proxy' keyword is provided" + ); + + this.assertDeepEqual( + [ value, null, cid, instCallback, name ], + this.proxyFactoryCall, + "Proxy factory should be called with proper arguments" + ); + + // ensure it was properly generated (use a strict check to ensure the + // *proper* value is returned) + this.assertStrictEqual( + this.proxyReturnValue, + this.members[ 'public' ][ name ], + "Generated proxy method should be properly assigned to members" + ); + }, } ); diff --git a/test/MemberBuilder/PropTest.js b/test/MemberBuilder/PropTest.js index 97631e2..47b5e26 100644 --- a/test/MemberBuilder/PropTest.js +++ b/test/MemberBuilder/PropTest.js @@ -65,7 +65,7 @@ require( 'common' ).testCase( ); this.sut = this.require( 'MemberBuilder' )( - stubFactory, stubFactory, + stubFactory, stubFactory, stubFactory, this.mockValidate = this.getMock( 'MemberBuilderValidator' ) ); diff --git a/test/MemberBuilder/VisibilityTest.js b/test/MemberBuilder/VisibilityTest.js index 7b457a7..fa05f99 100644 --- a/test/MemberBuilder/VisibilityTest.js +++ b/test/MemberBuilder/VisibilityTest.js @@ -214,7 +214,7 @@ require( 'common' ).testCase( ); this.sut = this.require( 'MemberBuilder' )( - stubFactory, stubFactory, + stubFactory, stubFactory, stubFactory, this.getMock( 'MemberBuilderValidator' ) ); diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index 781705d..750a312 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -378,5 +378,54 @@ require( 'common' ).testCase( _self.quickVisChangeTest( cur[ 0 ], cur[ 1 ], true, 'conflict' ); } ); }, + + + /** + * Proxies forward calls to other properties of a given instance. The only + * way to represent those properties is by name, which we will use a string + * to accomplish. Therefore, the value of a proxy method must be the name of + * the property to proxy to (as a string). + */ + "`proxy' keyword must provide string value": function() + { + var name = 'foo', + _self = this; + + this.quickFailureTest( name, 'string value expected', function() + { + // provide function instead of string + _self.sut.validateMethod( + name, function() {}, { 'proxy': true }, {}, {} + ); + } ); + }, + + + /** + * Similar to the above test, but asserts that string values are permitted. + */ + "`proxy' keyword can provide string value": function() + { + var _self = this; + + this.assertDoesNotThrow( function() + { + _self.sut.validateMethod( + 'foo', 'dest', { 'proxy': true }, {}, {} + ); + }, TypeError ); + }, + + + /** + * It does not make sense for a proxy to be abstract; proxies are concrete + * by definition (in ease.js' context, at least). + */ + 'Method proxy cannot be abstract': function() + { + this.quickKeywordMethodTest( [ 'proxy', 'abstract' ], + 'cannot be abstract' + ); + }, } ); diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js index 0b39701..2f65cf2 100644 --- a/test/MemberBuilderValidator/inc-common.js +++ b/test/MemberBuilderValidator/inc-common.js @@ -113,8 +113,11 @@ exports.quickKeywordTest = function( var testfunc = function() { + // proxies use strings, while all others use functions + var val = ( keyword_obj[ 'proxy' ] ) ? 'proxyDest': function() {}; + _self.sut[ type ]( - name, function() {}, keyword_obj, prev_data, prev_obj + name, val, keyword_obj, prev_data, prev_obj ); }; diff --git a/test/MethodWrapperFactoryTest.js b/test/MethodWrapperFactoryTest.js index ed9e302..01d1da8 100644 --- a/test/MethodWrapperFactoryTest.js +++ b/test/MethodWrapperFactoryTest.js @@ -64,10 +64,13 @@ var common = require( './common' ), super_method = function() {}, cid = 55, getInst = function() {}, + name = 'someMethod', retval = 'foobar'; var result = Sut( - function( given_method, given_super, given_cid, givenGetInst ) + function( + given_method, given_super, given_cid, givenGetInst, givenName + ) { called = true; @@ -87,9 +90,13 @@ var common = require( './common' ), "Factory method should be provided with proper inst function" ); + assert.equal( givenName, name, + "Factory method should be provided with proper method name" + ); + return retval; } - ).wrapMethod( method, super_method, cid, getInst ); + ).wrapMethod( method, super_method, cid, getInst, name ); // we'll include this in addition to the following assertion (which is // redundant) to make debugging more clear diff --git a/test/MethodWrappersTest.js b/test/MethodWrappersTest.js index 868659e..a5a1564 100644 --- a/test/MethodWrappersTest.js +++ b/test/MethodWrappersTest.js @@ -180,3 +180,165 @@ var common = require( './common' ), ); } )(); + +/** + * The proxy wrapper should forward all arguments to the provided object's + * appropriate method. The return value should also be proxied back to the + * caller. + */ +( function testProxyWillProperlyForwardCallToDestinationObject() +{ + var name = 'someMethod', + propname = 'dest', + + args = [ 1, {}, 'three' ], + args_given = [], + + getInst = function() + { + return inst; + }, + + method_retval = {}, + dest = { + someMethod: function() + { + args_given = Array.prototype.slice.call( arguments ); + return method_retval; + }, + }, + + // acts like a class instance + inst = { dest: dest }, + + proxy = sut.standard.wrapProxy( propname, null, 0, getInst, name ) + ; + + assert.strictEqual( method_retval, proxy.apply( inst, args ), + "Proxy call should return the value from the destination" + ); + + assert.deepEqual( args, args_given, + "All arguments should be properly forwarded to the destination" + ); +} )(); + + +/** + * If the destination object returns itself, then we should return the context + * in which the proxy was called; this ensures that we do not break + * encapsulation. Consequently, it also provides a more consistent and sensical + * API and permits method chaining. + * + * If this is not the desired result, then the user is free to forefit the proxy + * wrapper and instead use a normal method, manually proxying the call. + */ +( function testProxyReturnValueIsReplacedWithContextIfDestinationReturnsSelf() +{ + var propname = 'foo', + method = 'bar', + + foo = { + bar: function() + { + // return "self" + return foo; + } + }, + + inst = { foo: foo }, + + ret = sut.standard.wrapProxy( + propname, null, 0, + function() + { + return inst; + }, + method + ).call( inst ) + ; + + assert.strictEqual( inst, ret, + "Proxy should return instance in place of destination, if returned" + ); +} )(); + + +// common assertions between a couple of proxy tests +function proxyErrorAssertCommon( e, prop, method ) +{ + assert.ok( + e.message.search( 'Unable to proxy' ) > -1, + "Unexpected error received: " + e.message + ); + + assert.ok( + ( ( e.message.search( prop ) > -1 ) + && ( e.message.search( method ) > -1 ) + ), + "Error should contain property and method names" + ); +} + + +/** + * Rather than allowing a cryptic error to be thrown by the engine, take some + * initiative and attempt to detect when a call will fail due to the destination + * not being an object. + */ +( function testProxyThrowsErrorIfCallWillFailDueToNonObject() +{ + var prop = 'noexist', + method = 'foo'; + + try + { + // should fail because 'noexist' does not exist on the object + sut.standard.wrapProxy( + prop, null, 0, + function() { return {}; }, + method + )(); + } + catch ( e ) + { + proxyErrorAssertCommon( e, prop, method ); + return; + } + + assert.fail( + "Error should be thrown if proxy would fail due to a non-object" + ); +} )(); + + +/** + * Rather than allowing a cryptic error to be thrown by the engine, take some + * initiative and attempt to detect when a call will fail due to the destination + * method not being a function. + */ +( function testProxyThrowsErrorIfCallWillFailDueToNonObject() +{ + var prop = 'dest', + method = 'foo'; + + try + { + // should fail because 'noexist' does not exist on the object + sut.standard.wrapProxy( + prop, null, 0, + function() { return { dest: { foo: 'notafunc' } }; }, + method + )(); + } + catch ( e ) + { + proxyErrorAssertCommon( e, prop, method ); + return; + } + + assert.fail( + "Error should be thrown if proxy would fail due to a non-function" + ); +} )(); + diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index 7bc40c2..9ca321a 100644 --- a/test/test-class-abstract.js +++ b/test/test-class-abstract.js @@ -461,13 +461,10 @@ var ConcreteFoo = Class.extend( AbstractFoo, */ ( function testImplementingInterfacesWillPreserveAbstractClassDeclaration() { - assert.doesNotThrow( function() - { // if not considered abstract, extend() will fail, as it will contain // abstract member foo AbstractClass( 'TestImplExtend' ) .implement( Interface( { foo: [] } ) ) .extend( {} ); - }, Error, 'Class should still be abstract after implement().extend()' ); } )() diff --git a/test/test-class_builder-const.js b/test/test-class_builder-const.js index 6668b44..5279d72 100644 --- a/test/test-class_builder-const.js +++ b/test/test-class_builder-const.js @@ -38,6 +38,7 @@ require( 'common' ).testCase( this.require( '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), MethodWrapperFactory( wrappers.wrapOverride ), + MethodWrapperFactory( wrappers.wrapProxy ), this.getMock( 'MemberBuilderValidator' ) ), this.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() diff --git a/test/test-class_builder-static.js b/test/test-class_builder-static.js index 33d6669..0557af1 100644 --- a/test/test-class_builder-static.js +++ b/test/test-class_builder-static.js @@ -40,6 +40,7 @@ require( 'common' ).testCase( common.require( '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), MethodWrapperFactory( wrappers.wrapOverride ), + MethodWrapperFactory( wrappers.wrapProxy ), this.getMock( 'MemberBuilderValidator' ) ), common.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() diff --git a/test/test-class_builder-visibility.js b/test/test-class_builder-visibility.js index 9c5d559..f2bc98f 100644 --- a/test/test-class_builder-visibility.js +++ b/test/test-class_builder-visibility.js @@ -43,6 +43,7 @@ require( 'common' ).testCase( this.require( '/MemberBuilder' )( MethodWrapperFactory( wrappers.wrapNew ), MethodWrapperFactory( wrappers.wrapOverride ), + MethodWrapperFactory( wrappers.wrapProxy ), this.getMock( 'MemberBuilderValidator' ) ), this.require( '/VisibilityObjectFactoryFactory' ).fromEnvironment() diff --git a/test/test-util-prop-parse.js b/test/test-util-prop-parse.js index b3d3935..6ca81df 100644 --- a/test/test-util-prop-parse.js +++ b/test/test-util-prop-parse.js @@ -46,6 +46,9 @@ var data = { // abstract method abstractMethod: util.createAbstractMethod(), + + // proxy + 'proxy someProxy': 'dest', }; // only add getter/setter if it's supported by our engine @@ -82,6 +85,13 @@ util.propParse( data, { { delete chk_each[ name ]; } + + // TODO: Odd case. Perhaps this doesn't belong here or we can rewrite + // this test. + if ( name === 'someProxy' ) + { + delete chk_each[ 'proxy someProxy' ]; + } }, property: function( name, value ) @@ -209,3 +219,18 @@ assert.equal( }, SyntaxError, 'Valid var names as args should not throw exceptions' ); } )(); + +/** + * Proxies, since their values are strings, would conventionally be considered + * properties. Therefore, we must ensure that the `proxy' keyword is properly + * applied to return a method rather than a property. + */ +( function testProxiesAreConsideredMethodsDespiteTheirStringValues() +{ + assert.equal( + methods.someProxy, + data[ 'proxy someProxy' ], + "Properties with `proxy' keyword should be considered to be methods" + ); +} )(); +