From 9ff06eec6e82bd90a29a37dd34ecba931859f5f5 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Wed, 25 May 2022 11:16:39 +0200 Subject: [PATCH] v10: Instantly reload ConnectionStrings after saving configuration (#12475) * Do not replace DataDirectory placeholder when setting connection string * Ensure ConnectionStrings options are updated when configuration is reloaded * Use CurrentValue to get default Umbraco connection string --- .../Configuration/Models/ConnectionStrings.cs | 11 +-- .../UmbracoBuilder.Configuration.cs | 1 + .../Extensions/ConfigurationExtensions.cs | 20 +--- .../Configuration/JsonConfigManipulator.cs | 92 +++++++++++++------ .../Install/InstallHelper.cs | 22 +++-- .../InstallSteps/DatabaseConfigureStep.cs | 5 +- .../InstallSteps/DatabaseUpgradeStep.cs | 22 ++--- .../Install/InstallSteps/NewInstallStep.cs | 53 ++++------- .../Migrations/Install/DatabaseBuilder.cs | 16 ++-- .../Persistence/UmbracoDatabaseFactory.cs | 2 +- 10 files changed, 123 insertions(+), 121 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs index 333b655626..f4703adf92 100644 --- a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs +++ b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs @@ -7,8 +7,6 @@ namespace Umbraco.Cms.Core.Configuration.Models; /// public class ConnectionStrings // TODO: Rename to [Umbraco]ConnectionString (since v10 this only contains a single connection string) { - private string? _connectionString; - /// /// The default provider name when not present in configuration. /// @@ -39,14 +37,7 @@ public class ConnectionStrings // TODO: Rename to [Umbraco]ConnectionString (sin /// /// The connection string. /// - /// - /// When set, the will be replaced with the actual physical path. - /// - public string? ConnectionString - { - get => _connectionString; - set => _connectionString = ConfigurationExtensions.ReplaceDataDirectoryPlaceholder(value); - } + public string? ConnectionString { get; set; } /// /// Gets or sets the name of the provider. diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 7f5989e8b2..a4f34fb753 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -85,6 +85,7 @@ public static partial class UmbracoBuilderExtensions .AddUmbracoOptions() .AddUmbracoOptions(); + // Configure connection string and ensure it's updated when the configuration changes builder.Services.AddSingleton, ConfigureConnectionStrings>(); builder.Services.AddSingleton, ConfigurationChangeTokenSource>(); diff --git a/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs b/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs index d719876d9f..e5428fb4c4 100644 --- a/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs +++ b/src/Umbraco.Core/Extensions/ConfigurationExtensions.cs @@ -74,7 +74,11 @@ public static class ConfigurationExtensions if (!string.IsNullOrEmpty(connectionString)) { // Replace data directory - connectionString = ReplaceDataDirectoryPlaceholder(connectionString); + string? dataDirectory = AppDomain.CurrentDomain.GetData(DataDirectoryName)?.ToString(); + if (!string.IsNullOrEmpty(dataDirectory)) + { + connectionString = connectionString.Replace(DataDirectoryPlaceholder, dataDirectory); + } // Get provider name providerName = configuration.GetConnectionStringProviderName(name); @@ -86,18 +90,4 @@ public static class ConfigurationExtensions return connectionString; } - - internal static string? ReplaceDataDirectoryPlaceholder(string? connectionString) - { - if (!string.IsNullOrEmpty(connectionString)) - { - string? dataDirectory = AppDomain.CurrentDomain.GetData(DataDirectoryName)?.ToString(); - if (!string.IsNullOrEmpty(dataDirectory)) - { - return connectionString.Replace(DataDirectoryPlaceholder, dataDirectory); - } - } - - return connectionString; - } } diff --git a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs index 48c0540c67..e9e1236a9e 100644 --- a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs +++ b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs @@ -1,5 +1,3 @@ -using System; -using System.IO; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Json; using Microsoft.Extensions.DependencyInjection; @@ -14,6 +12,9 @@ namespace Umbraco.Cms.Core.Configuration { public class JsonConfigManipulator : IConfigManipulator { + private const string UmbracoConnectionStringPath = $"ConnectionStrings:{Cms.Core.Constants.System.UmbracoConnectionName}"; + private const string UmbracoConnectionStringProviderNamePath = UmbracoConnectionStringPath + ConnectionStrings.ProviderNamePostfix; + private readonly IConfiguration _configuration; private readonly ILogger _logger; private readonly object _locker = new object(); @@ -23,18 +24,24 @@ namespace Umbraco.Cms.Core.Configuration : this(configuration, StaticServiceProvider.Instance.GetRequiredService>()) { } - public JsonConfigManipulator( - IConfiguration configuration, - ILogger logger) + public JsonConfigManipulator(IConfiguration configuration, ILogger logger) { _configuration = configuration; _logger = logger; } - public string UmbracoConnectionPath { get; } = $"ConnectionStrings:{Cms.Core.Constants.System.UmbracoConnectionName}"; + [Obsolete] + public string UmbracoConnectionPath { get; } = UmbracoConnectionStringPath; + public void RemoveConnectionString() { - var provider = GetJsonConfigurationProvider(UmbracoConnectionPath); + // Update and reload configuration + _configuration[UmbracoConnectionStringPath] = null; + _configuration[UmbracoConnectionStringProviderNamePath] = null; + (_configuration as IConfigurationRoot)?.Reload(); + + // Remove keys from JSON + var provider = GetJsonConfigurationProvider(UmbracoConnectionStringPath); var json = GetJson(provider); if (json is null) @@ -43,13 +50,20 @@ namespace Umbraco.Cms.Core.Configuration return; } - RemoveJsonKey(json, UmbracoConnectionPath); + RemoveJsonKey(json, UmbracoConnectionStringPath); + RemoveJsonKey(json, UmbracoConnectionStringProviderNamePath); SaveJson(provider, json); } public void SaveConnectionString(string connectionString, string? providerName) { + // Update and reload configuration + _configuration[UmbracoConnectionStringPath] = connectionString; + _configuration[UmbracoConnectionStringProviderNamePath] = providerName; + (_configuration as IConfigurationRoot)?.Reload(); + + // Save keys to JSON var provider = GetJsonConfigurationProvider(); var json = GetJson(provider); @@ -60,18 +74,21 @@ namespace Umbraco.Cms.Core.Configuration } var item = GetConnectionItem(connectionString, providerName); - if (item is not null) { - json?.Merge(item, new JsonMergeSettings()); + json.Merge(item, new JsonMergeSettings()); } SaveJson(provider, json); } - public void SaveConfigValue(string key, object value) { + // Update and reload configuration + _configuration[key] = value?.ToString(); + (_configuration as IConfigurationRoot)?.Reload(); + + // Save key to JSON var provider = GetJsonConfigurationProvider(); var json = GetJson(provider); @@ -101,11 +118,15 @@ namespace Umbraco.Cms.Core.Configuration } SaveJson(provider, json); - } public void SaveDisableRedirectUrlTracking(bool disable) { + // Update and reload configuration + _configuration["Umbraco:CMS:WebRouting:DisableRedirectUrlTracking"] = disable.ToString(); + (_configuration as IConfigurationRoot)?.Reload(); + + // Save key to JSON var provider = GetJsonConfigurationProvider(); var json = GetJson(provider); @@ -116,10 +137,9 @@ namespace Umbraco.Cms.Core.Configuration } var item = GetDisableRedirectUrlItem(disable); - if (item is not null) { - json?.Merge(item, new JsonMergeSettings()); + json.Merge(item, new JsonMergeSettings()); } SaveJson(provider, json); @@ -127,6 +147,11 @@ namespace Umbraco.Cms.Core.Configuration public void SetGlobalId(string id) { + // Update and reload configuration + _configuration["Umbraco:CMS:Global:Id"] = id; + (_configuration as IConfigurationRoot)?.Reload(); + + // Save key to JSON var provider = GetJsonConfigurationProvider(); var json = GetJson(provider); @@ -137,10 +162,9 @@ namespace Umbraco.Cms.Core.Configuration } var item = GetGlobalIdItem(id); - if (item is not null) { - json?.Merge(item, new JsonMergeSettings()); + json.Merge(item, new JsonMergeSettings()); } SaveJson(provider, json); @@ -197,8 +221,13 @@ namespace Umbraco.Cms.Core.Configuration writer.WriteStartObject(); writer.WritePropertyName(Constants.System.UmbracoConnectionName); writer.WriteValue(connectionString); - writer.WritePropertyName($"{Constants.System.UmbracoConnectionName}{ConnectionStrings.ProviderNamePostfix}"); - writer.WriteValue(providerName); + + if (!string.IsNullOrEmpty(providerName)) + { + writer.WritePropertyName(Constants.System.UmbracoConnectionName + ConnectionStrings.ProviderNamePostfix); + writer.WriteValue(providerName); + } + writer.WriteEndObject(); writer.WriteEndObject(); @@ -216,8 +245,13 @@ namespace Umbraco.Cms.Core.Configuration token?.Parent?.Remove(); } - private void SaveJson(JsonConfigurationProvider provider, JObject? json) + private void SaveJson(JsonConfigurationProvider? provider, JObject? json) { + if (provider is null) + { + return; + } + lock (_locker) { if (provider.Source.FileProvider is PhysicalFileProvider physicalFileProvider) @@ -243,8 +277,13 @@ namespace Umbraco.Cms.Core.Configuration } } - private JObject? GetJson(JsonConfigurationProvider provider) + private JObject? GetJson(JsonConfigurationProvider? provider) { + if (provider is null) + { + return null; + } + lock (_locker) { if (provider.Source.FileProvider is not PhysicalFileProvider physicalFileProvider) @@ -269,22 +308,21 @@ namespace Umbraco.Cms.Core.Configuration } } - private JsonConfigurationProvider GetJsonConfigurationProvider(string? requiredKey = null) + private JsonConfigurationProvider? GetJsonConfigurationProvider(string? requiredKey = null) { if (_configuration is IConfigurationRoot configurationRoot) { foreach (var provider in configurationRoot.Providers) { - if (provider is JsonConfigurationProvider jsonConfigurationProvider) + if (provider is JsonConfigurationProvider jsonConfigurationProvider && + (requiredKey is null || provider.TryGet(requiredKey, out _))) { - if (requiredKey is null || provider.TryGet(requiredKey, out _)) - { - return jsonConfigurationProvider; - } + return jsonConfigurationProvider; } } } - throw new InvalidOperationException("Could not find a writable json config source"); + + return null; } /// diff --git a/src/Umbraco.Infrastructure/Install/InstallHelper.cs b/src/Umbraco.Infrastructure/Install/InstallHelper.cs index 671dc85c4f..0b6c5cc32b 100644 --- a/src/Umbraco.Infrastructure/Install/InstallHelper.cs +++ b/src/Umbraco.Infrastructure/Install/InstallHelper.cs @@ -1,5 +1,3 @@ -using System; -using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core; @@ -28,7 +26,8 @@ namespace Umbraco.Cms.Infrastructure.Install private readonly IUmbracoDatabaseFactory _umbracoDatabaseFactory; private InstallationType? _installationType; - public InstallHelper(DatabaseBuilder databaseBuilder, + public InstallHelper( + DatabaseBuilder databaseBuilder, ILogger logger, IUmbracoVersion umbracoVersion, IOptionsMonitor connectionStrings, @@ -75,10 +74,17 @@ namespace Umbraco.Cms.Infrastructure.Install dbProvider = _umbracoDatabaseFactory.SqlContext.SqlSyntax.DbProvider; } - var installLog = new InstallLog(installId: installId, isUpgrade: IsBrandNewInstall == false, - installCompleted: isCompleted, timestamp: DateTime.Now, versionMajor: _umbracoVersion.Version.Major, - versionMinor: _umbracoVersion.Version.Minor, versionPatch: _umbracoVersion.Version.Build, - versionComment: _umbracoVersion.Comment, error: errorMsg, userAgent: userAgent, + var installLog = new InstallLog( + installId: installId, + isUpgrade: IsBrandNewInstall == false, + installCompleted: isCompleted, + timestamp: DateTime.Now, + versionMajor: _umbracoVersion.Version.Major, + versionMinor: _umbracoVersion.Version.Minor, + versionPatch: _umbracoVersion.Version.Build, + versionComment: _umbracoVersion.Comment, + error: errorMsg, + userAgent: userAgent, dbProvider: dbProvider); await _installationService.LogInstall(installLog); @@ -96,7 +102,7 @@ namespace Umbraco.Cms.Infrastructure.Install /// true if this is a brand new install; otherwise, false. /// private bool IsBrandNewInstall => - _connectionStrings.Get(Constants.System.UmbracoConnectionName).IsConnectionStringConfigured() == false || + _connectionStrings.CurrentValue.IsConnectionStringConfigured() == false || _databaseBuilder.IsDatabaseConfigured == false || _databaseBuilder.CanConnectToDatabase == false || _databaseBuilder.IsUmbracoInstalled() == false; diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseConfigureStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseConfigureStep.cs index f01b65aaa7..2a9dacfe5e 100644 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseConfigureStep.cs +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseConfigureStep.cs @@ -50,9 +50,8 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps private bool ShouldDisplayView() { - // If the connection string is already present in web.config we don't need to show the settings page and we jump to installing/upgrading. - var databaseSettings = _connectionStrings.Get(Core.Constants.System.UmbracoConnectionName); - if (databaseSettings.IsConnectionStringConfigured()) + // If the connection string is already present in config we don't need to show the settings page and we jump to installing/upgrading. + if (_connectionStrings.CurrentValue.IsConnectionStringConfigured()) { try { diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs index 25494ff925..1322aaa3b8 100644 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs @@ -1,6 +1,3 @@ -using System; -using System.Linq; -using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Cms.Core; @@ -16,8 +13,7 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Install.InstallSteps { - [InstallSetupStep(InstallationType.Upgrade | InstallationType.NewInstall, - "DatabaseUpgrade", 12, "")] + [InstallSetupStep(InstallationType.Upgrade | InstallationType.NewInstall, "DatabaseUpgrade", 12, "")] public class DatabaseUpgradeStep : InstallSetupStep { private readonly DatabaseBuilder _databaseBuilder; @@ -66,28 +62,28 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps public override bool RequiresExecution(object model) { - //if it's properly configured (i.e. the versions match) then no upgrade necessary + // If it's properly configured (i.e. the versions match) then no upgrade necessary if (_runtime.Level == RuntimeLevel.Run) + { return false; + } + // This step relies on the previous one completed - because it has stored some information we need var installSteps = InstallStatusTracker.GetStatus().ToArray(); - //this step relies on the previous one completed - because it has stored some information we need if (installSteps.Any(x => x.Name == "DatabaseInstall" && x.AdditionalData.ContainsKey("upgrade")) == false) { return false; } - var databaseSettings = _connectionStrings.Get(Core.Constants.System.UmbracoConnectionName); - - if (databaseSettings.IsConnectionStringConfigured()) + if (_connectionStrings.CurrentValue.IsConnectionStringConfigured()) { - // a connection string was present, determine whether this is an install/upgrade - // return true (upgrade) if there is an installed version, else false (install) + // A connection string was present, determine whether this is an install/upgrade + // Return true (upgrade) if there is an installed version, else false (install) var result = _databaseBuilder.ValidateSchema(); return result?.DetermineHasInstalledVersion() ?? false; } - //no connection string configured, probably a fresh install + // No connection string configured, probably a fresh install return false; } } diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/NewInstallStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/NewInstallStep.cs index 4f5dbc3cc2..582faab76c 100644 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/NewInstallStep.cs +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/NewInstallStep.cs @@ -131,56 +131,44 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps } } - public override string View - { - get - { - return ShowView() - // the user UI - ? "user" - // continue install UI - : "continueinstall"; - } - } + public override string View => ShowView() + // the user UI + ? "user" + // continue install UI + : "continueinstall"; private InstallState GetInstallState() { var installState = InstallState.Unknown; - - // TODO: we need to do a null check here since this could be entirely missing and we end up with a null ref - // exception in the installer. - - var databaseSettings = _connectionStrings.Get(Constants.System.UmbracoConnectionName); - - var hasConnString = databaseSettings != null && _databaseBuilder.IsDatabaseConfigured; - if (hasConnString) + if (_databaseBuilder.IsDatabaseConfigured) { installState = (installState | InstallState.HasConnectionString) & ~InstallState.Unknown; } - var connStringConfigured = databaseSettings?.IsConnectionStringConfigured() ?? false; - if (connStringConfigured) + var umbracoConnectionString = _connectionStrings.CurrentValue; + + var isConnectionStringConfigured = umbracoConnectionString.IsConnectionStringConfigured(); + if (isConnectionStringConfigured) { installState = (installState | InstallState.ConnectionStringConfigured) & ~InstallState.Unknown; } - - var factory = _dbProviderFactoryCreator.CreateFactory(databaseSettings?.ProviderName); - var canConnect = connStringConfigured && DbConnectionExtensions.IsConnectionAvailable(databaseSettings?.ConnectionString, factory); - if (canConnect) + var factory = _dbProviderFactoryCreator.CreateFactory(umbracoConnectionString.ProviderName); + var isConnectionAvailable = isConnectionStringConfigured && DbConnectionExtensions.IsConnectionAvailable(umbracoConnectionString.ConnectionString, factory); + if (isConnectionAvailable) { installState = (installState | InstallState.CanConnect) & ~InstallState.Unknown; } - var umbracoInstalled = canConnect ? _databaseBuilder.IsUmbracoInstalled() : false; - if (umbracoInstalled) + var isUmbracoInstalled = isConnectionAvailable && _databaseBuilder.IsUmbracoInstalled(); + if (isUmbracoInstalled) { installState = (installState | InstallState.UmbracoInstalled) & ~InstallState.Unknown; } - var hasNonDefaultUser = umbracoInstalled ? _databaseBuilder.HasSomeNonDefaultUser() : false; - if (hasNonDefaultUser) + var hasSomeNonDefaultUser = isUmbracoInstalled && _databaseBuilder.HasSomeNonDefaultUser(); + if (hasSomeNonDefaultUser) { installState = (installState | InstallState.HasNonDefaultUser) & ~InstallState.Unknown; } @@ -192,14 +180,12 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps { var installState = GetInstallState(); - return installState.HasFlag(InstallState.Unknown) - || !installState.HasFlag(InstallState.UmbracoInstalled); + return installState.HasFlag(InstallState.Unknown) || !installState.HasFlag(InstallState.UmbracoInstalled); } public override bool RequiresExecution(UserModel model) { var installState = GetInstallState(); - if (installState.HasFlag(InstallState.Unknown)) { // In this one case when it's a brand new install and nothing has been configured, make sure the @@ -207,8 +193,7 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps _cookieManager.ExpireCookie(_securitySettings.AuthCookieName); } - return installState.HasFlag(InstallState.Unknown) - || !installState.HasFlag(InstallState.HasNonDefaultUser); + return installState.HasFlag(InstallState.Unknown) || !installState.HasFlag(InstallState.HasNonDefaultUser); } [Flags] diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index c4a37d4374..2f8bd3f4ec 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -155,28 +155,24 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install var connectionString = providerMeta.GenerateConnectionString(databaseSettings); var providerName = databaseSettings.ProviderName ?? providerMeta.ProviderName; - if (providerMeta.RequiresConnectionTest && !CanConnect(connectionString, providerName!)) + if (string.IsNullOrEmpty(connectionString) || string.IsNullOrEmpty(providerName) || + (providerMeta.RequiresConnectionTest && !CanConnect(connectionString, providerName))) { return false; } if (!isTrialRun) { - _configManipulator.SaveConnectionString(connectionString!, providerName); - Configure(connectionString!, providerName, _globalSettings.CurrentValue.InstallMissingDatabase || providerMeta.ForceCreateDatabase); + _configManipulator.SaveConnectionString(connectionString, providerName); + Configure(_globalSettings.CurrentValue.InstallMissingDatabase || providerMeta.ForceCreateDatabase); } return true; } - private void Configure(string connectionString, string? providerName, bool installMissingDatabase) + private void Configure(bool installMissingDatabase) { - // Update existing connection string - var umbracoConnectionString = _connectionStrings.Get(Core.Constants.System.UmbracoConnectionName); - umbracoConnectionString.ConnectionString = connectionString; - umbracoConnectionString.ProviderName = providerName; - - _databaseFactory.Configure(umbracoConnectionString); + _databaseFactory.Configure(_connectionStrings.CurrentValue); if (installMissingDatabase) { diff --git a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseFactory.cs b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseFactory.cs index 8677bc1c75..6257f7367f 100644 --- a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseFactory.cs @@ -97,7 +97,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _loggerFactory = loggerFactory; - ConnectionStrings umbracoConnectionString = connectionStrings.Get(Constants.System.UmbracoConnectionName); + ConnectionStrings umbracoConnectionString = connectionStrings.CurrentValue; if (!umbracoConnectionString.IsConnectionStringConfigured()) { logger.LogDebug("Missing connection string, defer configuration.");