NH-4008 - Split database providers into own projects. #626

ngbrown · 2017-05-17T15:47:38Z

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 · 2017-05-17T21:34:42Z

@ngbrown great work, as usual.

hazzik · 2017-05-17T21:35:12Z

Shouldn't dialects be in separate packages?

ngbrown · 2017-05-17T21:42:53Z

@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 · 2017-05-17T21:46:38Z

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 · 2017-05-18T01:26:04Z

@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 · 2017-07-13T14:43:14Z

@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 · 2017-07-15T03:47:38Z

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 · 2017-07-15T21:34:24Z

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

Maybe the second throw in the ConnectionProvider.ConfigureDriver should be adjusted for hinting at including relevant Nuget package.

About the Nuget packages, still waiting for the release build (supposed to be triggered by teamcity once per day).

fredericDelaporte · 2017-07-15T20:55:38Z

ShowBuildMenu.bat

@@ -15,7 +15,7 @@ echo.
 echo --- TESTING ---
 echo B. (Step 1) Set up a new test configuration for a particular database.
 echo C. (Step 2) Activate a test configuration.
-echo D. (Step 3) Run tests using active configuration.
+echo D. (Step 3) Run tests using active configuration (Build in Visual Studio).


You mean "Built"?

No, I mean to run this test step, you have to build in Visual Studio first. Performing the build from the menu outputs to a different location.

Precisely, I would use "built" for meaning this. It should have already been built in Visual Studio.

Reworded to "Needs built in Visual Studio".

fredericDelaporte · 2017-07-15T20:57:55Z

ShowBuildMenu.bat

-start "nunit3-console" cmd /K %NUNIT% --x86 --agents=1 --process=separate NHibernate.nunit
+SET NUNITPLATFORM=
+IF /I "%PLATFORM%" NEQ "x64" set NUNITPLATFORM=--x86
+start "nunit3-console" cmd /K %NUNIT% %NUNITPLATFORM% --agents=1 --process=separate NHibernate.nunit


Is switching the tests to 64bits related to NH-4008?

I was just trying to get it to work correctly from a mistake I had made in NH-3900 (update to NUnit 3). There were 64 bit configurations available (SQLite and SqlServer.Compact) that weren't running correctly.

fredericDelaporte · 2017-07-15T21:21:11Z

src/NHibernate.Test/NHibernate.Test.csproj

@@ -35,6 +35,8 @@
    <StartProgram>$(MSBuildProjectDirectory)\..\..\Tools\nunit\nunit-x86.exe</StartProgram>
    <StartArguments>NHibernate.Test.dll</StartArguments>
    <TargetFrameworkProfile />
+    <NuGetPackageImportStamp>
+    </NuGetPackageImportStamp>


Seems to be an old thing from VS2013.

Removed, but came from VS 2017...

fredericDelaporte · 2017-07-15T21:29:59Z

src/NHibernate/SqlCommand/Parser/MsSqlSelectParser.cs

-	using System.Linq;
-
-	/// <summary>
+    /// <summary>
 	/// Represents SQL Server SELECT query parser, primarily intended to support generation of 


Whitespace miss.

fredericDelaporte · 2017-07-15T21:44:18Z

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 · 2017-07-16T11:25:15Z

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

ngbrown · 2017-07-16T20:03:21Z

@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 · 2017-07-16T20:11:36Z

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.

fredericDelaporte · 2017-07-16T11:20:50Z

build-common/teamcity-hibernate.cfg.xml

@@ -15,7 +15,7 @@
 			<property name="format_sql">true</property>

 			<!-- This is the System.Data.dll provider for MSSQL Server -->
-			<property name="connection.driver_class">NHibernate.Driver.SqlClientDriver</property>
+			<property name="connection.driver_class">NHibernate.Driver.SqlClientDriver, NHibernate.Driver.SqlClient</property>


Should be NHibernate.Driver.SqlServerDriver, NHibernate.Driver.SqlServer, shouldn't it?

Yes. Fixed. Must not be used for TeamCity builds though...

Indeed, that is overridden by default values from nhibernate-properties.xml or specific values defined for the build.

ngbrown · 2018-03-13T13:36:53Z

I split off some pre-work in #1610 that can be merged quickly.

For this pull request I needed to put some more effort into the tests, and was hoping the effort wouldn't be wasted.

ngbrown · 2018-03-13T20:02:54Z

This pull request is now rebased.

The commit, 'In psake.ps1, build and test in one step.' is needed because not every project can (or should) target netcoreapp2.0.

…inaries.

fredericDelaporte

NHibernate.sln.DotSettings should be reverted and a condition is lacking in my opinion in NHibernate.Driver.SQLite.csproj.

fredericDelaporte · 2018-03-16T11:25:55Z

src/NHibernate.Driver.Oracle.Managed/NHibernate.Driver.Oracle.Managed.csproj

+    <Description>Driver for Oracle databases to be used with NHibernate 5.</Description>
+    <PackageTags>NHibernate; Driver; ODP.Net; Oracle; ADO.Net</PackageTags>
+
+    <TargetFramework>net461</TargetFramework>


For information, a beta version of ODP.Net for .Net Core 2.0 is available apparently since mid-February. But is it worth trying it, since this would also imply investigating testing Oracle at least with Travis?

And their statement of direction also includes:

ODP.NET Core beta requires Microsoft .NET Core 2.0.2 or higher. .NET Core 2.0 added applicationprogramming
interfaces that are required for ODP.NET Core to work properly. The ODP.NET Core
production release may have different minimum .NET Core version requirements than the beta.

Since the beta version of Oracle.ManagedDataAccess with .NET Core support isn't yet on NuGet, it shouldn't be added. Once it is, then yes, that would be an option.

I addressed the rest of the feedback as well.

fredericDelaporte · 2018-03-16T11:35:58Z

src/NHibernate.Driver.SQLite/NHibernate.Driver.SQLite.csproj

+  </ItemGroup>
+
+  <!-- This next section is in addition to the .target file for project references. -->
+  <ItemGroup>


Shouldn't this be guarded by a Condition="'$(TargetFramework)' == 'net461'" for consistency?

(SqlServer.Compact is somewhat similar here, but I am not adding a comment in it because it will normally never have a .Net Core version.)

fredericDelaporte · 2018-03-16T11:49:11Z

src/NHibernate.Test/DriverTest/OracleDataClientDriverFixture.cs

+#if NETFX
+					(OracleDataClientDriverBase)new OracleManagedDriver()
+#else
+					throw new NotImplementedException("OracleManagedDriver")


I was wondering why an exception rather than an ignore, but indeed it makes sens. Since by the time this code path ends up actually executed, we should have added a .Net Core managed driver.

fredericDelaporte · 2018-03-16T11:57:01Z

src/NHibernate.sln.DotSettings

@@ -1,13 +1,16 @@
 <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
-	<s:String x:Key="/Default/CodeStyle/CodeFormatting/CommonFormatter/ALIGNMENT_TAB_FILL_STYLE/@EntryValue">USE_SPACES</s:String>
+


Last time I have attempted letting this annoying DotSettings recurrent update slip in a PR, I was asked to revert it. Better do the same I think. (I have personally fixed that annoyance by removing write rights on the file... Maybe the old readonly flag would do it too.)

* Revert ReSharper .DotSettings file migrations * Add guard to SQLite driver package.

ngbrown · 2018-03-26T15:07:02Z

Can this be in 5.2 then?

fredericDelaporte · 2018-04-05T09:47:42Z

With the requests on #1635 and #1638, maybe should we go some steps more far, splitting them in a new sub repository like nhibernate/providers. This would also allow to publish new providers without having to wait for a nhibernate-core release.

I may work on this later.

ngbrown · 2018-04-05T16:28:47Z

There are several reasons to keep all drivers in this main repository:

The drivers will still be needed for the various unit tests of each of the databases.
This repository already has the tooling and scripts for building, testing, and releasing any number of NuGet packages.
The implementation of the separate NuGet packages needs to stay synchronized with the reflection based driver until they are removed (NHibernate 6.0, if this pull request get in on a 5.x release).
There's not a lot of code in these drivers, so they aren't going to need a lot of updates.
When adding new drivers, back-versioning them to the last NHibernate release when adding them to NuGet shouldn't be a problem.
The driver's don't have to stay up to date with each ADO.Net release because the NuGet reference to the 3rd party ADO.Net driver is only a minimum version; an end application can force an update to a newer version by adding a direct reference to the ADO.Net driver.

maulik-modi · 2021-09-04T09:00:10Z

@ngbrown and @fredericDelaporte , This is excellent feature so individual project can be upgraded without affecting other drivers. Moreover, drivers such as System.DAta.OracleClient could be removed in future.

We are eagerly awaiting this feature. What is holding us here?

ngbrown force-pushed the NH-4008 branch from a6d9d07 to 91bb4ac Compare May 17, 2017 20:41

ngbrown force-pushed the NH-4008 branch 2 times, most recently from 4099f92 to 39a1466 Compare May 18, 2017 01:14

ngbrown force-pushed the NH-4008 branch from 39a1466 to 4abb75f Compare May 27, 2017 00:58

This was referenced May 28, 2017

NH-3807 - Support for .NET Core 2.0 #633

Closed

NH-4026 - Upgrade Firebird driver and use server #639

Merged

ngbrown force-pushed the NH-4008 branch 2 times, most recently from 5c5e6c9 to 47da70e Compare June 14, 2017 13:35

ngbrown force-pushed the NH-4008 branch 2 times, most recently from 8c3815d to 969deee Compare June 27, 2017 02:53

ngbrown force-pushed the NH-4008 branch 3 times, most recently from 8656d25 to ededf50 Compare July 13, 2017 08:17

ngbrown force-pushed the NH-4008 branch 2 times, most recently from c715398 to 57fd898 Compare July 15, 2017 03:44

fredericDelaporte reviewed Jul 15, 2017

View reviewed changes

ngbrown force-pushed the NH-4008 branch from 57fd898 to 04c9ee2 Compare July 16, 2017 19:57

fredericDelaporte reviewed Jul 16, 2017

View reviewed changes

ngbrown mentioned this pull request Mar 13, 2018

Move MsSql constants from driver to dialect. #1610

Merged

ngbrown force-pushed the NH-4008 branch 2 times, most recently from 7db26f7 to 820f754 Compare March 13, 2018 19:50

ngbrown force-pushed the NH-4008 branch from 13ccf4e to 887f1c8 Compare March 14, 2018 12:18

ngbrown and others added 7 commits March 14, 2018 05:53

NH-4008 - Centralize all driver detection for tests.

e2c50ad

NH-4008 - Add separate driver projects for NuGet dependencies.

d7a5541

NH-4008 - Mark all usages of obsolete drivers so there's no errors.

9cb4978

NH-4008 - Enable separate drivers for tests.

fc338c7

NH-4008 - document driver changes.

452f1d4

In psake.ps1, build and test in one step.

912fcd5

NH-4008 - Enable separate drivers for tests in Appveyor and Travis-CI.

527c26d

ngbrown force-pushed the NH-4008 branch from 887f1c8 to 527c26d Compare March 14, 2018 12:53

ngbrown added 2 commits March 15, 2018 17:54

Merge branch 'master' into NH-4008

83cf9d7

NH-4008 - Improve project to project dependencies for SQLite native b…

c8566cc

…inaries.

fredericDelaporte reviewed Mar 16, 2018

View reviewed changes

fredericDelaporte mentioned this pull request Mar 16, 2018

5.1.0 release #1525

Merged

NH-4008 - Addressed feedback

8717ec5

* Revert ReSharper .DotSettings file migrations * Add guard to SQLite driver package.

fredericDelaporte approved these changes Mar 16, 2018

View reviewed changes

This was referenced Apr 4, 2018

IdentitySelectString implementation is inconsistent #1635

Closed

SQLite not supported on CoreFX #1638

Closed

Merge branch 'master' into NH-4008

6cddb06

fredericDelaporte mentioned this pull request Apr 13, 2018

Add support for SAP HANA #1662

Merged

hazzik added this to the 6.0 milestone Apr 11, 2019

fredericDelaporte mentioned this pull request Aug 8, 2021

SAP HANA and .NET Core #2154

Closed

		@@ -1,13 +1,16 @@
		<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
		<s:String x:Key="/Default/CodeStyle/CodeFormatting/CommonFormatter/ALIGNMENT_TAB_FILL_STYLE/@EntryValue">USE_SPACES</s:String>

NH-4008 - Split database providers into own projects. #626

Are you sure you want to change the base?

NH-4008 - Split database providers into own projects. #626

Conversation

ngbrown commented May 17, 2017 • edited Loading

hazzik commented May 17, 2017

hazzik commented May 17, 2017

ngbrown commented May 17, 2017

ngbrown commented May 17, 2017 • edited Loading

ngbrown commented May 18, 2017

ngbrown commented Jul 13, 2017

ngbrown commented Jul 15, 2017

fredericDelaporte commented Jul 15, 2017

fredericDelaporte left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredericDelaporte commented Jul 15, 2017 • edited Loading

fredericDelaporte commented Jul 16, 2017

ngbrown commented Jul 16, 2017

fredericDelaporte commented Jul 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngbrown commented Mar 13, 2018

ngbrown commented Mar 13, 2018 • edited Loading

fredericDelaporte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngbrown Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngbrown commented Mar 26, 2018

fredericDelaporte commented Apr 5, 2018

ngbrown commented Apr 5, 2018

maulik-modi commented Sep 4, 2021

ngbrown commented May 17, 2017 •

edited

Loading

ngbrown commented May 17, 2017 •

edited

Loading

fredericDelaporte left a comment •

edited

Loading

fredericDelaporte commented Jul 15, 2017 •

edited

Loading

ngbrown commented Mar 13, 2018 •

edited

Loading

ngbrown Mar 16, 2018 •

edited

Loading