From ce2da183041412e0802c6576f6b1def332b16973 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Mon, 14 Sep 2020 21:27:31 +0200 Subject: [PATCH] Fixed based on review comments Signed-off-by: Bjarke Berg --- .../HealthChecks/DisabledHealthCheck.cs | 11 +++ .../HealthChecks/IDisabledHealthCheck.cs | 11 --- .../Configuration/Models/ConnectionStrings.cs | 22 +++-- .../Configuration/Models/ContentErrorPage.cs | 15 ++++ .../Configuration/Models/ContentSettings.cs | 42 +-------- .../Models/HealthChecksSettings.cs | 86 +------------------ .../Models/ImagingCacheSettings.cs | 6 +- .../UmbracoSettings/IContentErrorPage.cs | 14 --- .../HealthCheck/HealthCheckResults.cs | 2 +- .../Routing/NotFoundHandlerHelper.cs | 7 +- .../UmbracoAuthorizedApiController.cs | 2 +- src/Umbraco.Web.UI.NetCore/appsettings.json | 4 +- .../Mvc/UmbracoViewPageOfTModel.cs | 2 + src/Umbraco.Web/PublishedContentExtensions.cs | 4 +- 14 files changed, 60 insertions(+), 168 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/HealthChecks/DisabledHealthCheck.cs delete mode 100644 src/Umbraco.Core/Configuration/HealthChecks/IDisabledHealthCheck.cs create mode 100644 src/Umbraco.Core/Configuration/Models/ContentErrorPage.cs delete mode 100644 src/Umbraco.Core/Configuration/UmbracoSettings/IContentErrorPage.cs diff --git a/src/Umbraco.Core/Configuration/HealthChecks/DisabledHealthCheck.cs b/src/Umbraco.Core/Configuration/HealthChecks/DisabledHealthCheck.cs new file mode 100644 index 0000000000..c962014bd5 --- /dev/null +++ b/src/Umbraco.Core/Configuration/HealthChecks/DisabledHealthCheck.cs @@ -0,0 +1,11 @@ +using System; + +namespace Umbraco.Core.Configuration.HealthChecks +{ + public class DisabledHealthCheck + { + public Guid Id { get; set; } + public DateTime DisabledOn { get; set; } + public int DisabledBy { get; set; } + } +} diff --git a/src/Umbraco.Core/Configuration/HealthChecks/IDisabledHealthCheck.cs b/src/Umbraco.Core/Configuration/HealthChecks/IDisabledHealthCheck.cs deleted file mode 100644 index 4ea63048f8..0000000000 --- a/src/Umbraco.Core/Configuration/HealthChecks/IDisabledHealthCheck.cs +++ /dev/null @@ -1,11 +0,0 @@ -using System; - -namespace Umbraco.Core.Configuration.HealthChecks -{ - public interface IDisabledHealthCheck - { - Guid Id { get; } - DateTime DisabledOn { get; } - int DisabledBy { get; } - } -} diff --git a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs index 056611a0ed..db7be894db 100644 --- a/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs +++ b/src/Umbraco.Core/Configuration/Models/ConnectionStrings.cs @@ -6,6 +6,8 @@ namespace Umbraco.Core.Configuration.Models { public class ConnectionStrings { + + // Backing field for UmbracoConnectionString to load from configuration value with key umbracoDbDSN. // Attributes cannot be applied to map from keys that don't match, and have chosen to retain the key name // used in configuration for older Umbraco versions. @@ -13,21 +15,27 @@ namespace Umbraco.Core.Configuration.Models private string umbracoDbDSN { get => string.Empty; - set => UmbracoConnectionString = value; + set + { + UmbracoConnectionString = value; + + ConnectionStringDictionary[Constants.System.UmbracoConnectionName] = value; + } } - public string UmbracoConnectionString { get; set; } + public string UmbracoConnectionString + { + get => ConnectionStringDictionary[Constants.System.UmbracoConnectionName]; + set => ConnectionStringDictionary[Constants.System.UmbracoConnectionName] = value; + } - private Dictionary AsDictionary() => new Dictionary - { - { Constants.System.UmbracoConnectionName, UmbracoConnectionString } - }; + private Dictionary ConnectionStringDictionary { get; } = new Dictionary(); public ConfigConnectionString this[string key] { get { - var connectionString = this.AsDictionary()[key]; + var connectionString = ConnectionStringDictionary[key]; var provider = ParseProvider(connectionString); return new ConfigConnectionString(connectionString, provider, key); } diff --git a/src/Umbraco.Core/Configuration/Models/ContentErrorPage.cs b/src/Umbraco.Core/Configuration/Models/ContentErrorPage.cs new file mode 100644 index 0000000000..e04362eafd --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/ContentErrorPage.cs @@ -0,0 +1,15 @@ +using System; + +namespace Umbraco.Core.Configuration.Models +{ + public class ContentErrorPage + { + //TODO introduce valiation, to check only one of key/id/xPath is used. + public int ContentId { get; } + public Guid ContentKey { get; } + public string ContentXPath { get; } + public bool HasContentId { get; } + public bool HasContentKey { get; } + public string Culture { get; set; } + } +} diff --git a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs index 31a97e6615..5158a5c746 100644 --- a/src/Umbraco.Core/Configuration/Models/ContentSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ContentSettings.cs @@ -16,15 +16,7 @@ namespace Umbraco.Core.Configuration.Models public bool ResolveUrlsFromTextString { get; set; } = false; - // TODO: to retain previous configuration structure, this should come from a nested collection, - // "Errors:Error404". Although we can use JsonPropertyName to map when the JSON field name differs - // from the one in this class, not sure how we'd "flatten" a collection like this. - public IEnumerable Error404Collection { get; set; } - - // public IEnumerable Error404Collection => _configuration - // .GetSection(Prefix + "Errors:Error404") - // .GetChildren() - // .Select(x => new ContentErrorPage(x)); + public IEnumerable Error404Collection { get; set; } = Array.Empty(); public string PreviewBadge { get; set; } = DefaultPreviewBadge; @@ -38,38 +30,6 @@ namespace Umbraco.Core.Configuration.Models public string LoginBackgroundImage { get; set; } = "assets/img/login.jpg"; - /* - private class ContentErrorPage : IContentErrorPage - { - public ContentErrorPage(IConfigurationSection configurationSection) - { - Culture = configurationSection.Key; - var value = configurationSection.Value; - - if (int.TryParse(value, out var contentId)) - { - HasContentId = true; - ContentId = contentId; - } - else if (Guid.TryParse(value, out var contentKey)) - { - HasContentKey = true; - ContentKey = contentKey; - } - else - { - ContentXPath = value; - } - } - - public int ContentId { get; } - public Guid ContentKey { get; } - public string ContentXPath { get; } - public bool HasContentId { get; } - public bool HasContentKey { get; } - public string Culture { get; set; } - } - */ } } diff --git a/src/Umbraco.Core/Configuration/Models/HealthChecksSettings.cs b/src/Umbraco.Core/Configuration/Models/HealthChecksSettings.cs index 37580195d6..0903a8f242 100644 --- a/src/Umbraco.Core/Configuration/Models/HealthChecksSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/HealthChecksSettings.cs @@ -6,37 +6,10 @@ namespace Umbraco.Core.Configuration.Models { public class HealthChecksSettings { - // TODO: implement - public IEnumerable DisabledChecks { get; set; } = Enumerable.Empty(); + public IEnumerable DisabledChecks { get; set; } = Enumerable.Empty(); public HealthCheckNotificationSettings NotificationSettings { get; set; } = new HealthCheckNotificationSettings(); - /* - public IEnumerable DisabledChecks => _configuration - .GetSection(Prefix+"DisabledChecks") - .GetChildren() - .Select( - x => new DisabledHealthCheck - { - Id = x.GetValue("Id"), - DisabledOn = x.GetValue("DisabledOn"), - DisabledBy = x.GetValue("DisabledBy") - }); - - public IHealthCheckNotificationSettings NotificationSettings => - new HealthCheckNotificationSettings( - _configuration.GetSection(Prefix+"NotificationSettings")); - - - private class DisabledHealthCheck : IDisabledHealthCheck - { - public Guid Id { get; set; } - public DateTime DisabledOn { get; set; } - public int DisabledBy { get; set; } - } - */ - - // TODO: move to new file public class HealthCheckNotificationSettings { public bool Enabled { get; set; } = false; @@ -45,64 +18,9 @@ namespace Umbraco.Core.Configuration.Models public int PeriodInHours { get; set; } = 24; - // TODO: implement - public IReadOnlyDictionary NotificationMethods { get; set; } = new Dictionary(); - public IEnumerable DisabledChecks { get; set; } = Enumerable.Empty(); - - /* - public IReadOnlyDictionary NotificationMethods => _configurationSection - .GetSection("NotificationMethods") - .GetChildren() - .ToDictionary(x => x.Key, x => (INotificationMethod) new NotificationMethod(x.Key, x), StringComparer.InvariantCultureIgnoreCase); - - public IEnumerable DisabledChecks => _configurationSection - .GetSection("DisabledChecks").GetChildren().Select( - x => new DisabledHealthCheck - { - Id = x.GetValue("Id"), - DisabledOn = x.GetValue("DisabledOn"), - DisabledBy = x.GetValue("DisabledBy") - }); - */ + public IEnumerable DisabledChecks { get; set; } = Enumerable.Empty(); } - - /* - private class NotificationMethod : INotificationMethod - { - private readonly IConfigurationSection _configurationSection; - - public NotificationMethod(string alias, IConfigurationSection configurationSection) - { - Alias = alias; - _configurationSection = configurationSection; - } - - public string Alias { get; } - public bool Enabled => _configurationSection.GetValue("Enabled", false); - - public HealthCheckNotificationVerbosity Verbosity => - _configurationSection.GetValue("Verbosity", HealthCheckNotificationVerbosity.Summary); - - public bool FailureOnly => _configurationSection.GetValue("FailureOnly", true); - - public IReadOnlyDictionary Settings => _configurationSection - .GetSection("Settings").GetChildren().ToDictionary(x => x.Key, - x => (INotificationMethodSettings) new NotificationMethodSettings(x.Key, x.Value), StringComparer.InvariantCultureIgnoreCase); - } - - private class NotificationMethodSettings : INotificationMethodSettings - { - public NotificationMethodSettings(string key, string value) - { - Key = key; - Value = value; - } - - public string Key { get; } - public string Value { get; } - } - */ } } diff --git a/src/Umbraco.Core/Configuration/Models/ImagingCacheSettings.cs b/src/Umbraco.Core/Configuration/Models/ImagingCacheSettings.cs index 9ab3386129..d2395b6da9 100644 --- a/src/Umbraco.Core/Configuration/Models/ImagingCacheSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ImagingCacheSettings.cs @@ -1,4 +1,6 @@ -namespace Umbraco.Core.Configuration.Models +using System.IO; + +namespace Umbraco.Core.Configuration.Models { public class ImagingCacheSettings { @@ -8,6 +10,6 @@ public uint CachedNameLength { get; set; } = 8; - public string CacheFolder { get; set; } = "../App_Data/Cache"; + public string CacheFolder { get; set; } = Path.Combine("~", "Umbraco", "Cache"); } } diff --git a/src/Umbraco.Core/Configuration/UmbracoSettings/IContentErrorPage.cs b/src/Umbraco.Core/Configuration/UmbracoSettings/IContentErrorPage.cs deleted file mode 100644 index 3ea00d7c74..0000000000 --- a/src/Umbraco.Core/Configuration/UmbracoSettings/IContentErrorPage.cs +++ /dev/null @@ -1,14 +0,0 @@ -using System; - -namespace Umbraco.Core.Configuration.UmbracoSettings -{ - public interface IContentErrorPage - { - int ContentId { get; } - Guid ContentKey { get; } - string ContentXPath { get; } - bool HasContentId { get; } - bool HasContentKey { get; } - string Culture { get; set; } - } -} diff --git a/src/Umbraco.Infrastructure/HealthCheck/HealthCheckResults.cs b/src/Umbraco.Infrastructure/HealthCheck/HealthCheckResults.cs index 0b27b34875..8f3c05f5bd 100644 --- a/src/Umbraco.Infrastructure/HealthCheck/HealthCheckResults.cs +++ b/src/Umbraco.Infrastructure/HealthCheck/HealthCheckResults.cs @@ -71,7 +71,7 @@ namespace Umbraco.Web.HealthCheck foreach (var checkResult in checkResults) { - Logger.Info("'Result for {HealthCheckName}: {HealthCheckResult}, Message: '{HealthCheckMessage}'", checkName, checkResult.ResultType, checkResult.Message); + Logger.Info("Result for {HealthCheckName}: {HealthCheckResult}, Message: '{HealthCheckMessage}'", checkName, checkResult.ResultType, checkResult.Message); } } } diff --git a/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs b/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs index 335e1f868a..20fbe9a1ef 100644 --- a/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs +++ b/src/Umbraco.Infrastructure/Routing/NotFoundHandlerHelper.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Linq; using Umbraco.Composing; using Umbraco.Core; +using Umbraco.Core.Configuration.Models; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -28,7 +29,7 @@ namespace Umbraco.Web.Routing /// /// internal static int? GetCurrentNotFoundPageId( - IContentErrorPage[] error404Collection, + ContentErrorPage[] error404Collection, string requestServerName, IEntityService entityService, IPublishedContentQuery publishedContentQuery, @@ -38,7 +39,7 @@ namespace Umbraco.Web.Routing } internal static int? GetCurrentNotFoundPageId( - IContentErrorPage[] error404Collection, + ContentErrorPage[] error404Collection, IEntityService entityService, IPublishedContentQuery publishedContentQuery, CultureInfo errorCulture) @@ -67,7 +68,7 @@ namespace Umbraco.Web.Routing /// /// /// - internal static int? GetContentIdFromErrorPageConfig(IContentErrorPage errorPage, IEntityService entityService, IPublishedContentQuery publishedContentQuery) + internal static int? GetContentIdFromErrorPageConfig(ContentErrorPage errorPage, IEntityService entityService, IPublishedContentQuery publishedContentQuery) { if (errorPage.HasContentId) return errorPage.ContentId; diff --git a/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedApiController.cs b/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedApiController.cs index 85c92d1139..c3e1a71b86 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedApiController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedApiController.cs @@ -18,7 +18,7 @@ namespace Umbraco.Web.BackOffice.Controllers [UmbracoAuthorize] [DisableBrowserCache] [UmbracoWebApiRequireHttps] - [CheckIfUserTicketDataIsStale] + //[CheckIfUserTicketDataIsStale] //TODO reintroduce //[UnhandedExceptionLoggerConfiguration] //TODO reintroduce //[EnableDetailedErrors] //TODO reintroduce public abstract class UmbracoAuthorizedApiController : UmbracoApiController diff --git a/src/Umbraco.Web.UI.NetCore/appsettings.json b/src/Umbraco.Web.UI.NetCore/appsettings.json index f28d3bca6c..3bbb4e7da4 100644 --- a/src/Umbraco.Web.UI.NetCore/appsettings.json +++ b/src/Umbraco.Web.UI.NetCore/appsettings.json @@ -1,6 +1,6 @@ { "ConnectionStrings": { - "umbracoDbDSN": "Server=(LocalDB)\\Umbraco;Database=NetCore;Integrated Security=true" + "umbracoDbDSN": "" }, "Umbraco": { "CMS": { @@ -61,4 +61,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs b/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs index 4a651fd04a..f461fb9a22 100644 --- a/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs +++ b/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs @@ -115,6 +115,8 @@ namespace Umbraco.Web.Mvc protected UmbracoViewPage(ServiceContext services, AppCaches appCaches, IOptions globalSettings, IOptions contentSettings) { + if (globalSettings == null) throw new ArgumentNullException(nameof(globalSettings)); + if (contentSettings == null) throw new ArgumentNullException(nameof(contentSettings)); Services = services; AppCaches = appCaches; _globalSettings = globalSettings.Value; diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index e132b8fcdd..487212840b 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -75,7 +75,7 @@ namespace Umbraco.Web Current.Services.ContentTypeService, /*Current.Configs.WebRouting().DisableAlternativeTemplates, Current.Configs.WebRouting().ValidateAlternativeTemplates, - TODO*/ + TODO get values from config*/ false, false, templateId); } @@ -87,7 +87,7 @@ namespace Umbraco.Web Current.Services.ContentTypeService, /*Current.Configs.WebRouting().DisableAlternativeTemplates, Current.Configs.WebRouting().ValidateAlternativeTemplates, - TODO*/ + TODO get values from config*/ false, false, templateAlias); }