OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.4k stars 2.39k forks source link

Should Orchard's core components be decoupled from ASP.NET Core? #2971

Closed kevinchalet closed 5 months ago

kevinchalet commented 5 years ago

A few notes from a discussion I had with @sebastienros last year:

Currently, most of OrchardCore's packages depend on ASP.NET Core and its primitives (including the infamous IHttpContextAccessor), which makes it hard to use for non-ASP.NET Core scenarios like gRPC, Orleans or Service Fabric micro-services.

If these are scenarios we'd like to support in the future, we should consider decoupling OC's core packages (i.e the framework) from ASP.NET Core and moving the ASP.NET Core-specific components to the modules layer.

I'd love to hear your thoughts about that.

sebastienros commented 5 years ago

I can already see the "Orchard is over-engineered" comments

kevinchalet commented 5 years ago

Even without a refactoring, you'll see them until you decide to add a big warning message in the README saying "hey dude, you want to use OrchardCore? Don't use the massive solution from here, just reference the packages you need, and here's a sample that demonstrates that" :smile:

carlwoodhouse commented 5 years ago

I think it's pretty interesting - we are already using (well its under development atm) OC as microservices behind azure application gateway (atm they are just individual docker containers but the plan is to use service fabric)

For us, we're using one to expose/injest content via graphql, we've written some custom logic to spin them automatically configured based on env variables - with a recipe per microservice (so one solution but each service uses a diff recipe)

Now i can certainly see that some of the lighter microservices could not need aspnetcore, so if we didnt need to include it that might be kinda neat :D everything to make builds/deployments/overheads faster eh.

I guess a lot of people might not grasp it yet as it seems many are developing from source rather then nu/my get packages - but there is a lot of REALLY neat stuff in ocore that i might wanna use without aspnetcore by only referencing a few packages.

alexdresko commented 5 years ago

I can already see the "Orchard is over-engineered" comments

You mean OC isn't already over-engineered? :) I'm still struggling to grasp OC as it is. But, to be honest, it's a strikingly beautiful architecture. I've never seen anything like it. It's a learning curve I'm willing to invest in, so adding a few more layers of abstraction to the mix isn't going to thwart my efforts.

That being said, I currently have no use for the suggested use cases, so it doesn't hurt my feelings that the separation doesn't exist.

sebastienros commented 5 years ago

I was being sarcastic, yes everyone thinks at first it is over-engineered ... until you realize how much it does

jpiquot commented 5 years ago

Yes, OC is well ingeneered. If it was a bit less coupled with YesSql, it would be a perfect application framework.

Skrypt commented 5 years ago

I agree with that but right now the ContentManager, Migration are strongly coupled with YesSQL. Maybe something that could be abstracted eventually.

sebastienros commented 5 years ago

Agreed that we should have some experiences that work without yessql, at least for the modules that are not related to the CMS.

JoshTango commented 1 year ago

Um I came across this exact issue when looking at the method to create a tenant. I noticed the method required a reference to the ASP.NET Core current context. I didn't think that this method should know anything about the current context since that is a ASP.NET Core concept. The parent method inside of ASP.NET Core should do whatever needs to be done that requires the context.

sebastienros commented 1 year ago

@JoshTango maybe, if you can come with a PR that fixes this dependency it would be fine.