Statically finding methods that need @Transactional but don't have it

Question

In order to avoid database connection leaks, it's possible to configure c3p0 to let you know when they occur, and point to the problem code:

<property name="unreturnedConnectionTimeout" value="900"/>  <!-- seconds -->
<property name="debugUnreturnedConnectionStackTraces" value="true"/>

It'd be better to catch these before they are ever deployed. That is, catch them statically at build time. It seems that it would possible to 1) Identify classes that are DAO-type classes, either annotated as such or via an hbm file, and then 2) trace call trees upward and flag methods that are not marked @Transactional. Even if there were some false positives, that'd be a useful tool to help eliminate this issue. IDEs such as IntelliJ and Eclipse already know how to find a method's callers.

Is there an opensource tool that does something like this? Even if it didn't do the first step, it's easy enough to identify DAOs manually. It's the second part that would benefit most from an automated solution.

Answer 1

I faced a similar problem in a Spring application, and wrote a bean processer that ran at application startup, and examined each bean after it was initialized. It's not a compile-time solution, but assuming you run the application somewhere before production, you'll have an opportunity to find methods lacking the annotation in question.

It looked for classes with the @Service annotation, iterated over interfaces implemented by that class, and inspected each interface's methods. It looked for methods that either didn't have any method-level security defined, or had a custom @Unsecured annotation, indicating we had already discovered that method lacked security but determined that it didn't need security. Any methods that lacked security and the @Unsecured annotation were logged.

The following code worked at one point, but was used with an older version of Spring. I don't know if it will work with Spring 3.x, but if not, a similar approach should still work. You might need to adjust the inspection logic to suite your needs (e.g. look for classes with a different class-level annotation, inspect methods on the class itself instead of methods in interfaces implemented by the class, etc). Note that my approach didn't try to traverse call trees, so you'd probably get more false positives (i.e. not all service methods end up calling DAO methods).

public class UnsecuredServiceMethodProcessor implements BeanPostProcessor {
    private static final Logger logger = LogManager.getLogger(UnsecuredServiceMethodProcessor.class);
    private final MethodSecurityInterceptor interceptor;

    public UnsecuredServiceMethodProcessor(MethodSecurityInterceptor interceptor) {
        this.interceptor = interceptor;
    }

    @Override
    public Object postProcessAfterInitialization(Object bean, String beanName)
            throws BeansException {
        Class<?> beanClass = bean.getClass();
        if (logger.isInfoEnabled()) {
            logger.info("checking bean " + beanName + " of type " + beanClass.getName());
        }
        for (Class<?> interfaceClass: beanClass.getInterfaces()) {
            checkClass(beanClass, interfaceClass);
        }

        return bean;
    }

    /**
     * @param beanClass
     * @param interfaceClass
     */
    private void checkClass(Class<?> beanClass, Class<?> interfaceClass) {
        if (interfaceClass.isAnnotationPresent(Service.class)) {
            if (logger.isDebugEnabled()) {
                logger.debug("found service implementation: " + interfaceClass + " on " + beanClass);
            }
            for (Method method: interfaceClass.getMethods()) {
                if (!method.isAnnotationPresent(Unsecured.class)) {
                    if (logger.isDebugEnabled()) {
                        logger.debug("checking " + method.getName());
                    }
                    MethodSecurityMetadataSource msms = interceptor.getSecurityMetadataSource();
                    Collection<ConfigAttribute> atts = msms.getAttributes(method, interfaceClass);
                    if (atts == null || atts.size() == 0) {
                        logger.warn("unsecured method: " +  method.getDeclaringClass().getName() + "." + method.getName());
                    }
                }
            }
        }
    }

    @Override
    public Object postProcessBeforeInitialization(Object bean, String beanName)
            throws BeansException {
        return bean;
    }

}

You'll need to either explicitly define a bean of this type in the application context, or add the @Component class-level annotation and scan for it.