From 98c9124735be46da607c9abf3400fe2965ca304f Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Fri, 12 Aug 2022 10:14:15 +0100 Subject: [PATCH] Refactored logic for calculating the first run time for a recurring hosted service. (#12828) * Refactors the logic for calculating the first run time for a recurring task for easier re-use by similar hostservices. * Renamed method to match wider usage. --- .../HealthCheckSettingsExtensions.cs | 3 + .../Configuration/NCronTabParser.cs | 5 ++ .../HostedServices/HealthCheckNotifier.cs | 2 +- .../RecurringHostedServiceBase.cs | 55 ++++++++++++++++ .../HealthCheckSettingsExtensionsTests.cs | 44 ------------- .../RecurringHostedServiceBaseTests.cs | 62 +++++++++++++++++++ 6 files changed, 126 insertions(+), 45 deletions(-) delete mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBaseTests.cs diff --git a/src/Umbraco.Core/Extensions/HealthCheckSettingsExtensions.cs b/src/Umbraco.Core/Extensions/HealthCheckSettingsExtensions.cs index bbf8c67db5..a029963805 100644 --- a/src/Umbraco.Core/Extensions/HealthCheckSettingsExtensions.cs +++ b/src/Umbraco.Core/Extensions/HealthCheckSettingsExtensions.cs @@ -3,6 +3,9 @@ using Umbraco.Cms.Core.Configuration.Models; namespace Umbraco.Extensions; +// TODO (V12): Remove this class that's no longer used. + +[Obsolete("Please use RecurringHostedServiceBase.GetDelay(). This class is no longer used within Umbraco and will be removed in V12.")] public static class HealthCheckSettingsExtensions { public static TimeSpan GetNotificationDelay(this HealthChecksSettings settings, ICronTabParser cronTabParser, DateTime now, TimeSpan defaultDelay) diff --git a/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs b/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs index 09cd2282da..5feee83f23 100644 --- a/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs +++ b/src/Umbraco.Infrastructure/Configuration/NCronTabParser.cs @@ -2,8 +2,12 @@ using NCrontab; namespace Umbraco.Cms.Core.Configuration; +/// +/// Implements using the NCrontab library +/// public class NCronTabParser : ICronTabParser { + /// public bool IsValidCronTab(string cronTab) { var result = CrontabSchedule.TryParse(cronTab); @@ -11,6 +15,7 @@ public class NCronTabParser : ICronTabParser return !(result is null); } + /// public DateTime GetNextOccurrence(string cronTab, DateTime time) { var result = CrontabSchedule.Parse(cronTab); diff --git a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs index 47e72cc8c5..30d164276a 100644 --- a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs +++ b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs @@ -59,7 +59,7 @@ public class HealthCheckNotifier : RecurringHostedServiceBase : base( logger, healthChecksSettings.CurrentValue.Notification.Period, - healthChecksSettings.CurrentValue.GetNotificationDelay(cronTabParser, DateTime.Now, DefaultDelay)) + GetDelay(healthChecksSettings.CurrentValue.Notification.FirstRunTime, cronTabParser, logger, DefaultDelay)) { _healthChecksSettings = healthChecksSettings.CurrentValue; _healthChecks = healthChecks; diff --git a/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs b/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs index ab807b5f97..68b311f8cd 100644 --- a/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs +++ b/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Configuration; namespace Umbraco.Cms.Infrastructure.HostedServices; @@ -58,6 +59,60 @@ public abstract class RecurringHostedServiceBase : IHostedService, IDisposable GC.SuppressFinalize(this); } + /// + /// Determines the delay before the first run of a recurring task implemented as a hosted service when an optonal + /// configuration for the first run time is available. + /// + /// The configured time to first run the task in crontab format. + /// An instance of + /// The logger. + /// The default delay to use when a first run time is not configured. + /// The delay before first running the recurring task. + protected static TimeSpan GetDelay( + string firstRunTime, + ICronTabParser cronTabParser, + ILogger logger, + TimeSpan defaultDelay) => GetDelay(firstRunTime, cronTabParser, logger, DateTime.Now, defaultDelay); + + /// + /// Determines the delay before the first run of a recurring task implemented as a hosted service when an optonal + /// configuration for the first run time is available. + /// + /// The configured time to first run the task in crontab format. + /// An instance of + /// The logger. + /// The current datetime. + /// The default delay to use when a first run time is not configured. + /// The delay before first running the recurring task. + /// Internal to expose for unit tests. + internal static TimeSpan GetDelay( + string firstRunTime, + ICronTabParser cronTabParser, + ILogger logger, + DateTime now, + TimeSpan defaultDelay) + { + // If first run time not set, start with just small delay after application start. + if (string.IsNullOrEmpty(firstRunTime)) + { + return defaultDelay; + } + + // If first run time not a valid cron tab, log, and revert to small delay after application start. + if (!cronTabParser.IsValidCronTab(firstRunTime)) + { + logger.LogWarning("Could not parse {FirstRunTime} as a crontab expression. Defaulting to default delay for hosted service start.", firstRunTime); + return defaultDelay; + } + + // Otherwise start at scheduled time according to cron expression, unless within the default delay period. + DateTime firstRunOccurance = cronTabParser.GetNextOccurrence(firstRunTime, now); + TimeSpan delay = firstRunOccurance - now; + return delay < defaultDelay + ? defaultDelay + : delay; + } + /// public Task StartAsync(CancellationToken cancellationToken) { diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs deleted file mode 100644 index f750fb4929..0000000000 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Configuration/Models/Extensions/HealthCheckSettingsExtensionsTests.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System; -using NUnit.Framework; -using Umbraco.Cms.Core.Configuration; -using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Extensions; - -namespace Umbraco.Cms.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)] - [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 = firstRunTimeCronExpression }, - }; - var now = new DateTime(2020, 10, 31, 12, 0, 0); - var result = settings.GetNotificationDelay(CronTabParser, now, TimeSpan.Zero); - Assert.AreEqual(expectedDelayInMinutes, result.TotalMinutes); - } - - [Test] - public void Returns_Notification_Delay_From_Default_When_Provided_Time_Too_Close_To_Current_Time() - { - var settings = new HealthChecksSettings - { - Notification = new HealthChecksNotificationSettings { FirstRunTime = "30 12 * * *" }, - }; - 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/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBaseTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBaseTests.cs new file mode 100644 index 0000000000..ae260d09f8 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBaseTests.cs @@ -0,0 +1,62 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; +using Microsoft.Extensions.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Infrastructure.HostedServices; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices; + +[TestFixture] +public class RecurringHostedServiceBaseTests +{ + [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 firstRunTime, int expectedDelayInMinutes) + { + var cronTabParser = new NCronTabParser(); + var logger = Mock.Of(); + var now = new DateTime(2020, 10, 31, 12, 0, 0); + var result = RecurringHostedServiceBase.GetDelay(firstRunTime, cronTabParser, logger, now, TimeSpan.Zero); + Assert.AreEqual(expectedDelayInMinutes, result.TotalMinutes); + } + + [Test] + public void Returns_Notification_Delay_From_Default_When_Provided_Time_Too_Close_To_Current_Time() + { + var firstRunTime = "30 12 * * *"; + var cronTabParser = new NCronTabParser(); + var logger = Mock.Of(); + var now = new DateTime(2020, 10, 31, 12, 25, 0); + var defaultDelay = TimeSpan.FromMinutes(10); + var result = RecurringHostedServiceBase.GetDelay(firstRunTime, cronTabParser, logger, now, defaultDelay); + Assert.AreEqual(defaultDelay.TotalMinutes, result.TotalMinutes); + } + + [Test] + public void Logs_And_Returns_Notification_Delay_From_Default_When_Provided_Time_Is_Not_Valid() + { + var firstRunTime = "invalid"; + var cronTabParser = new NCronTabParser(); + var logger = new Mock(); + var now = new DateTime(2020, 10, 31, 12, 25, 0); + var defaultDelay = TimeSpan.FromMinutes(10); + var result = RecurringHostedServiceBase.GetDelay(firstRunTime, cronTabParser, logger.Object, now, defaultDelay); + Assert.AreEqual(defaultDelay, result); + + logger.Verify( + logger => logger.Log( + It.Is(y => y == LogLevel.Warning), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once); + } +}