Feedback Requested for Method Interception API

[maxim.porges]

Hey guys,

I need some feedback on my plans for the interception API. Please see
the code sample below from my test suite - this is a test method.

public function testInterception() : void
{
var fixture : DynamicSubClass = new DynamicSubClass
("constructorArg");
fixture.__loomSetClosure(
"methodCallOne",
function(invocation : MethodInvocation) : void
{
trace("Before method: " +
invocation.methodName);
assertEquals(100, invocation.proceed());
trace("After method: " +
invocation.methodName);
}
);

fixture.methodCallOne("arg1", 100);
}

DynamicSubClass is a hard-coded implementation of a proxy, which I'm
using as a template to determine which opcodes Loom needs to weave.
The method call

__loomSetClosure(methodName : String, closure : Function)

will be weaved in to proxies and will allow you to provide a closure
that will be invoked instead of the method specified by name. As you
can see, in the test, I am tracing before and after the method call,
so you could call this around advice. The call to invocation.proceed()
(nested in the call to assertEquals()) calls the original method on
the proxy's superclass with the original arguments, and will return
whatever the superclass method returns.

All interception closures will receive a single argument, which will
be a MethodInvocation. MethodInvocation looks like this.

package loom.template
{
public class MethodInvocation
{
private var _instance : *;
private var _methodName : *;
private var _args : *;
private var _originalMethodReference : Function;

public function MethodInvocation(instance : *, methodName :
String, args : Array, originalMethodReference : Function)
{
_instance = instance;
_methodName = methodName;
_args = args;
_originalMethodReference = originalMethodReference;
}

public function proceed() : *
{
return originalMethodReference.apply(instance, args);
}

public function get instance() : *
{
return _instance;
}

public function get methodName() : String
{
return _methodName;
}

public function get args() : Array
{
return _args;
}

public function get originalMethodReference() : Function
{
return _originalMethodReference;
}
}
}

So, you should be able to get everything that you need from
MethodInvocation in order to determine what kind of advice you want to
apply. Now that I look at it, I think I'll also add the Class
reference for the original Class the method is being invoked on.
Perhaps the namespace (private, protected, public, custom) would be
useful too?

My questions for the list are:

1) What else should MethodInvocation make available?
2) Does the API look suitable for your needs and easy to use to apply
interception?
3) What other suggestions or changes would you have? Is there anything
you can see that I am missing?

After looking at the ABC opcode for the hard-coded template, it's a
little more complex than I had originally thought, but I still feel
good about having something useful ready next week.

Cheers,

- max

[Josh McDonald]

Hi Max,

It's looking good mate :) A few thoughts:

1) I think we shoud be using the world handler instead of closure. You can't have a Function without a closure (or a closure without a Function) in ActionScript, but they're not really the same thing, and if I don't at least suggest a change, Ola Bini is gonna come looking for me :)

2) rather than __loomSetClosure, why not a namespace? loom::setClosure looks nicer to me, but that's mainly a matter of taste.

3) What happens for getter and setter functions? Is this exposed in MethodInvocation?

4) Also, vars? Can they be wrapped in get/set functions at run-time, the way MXMLC does at compile-time when encountering [Bindable]?

I know, I'm a pain - but feedback it is :)

Cheers,
-Josh

2009/3/26 maxim.porges <maxim....@gmail.com>

--
"Therefore, send not to know For whom the bell tolls. It tolls for thee."

Josh 'G-Funk' McDonald
  -  jo...@joshmcdonald.info
  -  http://twitter.com/sophistifunk
  -  http://flex.joshmcdonald.info/

[maxim.porges]

Hey Josh,

Thanks for the rapid feedback.

> 1) I think we shoud be using the world handler instead of closure.

I'm fine with that - I've always referred to anonymous Function
definitions as closures. As often happens when I talk about
programming, I'm probably using the term incorrectly... :)

> 2) rather than __loomSetClosure, why not a namespace? loom::setClosure looks
> nicer to me, but that's mainly a matter of taste.

That's a good point. There's nothing stopping me from using a
namespace instead of an obfuscated method name, so I will head in that
direction instead. This will require the developer setting the closure
to use the namespace but that's hardly a big deal.

> 3) What happens for getter and setter functions? Is this exposed in
> MethodInvocation?

Getters and Setters are identical to methods from the AVM spec's
perspective, so they should play well with the existing proposal for
the interception API. The method name in the MethodInvocation will
just end up being the property name. Would it be useful to know if a
method invocation is a getter/setter? I can include that information
in the MethodInvocation object.

> 4) Also, vars? Can they be wrapped in get/set functions at run-time, the way
> MXMLC does at compile-time when encountering [Bindable]?

Hmm... yes, they could, but you might end up with potential conflicts.
I suppose we could take any public properties and replace them with
private properties wrapped by getters/setters that end up creating a
Loom invocation, which you could then intercept - what do you think? I
can't remember how AS3 responds if you take a property that was a
public var in the superclass and supersede (or subsede? :) ) it in a
subclass with a public property accessor; is that allowed? If so, that
sounds like a cool feature and I could definitely see this going in a
Loom point release.

Don't worry about being a pain - this is exactly the constructive
feedback I was hoping to get from you guys before the library goes
public.

Cheers,

- max

On Mar 26, 12:09 am, Josh McDonald <j...@joshmcdonald.info> wrote:
> Hi Max,
>
> It's looking good mate :) A few thoughts:
>
> 1) I think we shoud be using the world handler instead of closure. You can't
> have a Function without a closure (or a closure without a Function) in
> ActionScript, but they're not really the same thing, and if I don't at least
> suggest a change, Ola Bini is gonna come looking for me :)
>
> 2) rather than __loomSetClosure, why not a namespace? loom::setClosure looks
> nicer to me, but that's mainly a matter of taste.
>
> 3) What happens for getter and setter functions? Is this exposed in
> MethodInvocation?
>
> 4) Also, vars? Can they be wrapped in get/set functions at run-time, the way
> MXMLC does at compile-time when encountering [Bindable]?
>
> I know, I'm a pain - but feedback it is :)
>
> Cheers,
> -Josh
>

> 2009/3/26 maxim.porges <maxim.por...@gmail.com>

>   -  j...@joshmcdonald.info

[maxim.porges]

Anecdotally, my lack of correct terminology is documented with uncanny
accuracy on Wikipedia... guilty as charged. :)

http://en.wikipedia.org/wiki/Closure_(computer_science)

"The term closure is often mistakenly used to mean anonymous function.
This is probably because most languages implementing anonymous
functions allow them to form closures and programmers are usually
introduced to both concepts at the same time. These are, however,
distinct concepts."

- max


On Mar 26, 12:09 am, Josh McDonald <j...@joshmcdonald.info> wrote:

> Hi Max,
>
> It's looking good mate :) A few thoughts:
>
> 1) I think we shoud be using the world handler instead of closure. You can't
> have a Function without a closure (or a closure without a Function) in
> ActionScript, but they're not really the same thing, and if I don't at least
> suggest a change, Ola Bini is gonna come looking for me :)
>
> 2) rather than __loomSetClosure, why not a namespace? loom::setClosure looks
> nicer to me, but that's mainly a matter of taste.
>
> 3) What happens for getter and setter functions? Is this exposed in
> MethodInvocation?
>
> 4) Also, vars? Can they be wrapped in get/set functions at run-time, the way
> MXMLC does at compile-time when encountering [Bindable]?
>
> I know, I'm a pain - but feedback it is :)
>
> Cheers,
> -Josh
>

> 2009/3/26 maxim.porges <maxim.por...@gmail.com>

>   -  j...@joshmcdonald.info

[Josh McDonald]

I'm just lucky to have the likes of Ola and my mate Dhanji and a few others yelling at me on twitter when I do something stupid :)

With getters and setters, is it two methods with the same name? I'm also fairly certain ASC / AVM2 doesn't give a toss when you override properties with get/set pairs and vice versa, so long as you don't change the privacy qualifier, or replace a var with only a getter or a setter instead of both.

-Josh

2009/3/26 maxim.porges <maxim....@gmail.com>

  -  jo...@joshmcdonald.info

[maxim.porges]

Doesn't look like you can override a public property with a getter/
setter combo in a subclass. The following produces the compiler error:
"1023: Incompatible override". Taking off the override produces:
"1024: Overriding a function that is not marked for override".




public class BaseClass
{
public var stuff : String;
...

public class DynamicSubClass extends BaseClass
{
private var __loomProxies : Dictionary;

override public function get stuff() : String
{
return "";
}

override public function set stuff(value : String) : void
{
// nothing

}


On Mar 26, 12:52 am, Josh McDonald <j...@joshmcdonald.info> wrote:
> I'm just lucky to have the likes of Ola and my mate Dhanji and a few others
> yelling at me on twitter when I do something stupid :)
>
> With getters and setters, is it two methods with the same name? I'm also
> fairly certain ASC / AVM2 doesn't give a toss when you override properties
> with get/set pairs and vice versa, so long as you don't change the privacy
> qualifier, or replace a var with only a getter or a setter instead of both.
>
> -Josh
>

> 2009/3/26 maxim.porges <maxim.por...@gmail.com>

>
>
>
>
>
> > Anecdotally, my lack of correct terminology is documented with uncanny
> > accuracy on Wikipedia... guilty as charged. :)
>

> >http://en.wikipedia.org/wiki/Closure_(computer_science)<http://en.wikipedia.org/wiki/Closure_%28computer_science%29>

[Drew Bourne]

Hi Max,

Looking good.

I concur about using namespaces, this type of operation is an ideal
use-case as the the Loom methods should not appear as part of the
public API for the classes that are modified.

In the case of mocks and expectations knowing whether it is a method
or property that was invoked is required in order to evaluate that
invocation against the expectations.

Is there any reason for not allowing modification of the arguments
that will be passed to the original method? or the return value? Am I
just getting ahead of ourselves and its a to-do?

cheers,
Drew

[Drew Bourne]

NM, If I properly read the MethodInvocation class I would seen the
answer to my last question.

cheers,
Drew

[Maxim Porges]

Thanks Drew. Yeah, you should be able to use the bits and pieces from
the MethodInvocation to make your own proceed() call with your own
arguments. That being said, it seems like a convenience option to
proceed with different arguments is a sensible addition.

I'll plan on making an input array to proceed() an optional argument
defaulted to null, with the understanding that if nothing is specified
the original arguments are passed in Obviously, if arguments are
passed to proceed() they will stomp the original method arguments. Let
me know if you guys have any other suggestions.

- max

[Maxim Porges]

I was just thinking, we need a way to deal with throws advice as well.
I'll add that to the todo list.

- max

[Brian Kotek]

That sounds good as long as it will play nicely when more than one advice (i.e. an advice chain) is applied to a given method. It seems like it should technically *work*, but it might mean that folks who are changing the arguments may need to be careful since the target of the proceed() call may very well be another interceptor and not the original object, correct?

[Maxim Porges]

Hey Brian,

Yes, you are correct. However, the primary responsibility for Loom is to intercept a method invocation and hand off the invocation to a handler - not to provide the advice. A decent AOP-based framework wrapping Loom would create a single handler instance for each method call, which would then apply multiple sets of advice per method invocation by looping over a collection of handlers and supplying each with the MethodInvocation instance. This would be pretty easy to do through traditional means using the AS3 compiler rather than weaving it in to the bytecode of the proxy.

I'm going to put together a basic AOP sample app that does something like this as part of the Loom public beta. However, I believe Chris Scott was planning on putting an AOP framework around Loom that was independent of Swiz. Assuming he's still planning on doing this, I see Loom being to Chris's AOP framework what CGLIB is to AspectJ in the Java world. Likewise, Drew had plans for a mocking library, so the analogy applies to the way CGLIB supports JMock in Java.

- max

[Brian Kotek]

Ah, cool, thanks Max. I can see my understanding of how AOP is actually DONE in something like Java is pretty low. I didn't realize Spring and AspectJ used some other library to handle the actual weaving. I always thought it was something they "just did". Very interesting and now your response makes perfect sense.

[Maxim Porges]

Thanks Brian, glad that helped. Although, don't put too much credence in what I said about CGLIB and AspectJ - a lot of that is conjecture. I have a rudimentary enough understanding of these libraries to apply the concepts that make Loom work but I've never cracked the source trees for either project.

- max