diff --git a/src/Umbraco.Core/Configuration/ICronTabParser.cs b/src/Umbraco.Core/Configuration/ICronTabParser.cs new file mode 100644 index 0000000000..2238be4a4c --- /dev/null +++ b/src/Umbraco.Core/Configuration/ICronTabParser.cs @@ -0,0 +1,10 @@ +using System; + +namespace Umbraco.Core.Configuration +{ + public interface ICronTabParser + { + bool IsValidCronTab(string cronTab); + DateTime GetNextOccurrence(string cronTab, DateTime time); + } +} diff --git a/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs b/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs index 343ba5656b..6c0af9802b 100644 --- a/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs +++ b/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs @@ -42,17 +42,7 @@ namespace Umbraco.Core.Configuration.Models.Validation return true; } - public bool ValidateOptionalCronTab(string configPath, string value, out string message) - { - if (!string.IsNullOrEmpty(value) && !value.IsValidCronTab()) - { - message = $"Configuration entry {configPath} contains an invalid cron expression."; - return false; - } - message = string.Empty; - return true; - } } } diff --git a/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs b/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs index 9ecb6fb118..f26fce9bd9 100644 --- a/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs +++ b/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs @@ -4,6 +4,13 @@ namespace Umbraco.Core.Configuration.Models.Validation { public class HealthChecksSettingsValidator : ConfigurationValidatorBase, IValidateOptions { + private readonly ICronTabParser _cronTabParser; + + public HealthChecksSettingsValidator(ICronTabParser cronTabParser) + { + _cronTabParser = cronTabParser; + } + public ValidateOptionsResult Validate(string name, HealthChecksSettings options) { if (!ValidateNotificationFirstRunTime(options.Notification.FirstRunTime, out var message)) @@ -18,5 +25,17 @@ namespace Umbraco.Core.Configuration.Models.Validation { return ValidateOptionalCronTab($"{Constants.Configuration.ConfigHealthChecks}:{nameof(HealthChecksSettings.Notification)}:{nameof(HealthChecksSettings.Notification.FirstRunTime)}", value, out message); } + + public bool ValidateOptionalCronTab(string configPath, string value, out string message) + { + if (!string.IsNullOrEmpty(value) && !_cronTabParser.IsValidCronTab(value)) + { + message = $"Configuration entry {configPath} contains an invalid cron expression."; + return false; + } + + message = string.Empty; + return true; + } } } diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index 0cf8158518..67b0b49a45 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -1478,25 +1478,5 @@ namespace Umbraco.Core { return shortStringHelper.CleanStringForSafeFileName(text, culture); } - - /// - /// Validates a string matches a cron tab (for length only). - /// - /// String with timespan representation (in cron tab format: https://github.com/atifaziz/NCrontab/wiki/Crontab-Expression) - /// True if string matches a valid cron expression, false if not. - /// - /// Considering an expression as valid if it's supported by https://github.com/atifaziz/NCrontab/wiki/Crontab-Expression, - /// so only 5 or 6 values are expected. - /// - public static bool IsValidCronTab(this string input) - { - if (string.IsNullOrEmpty(input)) - { - return false; - } - - var parts = input.Split(new string[] { " " }, StringSplitOptions.RemoveEmptyEntries); - return parts.Length == 5 || parts.Length == 6; - } } } diff --git a/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs index 57ae58928d..92c3608ac1 100644 --- a/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs +++ b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs @@ -1,12 +1,13 @@ using System; using NCrontab; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; namespace Umbraco.Infrastructure.Configuration.Extensions { public static class HealthCheckSettingsExtensions { - public static TimeSpan GetNotificationDelay(this HealthChecksSettings settings, DateTime now, TimeSpan defaultDelay) + public static TimeSpan GetNotificationDelay(this HealthChecksSettings settings, ICronTabParser cronTabParser, DateTime now, TimeSpan defaultDelay) { // If first run time not set, start with just small delay after application start. var firstRunTime = settings.Notification.FirstRunTime; @@ -17,8 +18,7 @@ namespace Umbraco.Infrastructure.Configuration.Extensions else { // Otherwise start at scheduled time according to cron expression, unless within the default delay period. - var firstRunTimeCronExpression = CrontabSchedule.Parse(firstRunTime); - var firstRunOccurance = firstRunTimeCronExpression.GetNextOccurrence(now); + var firstRunOccurance = cronTabParser.GetNextOccurrence(firstRunTime, now); var delay = firstRunOccurance - now; return delay < defaultDelay ? defaultDelay diff --git a/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs b/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs new file mode 100644 index 0000000000..5642c882e6 --- /dev/null +++ b/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs @@ -0,0 +1,24 @@ +using System; +using NCrontab; + +namespace Umbraco.Core.Configuration +{ + + public class NCronTabParser : ICronTabParser + { + public bool IsValidCronTab(string cronTab) + { + var result = CrontabSchedule.TryParse(cronTab); + + return !(result is null); + } + + public DateTime GetNextOccurrence(string cronTab, DateTime time) + { + var result = CrontabSchedule.Parse(cronTab); + + return result.GetNextOccurrence(time); + } + } + +} diff --git a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs index c3cd2864dc..58decb80c4 100644 --- a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs +++ b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Core; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.HealthCheck; using Umbraco.Core.Logging; @@ -39,9 +40,10 @@ namespace Umbraco.Infrastructure.HostedServices IMainDom mainDom, IScopeProvider scopeProvider, ILogger logger, - IProfilingLogger profilingLogger) + IProfilingLogger profilingLogger, + ICronTabParser cronTabParser) : base(healthChecksSettings.Value.Notification.Period, - healthChecksSettings.Value.GetNotificationDelay(DateTime.Now, DefaultDelay)) + healthChecksSettings.Value.GetNotificationDelay(cronTabParser, DateTime.Now, DefaultDelay)) { _healthChecksSettings = healthChecksSettings.Value; _healthChecks = healthChecks; @@ -53,7 +55,7 @@ namespace Umbraco.Infrastructure.HostedServices _logger = logger; _profilingLogger = profilingLogger; } - + internal override async Task PerformExecuteAsync(object state) { if (_healthChecksSettings.Notification.Enabled == false) diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index 01bfb79e26..ca282fc6ab 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -208,7 +208,7 @@ namespace Umbraco.Core.Runtime // Config manipulator composition.Services.AddUnique(); - + // register the umbraco context factory // composition.Services.AddUnique(); composition.Services.AddUnique(); @@ -370,6 +370,8 @@ namespace Umbraco.Core.Runtime composition.Services.AddUnique(); composition.Services.AddUnique(); + + composition.Services.AddUnique(); } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs index d1819fd83e..833e5c865b 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs @@ -1,5 +1,6 @@ using System; using NUnit.Framework; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Infrastructure.Configuration.Extensions; @@ -8,6 +9,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Extensions [TestFixture] public class HealthCheckSettingsExtensionsTests { + private ICronTabParser CronTabParser => new NCronTabParser(); + [TestCase("30 12 * * *", 30)] [TestCase("15 18 * * *", 60 * 6 + 15)] [TestCase("0 3 * * *", 60 * 15)] @@ -23,7 +26,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Extensions } }; var now = new DateTime(2020, 10, 31, 12, 0, 0); - var result = settings.GetNotificationDelay(now, TimeSpan.Zero); + var result = settings.GetNotificationDelay(CronTabParser, now, TimeSpan.Zero); Assert.AreEqual(expectedDelayInMinutes, result.TotalMinutes); } @@ -38,7 +41,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Extensions } }; var now = new DateTime(2020, 10, 31, 12, 25, 0); - var result = settings.GetNotificationDelay(now, TimeSpan.FromMinutes(10)); + var result = settings.GetNotificationDelay(CronTabParser, now, TimeSpan.FromMinutes(10)); Assert.AreEqual(10, result.TotalMinutes); } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidatorTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidatorTests.cs index d38b41720e..64a8be79a8 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidatorTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidatorTests.cs @@ -1,5 +1,6 @@ using System; using NUnit.Framework; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.Configuration.Models.Validation; @@ -11,7 +12,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Validation [Test] public void Returns_Success_ForValid_Configuration() { - var validator = new HealthChecksSettingsValidator(); + var validator = new HealthChecksSettingsValidator(new NCronTabParser()); var options = BuildHealthChecksSettings(); var result = validator.Validate("settings", options); Assert.True(result.Succeeded); @@ -20,7 +21,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Validation [Test] public void Returns_Fail_For_Configuration_With_Invalid_Notification_FirstRunTime() { - var validator = new HealthChecksSettingsValidator(); + var validator = new HealthChecksSettingsValidator(new NCronTabParser()); var options = BuildHealthChecksSettings(firstRunTime: "0 3 *"); var result = validator.Validate("settings", options); Assert.False(result.Succeeded); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configurations/NCronTabParserTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configurations/NCronTabParserTests.cs new file mode 100644 index 0000000000..4a38de831d --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Configurations/NCronTabParserTests.cs @@ -0,0 +1,32 @@ +using NUnit.Framework; +using Umbraco.Core.Configuration; + +namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configurations +{ + [TestFixture] + public class NCronTabParserTests + { + private ICronTabParser Sut => new NCronTabParser(); + + [TestCase("", ExpectedResult = false)] + [TestCase("* * * * 1", ExpectedResult = true)] + [TestCase("* * * * * 1", ExpectedResult = false)] + [TestCase("* * * 1", ExpectedResult = false)] + [TestCase("Invalid", ExpectedResult = false)] + [TestCase("I n v a l", ExpectedResult = false)] + [TestCase("23 0-20/2 * * *", ExpectedResult = true)] + [TestCase("5 4 * * sun", ExpectedResult = true)] + [TestCase("0 0,12 1 */2 *", ExpectedResult = true)] + [TestCase("0 0 1,15 * 3", ExpectedResult = true)] + [TestCase("5 0 * 8 *", ExpectedResult = true)] + [TestCase("22 * * 1-5 *", ExpectedResult = true)] + [TestCase("23 0-20/2 * * *", ExpectedResult = true)] + [TestCase("23 0-20/2 * * sun-sat", ExpectedResult = true)] + [TestCase("23 0-20/2 * jan-dec sun-sat", ExpectedResult = true)] + [TestCase("* * 32 * *", ExpectedResult = false)] + public bool IsValidCronTab(string input) + { + return Sut.IsValidCronTab(input); + } + } +} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs index 427ae532f5..03c84a03b8 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs @@ -301,14 +301,5 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper Assert.AreEqual(expected, output); } - [TestCase("", false)] - [TestCase("* * * * 1", true)] - [TestCase("* * * * * 1", true)] - [TestCase("* * * 1", false)] - public void IsValidCronTab(string input, bool expected) - { - var result = input.IsValidCronTab(); - Assert.AreEqual(expected, result); - } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs index bbbb4035c5..6c1284854e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.HealthCheck; using Umbraco.Core.Logging; @@ -141,7 +142,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices return new HealthCheckNotifier(Options.Create(settings), checks, notifications, mockRunTimeState.Object, mockServerRegistrar.Object, mockMainDom.Object, mockScopeProvider.Object, - mockLogger.Object, mockProfilingLogger.Object); + mockLogger.Object, mockProfilingLogger.Object, Mock.Of()); } private void VerifyNotificationsNotSent()