From a0ce44c9fc0fbfb578945f2f0f037270b4614e75 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 31 Oct 2020 22:49:12 +0100 Subject: [PATCH] Changed configuration of first run time for health check notifier from a time string to a cron expression. --- .../Validation/ConfigurationValidatorBase.cs | 6 +-- .../HealthChecksSettingsValidator.cs | 2 +- src/Umbraco.Core/DateTimeExtensions.cs | 37 --------------- src/Umbraco.Core/StringExtensions.cs | 15 ++++-- .../HealthCheckSettingsExtensions.cs | 12 +++-- .../HostedServices/HealthCheckNotifier.cs | 2 +- .../Umbraco.Infrastructure.csproj | 1 + .../HealthCheckSettingsExtensionsTests.cs | 22 +++++---- .../Umbraco.Core/DateTimeExtensionsTests.cs | 46 ------------------- .../StringExtensionsTests.cs | 11 ++--- 10 files changed, 42 insertions(+), 112 deletions(-) rename src/{Umbraco.Core/Configuration/Models => Umbraco.Infrastructure/Configuration}/Extensions/HealthCheckSettingsExtensions.cs (54%) delete mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Core/DateTimeExtensionsTests.cs diff --git a/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs b/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs index 9b7f02a60c..343ba5656b 100644 --- a/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs +++ b/src/Umbraco.Core/Configuration/Models/Validation/ConfigurationValidatorBase.cs @@ -42,11 +42,11 @@ namespace Umbraco.Core.Configuration.Models.Validation return true; } - public bool ValidateOptionalTime(string configPath, string value, out string message) + public bool ValidateOptionalCronTab(string configPath, string value, out string message) { - if (!string.IsNullOrEmpty(value) && !value.IsValidTimeSpan()) + if (!string.IsNullOrEmpty(value) && !value.IsValidCronTab()) { - message = $"Configuration entry {configPath} contains an invalid time value."; + message = $"Configuration entry {configPath} contains an invalid cron expression."; return false; } diff --git a/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs b/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs index fe6a4d056b..9ecb6fb118 100644 --- a/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs +++ b/src/Umbraco.Core/Configuration/Models/Validation/HealthChecksSettingsValidator.cs @@ -16,7 +16,7 @@ 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); } } } 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..0cf8158518 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -1480,18 +1480,23 @@ namespace Umbraco.Core } /// - /// Validates a string matches a time stamp. + /// Validates a string matches a cron tab (for length only). /// - /// 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) + /// 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; } - return TimeSpan.TryParse(input, out var _); + var parts = input.Split(new string[] { " " }, StringSplitOptions.RemoveEmptyEntries); + return parts.Length == 5 || parts.Length == 6; } } } diff --git a/src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs similarity index 54% rename from src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs rename to src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs index 3fa37a0b19..57ae58928d 100644 --- a/src/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensions.cs +++ b/src/Umbraco.Infrastructure/Configuration/Extensions/HealthCheckSettingsExtensions.cs @@ -1,6 +1,8 @@ using System; +using NCrontab; +using Umbraco.Core.Configuration.Models; -namespace Umbraco.Core.Configuration.Models.Extensions +namespace Umbraco.Infrastructure.Configuration.Extensions { public static class HealthCheckSettingsExtensions { @@ -14,9 +16,11 @@ 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 firstRunTimeCronExpression = CrontabSchedule.Parse(firstRunTime); + var firstRunOccurance = firstRunTimeCronExpression.GetNextOccurrence(now); + var delay = firstRunOccurance - now; + return delay < defaultDelay ? defaultDelay : delay; } diff --git a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs index af0a0f335c..f4cf33fa61 100644 --- a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs +++ b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs @@ -4,11 +4,11 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Core; 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; diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index 2343ea806a..2f681efde3 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -20,6 +20,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..d1819fd83e 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,30 @@ using System; using NUnit.Framework; 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() + [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 now = new DateTime(2020, 10, 31, 12, 0, 0); var result = settings.GetNotificationDelay(now, TimeSpan.Zero); - Assert.AreEqual(30, result.Minutes); + Assert.AreEqual(expectedDelayInMinutes, result.TotalMinutes); } [Test] @@ -30,12 +34,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 now = new DateTime(2020, 10, 31, 12, 25, 0); var result = settings.GetNotificationDelay(now, TimeSpan.FromMinutes(10)); - Assert.AreEqual(10, result.Minutes); + Assert.AreEqual(10, result.TotalMinutes); } } } 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..427ae532f5 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs @@ -302,13 +302,12 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper } [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) + [TestCase("* * * * 1", true)] + [TestCase("* * * * * 1", true)] + [TestCase("* * * 1", false)] + public void IsValidCronTab(string input, bool expected) { - var result = input.IsValidTimeSpan(); + var result = input.IsValidCronTab(); Assert.AreEqual(expected, result); } }