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

Added Abp.ZeroCore.EntityFramework #2625

Merged
merged 1 commit into from Nov 14, 2017
Merged

Conversation

ryancyq
Copy link
Contributor

@ryancyq ryancyq commented Oct 23, 2017

As mention in #2600, i created Abp.ZeroCore.EntityFramework

@ryancyq ryancyq changed the title added Abp.ZeroCore.EntityFramework Added Abp.ZeroCore.EntityFramework Oct 23, 2017
@hikalkan
Copy link
Member

Thank you very much. Will review and merge it soon.

@hikalkan hikalkan added this to the v3.2 milestone Oct 23, 2017
@iyhammad
Copy link
Contributor

Dears,
I've tried this way before and I stuck in something that I wanted to tell you to avoid having the same issue I faced.
ISupportsExplicitLoading Interface is being implemented by EfCoreRepositoryBase but not implemented by EfRepositoryBase .
The Problem was after implementing Abp.ZeroCore.EntityFramework and tried to use EntityFramework with the AspNetCore, all the operations that depends on this interface breaks. below are example of them:
EnsureCollectionLoadedAsync depends on ISupportsExplicitLoading
if It's EntityFrameworkRepository and so it doesn't implement this interface, the result of that is the child collection will be null and this breaks in

And many other functionalities. I tried to implement ISupportsExplicitLoading interface using Entity Framework quickly but I couldn't.

Thanks,

@acjh
Copy link
Contributor

acjh commented Oct 23, 2017

@iyhammad Is that .NET Core 1 or 2? EF6 cannot target .NET Core 2; this PR targets net461 only.

@iyhammad
Copy link
Contributor

@acjh I think It doesn't matter here. @ryancyq is implementing Abp.ZeroCore.EntityFramework to be able to use it with Abp.ZeroCore What I'm saying is, there are classes and methods in Abp.ZeroCore will break because of the dependency on ISupportsExplicitLoading which is not implemented.
Am I right ? Or there is something I'm missing here?

@acjh
Copy link
Contributor

acjh commented Oct 23, 2017

Abp.ZeroCore won't break — Claims and Roles are virtual, so EF6 will lazy-load those.
ISupportsExplicitLoading was introduced because EF Core does not support lazy-loading.

@iyhammad
Copy link
Contributor

So, we just want to keep in mind that Lazy Loading is a must while using the new library. I believe It will be better if we null-check Claims, Roles and similar cases and load them if no ISupportsExplicitLoading and no lazy loading.

@acjh
Copy link
Contributor

acjh commented Oct 23, 2017

No, lazy-loading is not a must, but it is a given in EF6 for this case. Redundant code isn't better.

@acjh
Copy link
Contributor

acjh commented Oct 23, 2017

From your link, there are no "similar cases" in Abp.ZeroCore so please refer to the concerns directly.

@iyhammad
Copy link
Contributor

Hi @acjh ,
If we are using the new package Abp.ZeroCore.EntityFramework with Abp.ZeroCore with a DbContext that has lazy loading disabled for whatever reason.
These 2 lines

Will break or not ?
If they won't break, So forget what I'm saying and I'm wrong.
If Yes, They will break, So, Lazy Loading is a must in this particular case and we may raise a notice to the people who will use this package saying Please avoid disabling lazy loading.
Thanks,

@acjh
Copy link
Contributor

acjh commented Oct 23, 2017

As mentioned, lazy-loading is not a must "in this particular case" and can be safely disabled if ISupportsExplicitLoading is implemented.

@hikalkan hikalkan self-requested a review November 8, 2017 14:04
@hikalkan
Copy link
Member

We are calling EnsureCollectionLoaded which loads collections where lazy loading is not available.
However, it was not supported by EfRepositoryBase. I implemented it (#2683). So, now, we don't assume that lazy loading is enabled for EF 6.x (at least for Abp.ZeroCore package, I didn't check Abp.Zero package).

@hikalkan hikalkan merged commit bca80a3 into aspnetboilerplate:dev Nov 14, 2017
@iyhammad
Copy link
Contributor

I really love that. Thank you

@hikalkan
Copy link
Member

You're welcome :)

@emisand
Copy link
Contributor

emisand commented Nov 24, 2017

@ryancyq @hikalkan Do you have a working example of this package using the vanilla ASP.NET Boilerplate template with ASP.NET Core?

I have tried making a .EntityFramework project at the template based on code taken from the ASP.NET MVC 5 template, I get it to build but I get entity model validation exceptions when I try to create the initial migration.

Edit - This is the exception I get when I try to create the initial migration:

PM> Add-Migration InitialMigration
System.InvalidOperationException: The index with name 'IX_UserId_State_CreationTime' on table 'dbo.AbpUserNotifications' has the same column order of '1' specified for columns 'TenantId' and 'UserId'. Make sure a different order value is used for the IndexAttribute on each column of a multi-column index.
   at System.Data.Entity.Infrastructure.ConsolidatedIndex.Add(String columnName, IndexAttribute index)
   at System.Data.Entity.Infrastructure.ConsolidatedIndex.BuildIndexes(String tableName, IEnumerable`1 columns)
   at System.Data.Entity.Migrations.Infrastructure.EdmModelDiffer.<FindTargetIndexes>b__278(EntitySet es)
   at System.Linq.Enumerable.<SelectManyIterator>d__22`3.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at System.Data.Entity.Migrations.Infrastructure.EdmModelDiffer.Diff(ModelMetadata source, ModelMetadata target, Lazy`1 modificationCommandTreeGenerator, MigrationSqlGenerator migrationSqlGenerator, String sourceModelVersion, String targetModelVersion)
   at System.Data.Entity.Migrations.Infrastructure.EdmModelDiffer.Diff(XDocument sourceModel, XDocument targetModel, Lazy`1 modificationCommandTreeGenerator, MigrationSqlGenerator migrationSqlGenerator, String sourceModelVersion, String targetModelVersion)
   at System.Data.Entity.Migrations.DbMigrator.Scaffold(String migrationName, String namespace, Boolean ignoreChanges)
   at System.Data.Entity.Migrations.Design.MigrationScaffolder.Scaffold(String migrationName, Boolean ignoreChanges)
   at System.Data.Entity.Migrations.Design.ToolingFacade.ScaffoldRunner.Scaffold(MigrationScaffolder scaffolder)
   at System.Data.Entity.Migrations.Design.ToolingFacade.ScaffoldRunner.RunCore()
   at System.Data.Entity.Migrations.Design.ToolingFacade.BaseRunner.Run()
The index with name 'IX_UserId_State_CreationTime' on table 'dbo.AbpUserNotifications' has the same column order of '1' specified for columns 'TenantId' and 'UserId'. Make sure a different order value is used for the IndexAttribute on each column of a multi-column index.

This is the EntityFramework project I created at the AspNetCore vanilla template for ASP.NET Boilerplate
core-ef-structure

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

Successfully merging this pull request may close these issues.

None yet

5 participants