Friday, September 24, 2010

Interceptor Advisor Pattern

Your standard CDI/EJB Interceptor example uses a logger as an around advice. Generally this gives you an interceptor that looks like so:

@Log
public class LoggingInterceptor {

    private java.util.logging.Logger logger =
            java.util.logging.Logger.getLogger("theLogger");

    @AroundInvoke
    public Object intercept(InvocationContext context) throws Exception {
        logger.info("" + context.getMethod().getName());
        return context.proceed();
    }
}

As of the Interceptors 1.1 spec, you can bind that to a bean via creating our own javax.interceptor.InterceptorBinding annotation. In our example, we've created one called @Log:

@InterceptorBinding
@Target(value = {ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface Log {
}

Now we just apply that to the bean as follows.

@Log
public class FooBean {

    public void somethingCommon(){
        //...
    }

    public void somethingImportant() {
        //...
    }

    public void somethingNoteworthy() {
        //...
    }
}

Great! Now we are done. Every time that bean is invoked, the LoggerInterceptor will issue a log message on info level. Aren't interceptors wonderful! End of story, right? Not quite.

Fundamentally, our example is still very contrived. Who wants to log everything on the same level?

Here is a little pattern that you can use to better advise your LoggerInterceptor around advice. First, we create a couple new annotations for log levels: @Fine and @Info

@Target(value = {ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface Fine {
}

@Target(value = {ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface Info {
}

Then we apply those to the bean...

@Log
public class FooBean {

    public void somethingCommon(){
        //...
    }

    @Info
    public void somethingImportant() {
        //...
    }

    @Fine
    public void somethingNoteworthy() {
        //...
    }
}

Now we alter our LoggerInterceptor to check for our new annotations and alter its behavior.

@Log
public class LoggingInterceptor {

    private java.util.logging.Logger logger =
            java.util.logging.Logger.getLogger("theLogger");

    @AroundInvoke
    public Object intercept(InvocationContext context) throws Exception {
        final Method method = context.getMethod();
        if (method.isAnnotationPresent(Info.class)) {
            return info(context);
        } else if (method.isAnnotationPresent(Fine.class)) {
            return fine(context);
        } else {
            return finest(context);
        }
    }

    public Object info(InvocationContext context) throws Exception {
        logger.info("" + context.getMethod().getName());
        return context.proceed();
    }

    public Object fine(InvocationContext context) throws Exception {
        logger.finest("" + context.getMethod().getName());
        return context.proceed();
    }

    public Object finest(InvocationContext context) throws Exception {
        logger.finest("" + context.getMethod().getName());
        return context.proceed();
    }
}

Done! Now we have a pattern to advise our interceptor!

This will totally work today. But this pattern is so simple and elegant what if we could support it right inside the container? Imagine how cool it would be if we could just do this in our interceptor and the container would just figure it out.

@Log
public class LoggingInterceptor {

    private java.util.logging.Logger logger =
            java.util.logging.Logger.getLogger("theLogger");

    @Info
    public Object info(InvocationContext context) throws Exception {
        logger.info("" + context.getMethod().getName());
        return context.proceed();
    }

    @Fine
    public Object fine(InvocationContext context) throws Exception {
        logger.finest("" + context.getMethod().getName());
        return context.proceed();
    }

    @AroundInvoke
    public Object finest(InvocationContext context) throws Exception {
        logger.finest("" + context.getMethod().getName());
        return context.proceed();
    }
}

Definitely something I plan to propose for the next round of specifications....

As always, comments welcome.

Tuesday, September 21, 2010

EJB.next Interceptor Improvements - Method signatures

Interceptors are great, but some parts of them are odd when applied practically. For as fundamentally reflective as interceptors are, it isn't possible to pass all interceptable calls through a single method, like so:

import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import javax.ejb.PostActivate;
import javax.ejb.PrePassivate;
import javax.interceptor.AroundInvoke;
import javax.interceptor.AroundTimeout;
import javax.interceptor.InvocationContext;
import java.util.concurrent.TimeUnit;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

public class StatsInterceptor {

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundInvoke
    @AroundTimeout
    public Object intercept(InvocationContext context) throws Exception {
        final long start = System.nanoTime();
        try {
            return context.proceed();
        } finally {
            final long nanos = System.nanoTime() - start;
            final long millis = MILLISECONDS.convert(nanos, NANOSECONDS);
            System.out.println("invoke time "+ millis +" milleseconds");
        }
    }
}

This looks very nice, but currently is not legal. The spec currently requires different formats for callback-based interception vs business-method-based interception. This sounds like a nice safe and conservative rule, but is it benefiting users? Not really. Let's see what we have to do to our interceptor to make it compliant.

First, callback intercept methods are not allowed to return anything and must have a void return type. The logic here is that since callbacks have no return value, interceptors should know this. So fine, let's work around that and just make a new method:

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundInvoke
    public void callback(InvocationContext context) throws Exception {
        intercept(context);
    }

Second, callback intercept methods are not allowed to throw any exceptions. Again the rationale is that callbacks aren't allowed to throw checked exceptions, so interceptors should be aware of that as well. The trick is we can't simply remove the 'throws Exception' clause without doing any extra work. The InvocationContext.proceed() method throws java.lang.Exception but we can't, so we are forced to take responsibility for it. If we just catch and ignore the exception all hell will break loose, so instead we should at least wrap it as a RuntimeException.

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundTimeout
    public void callback(InvocationContext context) {
        try {
            intercept(context);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

We've just made a mistake. Did you see it? If there are several interceptors in the chain and all of them wrap the exception like this, when an actual exception is thrown it will end up wrapped N times -- once per interceptor -- before finally reaching the container. Instead, we should at least check if the exception needs wrapping and hope that other interceptors do the same:

    @PostConstruct
    @PreDestroy
    @PrePassivate
    @PostActivate
    @AroundTimeout
    public void callback(InvocationContext context) {
        try {
            intercept(context);
        } catch (Exception e) {
            if (e instanceof RuntimeException) {
                throw (RuntimeException) e;
            } else{
                throw new RuntimeException(e);
            }
        }
    }

Now we're done. Summary: all of the above is boilerplate code you need in any well implemented callback-based interceptor.

What have we achieved? Not much.

In terms of user code, it doesn't help you any to have these restrictions. If you wanted special logic for callback-based interceptor methods you can easily put that annotation on a separate method.

In terms of container code, there's little gain as well. Ultimately, most containers use reflection to kick off the interceptor chain. The java.lang.reflect.Method.invoke() method returns Object and throws InvocationTargetException. Restricting users from returning Object and throwing Exception have no real impact on the container code. It's a callback. Ultimately all exceptions, checked or not, go into the log file and no further and return values are simply ignored.

Summary: The restriction on returning void and throwing Exception should be lifted. For backwards compatibility we can just say this former requirement is now optional and allow interceptor signatures of 'Object <method>(InvocationContext c) throws Exception' even if what is being intercepted is a callback.