Remove System.Data.SqlClient from the NuGet dependency graph

[0xced]

Commit cff3c5b39d47aed9323976da5ef30a9196770d9e made it possible for the consumer to choose between System.Data.SqlClient or Microsoft.Data.SqlClient in order to avoid taking a hard dependency on either one.

So the System.Data.SqlClient dependency must also be removed from the nuspec else it will be used anyway.

[odinserj]

I think that System.Data.SqlClient default will be changed even for 1.7.31 (and also for 1.8.0 of course), because I see evidences with huge connection leaks in SQL Azure when this package is used instead of the newer Microsoft.Data.SqlClient. Also, if we remove references from the nuspec, some installations which don't have other packages that use the SQL Client may fail.

So it looks like it's better just to change the default to Microsoft.Data.SqlClient, and require this newer package for netstandard2.0 targets in nuspec instead of the old one. And require reference to the old one for compatibility reasons (while have the newer for defaults when installed).

I will modify this PR a bit later.

[odinserj]

What do you think?

[odinserj]

Also not sure we should dereference the System.Data.Common package as it contains the DbConnection class and not tied to the System.Data.SqlClient package.

[0xced]

I think it was a good call to prefer Microsoft.Data.SqlClient over System.Data.SqlClient if both are installed in e243711e76b987e1952e4a0b05b7683305c9ad0a.

Now, I went to great length in #2011 to use only reflection and avoid referencing either provider. Indeed, if none is referenced, an exception will be thrown. By the way, I just improved the exception message in a7e2c4910ef9ccd993a458d1b0b5ba4b5443ddbd so that it's clear what action must be taken:

Please add a NuGet package reference to either 'Microsoft.Data.SqlClient' or 'System.Data.SqlClient' in your application project. Hangfire.SqlServer supports both providers but let the consumer decide which one should be used.

In addition to this improved exception message, maybe the official documentation could mention that starting with version 1.8.0 a provider must be explicitly specified in the application project.

When I removed <PackageReference Include="System.Data.SqlClient" Version="4.4.0" /> in #2011 I thought that would remove the NuGet dependency because that's how it works if you dotnet pack a csproj. After I tried Hangfire.SqlServer 1.8.0-rc1 I came back and realised a manual nuspec and a custom build script is used, hence this pull request.

Also not sure we should dereference the System.Data.Common package as it contains the DbConnection class and not tied to the System.Data.SqlClient package.

Yep, I fixed this in 980bd6a36513381ae0c5ac84043c6609de319df5. (I rebased this pull request branch on dev on force pushed)

[odinserj]

The biggest issue with the transition to the Microsoft.Data.SqlClient package is that in version 4.0 they changed defaults for authentication, and the same connection string can now prevent from connecting to SQL Server, so it's a breaking change with a lot of issues reported (unfortunately lost issue reference on MD.SqlClient repository, but you can see my commits with "Try MD.SqlClient X.Y" pattern).

I even still struggling to determine the correct connection strings for AppVeyor's Ubuntu image – version 3.1 works fine, but neither 4.0, nor 4.1, nor 5.0 recently released doesn't work well.

So currently I'm leaning to leave everything as is for 1.7.X branch, but for 1.8 change defaults anyway, because of possible big problems in production – it's better to receive exception when application starts than to diagnose the leaked connections (possible problem with System.Data.SqlClient on SQL Azure). So I believe the following changes will be effective in 1.8:

• [x] Default package is Microsoft.Data.SqlClient when referenced regardless of the presence of System.Data.SqlClient, with breaking change documented in the upgrade guide. This is inevitable evil, but at least can be expected with such a big update.
• [x] Reference to System.Data.SqlClient is removed from both project and nuspec file as you proposed.
• [x] The new defaults are the same for both .NET Core and .NET Framework, since I've seen connection leak issues exactly on .NET Framework.
• [x] So System.Data.SqlClient will be used only when specified explicitly through the connection factory (in ctor or via the new option).

Sorry for the additional details, just want to summarise the changes in my head.