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

Request: RepoDb.SqLite remove dependency on EntityFramework #486

Closed
dalebrubaker opened this issue Jul 25, 2020 · 25 comments
Closed

Request: RepoDb.SqLite remove dependency on EntityFramework #486

dalebrubaker opened this issue Jul 25, 2020 · 25 comments
Assignees
Labels
deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed. request A request from the community.

Comments

@dalebrubaker
Copy link

When I pull RepoDb.SqLite from NuGet I also get a large EntityFramework.dll along with other undesired files. I think this is because both RepoDb and System.Data.SQLite are dependencies.
Could the dependency be System.Data.SQLite.Core instead of System.Data.SQLite?

@mikependon mikependon self-assigned this Jul 25, 2020
@mikependon mikependon added the request A request from the community. label Jul 25, 2020
@mikependon
Copy link
Owner

Absolutely! We will do this. When do you need the fix for this?

@mikependon mikependon changed the title RepoDb.SqLite remove dependency on EntityFramework Request: RepoDb.SqLite remove dependency on EntityFramework Jul 25, 2020
@dalebrubaker
Copy link
Author

No rush at all. Thank you.

@mikependon mikependon pinned this issue Jul 26, 2020
@mikependon
Copy link
Owner

We are working on this now. I am planning to upgrade the SqLite driver to use the Microsoft.Data.Sqlite.Core. Though, by upgrading it, 70% of the Integration Tests fail. We are fixing this as well. In anyway, do you have any comments before we do commit the changes?

@dalebrubaker
Copy link
Author

I don't feel qualified to comment. :)
I'll be using your package from .Net Framework 4.8, initially, but it appears that your new approach will still work with that. I'm sorry about the breaking integration tests, and I hope that this won't break too many of your existing users. But they may also like losing the unnecessary dependency on EF etc.

mikependon added a commit that referenced this issue Jul 28, 2020
@mikependon mikependon added the fixed The bug, issue, incident has been fixed. label Jul 30, 2020
@mikependon mikependon unpinned this issue Jul 30, 2020
@mikependon mikependon added the deployed Feature or bug is deployed at the current release label Aug 2, 2020
@mikependon
Copy link
Owner

@dalebrubaker - this request is now a part of the latest deployment (RepoDb.SqLite (v1.0.15)). Would you be able to validate in your side?

> Install-Package RepoDb.SqLite -version 1.0.15

Notes:

Please be reminded that the upgrade from System.Data.SQLite to Microsoft.Data.Sqlite.Core may affect some of the default coercion. That is not an issue of RepoDb, the reason for that are the huge changes between these 2 underlying libraries. If you encountered any problem on your existing code, please revert to me immediately. I am happy to help.

.NET Core:

image

.NET Framework:

image

@dalebrubaker
Copy link
Author

@mikependon

I installed 1.0.15 and changed
    using System.Data.SQLite to    using Microsoft.Data.Sqlite
    SQLiteConnection to SqliteConnection
When I do this: new SqliteConnection(ConnectionStringTradeData)
I get:
System.Exception
  HResult=0x80131500
  Message=Library e_sqlite3 not found
  Source=SQLitePCLRaw.nativelibrary
  StackTrace:
   at SQLitePCL.NativeLibrary.Load(String libraryName, Assembly assy, Int32 flags)

I don't see any x64 folder or any other e_sqlite3 dll in my output folder.
At this point I had this:
image

Installing Microsoft.Data.Sqlite didn't fix it.Installing RepoDb didn't fix itInstalling SQLitePCLRaw.bundle_e_sqlite3 got me past the new SqliteConnection() exception. But I do get this exception:System.Reflection.TargetInvocationException
  HResult=0x80131604
  Message=Exception has been thrown by the target of an invocation.
  Source=mscorlib
  StackTrace:
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)

Inner Exception 1:
InvalidOperationException: The process has no package identity. (Exception from HRESULT: 0x80073D54)

At this point I have this:
image

I don't see a way to get sqlitepclraw.batteries_v2 from NuGet search, but I think it's in the bundle and I have these files in bin\Debug.
image

Still no files in x64

Ignoring the InvalidOperationException I go on and try to insert two different Account records, using BaseRepository<Account, SqliteConnection>.Insert(account)
The first insert now inserts a record with autoincrement Id 0, and before it was Id 1. Inserting a second record with a different Name I get this exception. 
Microsoft.Data.Sqlite.SqliteException
  HResult=0x80004005
  Message=SQLite Error 19: 'UNIQUE constraint failed: Accounts.Id'.
  Source=Microsoft.Data.Sqlite
  StackTrace:
   at Microsoft.Data.Sqlite.SqliteException.ThrowExceptionForRC(Int32 rc, sqlite3 db)

The table definition is:create table Accounts(
    Id INTEGER primary key autoincrement,
    Name TEXT not null,
    CommissionName TEXT not null,
    DisplayName TEXT not null,
    Provider INTEGER not null,
    AccountStatus  INTEGER not null,
    FCM TEXT
)

@mikependon
Copy link
Owner

How about installing the Microsoft.Data.Sqlite.Core?

@mikependon
Copy link
Owner

@dalebrubaker - a community complaints about moving from SDS to MDS. I apology for this. Would you be able to wait for a new release for RepoDb.SqLite? I will re-reference the SDS as you recommended at first. I need to use the System.Data.SQLite.Core so the others would not be affected much. Can you wait for that?

@dalebrubaker
Copy link
Author

Done. No change in behavior.
image
"Can you wait for that?"
Sure, I'll continue with 1.0.14, no problem.

@mikependon
Copy link
Owner

@dalebrubaker and @FutureTD - the new modified version of RepoDb.SqLite with System.Data.SQLite.Core reference is now running in our own environment. We also made all the Unit Tests and Integration Tests passed as of writing this.

Though, what I noticed with MDS, they are good at PUSH operations (in which the MERGE works perfectly, same as SQL Server), while the SDS is not. On top of this, Microsoft my further improve this soon, as it is used by EF Core underneath. However, I truly believe that SDS is still the much superior and is still supported by the core team. There are no big change from the legacy library, and it is still faster and better.

Question: Being you as the user of this, which library do you prefer to use moving forward? SDS or MDS?

@dalebrubaker
Copy link
Author

It doesn't matter to me. Either SDS or MDS is fine.

@FutureTD
Copy link

FutureTD commented Aug 2, 2020

I would prefer SDS

@mikependon
Copy link
Owner

Thank you both. I already have a working branch, so it will only take few minutes to push it. But I will first gather more feedback to the community. Will update you further here. Thanks for the patience.

@dalebrubaker
Copy link
Author

Since you are this far along, and since the MDS version may become more valuable over time, you might consider offering a parallel package, like RepoDb.SqLite.MDS, in addition to your current one, someday. FWIW

@mikependon
Copy link
Owner

Same goes to #497. A new release has been pushed. Please install the RepoDb.SqLite (v1.0.16).

> Install-Package RepoDb.SqLite -version 1.0.16

Both the 'SDS' and 'MDS' are referenced on the package. The same strategy has been made with RepoDb.SqlServer package. Up until the community decided in the near future, one may be eliminated soon. That's still unclear when.

@mikependon
Copy link
Owner

Pertaining to your package problem. @dalebrubaker - I would recommend if you can uninstall RepoDb.SqLite first and then reinstall it. It seems a problem happens on your side during the installation. Would you be able to do that and revert?

@dalebrubaker
Copy link
Author

@mikependon

TestRepoDb.zip

This small zipped sln shows my problem with AutoIncrement using the current RepoDb.SqLite 1.0.16. This is a .Net Framework 4.8 project. You will have to install System.Data.SQLite from NuGet in order to get it to run.
When I insert a record without using FluentMap on the class, the AutoIncrement of Id works fine.
But when I insert a record after mapping, the AutoIncrement of Id fails (Line 34 of SimpleInserts.cs).
Looking at your code, SqLiteDbHelper.cs GetIdentityFieldName() is failing to "see" the identity field in this case. The sql variable from Line 82 comes back null from DbConnectionExtension.ExecutScalarInternal, so ParseTableFieldsFromSql(sql) does nothing in this case.

I've run this on 2 different PCs with the same result.

Best regards,
Dale

@mikependon
Copy link
Owner

Will look at this issue later. Thanks Dale.

@mikependon mikependon added deployed Feature or bug is deployed at the current release and removed fixed The bug, issue, incident has been fixed. deployed Feature or bug is deployed at the current release labels Aug 3, 2020
@mikependon mikependon added the fixed The bug, issue, incident has been fixed. label Aug 3, 2020
@mikependon
Copy link
Owner

mikependon commented Aug 3, 2020

I will create a separate issue for this later.

EDIT: Btw, do you know that you can create your customized DB Helper and inject it in RepoDb? You can inject it via DbHelperMapper.Add(typeof(SQLiteConnection), new CustomizedSqLiteDbHelper(), true);. You can temp edit the existing DB Helper and customize it. You can then debug it with your code :)

@dalebrubaker
Copy link
Author

Ah, thanks for the tip.
Also I just discovered that my problem disappears when I revert to 1.0.14

@mikependon
Copy link
Owner

mikependon commented Aug 3, 2020

@dalebrubaker - I had tested your solution locally, it is not really a big bug. The table cannot be found if you are quoting the mappings. Removing the quote would work (from [OrderEventArgs] to OrderEventArgs).

FluentMapper
	.Entity<OrderEventArgsMapped>()
	.Table("OrderEventArgs")
	.Column(e => e.CommentToMap, "[Comment]");

EDIT: Though, I will fix it on the underlying solution.

@mikependon
Copy link
Owner

Here is the project that I used. I had created a fresh project from scratch, using .NET Framework 4.7.2 (the latest framework installed on my local machine) and copied over all your files here.

TestRepoDbDale.zip

By the way, I was able to debug it without interfering the actual RepoDb code. I simply created a copy of the SqLiteDbHelper.cs class and injected it on this project. Below is the code in the line 16 of Program.cs. You can remove/comment this injection to check whether the current version is working if the mapping is not quoted.

DbHelperMapper.Add(typeof(SQLiteConnection), new CustomSqLiteDbHelper(), true);

Note: It seems that the package has issue in in C# 7.

@dalebrubaker
Copy link
Author

Thank you! I'm very impressed with this package!

@mikependon
Copy link
Owner

@dalebrubaker - can we now close this User Story? Please advise and thanks.

@dalebrubaker
Copy link
Author

Yes! Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed. request A request from the community.
Projects
None yet
Development

No branches or pull requests

3 participants