2

I am going threw my site with nhibernate profiler and I got this message

Alert: Use of implicit transactions is discouraged

http://nhprof.com/Learn/Alerts/DoNotUseImplicitTransactions

I see they are on every single select statement.

private readonly ISession session;

public OrderHistoryRepo(ISession session)
{
    this.session = session;
}

public void Save(OrderHistory orderHistory)
{
    session.Save(orderHistory);
}

public List<OrderHistory> GetOrderHistory(Guid Id)
{
    List<OrderHistory> orderHistories = session.Query<OrderHistory>().Where(x => x.Id == Id).ToList();
    return orderHistories;
}

public void Commit()
{
    using (ITransaction transaction = session.BeginTransaction())
    {
        transaction.Commit();
    }
}

Should I be wrapping my GetOrderHistory with a transaction like I have with my commit?

Edit

How would I wrap select statements around with a transaction? Would it be like this? But then "transaction" is never used.

    public List<OrderHistory> GetOrderHistory(Guid Id)
    {
        using (ITransaction transaction = session.BeginTransaction())
        {       

 List<OrderHistory> orderHistories = session.Query<OrderHistory>().Where(x => x.Id == Id).ToList();
        return orderHistories;
        }
    }

Edit

Ninject (maybe I can leverage it to help me out like I did with getting a session)

public class NhibernateSessionFactory
    {
        public ISessionFactory GetSessionFactory()
        {
           ISessionFactory fluentConfiguration = Fluently.Configure()
                                                  .Database(MsSqlConfiguration.MsSql2008.ConnectionString(c => c.FromConnectionStringWithKey("ConnectionString")))
                                                  .Mappings(m => m.FluentMappings.AddFromAssemblyOf<Map>().Conventions.Add(ForeignKey.EndsWith("Id")))
                                                  .ExposeConfiguration(cfg => cfg.SetProperty("adonet.batch_size", "20"))
                                                  //.ExposeConfiguration(BuidSchema)
                                                  .BuildSessionFactory();

            return fluentConfiguration;
        }

        private static void BuidSchema(NHibernate.Cfg.Configuration config)
        {
            new NHibernate.Tool.hbm2ddl.SchemaExport(config).Create(false, true);
        }
    }


public class NhibernateSessionFactoryProvider : Provider<ISessionFactory>
    {   
        protected override ISessionFactory CreateInstance(IContext context)
        {
            var sessionFactory = new NhibernateSessionFactory();
            return sessionFactory.GetSessionFactory();
        }
    }

  public class NhibernateModule : NinjectModule
    {
        public override void Load()
        {
            Bind<ISessionFactory>().ToProvider<NhibernateSessionFactoryProvider>().InSingletonScope();
            Bind<ISession>().ToMethod(context => context.Kernel.Get<ISessionFactory>().OpenSession()).InRequestScope();
        }
    }

Edit 3

If I do this

    public List<OrderHistory> GetOrderHistory(Guid Id)
    {
        using (ITransaction transaction = session.BeginTransaction())
        {       

 List<OrderHistory> orderHistories = session.Query<OrderHistory>().Where(x => x.Id == Id).ToList();
        return orderHistories;
        }
    }

I get this alert

If I do this

    public List<OrderHistory> GetOrderHistory(Guid Id)
    {
        using (ITransaction transaction = session.BeginTransaction())
        {       

 List<OrderHistory> orderHistories = session.Query<OrderHistory>().Where(x => x.Id == Id).ToList().ConvertToLocalTime(timezoneId);
        transaction.Commit();
        return orderHistories;
        }
    }

I can get rid of the errors but can get unexpected results.

For instance when I get orderHistories back I loop through all of them and convert the "purchase date" to the users local time. This is done through an extension method that I created for my list.

Once converted I set it to override the "purchase date" in the object. This way I don't have to create a new object for one change of a field.

Now if I do this conversion of dates before I call the commit nhibernate thinks I have updated the object and need to commit it.

So I am putting a bounty on this question.

  1. How can I create my methods so I don't have to wrap each method in a transaction? I am using ninject already for my sessions so maybe I can leverage that however some times though I am forced to do multiple transactions in a single request.

So I don't know have just one transaction per request is a soultion.

  1. how can I make sure that objects that I am changing for temporary use don't accidentally get commit?

  2. how can I have lazy loading that I am using in my service layer. I don't want to surround my lazy loading stuff in a transaction since it usually used in my service layer.

I am finding it very hard to find examples of how to do it when your using the repository pattern. With the examples everything is always written in the same transaction and I don't want to have transactions in my service layer(it is the job of the repo not my business logic)

µBio
  • 10,668
  • 6
  • 38
  • 56
chobo2
  • 83,322
  • 195
  • 530
  • 832
  • possible duplicate of [NHibernate Transactions on Reads](http://stackoverflow.com/questions/1657465/nhibernate-transactions-on-reads) – Vadim May 24 '11 at 01:11
  • Why don't you use a TransactionScope? http://msdn.microsoft.com/en-us/library/ms172152(v=vs.90).aspx – Simon Mourier Jun 03 '11 at 15:57
  • @Simon Mourier - I don't know what that is really. I will look more into that link but it almost looks the same as wrapping everything around in my repo. But I just looked at the code and read nothing yet so maybe it is totally different. – chobo2 Jun 03 '11 at 16:09

4 Answers4

4

The NHibernate community recommends that you wrap everything in a transaction, regardless of what you're doing.

To answer your second question, generally, it depends. If this is a web application, you should look at the session-per-request pattern. In most basic scenarios, what this means is that you'll create a single session per HTTP request in which the session (and transaction) is created when the request is made and committed/disposed of at the end of the request. I'm not saying that this is the right way for you, but it's a common approach that works well for most people.

There are a lot of examples out there showing how this can be done. Definitely worth taking the time to do a search and read through things.


EDIT: Example of how I do the session/transaction per request:

I have a SessionModule that loads the session from my dependency resolver (this is a MVC3 feature):

namespace My.Web
{
    public class SessionModule : IHttpModule {
        public void Init(HttpApplication context) {
            context.BeginRequest += context_BeginRequest;
            context.EndRequest += context_EndRequest;
        }

        void context_BeginRequest(object sender, EventArgs e) {
            var session = DependencyResolver.Current.GetService<ISession>();
            session.Transaction.Begin();
        }

        void context_EndRequest(object sender, EventArgs e) {
            var session = DependencyResolver.Current.GetService<ISession>();
            session.Transaction.Commit();
            session.Dispose(); 
        }

        public void Dispose() {}
    }
}

This is how I register the session (using StructureMap):

new Container(x => {

    x.Scan(a => {
        a.AssembliesFromApplicationBaseDirectory();
        a.WithDefaultConventions();
    });

    x.For<ISessionFactory>().Singleton().Use(...);
    x.For<ISession>().HybridHttpOrThreadLocalScoped().Use(sf => sf.GetInstance<ISessionFactory>().OpenSession());
    x.For<StringArrayType>().Use<StringArrayType>();

});

Keep in mind that this is something I've experimented with and have found to work well for the scenarios where I've used NHibernate. Other people may have different opinions (which are, of course, welcome).

csano
  • 13,266
  • 2
  • 28
  • 45
  • @cs - I already do a session-per-request the session that gets passed in to the repo is made by ninject(ioc) and is disposed of after the request. – chobo2 May 24 '11 at 02:00
  • @cs - I am wondering how if your doing lazy loading? Do you also need to wrap those too? – chobo2 May 24 '11 at 02:54
  • @chobo2 - Great. All you will need to do then is open the transaction in the same place where you are creating your session. This way you won't have to worry about opening/closing sessions and/or transactions in every repository method. Let me know if you want some more details and I'd be more than happy to provide them. – csano May 24 '11 at 02:55
  • @chobo2 - No. Since I do the session/transaction per request, any time lazy loading happens, it's within a request, therefore, within a session/transaction, so there's no wrapping. Does that make sense? – csano May 24 '11 at 02:56
  • @cs - could I see how to add the tranasction around? I will post what I have for my ninject session. I thought I need to always wrap the transaction around so I need to see a visual example. – chobo2 May 24 '11 at 03:02
  • @chobo2 - Updated my answer to include an example of how I handle sessions/transactions. Let me know if you have any questions. – csano May 24 '11 at 03:12
  • @cs - First it seems that if you don't wrap a transaction around lazy loading you get the same error. Your ioc is done differently then mine so I would have to modify it. I am just wondering though since you start the transaction and end the transaction at the end of each http request does that mean you do not commit anything till everything is done? How does that work if you have a couple get statements or have a statement that you must first insert then you need to do some select statements how do you handle these? – chobo2 May 24 '11 at 15:58
  • @chobo2 - If you want to lazy load something, it has to be loaded within the context of a session. With the session-per-request approach, you're pretty much guaranteed to always be in the context of a session. If, at any point, I need to persist data during the lifetime of my session, I flush it. – csano May 25 '11 at 01:11
  • @cs - hmm well how does it work if I have a repo that gets a session created and at the end of the request gets destroyed. I return back an object from that repo then do something like TableA.TableB so this would trigger lazy loading. Is this still in that session? – chobo2 May 25 '11 at 15:52
  • @chobo - The request ends when the rendered HTML is sent back to the browser. By this time, everything has already been processed. – csano May 25 '11 at 16:17
  • @cs - hmm then why does it come up when I use lazy loading that I need to wrap them around in a transaction as well? – chobo2 May 25 '11 at 16:21
  • @cs - I now have ninject doing all my transactions but it still does not work with lazy loading. – chobo2 Jun 05 '11 at 20:29
2

Well, i guess you could set a Transaction level that's appropriate for the kind of reads that you perform in your application, but the question is: should it really be required to do that within the application code? My guess is no, unless you have a use case that differs from the default transaction levels that (n)hibernate will apply by configuration.

Maybe you can set transaction levels in your nhibernate config.

Or maybe the settings for the profiler are a bit overzealous? Can't tell from here.

But: Have you tried commiting a read transaction? Should not do any harm.

mkro
  • 1,902
  • 14
  • 15
  • I am not sure too much about what these transaction levels are. – chobo2 May 24 '11 at 02:00
  • @mrko - Actually commit a read transaction can do some harm. I get back the results from the database and then convert some fields to local timezones and store it back in the same object variable. If I do a commit it thinks I updated the object and saves it. – chobo2 Jun 03 '11 at 04:00
2

You're passing the ISession into the repository's constructor, which is good. But that's the only thing I like about this class.

  • Save just calls session.Save, so it's not needed.
  • GetOrderHistory appears to be retrieving a single entity by ID, you should use session.Get<OrderHistory>(id) for this. You can put the result into a collection if needed.
  • The Commit method shouldn't be in a repository.

To answer your questions directly...

How can I create my methods so I don't have to wrap each method in a transaction? I am using ninject already for my sessions so maybe I can leverage that however some times though I am forced to do multiple transactions in a single request.

The pattern I recommend is below. This uses manual dependency injection but you could use Ninject to resolve your dependencies.

List<OrderHistory> orderHistories;
var session = GetSession(); // Gets the active session for the request
var repository = new OrderHistory(Repository);
// new up more repositories as needed, they will all participate in the same transaction
using (var txn = session.BeginTransaction())
{
    // use try..catch block if desired
    orderHistories = repository.GetOrderHistories();
    txn.Commit();
}

So I don't know have just one transaction per request is a soultion.

It's perfectly fine to have multiple transactions in a session. I don't like waiting until the request ends to commit because it's too late to provide good feedback to the user.

how can I make sure that objects that I am changing for temporary use don't accidentally get commit?

The only sure way is to use an IStatelessSession. Less sure ways are to Evict the object from the Session or Clear the session. But with NHibernate it's not recommended to modify persistent objects.

how can I have lazy loading that I am using in my service layer. I don't want to surround my lazy loading stuff in a transaction since it usually used in my service layer.

If you're using session-per-request this shouldn't be a problem. But you're right that lazy-loading can happen outside of the transaction. I ignore these warnings. I suppose you could "touch" every child object needed so that lazy loads are in a transaction but I don't bother.

I don't want to have transactions in my service layer(it is the job of the repo not my business logic)

I disagree with this. The UI or business logic should manage the transaction. The UI is where the user expresses their intent (save or cancel my changes) and is the natural place to manage the transaction.

Jamie Ide
  • 48,427
  • 16
  • 81
  • 117
0
  1. Recommended approach is unit of work (session+transaction) per request. Sure you can use NInject to manage session lifecycle, I blogged recently about similar approach using Castle Windsor.
  2. Here are 4 options:

    • Don't change entities temporary
    • Use stateless session in such cases
    • Detach objects when you are going to do temporary change
    • Rollback transaction

    I'd go with first one.

  3. You don't have to worry about lazy loading if you are using session-per-request pattern - it will be executed in same request and wrapped with transaction automatically.
xelibrion
  • 2,220
  • 1
  • 19
  • 24
  • Can the unit of work be used with the service layer and repository pattern? So why does it not seem like my transaction is being wrapped with lazy loading. I also went and did a ninject way and sometimes it seems like it does not wrap requests in transactions. Most of the time does but some it does not. – chobo2 Jun 09 '11 at 20:28
  • Sure, unit of work can be used with the service layer and repository pattern. Regarding wrapping requests with transaction - I guess it is result of requests that do not require session actually. Anyway, I don't have such problem - so you could try approach I wrote about – xelibrion Jun 09 '11 at 20:36