Refactoring toward frictionless & odorless codeWhat about transactions?
Originally posted at 3/30/2011
One thing that we haven’t done so far is manage transactions. I am strongly against having automatic transactions that wrap the entire request. However, I do like automatic transactions that wrap a single action. We can implement this as:
public class NHibernateActionFilter : ActionFilterAttribute { private static readonly ISessionFactory sessionFactory = BuildSessionFactory(); private static ISessionFactory BuildSessionFactory() { return new Configuration() .Configure() .BuildSessionFactory(); } public override void OnActionExecuting(ActionExecutingContext filterContext) { var sessionController = filterContext.Controller as SessionController; if (sessionController == null) return; sessionController.Session = sessionFactory.OpenSession(); sessionController.Session.BeginTransaction(); } public override void OnActionExecuted(ActionExecutedContext filterContext) { var sessionController = filterContext.Controller as SessionController; if (sessionController == null) return; using (var session = sessionController.Session) { if (session == null) return; if (!session.Transaction.IsActive) return; if (filterContext.Exception != null) session.Transaction.Rollback(); else session.Transaction.Commit(); } } }
This one is pretty simple, except that you should note that we are checking if the transaction is active before trying something. That is important because we might have got here because of an error in the opening the transaction, we would get a second error which would mask the first.
More posts in "Refactoring toward frictionless & odorless code" series:
- (12 Apr 2011) What about transactions?
- (11 Apr 2011) Getting rid of globals
- (10 Apr 2011) The case for the view model
- (09 Apr 2011) A broken home (controller)
- (08 Apr 2011) Limiting session scope
- (07 Apr 2011) Hiding global state
- (06 Apr 2011) The baseline
Comments
And what about child actions?
Check this related post also:
codebetter.com/.../
A couple of questions:
Is there a performance penalty for using transaction for all actions including the ones that are read-only.
If I use a BLL layer, how can I share the session used in UI and BLL?
Mag2,
1) you always need to use transactions with nhibernate
2) ambient session
@mag20 Since we are talking about a NHibernateActionFilter, this means the developer knows that the action manipulates a RDBMS, so even if it will run only one query, a transaction is needed (isolation from ACID is enforced by transactions).
@ayende How about using a TransactionScope ( msdn.microsoft.com/.../...ns.transactionscope.aspx)? If we have an action which spawns several child actions, each of these will just call the TransactionScope Complete in case everything is OK. Of course, this TransactionScope would have to be stored somewhere in order to make it available to all involved actions - maybe inside the HttpContext.Current ...
Be using a TransactionScope, we can enlist other types of resourcs, eg MSQM, etc.
@hazzik, child actions will run in their own transactions. Filters are applied on per action basis and child actions are run during result execution. If you got all the needed data in your session context, and you don't alter db in a child action, then everything is ok. Otherwise you have two (or more), separate transactions per request.
Bodgan,
If possible, avoid TransactionScope, it can lead to very bad perf.
@Bogdan Marian: look at what @hazzik pointed out. Where would you commit your TransactionScope?
@Scooletz The TransactionScope has a a method, Complete, which is called if the underlying tx should be commited. If a rollback is needed, no method is called, thus will make the underlying tx to be rolled back - more info can be found on MSDN (see the link of my first comment). The Complete method will be called inside the OnActionExecuted method.
What about actions that do not require db access at all? You are opening a session and a transaction ALWAYS. Perhaps it's better to make the Session property "lazy" and when first issued open the session and the transaction.
Then corresponding checks at ActionExecuted should be taken when that happens.
It still feels a bit awkward to have different sessions and transactions for EACH child action. I think it's better to have a wider session (i.e. close it at ResultExecuted) in the parent action and use the same session and transaction for the child actions (since generally those actions will be read only), just to present data in the view.
@Bogdan Marian, if so, the "child action problem" occur in the very same way, cause child action are run after OnActionExecuted method. They are run during ActionResult.ExecuteResult. Using TransactionScope adds nothing the Ayende's solution, but perf problems mentioned by Ayende.
@Leonardo - Creating a session isn't expensive, it's cheap, it's not going to hurt performance by creating a session thats never used for an action that does not do any database access.
However any action that does not require database access is probably a controller that does not require database access, so you wouldn't inherit the NH controller to begin with.
@Leonardo - Also it may feel awkward, but it's more logical, your session is a UoW, not a Collection of Work. If 1 action is saving a product to your shopping cart, and another action is querying the shopping cart. They are two completely separate things, and should be run under two separate units of work and two separate transactions.
@NC
I agree that session openings aren't expensive, but what about transaction opening?
I don't think that if I have an action that does not access the db then the whole controller isn't going to access the db.
Think of the case of a simple crud: The New action does not access the db (given the form is simple enough), but the Create action does.
In this scenario, you would be opening a transaction in the New action and that is not needed.
Leonardo,
No one solution is applicable for all scenarios.
I would point out that usually for New action, you need some lookup data, but that is mostly beside the point
<we>
IMHO, checking if transaction is active isn't really reliable in case of zombied transactions. The client may think the transaction is active when it's actually not -> you'll loose the first exception.
@Ayende
But if you make the Session property lazy, so that it opens when first issued (and opens a transaction also), you have a solution applicable to all cases.
@NC
I agree with your point, you're right that those are 2 separate things. It's just from the performance perspective where you could perhaps gain something. I'm thinking of a view that has several sections and each could be rendered by a separate action, retrieving different data.
For example this blog page. Main view for the post, then a placeholder for the comments, which would be a child action, then in the master of this view would also be different actions for each section in the sidebar from the right.
Do I really need to open that many transactions each time this page is requested?
@Leonardo, @Ayende
You can create Transaction Opt Out Attribute and inspect it through ActionExecutingContext in the filter above.
@Scooletz If the TransactionScope is sotred inside the HttpContext.Current, the action and all its children will share it. Each of them will call the Complete method to notify the transaction manager that their work is OK to be comitted. There is only one TransactionScope, so only one real transaction shared by all actions.
@Alex
That would bring the problem of explicitly having to say that you don't want a transaction, and you have to remember to put that on every action that does not access to the db.
Also, if you already have an action with that attribute and then you find out that it indeed needs db access (and you forget to remove the attribute), you will be accessing the session outside a transaction, which is not the correct usage.
If that commit() fails, do you still need to rollback()? Or, is that handled implicitly?
@Leonardo
If you have most Actions accessing db, then you make opt-in attribute global and apply opt-out as optimization technique (that can be postponed until you start optimization phase)
If you have most Actions not accessing db, then you only apply transaction opt-in attribute to them.
And, as usual, you cannot have it both ways
I meant "then you only apply transaction opt-in attribute to those accessing db"
@Alex
But why would you need to specify to not use transaction if you don't access the db? That's like saying things twice. If the session is not used anytime during the action, then that means you don't access the db and therefore you don't need a transaction.
And still I think that approach can lead to use the session outside a transaction if the attribute is not updated when the action is.
@Leonardo
Then you may want to look at this:
nhforge.org/.../...on-management-for-web-apps.aspx
@Alex
Yes, that's what I was talking about. Nice implementation using ICurrentSessionContext.
What are your reasons to be "strongly against having automatic transactions that wrap the entire request", I'm interested in this, since I've seen some NHibernate experts suggest you do wrap them
This is exactly my transaction boundary starts at the app service layer. My read layer does have this complexity and my command handler begins/commits my transactions.
I'm sure we all can think of some edge cases, but so far, no issues when building app layer from use cases.
@Eber,
I am with you on using session per request. This way child actions can sometimes benefit from using cached data.
As some guys mentioned here (my article). Instead of using an attribute to explcitly say where the transaction begin and end, i prefer delay the two things (OpenSession/BeginTransaction) to the first usage GetCurrentSession... and in the End of the request, if there is a transaction commit and/or close the session.
NHibernate need transaction for everything, so maintaining an attribute in the request that i will need to use the persistence is something that doesn't worth. I don't want to think how you test all this.
Hi,
How will this work among the Web Farms ?
Will one transaction be available in another farm ?
Thanks
As we have only single inheritance in C# and composition can be rather tricky with different base controllers, alternative approach to the "SessionController" would be to go with storing ISession in controllers (well, global) HttpContext and have an controller extension method to get the session from there.
Comment preview