OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Multitenancy and distributed shell restart #5036

Open orchardbot opened 9 years ago

orchardbot commented 9 years ago

MattGC0 created: https://orchard.codeplex.com/workitem/21207

Hi

I have a potential patch that I would like reviewed in order to notify web instances in an Azure Cloud Service installation of a new tenant. I found that the existing DistributedShellRestart had a problem with potential for a feedback loop and it also does not currently work for notifying new tenants only changed. This has been tested on 1.x (obviously) with the Redis message bus.

When creating a tenant I do so via a Web API call. This triggers a Changed event and subscribers are notified. This had a bug whereby the subscriber would notify back to the Default host because the DefaultOrchardHost.Saved event is called. I have resolved this with the code below..

First, this is how I trigger a restart for the tenant's shell in the Web API call (I am not suggesting adding this it is just to show what I am doing).... // add the new tenant to the restart collection var tenantsToRestart = (IList)HttpContext.Current.Items["DefaultOrchardHost.TenantsToRestart"]; if (tenantsToRestart.Any(t => t.Name.Equals(shellSettings.Name))) { tenantsToRestart.Remove(tenantsToRestart.FirstOrDefault(t => t.Name.Equals(shellSettings.Name))); } tenantsToRestart.Add(shellSettings); HttpContext.Current.Items["DefaultOrchardHost.TenantsToRestart"] = tenantsToRestart;

            var host = _orchardHost as DefaultOrchardHost;
            host.ActivateShell(shellSettings);

            // trigger a change in the tenant such that it gets refreshed by the default orchard host
            _events.Changed(shellDescriptor, viewModel.TenantName);
            //_savedEvents.Saved(shellSettings);

Now the change I made in DistributedShellStarter.cs... public void Activated() { _messageBus.Subscribe(Channel, (channel, message) => {

            using (var scope = _workContextAccessor.CreateWorkContextScope())
            {
                var shellSettings = scope.Resolve<ShellSettings>();
                if (shellSettings != null)
                {
                    var shellSettingsManager = scope.Resolve<IShellSettingsManager>();
                    var newShellSettings = shellSettingsManager.LoadSettings().FirstOrDefault(x => x.Name.Equals(message));
                    if (newShellSettings != null)
                    {
                        // commented out because this code causes a feedback loop. By triggering Saved it sends a message
                        // back to the host site that initiated the call which calls DefaultOrchardHost.Saved and round and round it goes.
                        //var shellSettingsManagerEventHandler = scope.Resolve<IShellSettingsManagerEventHandler>();
                        //shellSettingsManagerEventHandler.Saved(newShellSettings);
                        var orchardHost = scope.Resolve<IOrchardHost>() as DefaultOrchardHost;

                        if (orchardHost != null)
                        {
                            orchardHost.AddToShellsToRestart(newShellSettings);
                            var startUpdatedShellsMethod = typeof(DefaultOrchardHost).GetMethod("StartUpdatedShells", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
                            startUpdatedShellsMethod.Invoke(orchardHost, null);
                        }
                    }

                }
            }
        });
    }

And void IShellSettingsManagerEventHandler.Saved(ShellSettings settings) { AddToShellsToRestart(settings); }

    public void AddToShellsToRestart(ShellSettings settings)
    {
        Logger.Debug("Shell saved: " + settings.Name);

        // if a tenant has been created
        if (settings.State != TenantState.Invalid)
        {
            if (!_tenantsToRestart.GetState().Any(t => t.Name.Equals(settings.Name)))
            {
                Logger.Debug("Adding tenant to restart: " + settings.Name + " " + settings.State);
                _tenantsToRestart.GetState().Add(settings);
            }
        }
    }

Could someone advise whether this code is acceptable please? Seems to work in Azure both for new tenants added by the Default host site as well as for changes for tenants. Should I make the AddToShellsToRestart method private? For non-Azure web farm scenarios, I think this might break since the AppData folder would be duplicated rather than shared as it is in Azure. So thinking about it maybe I need to override DistributedShellStarter.cs with an Azure only version.

Thanks Matt

orchardbot commented 9 years ago

@Piedone commented:

I think we discussed this with Sebastien and the verdict was that: