1
0
Fork 0

Added `proxy' keyword support

The concept of proxy methods will become an important, core concept in ease.js
that will provide strong benefits for creating decorators and proxies, removing
boilerplate code and providing useful metadata to the system. Consider the
following example:

  Class( 'Foo',
  {
      // ...

      'public performOperation': function( bar )
      {
          this._doSomethingWith( bar );
          return this;
      },
  } );

  Class( 'FooDecorator',
  {
      'private _foo': null,

      // ...

      'public performOperation': function( bar )
      {
          return this._foo.performOperation( bar );
      },
  } );

In the above example, `FooDecorator` is a decorator for `Foo`. Assume that the
`getValueOf()` method is undecorated and simply needs to be proxied to its
component --- an instance of `Foo`. (It is not uncommon that a decorator, proxy,
or related class will alter certain functionality while leaving much of it
unchanged.) In order to do so, we can use this generic, boilerplate code

  return this.obj.func.apply( this.obj, arguments );

which would need to be repeated again and again for *each method that needs to
be proxied*. We also have another problem --- `Foo.getValueOf()` returns
*itself*, which `FooDecorator` *also* returns.  This breaks encapsulation, so we
instead need to return ourself:

  'public performOperation': function( bar )
  {
      this._foo.performOperation( bar );
      return this;
  },

Our boilerplate code then becomes:

  var ret = this.obj.func.apply( this.obj, arguments );
  return ( ret === this.obj )
      ? this
      : ret;

Alternatively, we could use the `proxy' keyword:

  Class( 'FooDecorator2',
  {
      'private _foo': null,

      // ...

      'public proxy performOperation': '_foo',
  } );

`FooDecorator2.getValueOf()` and `FooDecorator.getValueOf()` both perform the
exact same task --- proxy the entire call to another object and return its
result, unless the result is the component, in which case the decorator itself
is returned.

Proxies, as of this commit, accomplish the following:
  - All arguments are forwarded to the destination
  - The return value is forwarded to the caller
    - If the destination returns a reference to itself, it will be replaced with
      a reference to the caller's context (`this`).
  - If the call is expected to fail, either because the destination is not an
    object or because the requested method is not a function, a useful error
    will be immediately thrown (rather than the potentially cryptic one that
    would otherwise result, requiring analysis of the stack trace).

N.B. As of this commit, static proxies do not yet function properly.
perfodd
Mike Gerwitz 2012-05-02 13:26:47 -04:00
parent bd0158928c
commit d84b86b21b
No known key found for this signature in database
GPG Key ID: F22BB8158EE30EAB
21 changed files with 409 additions and 16 deletions

View File

@ -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
*

View File

@ -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 )
{

View File

@ -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 );
};

View File

@ -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;
};
},
};

View File

@ -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 )
{

View File

@ -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' )()
),

View File

@ -34,6 +34,7 @@ var _keywords = {
'const': true,
'virtual': true,
'override': true,
'proxy': true,
};

View File

@ -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,

View File

@ -68,7 +68,7 @@ require( 'common' ).testCase(
);
this.sut = this.require( 'MemberBuilder' )(
stubFactory, stubFactory,
stubFactory, stubFactory, stubFactory,
this.mockValidate = this.getMock( 'MemberBuilderValidator' )
);

View File

@ -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"
);
},
} );

View File

@ -65,7 +65,7 @@ require( 'common' ).testCase(
);
this.sut = this.require( 'MemberBuilder' )(
stubFactory, stubFactory,
stubFactory, stubFactory, stubFactory,
this.mockValidate = this.getMock( 'MemberBuilderValidator' )
);

View File

@ -214,7 +214,7 @@ require( 'common' ).testCase(
);
this.sut = this.require( 'MemberBuilder' )(
stubFactory, stubFactory,
stubFactory, stubFactory, stubFactory,
this.getMock( 'MemberBuilderValidator' )
);

View File

@ -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'
);
},
} );

View File

@ -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
);
};

View File

@ -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

View File

@ -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"
);
} )();

View File

@ -460,14 +460,11 @@ var ConcreteFoo = Class.extend( AbstractFoo,
* a bug.
*/
( 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()' );
} )()

View File

@ -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()

View File

@ -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()

View File

@ -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()

View File

@ -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"
);
} )();