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.36k stars 2.37k forks source link

OrchardCoreBuilder.ConfigureServices inconsistent behavior #9351

Closed rserj closed 3 years ago

rserj commented 3 years ago

I use OrchardCoreBuilder.ConfigureServices((services, serviceProvider) => extension. It's get called many times during Setup/Autosetup
(I think it happens when Features get enabled and Orchard rebuilding the whole Shell's Blueprint is this right? ) and sometimes I can not extract the configuration section for shell/tenant

private const string DataProtectionConfigSectionName = "DataProtection_Redis";
private const string RedisConnectionOptionName = "Connection";

 builder.ConfigureServices((services, serviceProvider) =>
                    {
                        // This Extension method used directly in the Web project (Not in the Module)
                        // We should make sure Tenant is ready before connecting to Redis
                        if (ShellScope.Context == null || ShellScope.Context.IsActivated == false)
                        {
                            return;
                        }

                        var settings = serviceProvider.GetRequiredService<ShellSettings>();
                        var dpapiConfigurationSection = settings.ShellConfiguration.GetSection(DataProtectionConfigSectionName);
                        var redisConnection = dpapiConfigurationSection?.GetValue<string>(RedisConnectionOptionName);
                        ...

- Sometimes during Setup `ShellScope.Context.Settings.State` is equal to "`Initializing`" but at the same time  `settings.State` is  equal to "`Running`"

### To Reproduce
In `launchSettings.json` add env variables
"OrchardCore__DataProtection_Redis__Connection": "localhost",
"OrchardCore__DataProtection_Redis__KeyPrefix": "MyApp"

Add the above Extension method code, directly in the Startup.cs of the web project       
### Expected behavior
- I think the State should be consistent and should have the same value `ShellScope.Context.Settings.State == serviceProvider.GetRequiredService<ShellSettings>().State
- `Tenant/Shell specific section should be always available
rserj commented 3 years ago

cc: @jtkech

jtkech commented 3 years ago

Yes during setup we use an internal shell with an internal shell settings at some point, but normally in a given scope you should get the same values, I will take a look tommorow

In place of testing ShellScope.Context.IsActivated try to check if ShellScope.Context.Settings.State is not initializing or just is running, because during setup a shell is activated multiple time while still being initializing. And because you have a service provider you can just use the resolved shellSettings.

We alreay have a redis module that includes e.g. a data protection feature

rserj commented 3 years ago

Yeh, I saw the feature OrchardCore.Redis.DataProtection, thank you

try to check if ShellScope.Context.Settings.State is not initializing

As I remember it was in Running State and I still couldn't get section values but can be mistaken. What I know is ShellScope.Context.Settings.State != serviceProvider.GetRequiredService<ShellSettings>().State

I will take a look tommorow

Thank you very much

jtkech commented 3 years ago

Okay, calling the modules ConfigureServices() (and the ones defined through our app level helpers) means that we are building the shell, so here it can't be activated, you are not executing in a scope context but in a tenant / shell context, the one we are composing, so the Context prop of ShellScope may be null.

What happens is that during a setup, or through some other services e.g. when a tenant depend on another one that is not yet composed, this is in the context of a given shell scope that we are building a shell context, so here you may get the ShellScope of a parent, which doesn't happen when really running in a scope context, which is not the case here.

The service provider that we pass here is a tenant service provider not a scoped one so no shell scope that can be used, this is in the Configure() methods when building the tenant pipeline that we pass a scoped service provider.

So here you should only use serviceProvider.GetRequiredService<ShellSettings>() to resolve the shellSettings which is a tenant singleton, and then check the settings state, e.g. if settings.State == TenantState.Running, as we do in our AddAntiForgery() extension.

rserj commented 3 years ago

@jtkech Thank you for detailed explanation