Switch to new EF Core plugin model

[roji]

@austindrenski @YohDeadfall @bricelam @ajcvickers here's a PR modifying the NetTopologySuite and NodaTime plugins to use the new EF Core plugin model introduced in 2.2. It's not a small change so I'd appreciate a review...

I did run into one annoying factoring issue when writing the type mapping source plugin for NodaTime. Although the NTS plugin is a bit special, the NodaTime plugin really has the same logic as the main NpgsqlTypeMappingSource - two lookup dictionaries (by store type, by CLR type) along with some non-trivial mapping lookup logic.. I experimented with refactoring the logic to avoid repetition but it isn't as easy it it looks. @ajcvickers it may be interesting for you to take a look; I know the type mapping APIs underwent quite a bit of change in the past, and there was a shift to a simpler model rather than trying to take care of too many things in the core. The relevant bits are the NodaTime plugin, and NpgsqlTypeMappingSource.

Some additional notes:

• Added some missing NTS translations
• The new EF Core plugin model doesn't yet support specifying evaluatable (Add plugin support to EvaluatableExpressionFilter dotnet/efcore#13454), so we currently hack that up inside the main provider using type names as strings.

Fixes #658

[austindrenski]

@roji I'm starting a review now, but it looks like the new file are missing the license block at the top. Could you update to include it?

[roji]

@austindrenski sorry about that. However, I've been wanting to simply get rid of these length messages, or at the very least drastically reduce them - I'm really not convinced having this big text in every single one of our source files helps in any way (just one at the repo root should be enough?). What do you think?

[austindrenski]

This looks awesome! See my review for one or two substantial issues (e.g. a missing ' in the SQL), a few questions, and some nits/suggestions.

This looks like it was a pretty substantial refactor, thanks for tackling it!

[austindrenski]

@austindrenski sorry about that. However, I've been wanting to simply get rid of these length messages, or at the very least drastically reduce them - I'm really not convinced having this big text in every single one of our source files helps in any way (just one at the repo root should be enough?). What do you think?

That's totally fine by me. I got into the habit to match the pre-existing style here, but I'd be happy to see them omitted.

[austindrenski]

Some additional notes:

• Added some missing NTS translations

Once merged, we should make a note in #457 of the newly added translations.

[austindrenski]

Not sure why the CI builds failed on repo access, but just restarted them and they seem to be running fine now.

[austindrenski]

Travis failing with:

[xUnit.net 00:02:14.70]     Npgsql.EntityFrameworkCore.PostgreSQL.Query.SpatialQueryNpgsqlGeographyTest.Area(isAsync: True) [FAIL]
[xUnit.net 00:02:14.79]     Npgsql.EntityFrameworkCore.PostgreSQL.Query.SpatialQueryNpgsqlGeographyTest.Area(isAsync: False) [FAIL]
Failed   Npgsql.EntityFrameworkCore.PostgreSQL.Query.SpatialQueryNpgsqlGeographyTest.Area(isAsync: True)
Error Message:
 Npgsql.PostgresException : XX000: ptarray_area_spheroid: cannot handle ptarray that crosses equator

[austindrenski]

The new test fixtures are currently in separate files. Is that copy-over from the upstream project? If so, could we nest the fixtures within a #region Support at the bottom of relevant test class?

If you would prefer to keep these fixtures separate, should we look at separating out our existing nested fixtures? This may help with some of the fixture-proliferation you recently mentioned.

I don't have strong feelings either way, just thinking about consistency.

[austindrenski]

Omit per #674

[roji]

Done, although I'm not too sure about the license status of these files which we copy-paste into the provider... Npgsql is licensed under the PostgreSQL license, which isn't the same as the Apache license which EF Core uses.

@ajcvickers, @divega anything to say here??

[roji]

@austindrenski,

The new test fixtures are currently in separate files. Is that copy-over from the upstream project? If so, could we nest the fixtures within a #region Support at the bottom of relevant test class?

You're referring to SpatialQueryNpgsql{Geometry,Geography}Fixture, right? If so then yes, it's a copy-over from EFCore. As elsewhere, for those files which copy over, I think it's a good idea to maintain as much parity to help with later maintenance. When I just started work on the EF Core provider, I originally refactored and re-edited things my way, and that led to a lot of pain as upstream changes had to be tracked...

If you would prefer to keep these fixtures separate, should we look at separating out our existing nested fixtures? This may help with some of the fixture-proliferation you recently mentioned.

I wouldn't necessarily go that far. For our own test suites, I don't see a reason to impose external fixtures just because upstream test suites do so. I'd treat it on a case-by-case basis - where we have a big fixture that makes sense as a separate file, we can do that, otherwise we can leave them inside the test suite.

[roji]

@austindrenski

Once merged, we should make a note in #457 of the newly added translations.

Done

[roji]

Regarding the Travis build failures (ptarray_area_spheroid: cannot handle ptarray that crosses equator).

My first thought was that this is due to an old version of PostGIS or something similar. Strangely on Appveyor we install 2.4.4 and there aren't any failures there - but I still suspect some environmental/version issue. When I tried to upgrade I found that there are no public packages for PostGIS 2.5 for PostgreSQL 10 for trusty. I can't be 100% sure that an upgrade to PostGIS 2.5 would fix this, but I honestly don't want to fight Travis anymore - it's really behind on everything - running on trusty in this day and age is simply unacceptable.

So for me this is the last straw, and as we were about to move to either Appveyor for Linux or to Azure DevOps, I think it makes sense to drop Travis for this PR. @austindrenski @YohDeadfall any objections?

[austindrenski]

Sounds good to me--no objections here.