Skip to content

Latest commit

 

History

History
178 lines (131 loc) · 9.03 KB

spring.md

File metadata and controls

178 lines (131 loc) · 9.03 KB
Raw

Clean Code Rules for the Spring Framework

Context

Spring beans should not be final

Rule-ID: spring.context-no-final-components

Classes that will be picked up as Spring Beans (@Component, @Service or similar) must not be final as final classes cannot be proxied (this is required in case you are using Spring AOP for things like @Transactional).

Functions annotated with @Cacheable, @CachePut or @CacheEvict should not be called from the same class

Rule-ID: spring.context-cacheable-annotated-functions-should-not-be-called-from-function-in-same-class

In order for annotations to work properly it has to be called from outside of the class. Calling functions internally will ignore the annotations. In this case the cacheables will be ignored.

Transaction Handling

In Spring-Boot based Java web projects the @Transactional annotation is typically used for transaction demarcation. There are various best-practices as well as several potential pitfalls out there:

  • Transactions with Spring and JPA
  • Understanding transaction pitfalls
  • Spring pitfalls: proxying
  • The goal is to provide a Cloudflight wide best-practice that is based on our needs and experienced problems from the past.

We want to:

  • Minimize the chance for unwanted side effects
  • Minimize the chance for performance issues at runtime
  • Minimize the chance for unwanted non-transactional behaviour and data corruption
  • Possibility to provide ArchUnit checks

Only annotate concrete classes with @Transactional

Springs annotation based declarative transaction management uses AOP-proxies (see Understanding the Spring Framework’s Declarative Transaction Implementation for further information). We want to follow the recommendation of the Spring team as stated in using @Transactional:

The Spring team recommends that you annotate only concrete classes (and methods of concrete classes) with the @Transactional annotation, as opposed to annotating interfaces. You certainly can place the @Transactional annotation on an interface (or an interface method), but this works only as you would expect it to if you use interface-based proxies. The fact that Java annotations do not inherit from interfaces means that, if you use class-based proxies (proxy-target-class="true") or the weaving-based aspect (mode="aspectj"), the transaction settings are not recognized by the proxying and weaving infrastructure, and the object is not wrapped in a transactional proxy.

See also Github Spring-Projects issue 18894 and issue 5423.

No @Transactional annotations on class level

Rule-ID: spring.tx-no-transactional-on-classlevel

It is compelling to just put the @Transactional on the class level, but a class-level annotation does not apply to ancestor classes up the class hierarchy; in such a scenario, methods need to be locally redeclared in order to participate in a subclass-level annotation. This can sometimes be troublesome when working with class-based proxies. Managing annotations on a class-level is also much more prone to side effects (see Pitfall #1), especially if your work in a shared codebase. Since the annotation changes the behavior of the code the scope that is affected by this change should be as small as possible.

Annotation inheritance in Java is only supported for type-level annotations. If type- and method-level annotations are mixed the actual behavior might differ from the expected as soon as you start overloading methods. Decision driver #1 is to minimize the chance of side effects, having only method-level annotations bears the least risk for such side effects, since they are never inherited and always have to be stated explicitly.

It is also considered best practice by others (see Spring @Transactional Annotation class or method).

Transaction demarcation is part of the service layer

Rule-ID: spring.tx-controller-methods-should-not-be-transactional

Even if it is tempting to open a transaction in the controller this can have a huge performance impact and might even make your application prone to denial-of-service attacks. Depending on the order of the different aspects applied to your controller data binding, input validation, authentication, and authorization might already be part of the transaction, meaning even unauthorized or fraudulent requests require a database connection. If you serialize complex datastructures or transfer larger amounts of data back to the user over a slow network, such operations might keep transactions open way longer than necessary. These are typical causes for spontaneous pool exhaustion or connection problems which you typically don't experience on your development machine or in isolated test environments. We therefore prohibit @Transactional on controller methods.

Controller methods should not access more than one transactional method

Rule-ID: spring.tx-controller-methods-should-not-access-more-than-one-transactional-method

The downside of the transaction demarcation rule from above is, that you might call multiple service methods within a single controller method causing potential database operations to be executed independently in separate transactions. This might be fine if you just read data to fill the view-model, but it might as well be problematic if accessed data is depending on each other. To minimize this pitfall we require that only one service call can be made within a single controller method.

Transactional methods should access other transactional methods or Repositories

Rule-ID: spring.tx-transactional-methods-should-access-other-transactional-methods-or-repositories

Being inside a transactional context is costly and should only be done if you access other transactional resources or the database itself via repositories. If there is no need to open a transaction, don't do it.

Repositories should only be called from transactional methods

Rule-ID: spring.tx-repository-only-from-transactional-methods

Whenever you access the database via Spring Data repositories, ensure you are within a transactional (i.e. it is called from a method that is annotated with @Transactional) to ensure ACID criteria when reading data from the DB. In case you are only reading from the database (and not writing), consider to use @Transactional(readOnly=true)

@Transactional annotated methods must not throw checked exceptions

Rule-ID: spring.tx-do-not-throw-exceptions

By default, only RuntimeException triggers a rollback of the current transaction, see declarative rolling back.

@Transactional from jakarta.transactional.Transactional should not be used

Rule-ID: spring.tx-no-jakarta-transactions-transactional-annotations

The Spring @Transactional annotation has more options than the jakarta @Transactional annotation and hence should be preferred.

Spring Web

Do not put @RequestMapping on top-level interface declarations

Rule-ID: spring.web-no-request-mapping-on-interface-top-level

Since Spring Cloud 2021.0.0 it is not supported anymore to create declarative Feign clients of interfaces which have a @RequestMapping annotation on the top level, that means that you can't create a Feign client from that interface any more:

@Api
@RequestMapping("/hello")
interface MyApi {

    @GetMapping("/world")
    fun world() : String
}

Instead, you need to merge that top-level annotation into the method annotations:

@Api
interface MyApi {

    @GetMapping("/hello/world")
    fun world() : String
}

or if you wanna keep the root context path a central constant:

@Api
interface MyApi {

    @GetMapping("$CONTEXT_PATH/world")
    fun world() : String

    companion object {
        private const val CONTEXT_PATH = "/hello"
    }
}

and in Java:

@Api
interface MyApi {

    String CONTEXT_PATH = "/hello";

    @GetMapping(CONTEXT_PATH + "/world")
    String world();
}