views:

635

answers:

2

In my Spring context file I have something like this:

<bean id="userCheck" class="a.b.c.UserExistsCheck"/>
<aop:config>
      <aop:aspect ref="userCheck">
         <aop:pointcut id="checkUser"
                expression="execution(* a.b.c.d.*.*(..)) &amp;&amp; args(a.b.c.d.RequestObject)"/>
         <aop:around pointcut-ref="checkUser" method="checkUser"/>
      </aop:aspect>
</aop:config>    

a.b.c.UserExistsCheck looks like this:

@Aspect
public class UserExistsCheck {

@Autowired
private UserInformation userInformation;

public Object checkUser(ProceedingJoinPoint pjp) throws Throwable {
    int userId = ... //get it from the RequestObject passed as a parameter
    if (userExists(userId)) {
        return pjp.proceed();
    } else {
        return new ResponseObject("Invalid user);
    }
}

And the class that is being intercepted with this stuff looks like this:

public class Klazz {
    public ResponseObject doSomething(RequestObject request) {...}
}

This works. UserExistCheck is executed as desired before the call is passed to Klazz. The problem is that this is the only way I got it working. To get this working by using annotations instead of the context file seems to be just too much for my small brain. So... how exactly should I annotate the methods in UserExistsCheck and Klazz? And do I still need something else too? Another class? Still something in the context file?

+2  A: 

Have you enabled annotation-based AOP? The documentation says you have to add

<aop:aspectj-autoproxy/>

to your spring configuration. Then you need to add an annotation in front of your checkUser method. It looks like you want @Around advice, as described here.

@Aspect
public class UserExistsCheck {

  @Around("execution(* a.b.c.d.*.*(..)) && args(a.b.c.d.RequestObject)")
  public Object checkUser(ProceedingJoinPoint pjp) throws Throwable {
John
Yes, I have enabled <aop:aspectj-autoproxy/> from the beginning. I tried this as well, but doesn't work, the checkUser method is never called :(
fish
Actually this _did_ work, I just had a stupid mistake in the args, thanks! Now I wonder what I did wrong originally before trying the context file approach...
fish
+2  A: 

From the example code you've provided, it appears that you're trying to create advice for a class that doesn't implement any interfaces. As described in the Proxying Mechanisms section of the Spring docs, if you're going to do this, you'll need to enable CGLIB:

<aop:aspectj-autoproxy proxy-target-class="true" />

I've personally found this to be a bit more finicky than the documentation indicates it should be, and though it does work if all of the stars are aligned just right, it's often easier--and preferable from a design standpoint--to declare your AOP advice on an interface, as follows. (Note that you'll need to obtain your instance of KlazzImpl from your BeanFactory/ApplicationContext.)

public class Klazz {
  ResponseObject doSomething(RequestObject request);
}

public class KlazzImpl implements Klazz {
  public ResponseObject doSomething(RequestObject request) {...}
}

Additionally, your use of the args expression is a little bit off. See the following:

@Aspect
public class UserExistsCheck {
  @Autowired
  private UserInformation userInformation;

  @Around("execution(* a.b.c.d.*.*(..)) && args(reqObj)")
  public Object checkUser(ProceedingJoinPoint pjp, a.b.c.d.RequestObject reqObj) throws Throwable {
      // ...
  }
}

These changes should do the job.

RTBarnard
Thanks for the tips, actually Klazz does implement an interface but I'm using Spring autowiring for creating the instance. Sorry for not informing about this, I thought it's not relevant. Anyway all the info is useful, thanks!
fish