Why we shouldn't make a Spring MVC controller @Transactional?
Asked Answered
B

4

94

There are already a few questions about the topic, but no response at all really provides arguments in order to explain why we shouldn't make a Spring MVC controller Transactional. See:

So, why?

  • Is there insurmountable technical issues?
  • Is there architectural issues?
  • Is there performance/deadlock/concurrency issues?
  • Are sometimes multiple separate transactions required? If yes, what are the use cases? (I like the simplifying design, that calls to server either completely succeed or completely fail. It sounds to be a very stable behavior)

Background: I worked a few years ago in a Team on a quite large ERP Software implemented in C#/NHibernate/Spring.Net. The round-trip to the server was exactly implemented like that: the transaction was opened before entering any controller logic and was committed or rollbacked after exiting the controller. The transaction was managed in the framework so that no one had to care about it. It was a brilliant solution: stable, simple, only a few architects had to care about transaction issues, the rest of the team just implemented features.

From my point of view, it is the best design I have ever seen. As I tried to reproduce the same design with Spring MVC, I entered in a nightmare with lazy-loading and transaction issues and every time the same answer: don't make the controller transactional, but why?

Thank you in advance for your founded answers!

Boon answered 16/4, 2014 at 19:46 Comment(0)
S
149

TLDR: this is because only the service layer in the application has the logic needed to identify the scope of a database/business transaction. The controller and persistence layer by design can't/shouldn't know the scope of a transaction.

The controller can be made @Transactional, but indeed it's a common recommendation to only make the service layer transactional (the persistence layer should not be transactional either).

The reason for this is not technical feasibility, but separation of concerns. The controller responsibility is to get the parameter requests, and then call one or more service methods and combine the results in a response that is then sent back to the client.

So the controller has a function of coordinator of the request execution, and transformer of the domain data to a format the client can consume such as DTOs.

The business logic resides on the service layer, and the persistence layer just retrieve / stores data back and forth from the database.

The scope of a database transaction is really a business concept as much as a technical concept: in an account transfer an account can only be debited if the other is credited etc., so only the service layer that contains the business logic can really know the scope of a bank account transfer transaction.

The persistence layer cannot know what transaction it's in, take for example a method customerDao.saveAddress. Should it run in it's own separate transaction always? there is no way to know, it depends on the business logic calling it. Sometimes it should run on a separate transaction, sometimes only save it's data if the saveCustomer also worked, etc.

The same applies to the controller: should saveCustomer and saveErrorMessages go in the same transaction? You might want to save the customer and if that fails then try to save some error messages and return a proper error message to the client, instead of rolling back everything including the error messages you wanted to save on the database.

In non transactional controllers, methods returning from the service layer return detached entities because the session is closed. This is normal, the solution is to either use OpenSessionInViewor do queries that eager fetch the results the controller knows it needs.

Having said that, it's not a crime to make controllers transactional, it's just not the most frequently used practice.

Soniferous answered 16/4, 2014 at 21:40 Comment(11)
Thank you! 1) You say: The persistence layer should not be transactional either. What do you mean exactly? Spring documentation: CRUD methods on repository instances are transactional by default. 2) You say: The controller responsibility is to combine the results in a response. Ok, but for that purpose, you need a spanned transaction over service calls performing the business logic and the conversion. 3) Is "Eager fetching" not a dependency inversion from services to controllers?Boon
'CRUD methods on repository instances are transactional by default' means that by default they run in propagation REQUIRED, meaning they join an ongoing transaction and if none is present run in their own transaction.Soniferous
'you need a spanned transaction over service calls performing the business logic and the conversion' - if it's a spanned transaction, then it probably should be in a business method in the service layer, as it can be reused elsewhere in the application. if it's in the controller then it would be a business method only callable via HTTP, this is not completelly wrong, just not the most frequent designSoniferous
Eager fetching" not a dependency inversion from services to controllers? - there is no clear answer to that, the posts on that are usually closed as opinion based. have a look at my blog post on the comment above for the usual pros and cons, indeed you have a point, but the alternatives such as using open session in view can have some pitfalls tooSoniferous
but the design you propose of making the controller transactional might be perfectly fine in many ocasions, if it simplifies your life by all means use it, there are no technical constraints, a controller can be transactional like any other bean. Hope it helpsSoniferous
yes, it helps! I don't want to take an arbitrary decision for my current project, so the more feedback I get, the better! Thank you for the time spent to answer!Boon
@jhadesdev Your example is logical. We should not write transactional in dao or in controller. But what do u mean in service layer? For instance I have rest style web service, where I have injected dao interface. So From controller I persist data. In this case, where should I write transactional annotation?Threshold
If there's a constraint on the bean whose validator uses a service (e.g. to check if an email address is unique) the controller is very much the right place to span the transaction.Flap
If I introduce service layer, should I introduce a new request model for service invocation ? Client Request model -> Service invocation model -> Service call.For complex business, there is no way to just use several parameter to call all method.Organization
The same applies to the controller: should saveCustomer and saveErrorMessages go in the same transaction? You might want to save the customer and if that fails then try to save some error messages and return a proper error message to the client, instead of rolling back everything including the error messages you wanted to save on the database." i have understood the meaning that rollback shouldn't be happened of saveErrorMessages but i am missing the context. still not cleared why transcational annotation not be used on controller. please explainHomogenesis
Kind of subquestion. Request often needs to be validated. Sometimes validators have to access database. E.g. user registration, check that user with given e-mail does not already exist. Checks should happen at the same transaction with business logic. So, validators should be called from services (which define transaction scope)?Uriia
M
35

I have seen both cases in practice, in medium- to large-sized business web applications, using various web frameworks (JSP/Struts 1.x, GWT, JSF 2, with Java EE and Spring).

In my experience, it's best to demarcate transactions at the highest level, ie, at the "controller" level.

In one case, we had a BaseAction class extending Struts' Action class, with an implementation for the execute(...) method that handled Hibernate session management (saved into a ThreadLocal object), transaction begin/commit/rollback, and the mapping of exceptions to user-friendly error messages. This method would simply rollback the current transaction if any exception got propagated up to this level, or if it was marked for rollback only; otherwise, it would commit the transaction. This worked in every case, where normally there is a single database transaction for the whole HTTP request/response cycle. Rare cases where multiple transactions were needed would be handled in use-case specific code.

In the case of GWT-RPC, a similar solution was implemented by a base GWT Servlet implementation.

With JSF 2, I've so far only used service-level demarcation (using EJB Session beans which automatically have "REQUIRED" transaction propagation). There are disadvantages here, as opposed to demarcating transactions at the level of the JSF backing beans. Basically, the problem is that in many cases the JSF controller needs to make several service calls, each one accessing the application database. With service-level transactions, this implies several separate transactions (all committed, unless an exception occurs), which taxes more the database server. It isn't just a performance disadvantage, though. Having multiple transactions for a single request/response can also lead to subtle bugs (I don't remember the details anymore, just that such issues did occur).

Other answer to this question talks about "logic needed to identify the scope of a database/business transaction". This argument doesn't make sense to me, since there is no logic associated with transaction demarcation at all, normally. Neither controller classes nor service classes need to actually "know" about transactions. In the vast majority of cases, in a web app each business operation occurs inside an HTTP request/response pair, with the scope of the transaction being all the individual operations being executed from the point the request is received up until the response being finished.

Occasionaly, a business service or controller may need to handle an exception in a particular way, then probably mark the current transaction for rollback only. In Java EE (JTA), this is done by calling UserTransaction#setRollbackOnly(). The UserTransaction object can be injected into a @Resource field, or obtained programmatically from some ThreadLocal. In Spring, the @Transactional annotation allows rollback to be specified for certain exception types, or code can obtain a thread-local TransactionStatus and call setRollbackOnly().

So, in my opinion and experience, making the controller transactional is the better approach.

Metternich answered 2/6, 2017 at 15:18 Comment(3)
Awarding a bounty of 100 as promised in the discussion below, thank you for sharing your point of view.Hypotrachelium
@Rogerio Using Transactional annotation on controller methods create proxy object for the controller class ? why it makes the proxy object if we use Transactional annotation. to my knowledge proxy is created to add some extra functionality at runtime,if so what is that extra functionality Transactional annotation adds to the controller methods ?Homogenesis
"A common pitfall when working with annotated controller classes happens when applying functionality that requires creating a proxy for the controller object (e.g. @Transactional methods). Usually you will introduce an interface for the controller in order to use JDK dynamic proxies. To make this work you must move the` @RequestMapping` annotations, as well as any other type and method-level annotations (e.g. @ModelAttribute, @InitBinder) to the interface as well as the mapping mechanism can only "see" the interface exposed by the proxy". added reference from spring mvc doc to above doubtHomogenesis
H
7

Sometimes you want to roll back a transaction when an exception is thrown, but at the same time you want to handle the exception create a proper response in the controller to it.

If you put @Transactional on the controller method the only way to enforce the rollback it to throw the transaction from the controller method, but then you cannot return a normal response object.

Update: A rollback can also be achieved programmatically, as outlined in Rodério's answer.

A better solution is to make your service method transactional and then handle a possible exception in the controller methods.

The following example shows a user service with a createUser method, that method is responsible to create the user and send an email to the user. If sending the mail fails we want to rollback the user creation:

@Service
public class UserService {

    @Transactional
    public User createUser(Dto userDetails) {

        // 1. create user and persist to DB

        // 2. submit a confirmation mail
        //    -> might cause exception if mail server has an error

        // return the user
    }
}

Then in your controller you can wrap the call to createUser in a try/catch and create a proper response to the user:

@Controller
public class UserController {

    @RequestMapping
    public UserResultDto createUser (UserDto userDto) {

        UserResultDto result = new UserResultDto();

        try {

            User user = userService.createUser(userDto);

            // built result from user

        } catch (Exception e) {
            // transaction has already been rolled back.

            result.message = "User could not be created " + 
                             "because mail server caused error";
        }

        return result;
    }
}

If you put a @Transaction on your controller method that is simply not possible.

Hypotrachelium answered 21/1, 2017 at 16:24 Comment(6)
No. Both in Java EE and Spring, the current transaction can be programatically marked as "rollback only". So, you can handle an exception and produce a "normal" result, while ensuring the transaction gets rolled back from a @Transactional controller. Also, catching exceptions in order to provide specific error messages is a poor solution; mapping of exceptions to error messages should be done once in a central place. Any custom exception handling should be extremely rare in a well-architected application.Timepleaser
Could you elaborate on that in a separate answer?Hypotrachelium
I could, but the fact there is already an accepted answer (which I, BTW, disagree with) makes it not worth the (considerable) effort to write a detailed answer.Timepleaser
I could offer a bounty of 100 reputation, if that would change your mind.Hypotrachelium
@Metternich I agree with centralized handling of errors but there is little support for different configurations for different parts of the application, eg: a public part has a more generic message for the same exception. So sometimes I had to use custom handling. Any suggestion to avoid it ?Canned
@Ianoxx: The reason custom mapping of exceptions exceptions to error message is rare is that you should have something like a request filter that catches all exceptions. That does the mapping. Therefore all endpoints will have consistent error mappings. If you've done your mapping well, it will be rare there's an exception which needs custom handling in a controller. Your controller can declare "throws Exception", in essence doing nothing with the exception, and it gets handled properly.Chavey
S
4

I think the best way is to put the transactional annotation at the layer where you transition from entities (persistent objects) to DTOs (transient objects). The reason for this is that entity relations may need to be traversed which may trigger lazy initializations and you don't want to use the open session in view anti-pattern.

One argument to put them at the controller level would be that REST already dictates the read-only or read-write properties of a transaction. (GET should be read-only and POST/PUT/DELETE should be read-write). Mind that this would only work if your exception to error response handling happens outside of the controller so a transaction is properly rolled back when this happens.

An argument to put them at the service layer is that the service probably relies on the proper transaction isolation/propagation for its inner workings.

Probably the best compromise is to design facade level services which only list DTO object in their API and contain the transaction annotations as part of their interface. It is important to design the interfaces with the transactional handling in mind, since calling a read-write method from a read-only method may have undesirable side effects.

Soilure answered 14/2, 2022 at 12:59 Comment(2)
This is the best answer I think. Thanks for suggesting facade level.Farwell
This together with the reported experience by @Metternich are really nice. They are clearly described and are really usefull in the decision making processVerism

© 2022 - 2024 — McMap. All rights reserved.