OrchardCoreContrib / OrchardCoreContrib.Modules

Unofficial Orchard Core modules that driven by the community who love Orchard Core CMS
BSD 3-Clause "New" or "Revised" License
37 stars 12 forks source link

Should we create a module to make Orchard support Entity Framework? #70

Open aaronamm opened 3 years ago

aaronamm commented 3 years ago

Hello, everyone.

When we want to add a new feature to Orchard website and make it as a new custom module. Sometime, it is valid to use a traditional relation database system and use Entity Framework as ORM for data access layer.

I am not sure if you like the idea to add a new custom module e.g. OrchardCore.EF to make Orchard supports EF and provide DbContext to other modules or not.

Here is the PoC project that I made. https://github.com/codesanook/OrchardCore.EF

If you agree with the idea, I am happy to make a PR for OrchardCore.EF and add other missing items:

Here is the code that consumes OrchardCore.EF module. https://github.com/codesanook/Codesanook.ThailandAdministrativeDivisionTool/blob/develop/Controllers/HomeController.cs

Service filter to auto commit a transaction. https://github.com/codesanook/OrchardCore.EF/blob/develop/Filters/TransactionActionServiceFilter.cs

A database migration https://github.com/codesanook/Codesanook.ThailandAdministrativeDivisionTool/blob/develop/Migrations.cs

Mapping class https://github.com/codesanook/Codesanook.ThailandAdministrativeDivisionTool/blob/develop/Mappings/ProvinceConfiguration.cs

Thanks.

hishamco commented 3 years ago

Thanks Aaron for opening this, it is a very good feature to abstract the some of the modules from YesSQL, It's a HUGE task, but we could manage this in many PRs

Content Management APIs and content modules are the major once who have a dependency on YesSQL, so let us think very well before we begin on this

IMHO the essential step is to make sure that OC.Data.Abstractions is not coupled on YesSQL which I think it's, then we can create OCC.Data.EntityFramework

aaronamm commented 3 years ago

@hishamco Thanks for prompt reply. I like your idea but at the beginning I don't think to have EF work for content part/content item like what we have in version 1 with NHibernate.

My idea is only allow some feature in a custom to have data access as normal relation database and use EF as ORM. My current implementation, the down side that I can see is I need to query with EF, YesSQL for content item and need to join in memory. I can't use only one SQL query join to get all required result.

hishamco commented 3 years ago

Regardless of Orchard 1, I think anything related to data store should interact with OC.Data.Abstractions at the end everything is depend on the the implementation part, so when we do query the the data abstraction shouldn't care about the implementation if it's YesSQL or EF or any other technology, what is important is to retrieve a list of objects

Regarding your question if you need some features to use EF, my first question is how the existing modules will aware about it? Unless your implementation is based on OC.Data.Abstractions

I can't use only one SQL query join to get all required result.

You can use SQL Queries or Lucene Queries, could you please let me know what kind of queries are you looking for, may be you didn't need EF at all. My aim when you start that issue was to bring EF into OC and eliminate the coupling from YesSQL

aaronamm commented 3 years ago

@hishamco This is interesting, I will prepare code example and let you know what query result I want. I still like how OrchardCore content part use store as a document for better performance. However, just only some models that I think using a traditional relation is a better approach.

BTW, I like your EF into OC and eliminate the coupling from YesSQL as well and I am happy if there is something that I can help.

hishamco commented 3 years ago

Please refer to your sample in the issues that opened in OC repo, and if you interest to contribute to make OCC.EntityFramework is a thing, I will be happy for your contribution

hishamco commented 3 years ago

BTW, I like your EF into OC and eliminate the coupling from YesSQL as well and I am happy if there is something that I can help.

Seems you modified your thread while I'm writing ;) so let us think about the changed required to make this happen, first we should revise OrchardCore.Data.Abstractions and OrchardCore.Data. If you have any suggestion you are welcome

aaronamm commented 3 years ago

@hishamco Thank you. BTW, have you discussed with your team to make sure that we really need Orchard/Content Item to support EF which is the same as Orchard 1 that supports NHibernate?

hishamco commented 3 years ago

Look @aaronamm OCC is another project that aims to fill the gabs that exists in OC, basically it should have unofficial modules, themes, translations which the community is LOVE, OC on other hand will not accept any modules or themes unless the team agreed, coz you know there are many impacts either from code, review, inter and intra communication with other OC parts

You issue there will open and hope to hear the feedback ASAP, but what you are asking could be manage easily but it needs some efforts. It's FROM community TO community

Thanks

aaronamm commented 3 years ago

@hishamco Thank you. To summarize:

For OrchardCore.Data.Abstractions and OrchardCore.Data, I've never looked into them before but I am happy to help for an implementation. After we are ready to start working, please assign some tasks to me.

Thanks.

hishamco commented 3 years ago

You will check required changes to make EF work with OC, revise OrchardCore.Data.Abstractions and OrchardCore.Data

I already started ;)

I will send you my query example that I need to run both YesSql and EF query.

Please post them in OC repo

hishamco commented 3 years ago

IScopedIndexProvider seems the only type that let OC.Data.Abstractions to have a dependency on YesSQL which is wrong IMHO, I think the first step is to remove this dependency as use it as a marker interface

Then we can formalize the needed abstraction APIs for the OCC.Data.Abstractions

hishamco commented 2 years ago

@aaronamm it's long time for complete the discussion on this, I just tried your sample, but I got

InvalidOperationException: Unable to resolve service for type 'OrchardCore.EF.OrchardDbContext' while attempting to activate 'OrchardCore.EF.Filters.TransactionActionServiceFilter'.

I already started a new APIs implementation for OrchardCoreContrib.Data.EntityFramework hopefullly I will push some local code soon, but I need to know why your sample doesn't work properly

Thanks

hishamco commented 2 years ago

FYI OC already clean up data abstractions and remove the dependency from YesSQL, which is why I started a new APIs after long time to reduce the efforts and to make it easy to integrate with OC

aaronamm commented 2 years ago

@hishamco Thanks for following up. I will be back to you soon. I have so many things to update to you.😀

hishamco commented 2 years ago

Take your time ;)

hishamco commented 2 years ago

@aaronamm can you find time to hear your updates on this?

hishamco commented 2 years ago

Related to https://github.com/OrchardCoreContrib/OrchardCoreContrib/issues/16