nhibernate / nhibernate-core

NHibernate Object Relational Mapper
https://nhibernate.info
GNU Lesser General Public License v2.1
2.13k stars 928 forks source link

HiLoGenerator is not aware of MultiTenancy #2834

Open mdshaner opened 3 years ago

mdshaner commented 3 years ago

While working to migrate over to use the MultiTenancy Database strategy, I ran into a few problems rooted around the HiLoGenerator.

Shared HiLo Values

The TableHiLoGenerator's values are shared across all tenants. For my use case, this is actually not too difficult to work around with a custom Generator that uses a ConcurrentDictionary to isolate the values for each tenant. Honestly I'm only mentioning it because it's also the entry point that I encounter for the next issue.

Isolated Work does not consider Tenancy when opening connection

With a MultiTenancy (database) strategy, isolated scope work does not consume the Tenant configuration. It appears to be because in AdoNetTransactionFactory we get a connection by going to the factory without considering the Tenant. I have a local change that appears to work, but it also seems like it may be too heavy :

        private static DbConnection GetConnection(ISessionImplementor session)
        {
            // We make an exception for SQLite and use the session's connection,
            // since SQLite only allows one connection to the database.
            if (session.Factory.Dialect is SQLiteDialect)
            {
                return session.Connection;
            }

            // When using database multi-tenancy strategy we need to retrieve the appropriate connection
            if (session.Factory.Settings.MultiTenancyStrategy == MultiTenancyStrategy.Database && session is AbstractSessionImpl sessionImpl)
            {
                var conn = session.Factory.Settings.MultiTenancyConnectionProvider.GetConnectionAccess(sessionImpl.TenantConfiguration, session.Factory);           
                return session.Factory.ConnectionProvider.GetConnection(conn.ConnectionString);
            }

            return session.Factory.ConnectionProvider.GetConnection();
        }

However, I do not know how to start with writing a test case due to needing at least two databases setup for the multitenancy scenario to make sense.

bahusoid commented 3 years ago

However, I do not know how to start with writing a test case due to needing at least two databases setup for the multitenancy scenario to make sense.

Yeah we don't have true multi-tenancy tests. Current tests just use MS SqlServer AppName feature. See https://github.com/nhibernate/nhibernate-core/blob/master/src/NHibernate.Test/MultiTenancy/DatabaseStrategyNoDbSpecificFixture.cs

bahusoid commented 3 years ago

Isolated Work does not consider Tenancy when opening connection

This one is fixed in 5.3.9 with #2835