Statary / google-gin

Automatically exported from code.google.com/p/google-gin
0 stars 0 forks source link

Implement AOP #38

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
It should be a fairly straight-forward project to implement Guice AOP in
Gin. We can write subclasses of intercepted types and wrap matched methods
in our pre/post-processing. The interesting part of this implementation is
likely going to be the inclusion and partial re-write (super-source) of
method interceptors as provided by the aop alliance.

See also
http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/Binder.ht
ml#bindInterceptor(com.google.inject.matcher.Matcher,%20com.google.inject.matche
r.Matcher,%20org.aopalliance.intercept.MethodInterceptor...)

Original issue reported on code.google.com by aragos on 26 May 2009 at 2:41

GoogleCodeExporter commented 9 years ago
Any chance that this could move up in priority? I've been thinking about adding 
gwt-log 
to our GWT projects, but the thought of filling the source code with cross 
cutting 
concerns gives me nightmares :P

Original comment by arthur.k...@gmail.com on 13 Aug 2009 at 2:25

GoogleCodeExporter commented 9 years ago

Original comment by aragos on 1 Dec 2009 at 1:49

GoogleCodeExporter commented 9 years ago
Here is a proposal for AOP:

Add a method in the GinBinder interface, to bind an interceptor giving class 
matchers, method matchers and a key:

  void bindInterceptor(Matcher<? super Class<?>> classMatcher,
      Matcher<? super Method> methodMatcher, Key<? extends MethodInterceptor> 
interceptorKey);

The interceptor key implies that the MethodInterceptor must be a GIN managed 
instance. MethodInterceptor instances should not be interceptables to avoid 
recursive 
interceptions and StackOverflowExceptions.

GIN should generate a Proxy class for each intercepted class, and override the 
intercepted methods to invoke the interceptors. The overriden methods must 
invoke a 
MethodInvoker instance. The MethodInvoker is an utility class to produce an 
invocation chain with a unique MethodInvocation instance:

public class MethodInvoker {

  private final Object target;
  private final MethodInterceptor[] interceptors;

  public MethodInvoker(Object target, MethodInterceptor... interceptors) { ... }

  public <T> T invoke(Object... arguments) { ... }
}

MethodInvoker instances are placed as instance fields in the generated proxy. 
The 
proxy constructor must receive as arguments the interceptors and super 
constructor 
parameters, and initialize the invoker fields:

public class Foo {

  Foo(String salutation) { ... }

  public String sayHello(String name) { ... }
}

public class proxy_Foo extends Foo {

  private final MethodInvoker sayHello_invoker;

  public proxy_Foo(MethodInterceptor interceptor_1, MethodInterceptor interceptor_2, 
String name) {

    super(name);

    sayHello_invoker = new MethodInvoker(interceptor_1, interceptor_2, new 
MethodInterceptor() {

       public Object invoke(MethodInvocation invocation) throws Throwable {
         return Foo.super.sayHello((String) invocation.getArguments()[0]);
       }

    });
  }

  @Override
  public String sayHello(String name) {
    return sayHello_invoker.invoke(name);
  }
}

This pattern could be optimized when there is only one interceptor intercepting 
the 
method. A CallProxyConstructorBinding, a specialized CreatorBinding, adds 
instantiations for interceptable classes.
There is an issue with intercepted classes with zero arg constructors because 
they 
can't be instantiated with GWT.create() done that their interceptors are 
required in 
the constructor argument list. This will break the assumption that GIN is 
invoking 
generators for zero arg constructors.

Open Questions:

  1) Should the ServiceAsync's be interceptable? Done the fact that ServiceAsync are 
interfaces, they will be exclude by the contract of Guice to only intercept 
instantiable implementations, but a main interest to bring AOP to GWT is to 
manage 
remote interaction. We could ignore the Guice contract here and produce 
wrappers for 
ServiceAsyncs:

interface FooAsync {
   void sayHello(String name, AsyncCallback<String> callback);
}

class proxy_FooAsync implements FooAsync {

   private final MethodInvoker sayHello_invoker; 

   public proxy_FooAsync(MethodInterceptor interceptor1, MethodInterceptor 
interceptor2) {

     final FooAsync wrapped = GWT.create(FooService.class);

     sayHello_invoker = new MethodInvoker(interceptor1, interceptor2, new 
MethodInterceptor() {

       public Object invoke(MethodInvocation invocation) throws Throwable {
         wrapped.sayHello((String) invocation.getArguments()[0], (AsyncCallback) 
invocation.getArguments()[1]);
         return null;
       }
     };
   }

   public void sayHello(String name, AsyncCallback callback) {
      sayHello_invoker.invoke(name, callback); 
   }
}

  2) Should java.lang.Method be instantiated? The java.lang.Method class could be 
softly emulated to compile Matchers and MethodInvocation implementations 
without 
requiring instantiations. But for mostly AOP applications, a method reference 
is 
required. In our company project we want support for cache and logging, and it 
can't 
be possible without Method instances.

  3) Interception of MethodInterceptor instances should be allowed? We could allow 
interception for non bound MethodInterceptor, without raising StackOverflow 
issues.

  4) Interception of GinModules should be allowed? Given that GinModules are managed 
as GIN instances they could be accidentally intercepted.

We have developed a proof of concept patch with the proposal exposed here 
without 
solve the open questions. I will soon commit the patch.

Original comment by andres.a...@gmail.com on 11 Mar 2010 at 4:47

GoogleCodeExporter commented 9 years ago
Thanks for looking into this! 

My main concern with AOP is its heavy reliance on reflection at runtime. We 
can't
offer any introspection on the code during application runtime and I think this
restriction will make most traditional AOP approaches useless. Providing 
"Method"
instances for example won't work - there is no java class information left once 
the
GWT compiler has done its work.

That doesn't mean that we can't use AOP, we probably just need another 
approach. One
option that I could see working is to generate (custom) interceptor code at 
compile
time, i.e. offer the possibility to run "interceptor generators". Gin would 
then wire
the code produced by these interceptor generators into the object creation 
process.

An example:

// Implemented by Gin. Note that there is no "getMethod" call.
class GinMethodInvocation {
  Object[] getArguments() {...};
  Object getThis() {...};
  Object proceed() {...};
}

// Implementation generated by developer's generator.
interface GinMethodInterceptor {
  Object invoke(GinMethodInvocation invocation);
}

interface InterceptorGenerator {

  // Returns name of generated class implementing GinMethodInterceptor.
  String generate(TreeLogger logger, GeneratorContext ctx, Method intercepted)
      throws UnableToCompleteException;
}

void bindInterceptor(Matcher<? super Class<?>> classMatcher,
    Matcher<? super Method> methodMatcher, 
    InterceptorGenerator generator);

For class Foo, Gin would generate the following:

class proxy_Foo extends Foo {

  proxy_Foo(String p1) {
    super(p1);
  }

  public String sayHello(String name) {

    GinMethodInvocation invocation = new GinMethodInvocation(
        GWT.create("GeneratedInterceptorOne.class"),
        GWT.create("GeneratedInterceptorTwo.class"),
        new GinMethodInterceptor() {
          public Object invoke(GinMethodInvocation invocation) {
            return proxy_Foo.super.sayHello(name);
          }
        });
    return invocation.proceed();
  }
}

================

Your questions are still valid - the "magic" GWT.create calls that Gin performs 
don't
go well with proxies. I think I'd be fine with your special solution for async
services. Since it isn't generally applicable to all proxies we might have to 
give
developers the choice: GWT.create magic or AOP but not both.

I also would say we don't allow interception on modules and interceptors - the 
former
are not constructed by using dependency injection anyhow and the latter might 
also be
created on the fly as in the example above.

Original comment by aragos on 20 Mar 2010 at 12:06

GoogleCodeExporter commented 9 years ago
Hi Peter. I understand that AOP needs heavy use of reflection, but I think 
Guice 
Matchers performs most of the reflective work. Could a MethodInfo class provide 
the 
reflective information?

Original comment by andres.a...@gmail.com on 29 Mar 2010 at 12:56

GoogleCodeExporter commented 9 years ago
We could provide a custom MethodInfo class that holds some static information 
about a
method. There are two caveats: 

We can't just return class objects for the parameters, enclosing class etc. 
because
those won't be available at runtime. The AOP code will have to look very 
different
from what it usually is like, not inspecting any java properties but potentially
doing a lot of string matching.

I'm also not sure how efficient (especially in terms of code-size) that would 
be -
we'd have to have a lot of method information stored, depending on the number of
methods you'd be able to intercept.

I still think that running AOP on a generator model might be useful, most 
importantly
it gives the GWT compiler the opportunity to optimize the AOP code across the 
project
(which would be more in line with Gin's approach to replacing reflection). This 
also
gives the AOP code the advantage of being able to use any reflection necessary 
to
perform its function.

Original comment by aragos on 30 Mar 2010 at 10:28

GoogleCodeExporter commented 9 years ago
You're Right. My concern about generator solution, is how to inject the 
interceptors
with GIN managed instances, i. e. Guice allows interceptor injection by mean of
requestInjection(interceptor).

Original comment by andres.a...@gmail.com on 30 Mar 2010 at 10:41

GoogleCodeExporter commented 9 years ago
Yeah, if you use a generator based solution you won't be able to do anything at 
Runtime, but as aragos points out, a generator based approach would be more 
efficient 
and would allow the compiler to do all its code optimization magic.

Original comment by arthur.k...@gmail.com on 31 Mar 2010 at 1:22

GoogleCodeExporter commented 9 years ago
Well, you can still do stuff at runtime: The generator you write is allowed to
generate anything. It could for instance implement the approach with 
MethodInfo. :)
Allowing developers to use generators will give them a lot more flexibility 
though.

Original comment by aragos on 31 Mar 2010 at 1:25

GoogleCodeExporter commented 9 years ago
Well yeah, but the functionality would be set at compile time :P. But AOP 
support in any form would be pretty 
killer :)

Original comment by arthur.k...@gmail.com on 31 Mar 2010 at 2:05

GoogleCodeExporter commented 9 years ago
What do you think about managing the InterceptorGenerator as an internal GIN 
instance, 
to access NameGenerator, SourceWriterUtil, MemberCollector, etc.?

Original comment by andres.a...@gmail.com on 5 Apr 2010 at 5:01

GoogleCodeExporter commented 9 years ago
Sure, that wouldn't be a problem. We can create a nice small generator util API 
for this.

Original comment by aragos on 5 Apr 2010 at 6:10

GoogleCodeExporter commented 9 years ago
What I can't figure is how to bind managed instances to generated interceptors. 
Guice 
solves interceptor injection by means of requestInjection(interceptor). How we 
can 
inject generated interceptors?

Original comment by andres.a...@gmail.com on 5 Apr 2010 at 7:03

GoogleCodeExporter commented 9 years ago
I suggest we create a gin-specific binding that takes the matchers and generator
class (instead of matchers and interceptor instance) as arguments. We then 
process
all such bindings in the gin generator, invoking the interceptor generators and
calling their generated code from the code generated by gin.

Original comment by aragos on 6 Apr 2010 at 12:49

GoogleCodeExporter commented 9 years ago
I have some questions about your proposal:

1) Should bindInterceptor() bind InterceptorGenerator instances or classes? 
I.e.:

  a) binder.bindInterceptor(Matchers.any(), Matchers.any(), new 
MyInterceptorGenerator())

  b) binder.bindInterceptor(Matchers.any(), Matchers.any(), 
MyInterceptorGenerator.class)

I like alternative 'a'.

2) Should interceptor instance be scoped as Eager Singleton, Singleton, or 
Default? I 
think Eager Singleton fits with Guice semantics.

3) Super source issues. bindInterceptor() requires sources for 
java.lang.Method, 
InterceptorGenerator, and InterceptorGenerator subclass. How can we deal with 
that?

4) Interceptor injection. Suposse an interceptor instance requiring a bound 
instance,  
for example:

class MyGeneratedInterceptor implements GinInterceptor {

  @Inject
  Provider<EventBus> eventBusProvider;

  public Object invoke(GinMethodInvocation invocation) {
    InterceptionEvent.fire(eventBusProvider.get());
    return invocation.proceed();
  }
}

How can we solve Provider<EventBus> injection if MyGeneratedInterceptor 
instanciation 
is performed by GWT.create() ?

Original comment by andres.a...@gmail.com on 6 Apr 2010 at 11:14

GoogleCodeExporter commented 9 years ago
Is there any new on this topic?

Original comment by andres.a...@gmail.com on 12 Apr 2010 at 4:13

GoogleCodeExporter commented 9 years ago
Sorry for the delay, I'm quite busy with work these days. Here are some answers:

I have some questions about your proposal:

> 1) Should bindInterceptor() bind InterceptorGenerator instances or classes? 
I.e.:
>
>   a) binder.bindInterceptor(Matchers.any(), Matchers.any(), new 
MyInterceptorGenerator())
>
>   b) binder.bindInterceptor(Matchers.any(), Matchers.any(), 
MyInterceptorGenerator.class)
>
> I like alternative 'a'.

This is really an implementation detail. However, if we use b) we can 
instantiate the
class ourselves, using Guice to inject it with all kinds of useful utilities (if
requested).

> 2) Should interceptor instance be scoped as Eager Singleton, Singleton, or 
Default?
I think Eager Singleton fits with Guice semantics.

Sounds fine to me.

> 3) Super source issues. bindInterceptor() requires sources for 
java.lang.Method, 
> InterceptorGenerator, and InterceptorGenerator subclass. How can we deal with 
that?

We could either emulate Method with super-source or offer Gin-specific 
matchers. The
emulation shouldn't be a problem since configure() is only run at compile time 
when
we actually have all the necessary code available. Not sure which is more 
efficient.

> 4) Interceptor injection. Suposse an interceptor instance requiring a bound
instance, for example:
>
> class MyGeneratedInterceptor implements GinInterceptor {
>
>   @Inject
>   Provider<EventBus> eventBusProvider;
>
>   public Object invoke(GinMethodInvocation invocation) {
>     InterceptionEvent.fire(eventBusProvider.get());
>     return invocation.proceed();
>   }
> }
>
> How can we solve Provider<EventBus> injection if MyGeneratedInterceptor
instanciation is performed by GWT.create() ?

This is indeed a problem and somewhat related to issue #95. Solving it will be
complicated - in this case we could introduce a mechanism by which the 
generator lets
us know what the generated class requires but that's a bad hack. Let's solve it
properly in the other issue and then use the solution here.

Original comment by aragos on 12 Apr 2010 at 9:13

GoogleCodeExporter commented 9 years ago
I'm interesting in seeing andres' patch. Would it be possible to have SVN 
branch for this AOP proposal?

Original comment by dusan.ma...@gmail.com on 28 Apr 2010 at 5:49