Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET Core #265

Open
Theoistic opened this issue Aug 21, 2016 · 24 comments
Open

.NET Core #265

Theoistic opened this issue Aug 21, 2016 · 24 comments

Comments

@Theoistic
Copy link

Hi, I've been trying to port Massive to .net core, still some work to be done, since there are a lot of base classes missing. Would be good to see a port of that in this repo.

@FransBouma
Copy link
Owner

If there are classes missing in .NET Core's bcl there's little you can do other than excluding that code from Massive. Microsoft will port a lot more classes to .NET core BCL in the near future however.

'A lot of base classes' means e.g. DataTable ?

@jimutt
Copy link

jimutt commented Nov 11, 2016

Just dropping by to say that a future .NET Core version of Massive would be very appreciated 👍

@FransBouma
Copy link
Owner

I'm currently porting LLBLGen Pro Runtime Framework to .net core (net standard1.6), and learning a lot about what's available and what isn't, so porting massive in the coming months isn't going to be a big challenge I think. I hope to have time for that :)

@avinashl3175
Copy link

I am also trying to port massive into dot net core but getting errors since some classes like DbProviderFactories not defined into System.Data.Common namespace , etc. Can you please suggest different ORM similar to massive or .NET Core version of massive .

@mikebeaton
Copy link
Contributor

I have ported Massive to .NET Core, I am almost ready to post an update in #302.

@FransBouma
Copy link
Owner

except for the datatable related features I recon? I initially ported it half-way then realized I could better wait for Netstandard2.0 and port everything over in 1 go as otherwise there will be 2 versions (one for netstandard1.6 without the datatable features and one for netstandard2.0 with the datatable feature)

@mikebeaton
Copy link
Contributor

Except for datatable features. But with stored procedures, named and directional params, multiple result sets, multiple providers at once... and it still drops in and runs the Massive 2 test suite with no changes, because I listened to you... ;

@mikebeaton
Copy link
Contributor

mikebeaton commented Mar 31, 2017

Man, I messed up before. I'm sorry. The features in the version I presented were real - maybe even worth something - but the way I presented them was... real stupid.

But writing code that people actually use is also ... worth something. And you do know that I can write good, clean code.

This is .NET Core 1.1+ support for Massive v2.0. Nothing else added or taken away. As I listened to you - because what you say about Massive is always sensible, and you do know where it needs to go, for its current users (not for pie-in-the-sky projects which need to live under separate names, I now understand that) I have also thought about, and done something hopefully reasonable about, the two DataTable methods, as well.

I don't know if you can take this, but I want to offer it - to you and the community.

@FransBouma
Copy link
Owner

FransBouma commented Mar 31, 2017

I looked at the PR, there are some things in the code which aren't OK ( I commented on a few) and some things which are simply not going to be merged, like the factory name collection in shared. I stopped there as it otherwise will take me a lot of time debating on every line of code and I simply don't have that time (as in: debating a lot of fine details is not going to work for me: the code offered should be OK, otherwise it's not really feasible. It will be otherwise a big piece of work for me to first describe everything that has to be changed, debate back/forth till it is OK. )

@mikebeaton
Copy link
Contributor

That's fine, understood.

Thank you for taking the time - again - to review it seriously and give feedback.

I think it's hard, this. @robconery not unreasonably commented that things can get toxic. Especially if someone comes barging in with big ideas, and very little experience (of OSS, even if not of coding), and very little tact. But even so, a very small commit looks like it's an attempt to waste someone's time. A very big change has so many pitfalls. It won't be done quite the way the maintainer would have done it. If it's too big, it's just unreviewable. If the comments added to the readme aren't authoritative, they're no use for future users. If they are, but they don't fit with how the maintainer thinks (even quite unintentionally so) they look dogmatic and bullshitty.

I have a lot to learn about this. I'll keep trying. But maybe not here, for now - you need some peace and quiet. ;)

Thank you, Frans, seriously, for at least taking this PR seriously enough to look at it.

Good luck with Massive, I still love it.

M

@FransBouma
Copy link
Owner

Never give up :). We all make mistakes and have made mistakes, myself included. Perhaps we'll meet again in the future, who knows.

@brgrz
Copy link

brgrz commented Feb 22, 2019

Hey @FransBouma so it's been 2 years, do you think Massive will ever get ported to .NET Core (which has also come a long way since then)? It'd be very much useful to have it in .NET Core world.

@brgrz
Copy link

brgrz commented Feb 22, 2019

Attached are the results of the .NET Portability analyzer and those are quite encouraging that a port is likely feasible

ApiPortAnalysis(4).xlsx

@FransBouma
Copy link
Owner

When I have time I'll look into it :)

@brgrz
Copy link

brgrz commented Feb 23, 2019

@FransBouma For my own needs I migrated everything to .net core 2.2 today and it was (almost) straightforward besides some issues with Nuget packages and tests. Feel free to take a look if/when you want.

@FransBouma
Copy link
Owner

If you could send me a PR, that would be great. I can then review it and merge it :) Thanks!

@brgrz
Copy link

brgrz commented Feb 25, 2019

I will but I still have some issues. Most of migration went fine but I had to install the System.Configuration.ConfigurationManager Nuget to support the old way configuration loading. Since Massive does not support passing in the connstring at any place (only the connection string name and then loads the actual connection string it from App.config or Web.config files). The configuration Nuget solved that issue but I am now running into an issue with lacking connection providers in .net core.

https://weblog.west-wind.com/posts/2017/nov/27/working-around-the-lack-of-dynamic-dbproviderfactory-loading-in-net-core

https://stackoverflow.com/questions/52007836/dbproviderfactories-getfactoryclasses-returns-no-results-after-installing-net-s

Hopefully I will be able to resolve this..

@brgrz
Copy link

brgrz commented Feb 25, 2019

@FransBouma oic, it is something you've been faced with already https://github.com/dotnet/corefx/issues/4571

What do you suggest, how should we proceed?

@FransBouma
Copy link
Owner

yeah that's an issue I ran into as well when I tried to do the migration (I think in the xmas break) and as it was more work than anticipated I stopped for now.

Thing is: you can't avoid introducing #ifdef's: you need to add conditional compilation parts for .net full and .netstandard, as the config file stuff and the dbproviderfactory stuff isn't compatible. I implemented DbProviderFactory support in .NET Core but it was contributed too late (due to MS politics and other crap) that it wasn't introduced in .netstandard2.0 (it's in .netstandard2.1). Besides, it requires a new .NET full version (4.7.2 or higher) anyway so that's not going to fly.

For LLBLGen Pro I introduced a simple way to configure the database connection data through a separate configuration class where you can specify the factory to use and the connection string. (See: https://www.llblgen.com/Documentation/5.5/LLBLGen%20Pro%20RTF/Using%20the%20generated%20code/gencode_runtimeconfiguration.htm) I think for Massive that's also the way to go (but it can be simpler, we just need the factory and the connection string).

So that configuration class can then be used from .net core and .net full, through a different ctor, which is also the target of the existing one (which simply fills in the configuration class instance from the config file and the factory pulled from DbProviderFactories using the specified invariant name).

Would that work?

@brgrz
Copy link

brgrz commented Feb 25, 2019

Couple of things:

  1. What if the current Massive v2 would be kept for .NET Full compatibility and new version was introduced to support .NET Core (eliminating the need for conditional code)?
  2. I ran into this issue after a call to GetFactory() failed with the following call stack
System.ArgumentException
  HResult=0x80070057
  Message=The specified invariant name 'System.Data.SqlClient' wasn't found in the list of registered .NET Data Providers.
  Source=System.Data.Common
  StackTrace:
   at System.Data.Common.DbProviderFactories.GetFactory(String providerInvariantName, Boolean throwOnError)
   at System.Data.Common.DbProviderFactories.GetFactory(String providerInvariantName)
   at Massive.DynamicModel..ctor(String connectionStringName, String tableName, String primaryKeyField, String descriptorField, String primaryKeyFieldSequence, IConnectionStringProvider connectionStringProvider) in H:\GitHub\brgrz\Massive\src\Massive.Shared.cs:line 256

So I don't know how that's possible but the DbProviderFactories.GetFactory() method does exist it just returns empty because nothing's registered. How is that possible (since you said it never made it in)?

Do you happen to know how to register data provider factories in asp.net core projects because I can't find any info anywhere?

@brgrz
Copy link

brgrz commented Feb 25, 2019

Also, what about an option which Rick Strahl discusses, to have specialized assemblies using the required provider factory. Massive already has specialized assemblies/projects, albeit in the same namespace but I think it would work for such a small projects as this.

https://weblog.west-wind.com/posts/2017/nov/27/working-around-the-lack-of-dynamic-dbproviderfactory-loading-in-net-core

Add specialized assemblies for each provider implementation One pretty common approach is to add specialized versions of the library that make the provider specific dependencies available. Something along the lines of: Westwind.Globalization.SqLite or Westwind.Globalization.MySql etc. That would work fine, but it's an administrative nightmare as each provider requires a separate project and a separate set of dependencies to keep in sync. On the plus side it guarantees the right providers are always available and loaded.

@brgrz
Copy link

brgrz commented Feb 25, 2019

Just a note. I was able to get it working by adding just the following line to register the factory to my static void Main() in Program.cs of the asp.net core app:

// Register the factory
DbProviderFactories.RegisterFactory("System.Data.SqlClient", SqlClientFactory.Instance);

@FransBouma
Copy link
Owner

On .NET Core, there's no dbprovider registered at the start, as there's no machine.config :) So on .NET core you have to register the factory first, then pull it from the table. On .NET Full the sqlserver one is always there. DbProviderFactories is supported in .NET Core 2.0+, I contributed that to corefx. It's not in netstandard2.0, that's something else :) So it IS in .net core, not in .netstandard.

So as you have to register the factory at the start of the app in any case on .net core, you can also just pass it to the app using some config class like I use in my orm as well.

Regarding specific assemblies: massive is a simple drop-in class file distributed system, so no specific dlls. It doesn't add anything specific here, anyway, only nuget packages which aren't present at the moment anyway.

@mikebeaton
Copy link
Contributor

mikebeaton commented Apr 23, 2019

I've finally put up a public release of the project which I was working on, which is highly compatible with Massive but effectively addresses this issue (#265) and #293, #289, #287, #263.

It's a rewrite, not a fork. I really have tried hard to keep it as compatible as possible, and it obviously benefits enormously from the code design and features introduced in Massive by @robconery and maintained and added to by @FransBouma, and equally from the significant amount of genuinely helpful feedback which @FransBouma kindly gave back when I was offering less well-formed versions of these new features as pull requests to the existing Massive. (Several of which, including Massive's support for MySQL, were accepted and are part of this codebase, btw... but then I got a bit carried away... wanting to add new features and upgrade things without checking at all properly in advance whether what I was trying to do was anything at all like what @FransBouma wanted for Massive. Sorry again for that.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants