NH-4008 - Split database providers into own projects.

[ngbrown]

Implements ~NH-4008~ #847.

This will also be critical in being clear as to what will be supported under .netstandard2.0, and what will not. It will also make it easier to pull all relevant dependencies together with the driver packages depending on both the main NuGet NHibernate package, and the end database NuGet package.

It separates all the implementations of DriverBase into separate projects to allow those individual projects to pull in NuGet dependencies. This means that connection.driver_class in configuration files will probably have to be assembly qualified now.

I also added NHibernate.Cfg.Loquacious helpers to enable quick configuration when using configure-by-code, much like how middleware is added to OWIN in Asp.Net projects.

The update matrix looks like this:

Previous connection.driver_class	New connection.driver_class	New Configure-by-code helper
NHibernate.Driver.FirebirdClientDriver	NHibernate.Driver.FirebirdClientDriver, NHibernate.Driver.Firebird	cfg.Connected.ByFirebirdClientDriver()
NHibernate.Driver.SqlClientDriver	NHibernate.Driver.SqlServerDriver, NHibernate.Driver.SqlServer	cfg.Connected.BySqlServerDriver()
NHibernate.Driver.Sql2008ClientDriver	NHibernate.Driver.SqlServer2008Driver, NHibernate.Driver.SqlServer	cfg.Connected.BySqlServer2008Driver()
NHibernate.Driver.MySqlDataDriver	NHibernate.Driver.MySqlDataDriver, NHibernate.Driver.MySql	cfg.Connected.ByMySqlDataDriver()
NHibernate.Driver.NpgsqlDriver	NHibernate.Driver.NpgsqlDriver, NHibernate.Driver.Npgsql	cfg.Connected.ByNpgsqlDriver()
NHibernate.Driver.SQLite20Driver	NHibernate.Driver.SQLiteDriver, NHibernate.Driver.SQLite	cfg.Connected.BySQLiteDriver()
NHibernate.Driver.SqlServerCeDriver	NHibernate.Driver.SqlServerCompactDriver, NHibernate.Driver.SqlServer.Compact	cfg.Connected.BySqlServerCompactDriver()
NHibernate.Driver.OracleManagedDataClientDriver	NHibernate.Driver.OracleManagedDataClientDriver, NHibernate.Driver.Oracle.Managed	cfg.Connected.ByOracleManagedDataClientDriver()

Over time, as more of the underlying official Ado.Net drivers become available on NuGet, they can be moved to their own package.

There are still 14 drivers that don't have official Ado.Net drivers on NuGet, so they will remain as reflection based for now.

[hazzik]

@ngbrown great work, as usual.

[hazzik]

Shouldn't dialects be in separate packages?

[ngbrown]

@hazzik I started that direction, but the only actual dependency was in the xmldoc comments, so that's why I didn't move them at first. I can circle around again and see if it makes sense moving them.

Then I can expand the use of configure-by-code to something like cfg.UsingNpgsql() to setup the driver and dialects at the same time.

[ngbrown]

I also really wanted to rename NHibernate.Driver.SqlClient to NHibernate.Driver.SqlServer to match how Entity Framework does it and the NHibernate.Driver.SqlServer.Compact. What do you think? I've made enough other changes, so I might as well make that one?

[ngbrown]

@hazzik I remember why I didn't pull the dialects into the per-database projects... A major reason was the 5 Oracle dialects have 4 different drivers, and everything mixes and matches... Only one of which is officially available on NuGet; the Oracle.ManagedDataAccess one.

Any ideas?

[ngbrown]

@hazzik @fredericDelaporte I noticed this isn't marked as going into v5.0.0. I think this is a worthy breaking change and would like to see it merged.

The last thing to do is update the NAnt build here to create NuGet packages per driver.

If the Support for .NET Core 2.0 pull request (#633) is going to follow quickly it could be deferred to that. Everything done here for NuGet package is thrown away in that pull request, since msbuild is used in #633 and NAnt is used here.

[ngbrown]

I've updated the NAnt build to build and publish the NuGet packages. Importing secondary database drivers works even better that I originally hoped, even importing the native drivers for SQLite and SqlServer.Compact.

I did rename NHibernate.Driver.SqlClient to NHibernate.Driver.SqlServer to be consistent with SqlServer.Compact and Entity Framework and other ORM's naming scheme.

Once the build is finished, the nuget packages should be published on TeamCity's feed. Can someone try it out?

[fredericDelaporte]

About the dialects, due to the Oracle (and Sybase, and potentially Firebird) situation, having many drivers usable for the same database, so many usable drivers for the same dialect, extracting dialect from the main NHibernate project would require to put them in their own dialect projects, letting then the user choosing which drivers and which dialects he wants. (Maybe could we then put a dependency from the driver project to the dialects project, but with the Sybase case, it is not even sure we can.)

I do not think it is worth it.

[fredericDelaporte]

The firebird driver still has a method, ClearPool, which uses reflection while it does no more need to, since it now has a direct reference on the data provider.

[fredericDelaporte]

I have tested with a dummy mvc project, referencing the resulting Nuget packages (from a local repository) for NHibernate and SqlServer. It works.

[ngbrown]

@fredericDelaporte using the NuGet packages really shines when it can pull database specific drivers (such as Npgsql or System.Data.SQLite.Core down along with the respective NHibernate.Driver package. I noticed the NuGet builds aren't really versioned in TeamCity (allways 5.0.0-Alpha1), so dependencies between the preview releases may get messed up sometimes.

[fredericDelaporte]

We do not actually publish them, so it should not cause issues.

Of course for testing with local repositories, this could cause issues. But since only contributors will likely do that, they should be able to fix it.

By the way I have not checked how was handled that versioning. I have not yet been involved in Nuget publishing. Alexander knows that subject way better than me.

[hazzik]

I noticed this isn't marked as going into v5.0.0. I think this is a worthy breaking change and would like to see it merged.

I don't think that there is a rush to implement this for a 5.0. In fact, this is just nice-to-have feature, which can be implemented without breaking things.

In the current state, it would be useful to provide "aliases"/redirects for dialects and drivers. Eg, user can still specify "NHibernate.Driver.SqlClientDriver" but will get an warning that they need to update a configuration if the correct package is present, and error if package is not found.

[ngbrown]

In the current state, it would be useful to provide "aliases"/redirects for dialects and drivers. Eg, user can still specify "NHibernate.Driver.SqlClientDriver" but will get an warning that they need to update a configuration if the correct package is present, and error if package is not found.

I think this would have to happen at the configuration level, because C# doesn't have weak linking as far as I know... Duplicate class names in the same namespace across assemblies may result in ambiguous name exceptions.

If there was a way to make it a warning (which may not be noticed), then it could be introduced in 4.x and then break in 5.0.

Historically NHibernate has gone through major versions fairly slow, so with .NET Core 2.0 arriving by November, when would v6 be? I think the timing for getting this into v5 is better. .NET Core doesn't have to be supported in v5.0, but it could then be supported in v5.x without any breaking changes for .NET Framework users.

If the thinking is to port all the .NET Core changes back to the current driver layout, I would recommend against that. I certainly don't want to do anymore manual linking, and supporting .NET Core is even more dependent on NuGet for bringing the right combination of dependency versions in for the myriad of framework and versions supported by each individual package.

[hazzik]

I think this would have to happen at the configuration level, because C# doesn't have weak linking as far as I know... Duplicate class names in the same namespace across assemblies may result in ambiguous name exceptions.

There are few options:

a. Add a code which will manually translate the old type to a new type somewhere in Dialect#GetDialect or ReflectHelper#GetClassForName. Warning to be generated there as well.

b. There is a type forwarding feature, but I'm not sure how it'll work if the forwarded assembly is not found, as I rarely tried it.

Historically NHibernate has gone through major versions fairly slow, so with .NET Core 2.0 arriving by November, when would v6 be? I think the timing for getting this into v5 is better.

Think it this way: more features we are willing to add to a version, less often we will be able to release. So, it's definitely would be better to drop this feature out of radar.

If the thinking is to port all the .NET Core changes back to the current driver layout, I would recommend against that. I certainly don't want to do anymore manual linking, and supporting .NET Core is even more dependent on NuGet for bringing the right combination of dependency versions in for the myriad of framework and version supported by each individual package.

I did not get this point. In my view everything should just work for the ADO.NET providers which do support the .NET core and have the same name as it's "grown-up" Full .NET FX counterparts.

[ngbrown]

I can setup some warning or redirection, but I think it will still be a breaking change.

The timing of v6 is my concern. If v5 is coming out soonish, and v6 by the end of the year, then I can be fine with that.

I did not get this point. In my view everything should just work for the ADO.NET providers which do support the .NET core and have the same name as it's "grown-up" Full .NET FX counterparts.

Tracking down all the correct versions can be a pain. With NuGet packages that contain the correct dependencies, the developer can just pick the relevant NHibernate.Driver.WhateverDB package, and then adding some configuration-by-code to tie it all together.

[hazzik]

Tracking down all the correct versions can be a pain.

Does this assume that we need to release intermediate packages with the every new release of relevant ado.net provider?

With NuGet packages that contain the correct dependencies, the developer can just pick the relevant NHibernate.Driver.WhateverDB package, and then adding some configuration-by-code to tie it all together.

We can still do this, but without breaking changes, just by providing relevant NHibernate.Driver.WhateverDB packages in addition to built-in providers.

[fredericDelaporte]

We can still do this, but without breaking changes, just by providing relevant NHibernate.Driver.WhateverDB packages in addition to built-in providers.

Meaning we would keep all the reflection based drivers while adding the possibility to use Nuget's ones? Sound a good way of not breaking. Maybe should we add in those reflection based drivers constructor some log4net warning for hinting at migrating to Nuget based ones.

[ngbrown]

Since the driver packages re-use the namespace and class names of the original reflection-based drivers, adding back all the original reflection classes gets ugly fast with needing to use alias and extern to disambiguate what assembly you are trying to refer too.

If an intermediate step is really needed, how about a "meta-package"? The now driverless NHibernate project would have a PackageId of NHibernate.Core, and a new meta-package of NHibernate would pull in every Driver package. The hibernate-configuration connection.driver_class parsing would point out to the correct assemblies with a stern error message in the log.

I would name this meta-package NHibernate.All, but by not changing the name, there is the potential that just updating the package version from 4.x to the new one could actually work.

I think with all the packages pulled in, and the logged errors, and a changed description in NuGet.org, people would get the hint to include only the one Driver package, and not the overall package.

[ngbrown]

I've recreated this pull request on top of released v5.0. I've paired back the scope and am focusing just on pulling out the drivers that have external, maintained (that is official or semi-official), NuGet packages to depend on.

All drivers released in v5.0 will continue working as before, but if they have an NHibernate.Driver.XXX NuGet package available, they will be marked as [Obsolete] and have a logged message indicating as such (for when pulled in through .xml config)

The major idea is that the .NetStandard 2.0 targeted version of NHibernate will have no drivers included in the main package. That targeted version of NHibernate would support only drivers that directly depend on an external NuGet packaged version of the ADO.Net driver. Even as the same-version .NET 4.6.1 targeted version of NHibernate keeps the original set of reflection based drivers.

.NET Core 2.0 also doesn't support ODBC or OleDb drivers, so in addition to the reflection based drivers, they would also not be available.

[ngbrown]

@hazzik This is now done in a completely non-breaking-change way. Can it be slated for v5.1?

[annymsMthd]

+1 This makes a lot of sense. I've been following the .net standard work that you have been doing and I'm looking at sending in a pr for FluentNHibernate. This driver breakup is pretty essential. Some of the driver's just aren't supported on a .net standard.

[x2764tech]

@annymsMthd How far along are you with FluentNHibernate port? There's some discussion at https://github.com/jagregory/fluent-nhibernate/issues/361

[fredericDelaporte]

Maybe NHibernate drivers needs to target the lowest data provider version possible, and let user choose to use a newer one if he wishes. But for supporting NetStandard 2.0, the lowest version will likely not be very low.

Oracle versions greater than 12.1.2400 fail NH-1171 tests, it seems it has its own bug with comments in SQL. And it is a known bug since quite long as shown here, but apparently with some versions suffering less of it.

Npgsql latest (3.2.5) does no more support cases where NHibernate closes the connection from the second phase of distributed transaction. (A thing that our current doc advise against anyway, urging the user to flush himself its sessions at least when he closes them inside the transaction scope, and to disable the feature delaying connection close to transaction scope completion.)

[ngbrown]

Maybe NHibernate drivers needs to target the lowest data provider version possible, and let user choose to use a newer one if he wishes.

So that was mostly my original approach, but with these new commits, I brought it up to the level of your pull request because I didn't want to be the one breaking the PostgreSQL build for everyone.

Maybe the biggest issue though, is how the .csproj new<PackageReference \> usage has changed things. With the old projects, when importing a NuGet package, it also imported all child dependencies. This made it very simple to update or not update any packages as you so wished. With the new project system, the dependencies multiple levels down are not directly referenced by the top level project, and the need has to be fairly great to directly reference them and update the version. Because of that, I think the approach should be to reference a reasonably recent, highly-compatible version, and set that as the minimum.

Oracle versions greater than 12.1.2400 ...

Saw you changed yours, and so I changed mine back.

Npgsql latest (3.2.5) does no more support...

Targeting just Npgsql 3.2.4.1 now, I originally was targeting 3.2.2, and for the .net core port, that wasn't so I had split the dependencies and gone with the most recent for .net core.

[fredericDelaporte]

Be it for Npgsql or Oracle, if we need to upgrade for some reason, I do not think the failures they introduce should prevent us of upgrading. We should instead adjust the tests to ignore them. For Npgsql, because what fail with 3.2.5 is anyway a thing we now advise against using (not disabling transaction.use_connection_on_system_prepare when disposing sessions inside scopes: implies identifying tests doing that and adding a condition for ignoring them), for Oracle, because it clearly seems to be an external issue.

I do not think the package reference change is really an issue, as long as a project like NHibernate.Test is still able of overriding a lower version with a higher one if it wishes to.

[ngbrown]

Hi @fredericDelaporte I'll address everything here in one comment:

The doc/reference needs some updates, especially the driver setting documentation.

I would be inclined to do the documentation as a second pull request once I was sure this was going in by being approved and merged.

I would rather avoid the external alias trick.

The end developer generally wouldn't have to deal with this because they would use either the assembly qualified name in XML or extension methods for fluent configuration. Renaming is an option, but it isn't strictly necessary, and I didn't want to incur too much confusion.

The alias won't be needed long if the old drivers are removed in NHibernate 6 and live exclusively in their separate assemblies.

New projects should have <TreatWarningsAsErrors>True</TreatWarningsAsErrors>...

Fixed.

I would rather write all that with a switch

You are referring to the new C# 7 pattern matching. It is clearer, and did catch an error that I had.

Dialect must be non-static. It is configured, and the configuration is not static and can change. Needs to be reverted since GetFbTypeForParam can not be static.

Ok.

why removing the <see cref="System.Transactions.Transaction"/>? code is for block code and what was the harm of see?

The driver packages don't need to referenceSystem.Transactions for anything but these comments to avoid warning CS1580 and CS1574. Instead of referencing that additional assembly, I just changed them to pure comments.

I did change the <code> to <c>.

Could be renamed MySqlDriver, if we accept to not use a naming hinting at the chosen data provider.

There is an alternate ADO.Net drivers for MySQL that already has an issue open to be added to NHibernate (#1375) so the driver name should be for MySQL should indicate which one it supports.

It seems ceasing to use reflection has not be done for avoiding duplicating the code, due to the complexity of keeping old drivers with their common reflection base with a new driver having a non reflection base.

Yes, since the merge of all the Oracle drivers happened, I didn't want to undo all of that and then have to maintain a separate copy of the special logic. I did add an an override of CreateConnection and CreateCommand though.

If we consider our naming should hint at the chosen data provider, it would then need to be renamed SystemDataSQLiteDriver. But personally I am more for a neutral naming like SQLiteDriver chosen here.

There's actually a Microsoft supported driver for SQLite that works on .NET Core that I've tried to get working with NHibernate, but it has limitations on nested transactions. If it worked out, I was planning on having the assembly named NHibernate.Driver.SQLite.Microsoft, since the one currently being used in NHibernate is the "official" SQLite driver.

[fredericDelaporte]

Hi,

The doc/reference needs some updates, especially the driver setting documentation.

I would be inclined to do the documentation as a second pull request once I was sure this was going in by being approved and merged.

I am in favor of doing that within the PR causing the change to be documented. Documentation is too often forgotten. Of course postponing it till the PR changes are settled is a valid option, but better add a last commit with the documentation before merging.

I would rather avoid the external alias trick.

The end developer generally wouldn't have to deal with this because they would use either the assembly qualified name in XML or extension methods for fluent configuration. Renaming is an option, but it isn't strictly necessary, and I didn't want to incur too much confusion.

The alias won't be needed long if the old drivers are removed in NHibernate 6 and live exclusively in their separate assemblies.

Those using the loquacious IConnectionConfiguration.By<TDriver> may have a hard time finding out how to disambiguate the class to use when wanting to fix the obsolete warning they will get if referencing an "old" reflection base driver. That would mean at least more documentation (like a big detailed obsolete message for class with name conflicts) for guiding them, hinting to alternate way for not referencing the class directly and otherwise hinting at the extern alias trick.

Changing more names looks less confusing to me than having the class conflict.

Could be renamed MySqlDriver, if we accept to not use a naming hinting at the chosen data provider.

There is an alternate ADO.Net drivers for MySQL that already has an issue open to be added to NHibernate (#1375) so the driver name should be for MySQL should indicate which one it supports.

Same for Firebird, SQLite (as you write later) and probably some others who have an "official" data provider and some alternates. I think it is reasonable to not hint at the chosen data provider when we use the official one, and hint to it when we use an alternate one. Or otherwise, for having a really consistent naming for all the drivers we manage, name all of them with their specific data provider name.

Currently we use the "official" data provider (or mainstream) with a mix of naming: sometimes using the data provider name, other times just the db name. Of course Oracle is a special case, since it has many official data providers...

[ngbrown]

The Obsolete warning does say: "There are also Loquacious configuration points: .Connection.ByNpgsqlDriver() and .DataBaseIntegration(x => x.NpgsqlDriver())."

[ngbrown]

Since it seems like naming is hanging this up, while I am indifferent to changing all the names, here is something that would work. Primary official drivers available on NuGet are named based on the database they support. Additional drivers would be named as derivatives of that.

Previous connection.driver_class	New connection.driver_class
NHibernate.Driver.FirebirdClientDriver	NHibernate.Driver.FirebirdDriver, NHibernate.Driver.Firebird
NHibernate.Driver.SqlClientDriver	NHibernate.Driver.SqlServer2000Driver, NHibernate.Driver.SqlServer
NHibernate.Driver.Sql2008ClientDriver	NHibernate.Driver.SqlServer2008Driver, NHibernate.Driver.SqlServer
NHibernate.Driver.MySqlDataDriver	NHibernate.Driver.MySqlDriver, NHibernate.Driver.MySql
NHibernate.Driver.NpgsqlDriver	NHibernate.Driver.PostgreSqlDriver, NHibernate.Driver.PostgreSql
NHibernate.Driver.SQLite20Driver	NHibernate.Driver.SQLiteDriver, NHibernate.Driver.SQLite
NHibernate.Driver.SqlServerCeDriver	NHibernate.Driver.SqlServerCompactDriver, NHibernate.Driver.SqlServer.Compact
NHibernate.Driver.OracleManagedDataClientDriver	NHibernate.Driver.OracleManagedDriver, NHibernate.Driver.Oracle.Managed

Alternative ADO.Net drivers that could be ~supported~ included in the future could named like this:

• NHibernate.Driver.MySqlConnectorDriver, NHibernate.Driver.MySql.MySqlConnector (#1375)
• NHibernate.Driver.SQLiteMicrosoftDriver, NHibernate.Driver.SQLite.Microsoft *
• NHibernate.Driver.PostgreDotConnectDriver, NHibernate.Driver.Postgre.DotConnect
• etc...

I do think that any alternative driver projects and NuGet package generation should live in the main NHibernate repository. These drivers are fairly thin and easy to maintain. This will enable catching any compile-time breakage. The drivers' NuGet packages could end up becoming the primary root of the NHibernate dependency tree for client applications, so having them all in this repository will enable simultaneous publishing.

If these names are fine, I'll rename the new projects and drivers. I'll also base the documentation on these new names.

Update: We should be steering people away from using the base SqlClientDriver unless they have a SQL Server version older than 2008, so I propose renaming that to SqlServer2000Driver to be clear that it is older. (SQL Server 2005 went out of extended support 2016-04-12).