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 9b7f02a60c..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 ValidateOptionalTime(string configPath, string value, out string message) - { - if (!string.IsNullOrEmpty(value) && !value.IsValidTimeSpan()) - { - message = $"Configuration entry {configPath} contains an invalid time value."; - 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 fe6a4d056b..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)) @@ -16,7 +23,19 @@ namespace Umbraco.Core.Configuration.Models.Validation private bool ValidateNotificationFirstRunTime(string value, out string message) { - return ValidateOptionalTime($"{Constants.Configuration.ConfigHealthChecks}:{nameof(HealthChecksSettings.Notification)}:{nameof(HealthChecksSettings.Notification.FirstRunTime)}", value, out message); + 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/DateTimeExtensions.cs b/src/Umbraco.Core/DateTimeExtensions.cs index 1920205b69..378d06a637 100644 --- a/src/Umbraco.Core/DateTimeExtensions.cs +++ b/src/Umbraco.Core/DateTimeExtensions.cs @@ -43,42 +43,5 @@ namespace Umbraco.Core Minute, Second } - - /// - /// Calculates the number of minutes from a date time, on a rolling daily basis (so if - /// date time is before the time, calculate onto next day). - /// - /// Date to start from - /// Time to compare against (in Hmm form, e.g. 330, 2200) - /// - public static int PeriodicMinutesFrom(this DateTime fromDateTime, string scheduledTime) - { - // Ensure time provided is 4 digits long - if (scheduledTime.Length == 3) - { - scheduledTime = "0" + scheduledTime; - } - - var scheduledHour = int.Parse(scheduledTime.Substring(0, 2)); - var scheduledMinute = int.Parse(scheduledTime.Substring(2)); - - DateTime scheduledDateTime; - if (IsScheduledInRemainingDay(fromDateTime, scheduledHour, scheduledMinute)) - { - scheduledDateTime = new DateTime(fromDateTime.Year, fromDateTime.Month, fromDateTime.Day, scheduledHour, scheduledMinute, 0); - } - else - { - var nextDay = fromDateTime.AddDays(1); - scheduledDateTime = new DateTime(nextDay.Year, nextDay.Month, nextDay.Day, scheduledHour, scheduledMinute, 0); - } - - return (int)(scheduledDateTime - fromDateTime).TotalMinutes; - } - - private static bool IsScheduledInRemainingDay(DateTime fromDateTime, int scheduledHour, int scheduledMinute) - { - return scheduledHour > fromDateTime.Hour || (scheduledHour == fromDateTime.Hour && scheduledMinute >= fromDateTime.Minute); - } } } diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index 93d664a2bd..67b0b49a45 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -1478,20 +1478,5 @@ namespace Umbraco.Core { return shortStringHelper.CleanStringForSafeFileName(text, culture); } - - /// - /// Validates a string matches a time stamp. - /// - /// String with timespan representation (in standard timespan format: https://docs.microsoft.com/en-us/dotnet/standard/base-types/standard-timespan-format-strings) - /// - public static bool IsValidTimeSpan(this string input) - { - if (string.IsNullOrEmpty(input)) - { - return false; - } - - return TimeSpan.TryParse(input, out var _); - } } } diff --git a/src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs similarity index 50% rename from src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs rename to src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs index 3fa37a0b19..92c3608ac1 100644 --- a/src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs +++ b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs @@ -1,10 +1,13 @@ using System; +using NCrontab; +using Umbraco.Core.Configuration; +using Umbraco.Core.Configuration.Models; -namespace Umbraco.Core.Configuration.Models.Extensions +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; @@ -14,9 +17,10 @@ namespace Umbraco.Core.Configuration.Models.Extensions } else { - // Otherwise start at scheduled time. - var delay = TimeSpan.FromMinutes(now.PeriodicMinutesFrom(firstRunTime)); - return (delay < defaultDelay) + // Otherwise start at scheduled time according to cron expression, unless within the default delay period. + var firstRunOccurance = cronTabParser.GetNextOccurrence(firstRunTime, now); + var delay = firstRunOccurance - now; + return delay < defaultDelay ? defaultDelay : delay; } 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 41de4d4cf3..58decb80c4 100644 --- a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs +++ b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs @@ -4,12 +4,13 @@ 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.Configuration.Models.Extensions; using Umbraco.Core.HealthCheck; using Umbraco.Core.Logging; using Umbraco.Core.Scoping; using Umbraco.Core.Sync; +using Umbraco.Infrastructure.Configuration.Extensions; using Umbraco.Infrastructure.HealthCheck; using Umbraco.Web.HealthCheck; @@ -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; diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index c6745103b0..081c1b4d10 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -370,6 +370,8 @@ namespace Umbraco.Core.Runtime composition.Services.AddUnique(); composition.Services.AddUnique(); + + composition.Services.AddUnique(); } } } diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index 54ac7817e1..b9368da89b 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -21,6 +21,7 @@ + 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 3ec9af0b5a..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,26 +1,33 @@ using System; using NUnit.Framework; +using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; -using Umbraco.Core.Configuration.Models.Extensions; +using Umbraco.Infrastructure.Configuration.Extensions; namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Extensions { [TestFixture] public class HealthCheckSettingsExtensionsTests { - [Test] - public void Returns_Notification_Delay_From_Provided_Time() + private ICronTabParser CronTabParser => new NCronTabParser(); + + [TestCase("30 12 * * *", 30)] + [TestCase("15 18 * * *", 60 * 6 + 15)] + [TestCase("0 3 * * *", 60 * 15)] + [TestCase("0 3 2 * *", 24 * 60 * 1 + 60 * 15)] + [TestCase("0 6 * * 3", 24 * 60 * 3 + 60 * 18)] + public void Returns_Notification_Delay_From_Provided_Time(string firstRunTimeCronExpression, int expectedDelayInMinutes) { var settings = new HealthChecksSettings { Notification = new HealthChecksNotificationSettings { - FirstRunTime = "1230", + FirstRunTime = firstRunTimeCronExpression, } }; - var now = DateTime.Now.Date.AddHours(12); - var result = settings.GetNotificationDelay(now, TimeSpan.Zero); - Assert.AreEqual(30, result.Minutes); + var now = new DateTime(2020, 10, 31, 12, 0, 0); + var result = settings.GetNotificationDelay(CronTabParser, now, TimeSpan.Zero); + Assert.AreEqual(expectedDelayInMinutes, result.TotalMinutes); } [Test] @@ -30,12 +37,12 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.Configuration.Models.Extensions { Notification = new HealthChecksNotificationSettings { - FirstRunTime = "1230", + FirstRunTime = "30 12 * * *", } }; - var now = DateTime.Now.Date.AddHours(12).AddMinutes(25); - var result = settings.GetNotificationDelay(now, TimeSpan.FromMinutes(10)); - Assert.AreEqual(10, result.Minutes); + var now = new DateTime(2020, 10, 31, 12, 25, 0); + 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 e6ca06a34b..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,13 +21,13 @@ 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 options = BuildHealthChecksSettings(firstRunTime: "25:00"); + var validator = new HealthChecksSettingsValidator(new NCronTabParser()); + var options = BuildHealthChecksSettings(firstRunTime: "0 3 *"); var result = validator.Validate("settings", options); Assert.False(result.Succeeded); } - private static HealthChecksSettings BuildHealthChecksSettings(string firstRunTime = "12:00") + private static HealthChecksSettings BuildHealthChecksSettings(string firstRunTime = "0 3 * * *") { return new HealthChecksSettings { 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/DateTimeExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/DateTimeExtensionsTests.cs deleted file mode 100644 index b1585aa17f..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/DateTimeExtensionsTests.cs +++ /dev/null @@ -1,46 +0,0 @@ -using System; -using NUnit.Framework; -using Umbraco.Core; - -namespace Umbraco.Tests.UnitTests.Umbraco.Core -{ - [TestFixture] - public class DateTimeExtensionsTests - { - [Test] - public void PeriodicMinutesFrom_PostTime_CalculatesMinutesBetween() - { - var nowDateTime = new DateTime(2017, 1, 1, 10, 30, 0); - var scheduledTime = "1145"; - var minutesBetween = nowDateTime.PeriodicMinutesFrom(scheduledTime); - Assert.AreEqual(75, minutesBetween); - } - - [Test] - public void PeriodicMinutesFrom_PriorTime_CalculatesMinutesBetween() - { - var nowDateTime = new DateTime(2017, 1, 1, 10, 30, 0); - var scheduledTime = "900"; - var minutesBetween = nowDateTime.PeriodicMinutesFrom(scheduledTime); - Assert.AreEqual(1350, minutesBetween); - } - - [Test] - public void PeriodicMinutesFrom_PriorTime_WithLeadingZero_CalculatesMinutesBetween() - { - var nowDateTime = new DateTime(2017, 1, 1, 10, 30, 0); - var scheduledTime = "0900"; - var minutesBetween = nowDateTime.PeriodicMinutesFrom(scheduledTime); - Assert.AreEqual(1350, minutesBetween); - } - - [Test] - public void PeriodicMinutesFrom_SameTime_CalculatesMinutesBetween() - { - var nowDateTime = new DateTime(2017, 1, 1, 10, 30, 0); - var scheduledTime = "1030"; - var minutesBetween = nowDateTime.PeriodicMinutesFrom(scheduledTime); - Assert.AreEqual(0, minutesBetween); - } - } -} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs index 1644b9049e..03c84a03b8 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs @@ -301,15 +301,5 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper Assert.AreEqual(expected, output); } - [TestCase("", false)] - [TestCase("12:34", true)] - [TestCase("1:14:23", true)] - [TestCase("25:03", false)] - [TestCase("18:61", false)] - public void IsValidTimeSpan(string input, bool expected) - { - var result = input.IsValidTimeSpan(); - 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()