NInject, nHibernate, and auditing in ASP.NET MVC

Question

I am working on an inherited application which makes use of NInject and nHibernate as part of an ASP.NET MVC (C#) application. Currently, I'm looking at a problem with the auditing of modifications. Each entity has ChangedOn/ChangedBy and CreatedOn/CreatedBy fields, which are mapped to database columns. However, these either get filled with the wrong username or no username at all. I think this is because it has been configured in the wrong way, but I don't know enough about nHibernate and NInject to solve the issue, so I hope someone can help. Below some code snippets to hopefully provide sufficient insight in the application.

Creating the session factory and session:

public class NHibernateModule : NinjectModule
{
    public override void Load()
    {
        Bind<ISessionFactory>().ToProvider(new SessionFactoryProvider()).InSingletonScope();

        Bind<ISession>().ToProvider(new SessionProvider()).InRequestScope();
        Bind<INHibernateUnitOfWork>().To<NHibernateUnitOfWork>().InRequestScope();
        Bind<User>().ToProvider(new UserProvider()).InRequestScope();
        Bind<IStamper>().ToProvider(new StamperProvider()).InRequestScope();
    }
}

public class SessionProvider : Provider<ISession>
{
    protected override ISession CreateInstance(IContext context)
    {
        // Create session
        var sessionFactory = context.Kernel.Get<ISessionFactory>();            
        var session = sessionFactory.OpenSession();            
        session.FlushMode = FlushMode.Commit;

        return session;
    }
}

public class SessionFactoryProvider : Provider<ISessionFactory>
{
    protected override ISessionFactory CreateInstance(IContext context)
    {
        var connectionString = ConfigurationManager.ConnectionStrings["DefaultConnectionString"].ToString();
        var stamper = context.Kernel.Get<IStamper>();

        return NHibernateHelper.CreateSessionFactory(connectionString, stamper);
    }
}

public class StamperProvider : Provider<IStamper>
{
    protected override IStamper CreateInstance(IContext context)
    {
        System.Security.Principal.IPrincipal user = HttpContext.Current.User;
        System.Security.Principal.IIdentity identity = user == null ? null : user.Identity;
        string name = identity == null ? "Unknown" : identity.Name;

        return new Stamper(name);
    }
}

public class UserProvider : Provider<User>
{
    protected override UserCreateInstance(IContext context)
    {
        var userRepos = context.Kernel.Get<IUserRepository>();

        System.Security.Principal.IPrincipal user = HttpContext.Current.User;
        System.Security.Principal.IIdentity identity = user == null ? null : user.Identity;
        string name = identity == null ? "" : identity.Name;

        var user = userRepos.GetByName(name);
        return user;
    }
}

Configuring the session factory:

public static ISessionFactory CreateSessionFactory(string connectionString, IStamper stamper)
    {
        // Info: http://wiki.fluentnhibernate.org/Fluent_configuration
        return Fluently.Configure()
                .Database(MsSqlConfiguration.MsSql2008
                    .ConnectionString(connectionString))
                .Mappings(m => 
                    {
                        m.FluentMappings
                            .Conventions.Add(PrimaryKey.Name.Is(x => "Id"))
                            .AddFromAssemblyOf<NHibernateHelper>();

                        m.HbmMappings.AddFromAssemblyOf<NHibernateHelper>();
                    })
                // Register 
                .ExposeConfiguration(c => {
                    c.EventListeners.PreInsertEventListeners = 
                        new IPreInsertEventListener[] { new EventListener(stamper) };
                    c.EventListeners.PreUpdateEventListeners =
                        new IPreUpdateEventListener[] { new EventListener(stamper) };
                })
                .BuildSessionFactory();
     }

Snippet from the eventlistener:

public bool OnPreInsert(PreInsertEvent e)
{
    _stamper.Insert(e.Entity as IStampedEntity, e.State, e.Persister);
    return false;
}

As you can see the session factory is in a singleton scope. Therefore the eventlistener and stamper also get instantiated in this scope (I think). And this means that when the user is not yet logged in, the username in the stamper is set to an empty string or "Unknown". I tried to compensate for this problem, by modifying the Stamper. It checks if the username is null or empty. If this is true, it tries to find the active user, and fill the username-property with that user's name:

    private string GetUserName()
    {
        if (string.IsNullOrWhiteSpace(_userName))
        {
            var user = ServiceLocator.Resolve<User>();

            if (user != null)
            {
                _userName = user.UserName;
            }
        }

        return _userName;
    }

But this results in a completely different user's name, which is also logged in to the application, being logged in the database. My guess this is because it resolves the wrong active user, being the last user logged in, instead of the user that started the transaction.

Answer 1

The offending parts are here:

Bind<ISessionFactory>().
    .ToProvider(new SessionFactoryProvider())
    .InSingletonScope();

Bind<IStamper>()
    .ToProvider(new StamperProvider())
    .InRequestScope();

And later on:

public class SessionFactoryProvider : Provider<ISessionFactory>
{
    protected override ISessionFactory CreateInstance(IContext context)
    {
        // Unimportant lines omitted
        var stamper = context.Kernel.Get<IStamper>();
        return NHibernateHelper.CreateSessionFactory(connectionString, stamper);
    }
}

public class StamperProvider : Provider<IStamper>
{
    protected override IStamper CreateInstance(IContext context)
    {
        // Unimportant lines omitted
        string name = /* whatever */
        return new Stamper(name);
    }
}

Let's analyze what's going on with the code:

• The ISessionFactory is bound as single-instance. There will only ever be one throughout the lifetime of the process. This is fairly typical.

• The ISessionFactory is initialized with SessionFactoryProvider which immediately goes out to get an instance of IStamper, and passes this as a constant argument to initialize the session factory.

• The IStamper in turn is initialized by the StamperProvider which initializes a Stamper class with a constant name set to the current user principal/identity.

The net result of this is that as long as the process is alive, every single "stamp" will be assigned the name of whichever user was first to log in. This might even be the anonymous user, which explains why you're seeing so many blank entries.

Whoever wrote this only got half the equation right. The IStamper is bound to the request scope, but it's being supplied to a singleton, which means that only one IStamper will ever be created. You're lucky that the Stamper doesn't hold any resources or have any finalizers, otherwise you'd probably end up with a lot of ObjectDisposedException and other weird errors.

There are three possible solutions to this:

1. (Recommended) - Rewrite the Stamper class to look up the current user on each call, instead of being initialized with static user info. Afterward, the Stamper class would no longer take any constructor arguments. You can the bind the IStamper InSingletonScope instead of InRequestScope.

2. Create an abstract IStamperFactory with a GetStamper method, and a concrete StamperFactory which implements it by wrapping the IKernel instance. Bind these together InSingletonScope. Have your concrete factory return kernel.Get<IStamper>(). Modify the session factory to accept and hold an IStamperFactory instead of an IStamper. Each time it needs to stamp, use the factory to get a new IStamper instance.

3. Change the ISessionFactory to be InRequestScope. Not recommended because it will hurt performance and potentially mess up ID generators if you don't use DB-generated identities, but it will solve your auditing problem.

Answer 2

Aaronaught, you're analysis describes exactly what I suspected. However, I found there is a fourth solution which is easier and more straightforward IMHO. I modified the sessionprovider, such that the call to OpenSession takes an instance of IInterceptor as argument. As it turns out, the event listeners aren't actually supposed to be used for auditing (a bit of a rant, but other than that he is right, according to Fabio as well).

The AuditInterceptor implements OnFlushDirty (for auditing existing entities) and OnSave (for auditing newly created entities). The SessionProvider looks as below:

public class SessionProvider : Provider<ISession>
{
    protected override ISession CreateInstance(IContext context)
    {
        // Create session
        System.Security.Principal.IPrincipal user = HttpContext.Current.User;
        System.Security.Principal.IIdentity identity = user == null ? null : user.Identity;
        string name = identity == null ? "" : identity.Name;

        var sessionFactory = context.Kernel.Get<ISessionFactory>();
        var session = sessionFactory.OpenSession(new AuditInterceptor(name));            
        session.FlushMode = FlushMode.Commit;

        return session;
    }
}