Remove false positives from Transactional Proxy Rule

[fletchgqc]

I think that the ProxyRule

no_classes_should_directly_call_other_methods_declared_in_the_same_class_that_are_annotated_with(Transactional)

creates false positives. We are checking that no method calls Transactional methods in the same class. However, if the calling method is also Transactional, there is probably no problem. The Transaction is started by the calling method and includes the called method. Thefore, the rule should really be something like:

no_methods_not_annotated_with_should_directly_call_other_methods_declared_in_the_same_class_that_are_annotated_with

Do you agree with my logic?

[fletchgqc]

OK, I worked out the solution. The methods in ProxyRules.java should return ArchCondition<JavaCodeUnit>, for example:

  public static ArchCondition<JavaCodeUnit> directly_call_other_methods_declared_in_the_same_class_that_are_annotated_with(Class<? extends Annotation> annotationType) {
        return directly_call_other_methods_declared_in_the_same_class_that(are(annotatedWith(annotationType)));
    }

This would allow us to apply these methods with methods(). Like this:

  public static final ArchRule no_transaction_bypassing_calls = ArchRuleDefinition.noMethods().that().areNotAnnotatedWith(Transactional.class).should(directly_call_other_methods_declared_in_the_same_class_that_are_annotated_with(Transactional.class));

I made the changes locally and it worked for me.

[rweisleder]

What about this class?

public class MyService {

    @Transactional
    public void a() [
        b();
    }

    private void b() [
        c();
    }

    @Transactional
    public void c() [
        // ...
    }
}

Should the method call in b() be a violation? Currently, it will only be invoked inside a transaction started by a().

Another example from Spring:

class MyRepo {

    @Cacheable("x")
    public String x() [
        y();
        // ...
    }

    @Cacheable("y")
    public int y() [
        // ...
    }
}

In this case, the method call in x() should be considered as violation, because the two @Cacheable are independent.

I'm not really happy with ProxyRules and not sure if ArchUnit is the right place for that. Clearly, the example in the documentation is not the best.

That's why I started archunit-spring and support for @Transactional is on my todo list: https://github.com/rweisleder/archunit-spring/issues/2 (In case you are working with Jakarta EE instead, I plan to create dedicated rules for that as well.)