Guice-AOP invok "synthetic" method

[GoogleCodeExporter]

The test case:

@Retention(RUNTIME)
@Target(ElementType.TYPE)
@Inherited
public @interface MyAnno {}

public interface IToto {}

public class TotoImpl implements IToto {}

@MyAnno
public interface ITotoService {

    /**
     * A great method which will do something
     *
     * @return
     */
    IToto doSomething();
}

public class TotoServiceImpl implements ITotoService {

    public TotoImpl doSomething() {
        return null;
    }
}

package tests.guice;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.matcher.Matchers;

public class Main {

    public static void main(String[] args) {
        Injector injector = Guice.createInjector(new TotoModule());
        ITotoService totoService = injector.getInstance(ITotoService.class);

        // Method call
        totoService.doSomething();
    }
}

class TotoModule extends AbstractModule {

    @Override
    protected void configure() {
        bind(ITotoService.class).to(TotoServiceImpl.class);
        bindInterceptor(Matchers.any(), new
TypeAnnotatedMatcher(MyAnno.class), new TotoInterceptor());
    }

}

class TotoInterceptor implements MethodInterceptor {

    public Object invoke(MethodInvocation mi) throws Throwable {
        System.out.println(mi.getMethod().getName() + " -> isSynthetic ? "
                + mi.getMethod().isSynthetic());
        return mi.proceed();
    }
}

class TypeAnnotatedMatcher extends AbstractMatcher<Method> {

    private final Class<? extends Annotation> annClass;

    /**
     * Constructor.
     *
     * @param annClass
     */
    public TypeAnnotatedMatcher(final Class<? extends Annotation> annClass) {
        this.annClass = annClass;

    }

    /**
     * Search for the method signature into all the interface annotated with a
     * given annotation.
     */
    public boolean matches(Method method) {
        // Hook to avoid twice interceptions of the same method
        if (method.isSynthetic()) {
            //return false;

        }

        boolean matches = false;
        Class<?> type = method.getDeclaringClass();
        Class<?>[] iTypes = ReflectUtils.getInterfaces(type, annClass);
        for (Class<?> iType : iTypes) {
            Method iMethod = null;

            try {
                iMethod = iType.getDeclaredMethod(method.getName(), method
                        .getParameterTypes());
            } catch (SecurityException e) {
                // Should never happen
            } catch (NoSuchMethodException e) {
                // Should never happen
            }
            if (iMethod != null) {
                matches = true;
                break;
            }
        }

        return matches;
    }
}

class ReflectUtils {
    /**
     * Get type annoted with the given annotation
     *
     * @param type
     *            Type where we have to search for annoted interfaces
     *
     * @param annClass
     *            Annotation to find
     *
     * @return Class<?>[]
     */
    public final static Class<?>[] getInterfaces(final Class<?> type,
            final Class<? extends Annotation> annClass) {
        List<Class<?>> annTypes = new ArrayList<Class<?>>();
        Class<?>[] iTypes = type.getInterfaces();
        for (Class<?> iType : iTypes) {
            if (iType.isAnnotationPresent(annClass)) {
                annTypes.add(iType);
            }
        }
        return annTypes.toArray(new Class<?>[annTypes.size()]);
    }
}

Original issue reported on code.google.com by anthony....@gmail.com on 29 Sep 2008 at 9:37

[GoogleCodeExporter]

I discover the origin of this problem:

interface ITotoService {
     IToto doSomething();
}

class TotoServiceImpl {
     Toto doSomething() {
        return ..... ;
     }
}

Observe the return type of both methods.

First, the interface, returns IToto.
Second, the implementation returns Toto.

In this case, the invoke() method of a Matcher<Method> class is called twice!

I think Guice should not ask for invocation when a method is "synthetic" 
("volatile",
"bridge", ... any methods generated by compiler)...

What's your point of view?

Regards,
Anthony MÜLLER

Original comment by anthony....@gmail.com on 29 Sep 2008 at 9:38

[GoogleCodeExporter]

Hey Anthony . . . first off, apologies for the extremely slow turnaround. 

I tried to create a JUnit testcase that reproduces this and I couldn't. Could 
you send a patch that adds a test to MethodInterceptionTest that demonstrates 
the problem? Once I can reproduce it, I can probably fix it.

Original comment by limpbizkit on 30 Dec 2008 at 11:53

[GoogleCodeExporter]

Below is a smaller JUnit-style testcase that reproduces the problem:

  public void testDoesNotInterceptTwiceOnSyntheticMethods() {
    Injector injector = Guice.createInjector(new AbstractModule() {
      @Override
      protected void configure() {
        bind(Store.class).to(ConeyIslandStore.class);
        bindInterceptor(Matchers.any(), new AbstractMatcher<Method>() {
          public boolean matches(Method t) {
            System.out.println("synthetic: " + t.isSynthetic() + ", t: " + t);
            return true;
          }          
        }, new CountingInterceptor());
      }
    });
    Store store = injector.getInstance(Store.class);
    store.cookDog();
    assertEquals(1, count.get());
  }

  interface HotDog {}
  static class NathansDog implements HotDog {}
  interface Store {
    HotDog cookDog();
  }
  static class ConeyIslandStore implements Store {
    public NathansDog cookDog() {
      return null;
    }
  }

(where CountingInterceptor is an interceptor that increments a 'count' 
variable).

To be honest, I'm not certain we should fix this.  Yes, it can intercept twice 
if your matcher is broad... but if we rule out all synthetic methods, then it 
may prevent your matcher from matching entirely, because it may only have 
wanted to match on the return value from the synthetic method.  Will think on 
this a bit more...

Original comment by sberlin on 19 Feb 2011 at 9:19

[GoogleCodeExporter]

Issue 640 has been merged into this issue.

Original comment by sberlin on 5 Jul 2011 at 2:13