diff --git a/src/Umbraco.Cms.Api.Management/Controllers/RedirectUrlManagement/SetStatusRedirectUrlManagementController.cs b/src/Umbraco.Cms.Api.Management/Controllers/RedirectUrlManagement/SetStatusRedirectUrlManagementController.cs index fa74526e38..45bd818123 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/RedirectUrlManagement/SetStatusRedirectUrlManagementController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/RedirectUrlManagement/SetStatusRedirectUrlManagementController.cs @@ -43,7 +43,7 @@ public class SetStatusRedirectUrlManagementController : RedirectUrlManagementCon // For now I'm not gonna change this to limit breaking, but it's weird to have a "disabled" switch, // since you're essentially negating the boolean from the get go, // it's much easier to reason with enabled = false == disabled. - _configManipulator.SaveDisableRedirectUrlTracking(!enable); + await _configManipulator.SaveDisableRedirectUrlTrackingAsync(!enable); // Taken from the existing implementation in RedirectUrlManagementController // TODO this is ridiculous, but we need to ensure the configuration is reloaded, before this request is ended. diff --git a/src/Umbraco.Core/Configuration/IConfigManipulator.cs b/src/Umbraco.Core/Configuration/IConfigManipulator.cs index 18ce8a5eca..8f5e0c0472 100644 --- a/src/Umbraco.Core/Configuration/IConfigManipulator.cs +++ b/src/Umbraco.Core/Configuration/IConfigManipulator.cs @@ -2,13 +2,81 @@ namespace Umbraco.Cms.Core.Configuration; public interface IConfigManipulator { + [Obsolete("Use RemoveConnectionStringAsync instead, scheduled for removal in V16.")] void RemoveConnectionString(); + [Obsolete("Use SaveConnectionStringAsync instead, scheduled for removal in V16.")] void SaveConnectionString(string connectionString, string? providerName); + [Obsolete("Use SaveConfigValueAsync instead, scheduled for removal in V16.")] void SaveConfigValue(string itemPath, object value); + [Obsolete("Use SaveDisableRedirectUrlTrackingAsync instead, scheduled for removal in V16.")] void SaveDisableRedirectUrlTracking(bool disable); + [Obsolete("Use SetGlobalIdAsync instead, scheduled for removal in V16.")] void SetGlobalId(string id); + + /// + /// Removes the connection string from the configuration file + /// + /// + Task RemoveConnectionStringAsync() + { + RemoveConnectionString(); + return Task.CompletedTask; + } + + /// + /// Saves the connection string to the configuration file + /// + /// + /// + /// + Task SaveConnectionStringAsync(string connectionString, string? providerName) + { + SaveConnectionString(connectionString, providerName); + return Task.CompletedTask; + } + + /// + /// Updates a value in the configuration file. + /// Will only update an existing key in the configuration file, if it does not exists nothing is saved + /// + /// Path to update, uses : as the separator. + /// The new value. + /// + Task SaveConfigValueAsync(string itemPath, object value) + { + SaveConfigValue(itemPath, value); + return Task.CompletedTask; + } + + /// + /// Updates the disableRedirectUrlTracking value in the configuration file. + /// + /// Will create the node if it does not already exist. + /// + /// + /// The value to save. + /// + Task SaveDisableRedirectUrlTrackingAsync(bool disable) + { + SaveDisableRedirectUrlTracking(disable); + return Task.CompletedTask; + } + + /// + /// Sets the global id in the configuration file. + /// + /// Will create the node if it does not already exist. + /// + /// + /// The ID to save. + /// + Task SetGlobalIdAsync(string id) + { + SetGlobalId(id); + return Task.CompletedTask; + } } diff --git a/src/Umbraco.Core/Telemetry/SiteIdentifierService.cs b/src/Umbraco.Core/Telemetry/SiteIdentifierService.cs index a7b5882ecc..6af5830cf5 100644 --- a/src/Umbraco.Core/Telemetry/SiteIdentifierService.cs +++ b/src/Umbraco.Core/Telemetry/SiteIdentifierService.cs @@ -65,7 +65,7 @@ internal class SiteIdentifierService : ISiteIdentifierService try { - _configManipulator.SetGlobalId(createdGuid.ToString()); + _configManipulator.SetGlobalIdAsync(createdGuid.ToString()).GetAwaiter().GetResult(); } catch (Exception ex) { diff --git a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs index fb0f8f4f18..7a07edfc7b 100644 --- a/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs +++ b/src/Umbraco.Infrastructure/Configuration/JsonConfigManipulator.cs @@ -1,329 +1,300 @@ +using System.Text.Json; +using System.Text.Json.Nodes; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.Json; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Cms.Core.DependencyInjection; -namespace Umbraco.Cms.Core.Configuration +namespace Umbraco.Cms.Infrastructure.Configuration; + +internal class JsonConfigManipulator : IConfigManipulator { - public class JsonConfigManipulator : IConfigManipulator + private const string ConnectionStringObjectName = "ConnectionStrings"; + private const string UmbracoConnectionStringPath = $"{ConnectionStringObjectName}:{Constants.System.UmbracoConnectionName}"; + private const string UmbracoConnectionStringProviderNamePath = UmbracoConnectionStringPath + ConnectionStrings.ProviderNamePostfix; + private const string CmsObjectPath = "Umbraco:CMS"; + private const string GlobalIdPath = $"{CmsObjectPath}:Global:Id"; + private const string DisableRedirectUrlTrackingPath = $"{CmsObjectPath}:WebRouting:DisableRedirectUrlTracking"; + + private readonly IConfiguration _configuration; + private readonly ILogger _logger; + private readonly SemaphoreSlim _lock = new(1, 1); + + public JsonConfigManipulator(IConfiguration configuration, ILogger logger) { - private const string UmbracoConnectionStringPath = $"ConnectionStrings:{Cms.Core.Constants.System.UmbracoConnectionName}"; - private const string UmbracoConnectionStringProviderNamePath = UmbracoConnectionStringPath + ConnectionStrings.ProviderNamePostfix; + _configuration = configuration; + _logger = logger; + } - private readonly IConfiguration _configuration; - private readonly ILogger _logger; - private readonly object _locker = new object(); + public void RemoveConnectionString() + => RemoveConnectionStringAsync().GetAwaiter().GetResult(); - public JsonConfigManipulator(IConfiguration configuration, ILogger logger) + /// + public async Task RemoveConnectionStringAsync() + { + JsonConfigurationProvider? provider = GetJsonConfigurationProvider(UmbracoConnectionStringPath); + + JsonNode? jsonNode = await GetJsonNodeAsync(provider); + + if (jsonNode is null) { - _configuration = configuration; - _logger = logger; + _logger.LogWarning("Failed to remove connection string from JSON configuration"); + return; } - [Obsolete] - public string UmbracoConnectionPath { get; } = UmbracoConnectionStringPath; + RemoveJsonNode(jsonNode, UmbracoConnectionStringPath); + RemoveJsonNode(jsonNode, UmbracoConnectionStringProviderNamePath); - public void RemoveConnectionString() + await SaveJsonAsync(provider, jsonNode); + } + + public void SaveConnectionString(string connectionString, string? providerName) + => SaveConnectionStringAsync(connectionString, providerName).GetAwaiter().GetResult(); + + /// + public async Task SaveConnectionStringAsync(string connectionString, string? providerName) + { + JsonConfigurationProvider? provider = GetJsonConfigurationProvider(); + JsonNode? node = await GetJsonNodeAsync(provider); + + if (node is null) { - // Remove keys from JSON - var provider = GetJsonConfigurationProvider(UmbracoConnectionStringPath); - - var json = GetJson(provider); - if (json is null) - { - _logger.LogWarning("Failed to remove connection string from JSON configuration."); - return; - } - - RemoveJsonKey(json, UmbracoConnectionStringPath); - RemoveJsonKey(json, UmbracoConnectionStringProviderNamePath); - - SaveJson(provider, json); + _logger.LogWarning("Was unable to load the configuration file to save the connection string"); + return; } - public void SaveConnectionString(string connectionString, string? providerName) + CreateOrUpdateJsonNode(node, UmbracoConnectionStringPath, connectionString); + if (providerName is not null) { - // Save keys to JSON - var provider = GetJsonConfigurationProvider(); - - var json = GetJson(provider); - if (json is null) - { - _logger.LogWarning("Failed to save connection string in JSON configuration."); - return; - } - - var item = GetConnectionItem(connectionString, providerName); - if (item is not null) - { - json.Merge(item, new JsonMergeSettings()); - } - - SaveJson(provider, json); + CreateOrUpdateJsonNode(node, UmbracoConnectionStringProviderNamePath, providerName); } - public void SaveConfigValue(string key, object value) + await SaveJsonAsync(provider, node); + } + + public void SaveConfigValue(string key, object value) + => SaveConfigValueAsync(key, value).GetAwaiter().GetResult(); + + /// + public async Task SaveConfigValueAsync(string itemPath, object value) + { + JsonConfigurationProvider? provider = GetJsonConfigurationProvider(); + JsonNode? node = await GetJsonNodeAsync(provider); + + if (node is null) { - // Save key to JSON - var provider = GetJsonConfigurationProvider(); - - var json = GetJson(provider); - if (json is null) - { - _logger.LogWarning("Failed to save configuration key \"{Key}\" in JSON configuration.", key); - return; - } - - JToken? token = json; - foreach (var propertyName in key.Split(new[] { ':' })) - { - if (token is null) - break; - token = CaseSelectPropertyValues(token, propertyName); - } - - if (token is null) - return; - - var writer = new JTokenWriter(); - writer.WriteValue(value); - - if (writer.Token is not null) - { - token.Replace(writer.Token); - } - - SaveJson(provider, json); + _logger.LogWarning("Failed to save configuration key \"{Key}\" in JSON configuration", itemPath); + return; } - public void SaveDisableRedirectUrlTracking(bool disable) + JsonNode? propertyNode = node; + foreach (var propertyName in itemPath.Split(':')) { - // Save key to JSON - var provider = GetJsonConfigurationProvider(); - - var json = GetJson(provider); - if (json is null) - { - _logger.LogWarning("Failed to save enabled/disabled state for redirect URL tracking in JSON configuration."); - return; - } - - var item = GetDisableRedirectUrlItem(disable); - if (item is not null) - { - json.Merge(item, new JsonMergeSettings()); - } - - SaveJson(provider, json); + propertyNode = FindChildNode(propertyNode, propertyName); } - public void SetGlobalId(string id) + if (propertyNode is null) { - // Save key to JSON - var provider = GetJsonConfigurationProvider(); - - var json = GetJson(provider); - if (json is null) - { - _logger.LogWarning("Failed to save global identifier in JSON configuration."); - return; - } - - var item = GetGlobalIdItem(id); - if (item is not null) - { - json.Merge(item, new JsonMergeSettings()); - } - - SaveJson(provider, json); + return; } - private object? GetGlobalIdItem(string id) + propertyNode.ReplaceWith(value); + await SaveJsonAsync(provider, node); + } + + public void SaveDisableRedirectUrlTracking(bool disable) + => SaveDisableRedirectUrlTrackingAsync(disable).GetAwaiter().GetResult(); + + /// + public async Task SaveDisableRedirectUrlTrackingAsync(bool disable) + => await CreateOrUpdateConfigValueAsync(DisableRedirectUrlTrackingPath, disable); + + public void SetGlobalId(string id) + => SetGlobalIdAsync(id).GetAwaiter().GetResult(); + + /// + public async Task SetGlobalIdAsync(string id) + => await CreateOrUpdateConfigValueAsync(GlobalIdPath, id); + + /// + /// Creates or updates a config value at the specified path. + /// This causes a rewrite of the configuration file. + /// + /// Path to update, uses : as the separator. + /// The value of the node. + private async Task CreateOrUpdateConfigValueAsync(string itemPath, object value) + { + JsonConfigurationProvider? provider = GetJsonConfigurationProvider(); + JsonNode? node = await GetJsonNodeAsync(provider); + + if (node is null) { - JTokenWriter writer = new JTokenWriter(); - - writer.WriteStartObject(); - writer.WritePropertyName("Umbraco"); - writer.WriteStartObject(); - writer.WritePropertyName("CMS"); - writer.WriteStartObject(); - writer.WritePropertyName("Global"); - writer.WriteStartObject(); - writer.WritePropertyName("Id"); - writer.WriteValue(id); - writer.WriteEndObject(); - writer.WriteEndObject(); - writer.WriteEndObject(); - writer.WriteEndObject(); - - return writer.Token; + _logger.LogWarning("Failed to save configuration key \"{Key}\" in JSON configuration", itemPath); + return; } - private JToken? GetDisableRedirectUrlItem(bool value) + CreateOrUpdateJsonNode(node, itemPath, value); + await SaveJsonAsync(provider, node); + } + + /// + /// Updates or creates a json node at the specified path. + /// + /// Will also create any missing nodes in the path. + /// + /// + /// Node to create or update. + /// Path to create or update, uses : as the separator. + /// The value of the node. + private static void CreateOrUpdateJsonNode(JsonNode node, string itemPath, object value) + { + // This is required because System.Text.Json has no merge function, and doesn't support patch + // this is a problem because we don't know if the key(s) exists yet, so we can't simply update it, + // we may have to create one ore more json objects. + + // First we find the inner most child that already exists. + var propertyNames = itemPath.Split(':'); + JsonNode propertyNode = node; + var index = 0; + foreach (var propertyName in propertyNames) { - JTokenWriter writer = new JTokenWriter(); + JsonNode? found = FindChildNode(propertyNode, propertyName); + if (found is null) + { + break; + } - writer.WriteStartObject(); - writer.WritePropertyName("Umbraco"); - writer.WriteStartObject(); - writer.WritePropertyName("CMS"); - writer.WriteStartObject(); - writer.WritePropertyName("WebRouting"); - writer.WriteStartObject(); - writer.WritePropertyName("DisableRedirectUrlTracking"); - writer.WriteValue(value); - writer.WriteEndObject(); - writer.WriteEndObject(); - writer.WriteEndObject(); - writer.WriteEndObject(); - - return writer.Token; + propertyNode = found; + index++; } - private JToken? GetConnectionItem(string connectionString, string? providerName) + + // We can now use the index to go through the remaining keys, creating them as we go. + while (index < propertyNames.Length) { - JTokenWriter writer = new JTokenWriter(); - - writer.WriteStartObject(); - writer.WritePropertyName("ConnectionStrings"); - writer.WriteStartObject(); - writer.WritePropertyName(Constants.System.UmbracoConnectionName); - writer.WriteValue(connectionString); - - if (!string.IsNullOrEmpty(providerName)) - { - writer.WritePropertyName(Constants.System.UmbracoConnectionName + ConnectionStrings.ProviderNamePostfix); - writer.WriteValue(providerName); - } - - writer.WriteEndObject(); - writer.WriteEndObject(); - - return writer.Token; + var propertyName = propertyNames[index]; + var newNode = new JsonObject(); + propertyNode.AsObject()[propertyName] = newNode; + propertyNode = newNode; + index++; } - private static void RemoveJsonKey(JObject? json, string key) - { - JToken? token = json; - foreach (var propertyName in key.Split(new[] { ':' })) - { - token = CaseSelectPropertyValues(token, propertyName); - } + // System.Text.Json doesn't like just setting an Object as a value, so instead we first create the node, + // and then replace the value + propertyNode.ReplaceWith(value); + } - token?.Parent?.Remove(); + private static void RemoveJsonNode(JsonNode node, string key) + { + JsonNode? propertyNode = node; + foreach (var propertyName in key.Split(':')) + { + propertyNode = FindChildNode(propertyNode, propertyName); } - private void SaveJson(JsonConfigurationProvider? provider, JObject? json) + propertyNode?.Parent?.AsObject().Remove(propertyNode.GetPropertyName()); + } + + private async Task SaveJsonAsync(JsonConfigurationProvider? provider, JsonNode jsonNode) + { + if (provider?.Source.FileProvider is not PhysicalFileProvider physicalFileProvider) { - if (provider is null) - { - return; - } - - lock (_locker) - { - if (provider.Source.FileProvider is PhysicalFileProvider physicalFileProvider) - { - var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path!); - - try - { - using (var sw = new StreamWriter(jsonFilePath, false)) - using (var jsonTextWriter = new JsonTextWriter(sw) - { - Formatting = Formatting.Indented, - }) - { - json?.WriteTo(jsonTextWriter); - } - } - catch (IOException exception) - { - _logger.LogWarning(exception, "JSON configuration could not be written: {path}", jsonFilePath); - } - } - } + return; } - private JObject? GetJson(JsonConfigurationProvider? provider) + await _lock.WaitAsync(); + + try { - if (provider is null) - { - return null; - } - - lock (_locker) - { - if (provider.Source.FileProvider is not PhysicalFileProvider physicalFileProvider) - { - return null; - } - - var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path!); - - try - { - var serializer = new JsonSerializer(); - using var sr = new StreamReader(jsonFilePath); - using var jsonTextReader = new JsonTextReader(sr); - return serializer.Deserialize(jsonTextReader); - } - catch (IOException exception) - { - _logger.LogWarning(exception, "JSON configuration could not be read: {path}", jsonFilePath); - return null; - } - } + var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path!); + await using var jsonConfigStream = new FileStream(jsonFilePath, FileMode.Create); + await using var writer = new Utf8JsonWriter(jsonConfigStream, new JsonWriterOptions { Indented = true }); + jsonNode.WriteTo(writer); } - - private JsonConfigurationProvider? GetJsonConfigurationProvider(string? requiredKey = null) + finally { - if (_configuration is IConfigurationRoot configurationRoot) - { - foreach (var provider in configurationRoot.Providers) - { - if (provider is JsonConfigurationProvider jsonConfigurationProvider && - (requiredKey is null || provider.TryGet(requiredKey, out _))) - { - return jsonConfigurationProvider; - } - } - } - - return null; - } - - /// - /// Returns the property value when case insensative - /// - /// - /// This method is required because keys are case insensative in IConfiguration. - /// JObject[..] do not support case insensative and JObject.Property(...) do not return a new JObject. - /// - private static JToken? CaseSelectPropertyValues(JToken? token, string name) - { - if (token is JObject obj) - { - foreach (var property in obj.Properties()) - { - if (name is null) - { - return property.Value; - } - - if (string.Equals(property.Name, name, StringComparison.OrdinalIgnoreCase)) - { - return property.Value; - } - } - } - - return null; + _lock.Release(); } } + + private async Task GetJsonNodeAsync(JsonConfigurationProvider? provider) + { + if (provider is null) + { + return null; + } + + await _lock.WaitAsync(); + if (provider.Source.FileProvider is not PhysicalFileProvider physicalFileProvider) + { + return null; + } + + var jsonFilePath = Path.Combine(physicalFileProvider.Root, provider.Source.Path!); + + try + { + using var streamReader = new StreamReader(jsonFilePath); + return await JsonNode.ParseAsync(streamReader.BaseStream); + } + catch (IOException exception) + { + _logger.LogWarning(exception, "JSON configuration could not be read: {Path}", jsonFilePath); + return null; + } + finally + { + _lock.Release(); + } + } + + private JsonConfigurationProvider? GetJsonConfigurationProvider(string? requiredKey = null) + { + if (_configuration is not IConfigurationRoot configurationRoot) + { + return null; + } + + foreach (IConfigurationProvider provider in configurationRoot.Providers) + { + if (provider is JsonConfigurationProvider jsonConfigurationProvider && + (requiredKey is null || provider.TryGet(requiredKey, out _))) + { + return jsonConfigurationProvider; + } + } + + return null; + } + + /// + /// Finds the immediate child with the specified name, in a case insensitive manner. + /// + /// + /// This is required since keys are case insensitive in IConfiguration. + /// But not in JsonNode. + /// + /// The node to search. + /// The key to search for. + /// The found node, null if no match is found. + private static JsonNode? FindChildNode(JsonNode? node, string key) + { + if (node is null) + { + return node; + } + + foreach (KeyValuePair property in node.AsObject()) + { + if (property.Key.Equals(key, StringComparison.OrdinalIgnoreCase)) + { + return property.Value; + } + } + + return null; + } } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index a60cef7e7f..808d6ceb6e 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -42,6 +42,7 @@ using Umbraco.Cms.Core.Templates; using Umbraco.Cms.Core.Trees; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Core.Webhooks; +using Umbraco.Cms.Infrastructure.Configuration; using Umbraco.Cms.Infrastructure.DeliveryApi; using Umbraco.Cms.Infrastructure.DistributedLocking; using Umbraco.Cms.Infrastructure.Examine; diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index 25dc48d23b..fc5f9e2ceb 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -219,7 +219,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install }); // Update configuration and wait for change - _configManipulator.SaveConnectionString(connectionString, providerName); + _configManipulator.SaveConnectionStringAsync(connectionString, providerName).GetAwaiter().GetResult(); if (!isChanged.WaitOne(10_000)) { throw new InstallException("Didn't retrieve updated connection string within 10 seconds, try manual configuration instead."); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/SiteIdentifierServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/SiteIdentifierServiceTests.cs index 4f47588655..50b779cb0c 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/SiteIdentifierServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/SiteIdentifierServiceTests.cs @@ -55,13 +55,13 @@ public class SiteIdentifierServiceTests if (shouldCreate) { - configManipulatorMock.Verify(x => x.SetGlobalId(It.IsAny()), Times.Once); + configManipulatorMock.Verify(x => x.SetGlobalIdAsync(It.IsAny()), Times.Once); Assert.AreNotEqual(Guid.Empty, identifier); Assert.IsTrue(result); } else { - configManipulatorMock.Verify(x => x.SetGlobalId(It.IsAny()), Times.Never()); + configManipulatorMock.Verify(x => x.SetGlobalIdAsync(It.IsAny()), Times.Never()); Assert.AreEqual(guidString.ToLower(), identifier.ToString()); Assert.IsTrue(result); }