Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSessionContext in ASP.NET Core 2 #1632

Open
leandro86 opened this issue Mar 30, 2018 · 39 comments
Open

WebSessionContext in ASP.NET Core 2 #1632

leandro86 opened this issue Mar 30, 2018 · 39 comments

Comments

@leandro86
Copy link

@leandro86 leandro86 commented Mar 30, 2018

Hello,

I'm trying to use the WebSessionContext in a ASP.NET Core 2 application, but I get the following error when CurrentSessionContext is accesed:

Could not load file or assembly 'System.Web, Version=4.0.30319.42000, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.

The problem seems to be in ReflectiveHttpContext.cs:

private static System.Type HttpContextType
{
	get
	{
		return
			System.Type.GetType(
				string.Format(
					"System.Web.HttpContext, System.Web, Version={0}, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
					Environment.Version));
	}
}

HttpContext is now in the assembly Microsoft.AspNetCore.Http.Abstractions. However, I don't believe it would be possible to load the current HttpContext by reflection, since now it doesn't have a "Current" property. ReflectiveHttpContext access the "Current" property of HttpContext here:

private static void CreateCurrentHttpContextGetter()
{
	PropertyInfo currentProperty = HttpContextType.GetProperty("Current", BindingFlags.Static | BindingFlags.Public | BindingFlags.FlattenHierarchy);
	Expression propertyExpression = Expression.Property(null, currentProperty);
	Expression convertedExpression = Expression.Convert(propertyExpression, typeof (object));
	HttpContextCurrentGetter = (Func<object>) Expression.Lambda(convertedExpression).Compile();		
}

The current HttpContext is now available through IHttpContextAccesor. See: https://stackoverflow.com/questions/31243068/access-the-current-httpcontext-in-asp-net-core

@fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Mar 31, 2018

Feature parity between full Framework/.Net Core about the HttpContext based session context seems hard to achieve, since the Core team is banning static accessors and tries hard to prevent/dissuade people of putting workarounds in place.

The main difficulties lie in the fact that NHibernate has been ported to .Net Core and not rewritten/re-architectured for it. It especially has not been rewritten as an injectable .Net Core service, meaning it has no mechanism out of the box for easily getting injected with other .Net Core services, like an IHttpContextAccessor. (Moreover this service is not guaranteed to be present. And many NHibernate setup may not require it either, so putting it as a mandatory dependency of NHibernate does not look as an option to me.)

I think this case would require a new kind of session context, which would also be a Core service to add in the pipeline like any service, allowing it to mandate and get any .Net Core dependencies it requires.

@leandro86
Copy link
Author

@leandro86 leandro86 commented Mar 31, 2018

Thanks for your answer. As a workaround, right now I'm doing this:

I created a new session context:

[Serializable]
public class AspNetCoreWebSessionContext : MapBasedSessionContext
{
	private const string SessionFactoryMapKey = "NHibernate.Context.AspNetCoreWebSessionContext.SessionFactoryMapKey";

	public static IHttpContextAccessor HttpContextAccessor { get; set; }

	public AspNetCoreWebSessionContext(ISessionFactoryImplementor factory) : base(factory)
	{
	}

	protected override IDictionary GetMap()
	{
		return HttpContextAccessor.HttpContext.Items[SessionFactoryMapKey] as IDictionary;
	}

	protected override void SetMap(IDictionary value)
	{
		HttpContextAccessor.HttpContext.Items[SessionFactoryMapKey] = value;
	}
}

And then, after building the session factory, I ask the container to resolve an instance of IHttpContextAccesor:

var sessionFactory = config.BuildSessionFactory();
AspNetCoreWebSessionContext.HttpContextAccessor = container.GetInstance<IHttpContextAccessor>();

It's also possible to create an instance of HttpContextAccessor directly inside AspNetCoreWebSessionContext, but that would require a dependency on Microsoft.AspNetCore.Http, instead of Microsoft.AspNetCore.Http.Abstractions.

What do you have in mind to do this the proper way?

@fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Mar 31, 2018

As a workaround you may use instead the AsyncLocalSessionContext, it should be suitable.

About providing a .Net Core compatible web session context, I would need to investigate the subject more.

@jpenniman
Copy link

@jpenniman jpenniman commented Apr 4, 2018

In our ASP.Net Core apps, we don't use the WebSessionContext at all. We manage the ISessionFactory and ISession life cycle with the dependency injection built into ASP.Net Core...

services.AddSingleton<ISessionFactory>((provider)=>{ 
    var cfg = new NHibernate.Cfg.Configuration();
    // your config stuff here
    return cfg.BuildSessionFactory();
});

services.AddScoped<ISession>((provider)=>{
    var factory = provider.GetService<ISessionFactory>();
    return factory.OpenSession();
});

Then just inject ISession into what ever needs it. For example (over simplified for brevity):

public class CustomerController : Controller
{
    ISession session;

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

    public IEnumerable<Customer> Get()
    {
        return session.QueryOver<Customer>().List();
    }
}

We simplified it even further and created an extension method on IServiceProvider to handle all the boilerplate, so the Startup.cs in our projects just contains...

services.AddNHibernate();

We have been using this approach for the last couple years (since ASP.Net Core 1.0) and it has worked well for us.

@hazzik
Copy link
Member

@hazzik hazzik commented Apr 4, 2018

I think the solution would be to throw PlatformNotSupportedException on access to WebSessionContext. Thoughts?

@fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Apr 4, 2018

Well, that was somewhat my first thought. But since we have avoided this in the call context case for feature parity sake, it would have been better to find a solution for web context too.

But it will be still better to throw that error than failing as it does currently.

@hazzik
Copy link
Member

@hazzik hazzik commented Apr 4, 2018

I don't think we would be able to find a viable solution at the moment.

@fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Apr 5, 2018

I think the solution would be to throw PlatformNotSupportedException on access to WebSessionContext.

Thinking about it again, that may not be straightforward to do. For netcoreapp target, this is easy. But for netstandard, it is another story, since we do not really know what will be the executing runtime at compile time. It could even be the full framework after all.

Currently, there is no standardized way of detecting this. This is a work in progress in dotnet/corefx#17452.
Some workarounds are provided in dotnet/corefx#22293, like using this.

But maybe (even if unlikely) some other runtime than the full .Net Framework may support the HttpContext. So overall we may have better keeping this context working or failing as it does currently.

@hazzik
Copy link
Member

@hazzik hazzik commented Apr 11, 2018

I think the simplest fix would be to pass throwOnError: true into System.Type.GetType method in ReflectiveHttpContext.

@fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Apr 11, 2018

I think the simplest fix would be to pass throwOnError: true into System.Type.GetType method in ReflectiveHttpContext.

While the code would be more correct this way (avoiding a potential null reference exception), it will not change anything to the case reported here, because that is a case which already throws even if throwOnError is false.

@danilobreda
Copy link

@danilobreda danilobreda commented Jun 4, 2018

A "deprecated" or something like that would be nice, because even on .net full, the use of a DI is suggestive for a right way that the plataform is supporting right now.

@shatl
Copy link

@shatl shatl commented Jun 4, 2018

@leandro86 Using DI instead would be a better approach.
I had a lot of issues with HttpContext based approach when decided to move some code to run in background thread.

@leandroaraujo
Copy link

@leandroaraujo leandroaraujo commented Aug 3, 2018

@jpenniman, @shatl

In our ASP.Net Core apps, we don't use the WebSessionContext at all. We manage the ISessionFactory and ISession life cycle with the dependency injection built into ASP.Net Core...

And about the CloseSession?
.Net Core DI disposes/close Session automatically or I need to close by my own?

@maca88
Copy link
Contributor

@maca88 maca88 commented Aug 3, 2018

.Net Core DI disposes/close Session automatically or I need to close by my own?

The session will be closed and disposed automatically when the service provider scope is disposed. .Net Core DI disposes all services that implements IDisposable interface when the scope that contains them is disposed. As NHibernate session implements IDisposable, it will be disposed when the scope ends and also closed if it not already closed.

@lissinho
Copy link

@lissinho lissinho commented Jun 29, 2019

.Net Core DI disposes/close Session automatically or I need to close by my own?

The session will be closed and disposed automatically when the service provider scope is disposed. .Net Core DI disposes all services that implements IDisposable interface when the scope that contains them is disposed. As NHibernate session implements IDisposable, it will be disposed when the scope ends and also closed if it not already closed.

We are using this approach but having trouble with ajax requests (odd "A command is already in progress" messages) and even "Session is closed!" sometimes.
Does anyone also have these problems? @jpenniman , @maca88 ?

@jpenniman
Copy link

@jpenniman jpenniman commented Jul 1, 2019

It sounds like your scoping is incorrect. Make sure ISessionFactory is a singleton, and ISession is request scoped. You'll get a "A command is already in progress" error if a session is running a sql command and you try to run another command on the same session.

services.AddSingleton<ISessionFactory>(...);
services.AddScoped<ISession>(...);

If you want to post your registrations here, I'll review them for any possible issues. If you don't want to post them publicly, you can email them to me.

We have ASP.net core applications using NHibernate handling thousands of transactions per second with no issues.

@lissinho
Copy link

@lissinho lissinho commented Jul 1, 2019

It sounds like your scoping is incorrect. Make sure ISessionFactory is a singleton, and ISession is request scoped. You'll get a "A command is already in progress" error if a session is running a sql command and you try to run another command on the same session.

services.AddSingleton<ISessionFactory>(...);
services.AddScoped<ISession>(...);

If you want to post your registrations here, I'll review them for any possible issues. If you don't want to post them publicly, you can email them to me.

We have ASP.net core applications using NHibernate handling thousands of transactions per second with no issues.

Thanks for answering, @jpenniman
Yeah, sounds like that. But we are registering just like you said. Here is the code:

            services.AddSingleton<ISessionFactory>((provider) => {
                var cfg = provider.GetService<ConfigManager>();
                return cfg.CreateSessionFactory();
            });

            services.AddScoped<NHibernate.ISession>((provider) => {
                var factory = provider.GetService<ISessionFactory>();
                return factory.OpenSession();
            });

ConfigManager is our class that, like it's name says, manage configuration. Here is CreateSessionFactory function code:

        public ISessionFactory CreateSessionFactory()
        {
            var dbc = Licenca?.dbConfigs.FirstOrDefault();

            if (dbc == null)
                throw new Exception("String de conexão não configurada !");

            IPersistenceConfigurer cfg;
            if (dbc.providerName == "OracleClient")
                cfg = OracleClientConfiguration.Oracle10
                                               .ConnectionString(c => c.Is(dbc.connectionString))
                                               .AdoNetBatchSize(0);
            else if (dbc.providerName == "System.Data.SqlClient")
            {
                cfg = MsSqlConfiguration.MsSql2008
                                        .ConnectionString(c => c.Is(dbc.connectionString))
                                        .AdoNetBatchSize(0);
            }
            else if (dbc.providerName == "Npgsql")
            {
                cfg = PostgreSQLConfiguration.PostgreSQL82
                                             .ConnectionString(c => c.Is(dbc.connectionString));
            }
            else
            {
                throw new Exception("Provider '" + dbc.providerName + "' não implementado !");
            }

            return Fluently.Configure()
                           .Database(cfg)
                           .Mappings(m =>
                                     m.FluentMappings.AddFromAssemblyOf<Entities.Classe>())
                           .BuildSessionFactory();
        }

We are using FluentNHibernate. Could you share your NH configuration?
Thanks again.

@lissinho
Copy link

@lissinho lissinho commented Jul 1, 2019

To give more information:
We inject ISession in a separate BLL project as services, and inject services in controllers. Debugging, I realized that the line "factory.OpenSession" is called several times, not once per request. Is it correct?

@jpenniman
Copy link

@jpenniman jpenniman commented Jul 3, 2019

Make sure your service classes in your BLL are also being registered in request scope. If you make them singletons, then you've effectively made ISession a singleton (sort of... per service).

I assume your ConfigManager is also a singlton? (It should be, since ISessionFactory is a singleton.)

How are you debugging? If you're setting a breakpoint and using the browser to initiate the call, you'll see multiple calls while debugging. The browser retries the request for you automatically, up to 3 times. When not debugging, you should only get one ISession instance per request.

We don't use FluentNHibernate, but it shouldn't matter, unless Fluent is doing some magic that NHibernate-Core does not.

Here's how we configure NHibernate with Asp.Net Core. Note that we do not use WebSession. I abbreviated the OrderService implementation for simplicity/clarity by removing routing, mappers, logging, etc.:

services.AddSingleton<Configuration>((provider) => {
    Configuration config = new Configuration();
           
    IDictionary<string, string> properties = new Dictionary<string, string> {
        { "connection.connection_string", this.Configuration.GetConnectionString("Default") },
        { "connection.driver_class", "MilestoneTG.NHibernate.TransientFaultHandling.SqlServer.SqlAzureClientDriver, MilestoneTG.NHibernate.TransientFaultHandling.SqlServer" },
        { "dialect", "NHibernate.Dialect.MsSql2012Dialect" },
        { "hbm2ddl.keywords", "auto-quote"},
        { "show_sql", "true"},
        { "generate_statistics", "true"}
    };

    config.AddProperties(properties);
    config.AddXmlFile("hibernate.hbm.xml");
});

services.AddSingleton<ISessionFactory>((provider) => {
    return provider.GetService<Configuration>().BuildSessionFactory();
});

services.AddScoped<ISession>((provider) => {
    ISessionFactory factory = provider.GetService<ISessionFactory>();
    ISession session = factory.OpenSession();
    return session;
});

services.AddScoped<IOrderService, OrderService>();

Our OrderService takes in the ISession:

public class OrderService : IOrderService
{
    ISession session;
    public OrderService(ISession session)
    {
        this.session = session;
    }

    public Task<Order> FindByIdAsync(long id)
    {
        return session.GetAsync<Order>(id);
    }

    public async Task<Order> Create(Order order)
    {
        var tran = session.BeginTransaction();
        try
        {
            await session.SaveAsync(order);
            tran.Commit();
            return order;
        }
        catch(Exception)
        {
            tran.Rollback();
            throw;
        }
    }
}

Our controller then takes in the IOrderService:

public class OrderController : ControllerBase
{
    IOrderService service;
    public OrderService(IOrderService service)
    {
        this.service = service;
    }

    public Task<IActionResult> Get(long id)
    {
        return service.GetAsync(id);
    }
}

For your random "Session is closed" error, make sure ISession isn't wrapped in a using block any where, or that any code isn't explicitly closing or disposing the session.

@lissinho
Copy link

@lissinho lissinho commented Jul 8, 2019

Thanks again, @jpenniman.
Yes, we are registering just like you've assumed:

  • services and session in request scope and confmanager and sessionFacory singleton;
  • in services, we are injecting Session in constructor;
  • in controllers, services are being injected in constructor;
  • and we are not using using block in anyone of the codes that had triggered the random "Session is closed" error
    So, everything seems to be exactly like you do. I am really lost here.

Does anyone have another suggestions?

@jpenniman
Copy link

@jpenniman jpenniman commented Jul 9, 2019

If your configuration is correct, then there must be something with your business logic. If you want to email me directly, I can work with your company to do a formal architecture and code review: jason.penniman@milestonetg.com

There is likely something in your code that is trying to use ISession to make concurrent database calls under certain conditions, whether intentionally or unintentionally.

A few more considerations for DI:

  • Make sure you're not creating any new scopes (provider.CreateScope()).
  • Make sure you're not using service location anywhere other than startup and not trying to resolve scoped services from ApplicationServices.
  • Make sure you're not injecting scoped or transient objects into the constructor of any middleware. Middleware instances are singletons.

Another issue I see a lot is improper use of threading patterns, Task and async/await. This can result in undesired and unpredictable behavior--such as concurrent calls to the same ISession instance and disposing of the scope before background work is finished.

  • Async code mixed with synchronous code should be awaited.
  • Don't use any "fire and forget" patterns.
  • Async scopes should be handled by the framework; don't create your own async scopes.
  • Similarly, don't try and make ISession or your service references AsyncLocal
  • Make sure there are no async/void methods; all async method should return Task.
  • Make sure you are not trying to call async methods synchronously (calling .Result or .GetAwaiter().GetResult()).
  • NHibernate supports async methods since 5.0. Microsoft recommends "async all the way" from your controllers down to your data access code.

Beyond that, it's difficult to know what is going on without looking at your code and design. At this point, I don't think it is an issue with NHibernate. ISession is not thread safe, and the same instance of ISession is being called concurrently by multiple threads in your code based on the errors you have described.

@lissinho
Copy link

@lissinho lissinho commented Jul 11, 2019

Thanks again, @jpenniman .
I've created a project from "web application mvc" template and put my configuration code to reproduce the error here:
https://github.com/lissinho/netcorenhibernateajaxerror

You will see that it happens without any complex code (the routines just get datetime from database server), just configure a valid connection string to a postgresql server, run the project and keep refreshing the first page until see the errors.

I really don't know if it is a fluent, nhibernate, .net core DI or npgsql error. But I think it is an important issue to report.

I will keep trying to solve here, if I get any news, I'll let you know.

@jpenniman
Copy link

@jpenniman jpenniman commented Jul 12, 2019

Your service is a static variable... there for a singleton... static instances exist only once per type.,,

namespace WebApplication5.Controllers
{
    public class HomeController : Controller
    {
        private static IServiceTeste _serviceTeste;

        public HomeController(IServiceTeste serviceTeste)
        {
            _serviceTeste = serviceTeste;
        }

It must be an instance variable. It should look like this:

namespace WebApplication5.Controllers
{
    public class HomeController : Controller
    {
        private IServiceTeste _serviceTeste;

        public HomeController(IServiceTeste serviceTeste)
        {
            _serviceTeste = serviceTeste;
        }
@lissinho
Copy link

@lissinho lissinho commented Jul 12, 2019

Perfect, @jpenniman !!!!! That's exactly what cause the errors!!!
Thanks a lot, man!!! I owe you a beer!!!
XD

@jpenniman
Copy link

@jpenniman jpenniman commented Jul 12, 2019

You're welcome. I'm glad it was something simple.

@gusgorman
Copy link

@gusgorman gusgorman commented Oct 28, 2019

Personally I think having the ISession in the DI container is an anti pattern, because you might not want to rewire the whole application ever time you create a new hibernate session. Its much better for the ISessionFactory to live in the container..

For example, in my current application I am reusing the DAL (more specifically, the DAOs) in some batch processes that do some data migration. Here I don't want to re-create all the DAOs every time I create a new NHibernate session.

In my current design each DAO has the ISessionFactory as a constructor dependency. If the dependency was changed to be an ISession this would cause a problem.

@jpenniman
Copy link

@jpenniman jpenniman commented Oct 31, 2019

@gusgorman, If you look at my example, the whole application is not being wired-up every time an ISession is requested. ISessionFactory is managed by the container as a singleton and is only built once.

I agree that you may not always want to resolve ISession from the container and some scenarios may warrant injecting ISessionFactory instead, but that does not make this an anti-pattern. There are exceptions to every rule. I cover this in my blog article on this topic: https://jpenniman.blogspot.com/2019/07/using-nhibernate-in-aspnet-core.html

If you do have a service the depends on NHibernate that should be a singleton, for example an in-memory cache or other service with an expensive instantiation, inject ISessionFactory instead, and use it to create a new session within the service itself.

In a typical service/application following DDD and/or Clean Code/Architecture principles, injecting ISession is generally appropriate, so that you are using the same session for the entire request as originally intended by the legacy WebSessionContext. Managing ISession inside each DAO can cause issues with lazy loaded associations. In DDD/Clean Code, your DAO should know nothing of your DTO or Mapper. In that scenario, you may no longer have your ISession available when the mapper walks the graph.

The other thing to be careful of when injecting ISessionFactory, as in your scenario, is you are now creating a separate database connection for each DAO needed for a given request. This can exhaust connection pools and connection limits (Oracle imposes hard limits on the number of connections), increase memory utilization on the database server, and have a negative performance impact on the application itself (creating connections is very expensive). I'm not saying your design has these issues, but just something to be concious of. In a typical use-case, injecting ISessionFactory instead of ISession would have all these issues. Again, there are always exceptions.

@gusgorman
Copy link

@gusgorman gusgorman commented Oct 31, 2019

@jpenniman Most of your reply is a straw man. I am not advocating managing the hibernate session inside each DAO, so there is no danger of lazy loading errors or creating a database connection for each DAO. This would be another anti pattern and I'm not sure why you have to come to conclusion that I am doing this, because I certainly didn't say anything to indicate that I am (I know you slipped the caveat in at the end say to you're not sure if my design have these issues, but I really don't think any of it was worth mentioning at all).

The normal and correct use of DAOs is of course to inject them into the Service layer (or other higher layer of the application).

In the system I am currently working on we have one hibernate session per Http Request for the web app, and one hibernate session per "Unit of Work/Transaction" in the data migration batch processes.

I have had a quick look at your blog post and interestingly you say this:

"A common mistake I see among developers new to dependency injection (and even some veterans) is registering the services as singletons. This causes undesired behavior because you’ve effectively made ISession a singleton as well, since it will never be instantiated again for furture requests"

This is where we disagree. I would say the mistake is injecting the ISession into anything at all, because doing so means that it dictates the scope of anything that needs to consume it. It isn't a good idea to tie the lifetime of any component to the lifetime of a hibernate session. Injecting the ISessionFactory is much more flexible.

@jpenniman
Copy link

@jpenniman jpenniman commented Oct 31, 2019

@gusgorman, You make a very interesting point. You mentioned that you are injecting ISessionFactory into the DAO:

In my current design each DAO has the ISessionFactory as a constructor dependency.

I made the assumption that you would have to call ISessionFactory.OpenSession() within the DAO which would create a new session/connection in every DAO, and I should not have assumed your usage; I apologize.

Can I ask, then, if you are injecting ISessionFactory into the DAO, how are you managing ISession? I am curious how your design works. I'm always open to learning different ways of tackling a problem.

Can you share a web app example, since this thread is about Asp.Net Core and the lack of a WebSessionContext? It would help me and others reading the thread to understand how to best use your approach.

I agree, your approach gives much finer grained/flexible control over session lifetime. I can update my article to include your approach (crediting you, of course).

@gusgorman
Copy link

@gusgorman gusgorman commented Nov 1, 2019

@jpenniman Apology 100% accepted!

tbh I don't have the time to prepare a full app to share at the moment, but my current design is really no different from that which was encouraged by Castle and Spring.NET 10 years ago.... and Java Spring and similar frameworks before that.

So the layers in a web app (from top to bottom):

  1. MVC (or Razor pages) - Controllers have Services injected into them
  2. Service layer. Transactions are managed here using AOP (we use a Transaction attribute which signals the DI container to wrap the service method in an interceptor). The code in the services has no reference to or knowledge of NHibernate whatsover. DAOs are injected into the services.
  3. Data Access Layer consisting of DAOs. The DAOs have the ISessionFactory injected into them. Data access logic and Nhibernate specific code are encapsulated within the DAOs.

We have one NHIbernate session per http request, which is created and disposed using IActionFilter and IResultFilter implementations. This is similar to the "OpenSessionInView" pattern used in old Spring apps. which some people considered an anti-pattern when it was originally used :) However, I find it works well (we only ever pass ViewModels into the views so the views are decoupled from the actual domain objects).

The only small hack in the current web app is that we are using a modified version of the AspNetCoreWebSessionContext suggested by leandro86 earlier in this thread. The only difference being that the static method to set the HttpAccessor looks like this:

        public static Func<IHttpContextAccessor> ContextAccessorInitialiser
        {
            set { _httpContextAccessor = new Lazy<IHttpContextAccessor>(value); }
        }

This is probably the only static property in our current architecture at the moment, which is a small shame but not the worse hack in the world.

The batch process have a similar architecture, the main difference being that the top layer is some C# code that compiles to an exe, and we are using a "Session per transaction" implementation of our Transaction interceptor, ie the interceptor implementation creates the Hibernate Session, and opens a transaction before invoking the wrapped service call, and then commits the transaction and closes then session after the call. This is flexible though - for example you can make sure your interceptor implementaion checks to see if a transaction is already running if you want to wrap multiple service calls in a single transaction or ISession.

I hope this makes sense! If you've got any questions fire away, or I'll try to dig up some code examples when I can find the time.

@jpenniman
Copy link

@jpenniman jpenniman commented Nov 1, 2019

@gusgorman, Ah, that was the missing piece... you have a custom WebSessionContext to manage session lifetime. Then you can rely on ISessionFactory.GetCurrentSession() to work correctly in both scenarios. That all makes perfect sense.

Yeah, some regard using ActionFilters for working with the NHibernate session as an anti-pattern because there are scenarios where an ActionFilter can be called more than once for a single request. For example, if you have an action that returns the results of another action method, which I think is a bad practice to begin with. The other way to handle that would be with a custom middleware and make it part of the request pipeline the you're guaranteed to only be called once. But, if it's working for you, great!

It would be nice to get something "official" into NHibernate itself (or an extension to) now that the full framework has been end-of-lifed (but will continue to get security updates for at least another 10 years). More and more companies are moving to .Net Core on Linux as part of their cloud migrations.

@felipeoriani
Copy link

@felipeoriani felipeoriani commented Nov 15, 2019

Hey guys. We have a huge project made in .Net Framework here where we use NHibernate as persistence layer. I was trying to implement a POC for a new version using Asp.Net Core with NHibernate. We have implemented the session per request on our current application and I was trying to find a way to implement a Middleware to do the same behaviour on the ASP.NET Core and after a lot of problems I found this thread. I would love to see some way to bind a session on some place and access it from SessionFactory.GetCurrentSession() but it seems to be not implemented/supported on .Net Core. I know a lot of people want it on .Net Core.

Is there any plans from NHIbernate Team to implement it? While it is not ready, what is a good approach to workaround on it (session/transaction per request)?

@danilobreda
Copy link

@danilobreda danilobreda commented Nov 15, 2019

@felipeoriani Using DI instead would be a better approach.
The idea of "scoped" on DI, is a session/transaction per request on asp net core, for example.
Here's a example of a DI Setup using SimpleInjector but you can use the DI that comes with aspnetcore.

A singleton sessionfactory.
kernel.RegisterSingleton<ISessionFactory>(() => MySessionFactoryProvider.CreateSessionFactory());

A scoped "ISession"
kernel.Register<IUnitOfWork, UnitOfWork>(Lifestyle.Scoped); (injection of isessionfactory on constructor)

thats only a example of the ideia

I still remain confident with my comment from almost 2 years ago.

A "deprecated" or something like that would be nice, because even on .net full, the use of a DI is suggestive for a right way that the plataform is supporting right now.

@felipeoriani
Copy link

@felipeoriani felipeoriani commented Nov 15, 2019

@danilobreda, yes I have been trying some ways to manage the ISession and ITransaction in a asp.net core web application and yes, it is the best way. I used to bind the ISession using the CurrentSessionContext but it is not available anymore because of the platform like I understand from this topic.

If we add the ISession from the AddScoped registry method on the ISessionCollection, I know it will just open the session when it is necessary to inject on any dependency and it's fine. But using Oracle, I have to clear the pools and close the Session. I have defined a Middleware to to do. Not sure if it is good but works fine.

public async Task InvokeAsync(HttpContext httpContext)
{
   await _next.Invoke(httpContext);

   ISession session = httpContext.RequestServices.GetService<ISession>();
   if (session != null)
   {
       ClearPool(session.Connection);

       if (session.IsOpen)
          session.Close();
               
    }
}

If someone looks how to close an ISession, it's working fine (for async/non-async requests).

Using Transactions over the ACtionFilter is a good pratice? If not, how to encapsulate it from the controller scoped? I do not want to perform transactional operations from the controller scope.

@erizzo
Copy link

@erizzo erizzo commented Mar 27, 2020

@jpenniman I have 2 questions about the DI managed pattern that you describe:

  1. Do you open a transaction when creating the session? In Java I would use the Spring annotations for declarative transaction boundaries, but I don't think .Net has the equivalent. And I don't like individual services or controllers having to explicitly manage transactions all the time (sometimes yes, but those are exceptions).
    On a related note, how does the transaction get rolled back if an exception occurs during the request? Is explicitly calling session.Transaction.Rollback() not necessary?
  2. What about things that aren't subject to DI injection? I'm thinking specifically of controller attributes (action filters) that might need the session.
@jpenniman
Copy link

@jpenniman jpenniman commented Mar 28, 2020

@erizzo, good questions. I'll try and answer as best I can...

  1. Do you open a transaction when creating the session?

That depends on the scenario. Personally, I always manage explicit transactions and do so as close to the database work as possible to shorten transaction times and reduce the likelihood of deadlocks or other performance issues related to long transactions. This is exactly what you would be doing with the @Transaction attribute in Spring Data JPA, but as you said, there is no .Net equivalent. (You could always recreate the behavior with PostSharp).

Let's look at some options...

  1. If you do nothing, NHibernate will fallback to implicit transactions. A transaction will be created and committed (or rolled back) automatically every time you call a persistence method such as ISession.Save(). This is not recommended and is discouraged in the docs:

We advise against such a pattern, because this causes each single interaction of the session with the database to be a transaction on its own. This causes overhead, breaks the unit of work in case of errors during flushing, and causes some features to be disabled like auto-flush and second level cache.

  1. Explicitly begin the transaction at the start of a request, and commit at the end of the request. You could handle this with a custom middleware. Rollback would be handled in a custom exception middleware/filter. However, this means that exceptions would have to bubble-up all the way to the middleware/global exception handler. This may not always be desirable. This approach also leaves a connection and transaction open for the entire request. This is a code smell even without NHibernate. It also means you end up with a session even when you don't need one.

  2. Wrap ISession in a custom UnitOfWork and Repository. I see this a lot. I personally find this approach redundant as ISession is an implementation of the Repository Pattern and UnitOfWork Pattern. However, this can add advantages in unit testing as well as an abstraction around transactions and when they happen.

  3. Do what I do and begin and end the transaction as close to the work as you can. Since I typically don't use repository classes when using NHibernate (since ISession is a repository) I do it in my service class. As I mentioned earlier, you could use PostSharp to put that boilerplate code in an aspect.

On a related note, how does the transaction get rolled back if an exception occurs during the request? Is explicitly calling session.Transaction.Rollback() not necessary?

Anything that has not been committed with rollback when the session is closed.

What about things that aren't subject to DI injection? I'm thinking specifically of controller attributes (action filters) that might need the session.

We have access to the container in ActionFilters in the context. So while they do no support constructor injection, you can use service location...

public override void OnActionExecuting(ActionExecutingContext context)
{
    ISession session  = context.HttpContext.RequestServices.GetService<ISession>();
}

Cusom middleware does support constructor injection and is the recommended way of providing singleton dependencies to middleware:

Middleware should follow the Explicit Dependencies Principle by exposing its dependencies in its constructor. Middleware is constructed once per application lifetime.

...but because middleware is only constructed once, you can use service location to grab the ISession instance. Again, the container is available on the context:

public async Task Invoke(HttpContext httpContext)
{
    ISession session  = httpContext.RequestServices.GetService<ISession>();
    await _next(httpContext);
}

... or Microsoft's recommended approach of method injection, though this will only work with Microsoft's DI...

Because middleware is constructed at app startup, not per-request, scoped lifetime services used by middleware constructors aren't shared with other dependency-injected types during each request. If you must share a scoped service between your middleware and other types, add these services to the Invoke method's signature. The Invoke method can accept additional parameters that are populated by DI

public async Task Invoke(HttpContext httpContext, ISession session)
{
    await _next(httpContext);
}

I hope this helps. Cheers!

@felipeoriani
Copy link

@felipeoriani felipeoriani commented Mar 28, 2020

Very nice answer @jpenniman,

I have been implementing the transaction from the controller scope with an actionFilter. We can access the container from the context.RequestServices and get the current ISession.

I know the ISession will be closed, but we have an enterprise application that can run over Sql Server, Oracle and PostgreSql. In the case of Oracle, I have to clear the pools from the DbConnection inside the ISession to avoid problems with the next connections. We created a custom middleware and just call the next and then give the correct treatment to the ISession.

public async Task InvokeAsync(HttpContext httpContext)
{
    await _next.Invoke(httpContext);

    ISession session = httpContext.RequestServices.GetService<ISession>();

    if (session != null)
    {
        ClearPool(session.Connection);

        if (session.IsOpen)
            session.Close();                
    }
}

I am not sure if it is a good pratice to create a middleware for it, but that's the solution I got. If you think there is a better way, please share with us.

Thank you!

@jpenniman
Copy link

@jpenniman jpenniman commented Mar 29, 2020

Thanks @felipeoriani. It's interesting that you have to clear the connection pool in Oracle. I haven't run across the need for it before. I used Oracle quite a bit with full framework, but not much with corefx (mostly because of poor driver support from Oracle). If you want to eliminate the need, you could try a different provider. Or open a ticket with Oracle. It sounds more like a connection pool bug than a NHibernate thing.

As far as best practice, I would say, if your solution works, isn't causing any performance issues, and you can test your code easily, then stick with it, and document it well, both in the code and out. The only downside I can see is it's always creating a session regardless if one is needed.

@pastidev
Copy link

@pastidev pastidev commented May 19, 2020

@jpenniman thank you to give some light about this topic, i've tried to understand why people keeps trying to use WebSessionContext and check the binding on the CurrentSessionContext in examples like NHibernate and can't get it, because like you say if you use DI (requestScope) of ISession you get the same session for all the request.

Now i'm using self hosted owin with autofac trying to emulate an old example made for web api with ninject and hibernate. If i use autofac and check the activation of a ISession only gets activated once, but the same using the original webapi2 and ninject (no owin) is activated each time a ISession is requested. I think that's because they used the check on CurrentSessionContext to Bind a new session or not. But i don't understand why with autofac works without that check.

Autofac code

container.RegisterInstance(sessionFactory).As<ISessionFactory>();
container.Register(x => {
    return x.Resolve<ISessionFactory>().OpenSession();           
}).As<ISession>().InstancePerRequest();

Ninject code

container.Bind<ISessionFactory>().ToConstant(sessionFactory);
container.Bind<ISession>().ToMethod(CreateSession).InRequestScope();
private ISession CreateSession(IContext context)
   {
        var sessionFactory = context.Kernel.Get<ISessionFactory>();
        if (!CurrentSessionContext.HasBind(sessionFactory))
        {
            var session = sessionFactory.OpenSession();
            CurrentSessionContext.Bind(session);
        }
        return sessionFactory.GetCurrentSession();
     }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet