Fix CronExpression not always returning correct next schedule for weekdayonly and month flags. Relates to #2331

src/Quartz.Tests.Integration/Impl/SmokeTestPerformer.cs

@@ -11,7 +11,6 @@
 using Quartz.Job;
 using Quartz.Spi;
 using Quartz.Tests.Integration.Impl.AdoJobStore;
-using Quartz.Tests.Integration.Utils;
 using Quartz.Util;
 
 namespace Quartz.Tests.Integration.Impl;

src/Quartz.Tests.Unit/CronExpressionTest.cs

@@ -650,8 +650,9 @@ public void TestGetTimeAfter_QRTZNET149()
     public void TestQRTZNET152_Nearest_Weekday_Expression_W_Does_Not_Work_In_CronTrigger()
     {
         CronExpression expression = new CronExpression("0 5 13 5W 1-12 ?");
-        DateTimeOffset test = new DateTimeOffset(2009, 3, 8, 0, 0, 0, TimeSpan.Zero);
+        DateTimeOffset test = new DateTimeOffset(2009, 3, 8, 0, 0, 0, TimeSpan.Zero); //Sunday
         DateTimeOffset d = expression.GetNextValidTimeAfter(test).Value;
+        // 2009-04-06 is Monday, Sunday is invalid for W
         Assert.AreEqual(new DateTimeOffset(2009, 4, 6, 13, 5, 0, TimeZoneUtil.GetUtcOffset(d, TimeZoneInfo.Local)).ToUniversalTime(), d);
         d = expression.GetNextValidTimeAfter(d).Value;
         Assert.AreEqual(new DateTimeOffset(2009, 5, 5, 13, 5, 0, TimeZoneUtil.GetUtcOffset(d, TimeZoneInfo.Local)), d);

src/Quartz.Tests.Unit/Impl/Calendar/CronCalendarTest.cs

@@ -19,6 +19,10 @@
 
 #endregion
 
+using System.Collections;
+
+using FluentAssertions;
+
 using NUnit.Framework;
 
 using Quartz.Impl.Calendar;
@@ -35,6 +39,13 @@ public CronCalendarTest(Type serializerType) : base(serializerType)
     {
     }
 
+    [TestCaseSource(typeof(CronWeekdayModifierTestData), nameof(CronWeekdayModifierTestData.TestCases))]
+    public void CronWeekdayModifierReturnsNextExpectedFireTimeSet(CronExpression cronExpression, DateTimeOffset timeAfterDate, DateTimeOffset expectedNextFireTime)
+    {
+        var nextFireTime = cronExpression.GetTimeAfter(timeAfterDate);
+        nextFireTime.Value.Date.Should().Be(expectedNextFireTime.Date, "NextFireTime was not correct");
+    }
+
     [Test]
     public void TestTimeIncluded()
     {
@@ -53,7 +64,7 @@ public void TestTimeIncluded()
     public void TestClone()
     {
         CronCalendar calendar = new CronCalendar("0/15 * * * * ?");
-        CronCalendar clone = (CronCalendar) calendar.Clone();
+        CronCalendar clone = (CronCalendar)calendar.Clone();
         Assert.AreEqual(calendar.CronExpression, clone.CronExpression);
     }
 
@@ -83,4 +94,23 @@ protected override void VerifyMatch(CronCalendar original, CronCalendar deserial
         Assert.AreEqual(original.CronExpression, deserialized.CronExpression);
         Assert.AreEqual(original.TimeZone, deserialized.TimeZone);
     }
+}
+
+public class CronWeekdayModifierTestData
+{
+    public static IEnumerable TestCases
+    {
+        get
+        {
+            // params are CronExpression cronExpression, DateTimeOffset timeAfterDate, DateTimeOffset expectedNextFireTime
+            // Sat 15th, schedule should be 14th
+            yield return new TestCaseData(new CronExpression("0 0 12 15W * ?"), new DateTimeOffset(2024, 5, 15, 12, 0, 0, TimeSpan.Zero), new DateTimeOffset(2024, 6, 14, 12, 0, 0, TimeSpan.Zero)); 
+            yield return new TestCaseData(new CronExpression("0 0 12 15W * ?"), new DateTimeOffset(2024, 5, 10, 12, 0, 0, TimeSpan.Zero), new DateTimeOffset(2024, 5, 15, 12, 0, 0, TimeSpan.Zero));
+            yield return new TestCaseData(new CronExpression("0 0 12 15W * ?"), new DateTimeOffset(2024, 5, 22, 12, 0, 0, TimeSpan.Zero), new DateTimeOffset(2024, 6, 14, 12, 0, 0, TimeSpan.Zero));
+            // Sunday 15th schedule will be 16th
+            yield return new TestCaseData(new CronExpression("0 0 12 15W * ?"), new DateTimeOffset(2024, 8, 15, 12, 0, 0, TimeSpan.Zero), new DateTimeOffset(2024, 9, 16, 12, 0, 0, TimeSpan.Zero));
+            // 15th is Sunday, schedule will be 16th
+            yield return new TestCaseData(new CronExpression("0 0 12 15W * ?"), new DateTimeOffset(2023, 12, 15, 12, 0, 0, TimeSpan.Zero), new DateTimeOffset(2024, 1, 15, 12, 0, 0, TimeSpan.Zero));
+        }
+    }
 }

src/Quartz.Tests.Unit/TestUtil.cs

@@ -17,8 +17,6 @@
  */
 #endregion
 
-using NUnit.Framework;
-
 using Quartz.Impl;
 using Quartz.Impl.Triggers;
 using Quartz.Job;

src/Quartz/CronExpression.cs

@@ -1469,95 +1469,145 @@ private NextFireTimeCursor ProgressNextFireTimeHour(DateTimeOffset d)
 
         return new NextFireTimeCursor(false, new DateTimeOffset(d.Year, d.Month, d.Day, hour, d.Minute, d.Second, d.Millisecond, d.Offset));
     }
+
+    private (SortedSet<int> daysOfMonthSet, bool dayHasNegativeOffset) CalculateDaysOfMonth(DateTimeOffset currentDate)
+{
+    var daysOfMonthSet = new SortedSet<int>(daysOfMonth);
+    var dayHasNegativeOffset = false;
 
-    private SortedSet<int> CalculateDaysOfMonth(DateTimeOffset dt)
+    if (lastDayOfMonth)
     {
-        var results = new SortedSet<int>(daysOfMonth);
-        if (lastDayOfMonth)
+        int lastDayOfMonthValue = GetLastDayOfMonth(currentDate.Month, currentDate.Year);
+        int lastDayOfMonthWithOffset = lastDayOfMonthValue - lastDayOffset;
+
+        if (nearestWeekday)
         {
-            var lastDayOfMonth = GetLastDayOfMonth(dt.Month, dt.Year);
-            var lastDayOfMonthWithOffset = lastDayOfMonth - lastDayOffset;
+            int calculatedLastDay = CalculateNearestWeekdayForLastDay(currentDate, lastDayOfMonthWithOffset);
+            daysOfMonthSet.Add(calculatedLastDay);
+        }
+        else
+        {
+            daysOfMonthSet.Add(lastDayOfMonthWithOffset);
+        }
+    }
+    else if (nearestWeekday)
+    {
+        (daysOfMonthSet, dayHasNegativeOffset) = CalculateNearestWeekdayForDaysOfMonth(currentDate, daysOfMonthSet);
+    }
 
-            if (nearestWeekday)
-            {
-                var checkDay = new DateTimeOffset(dt.Year, dt.Month, lastDayOfMonthWithOffset, dt.Hour, dt.Minute, dt.Second, dt.Millisecond, dt.Offset);
-                var calculatedDay = lastDayOfMonthWithOffset;
-                switch (checkDay.DayOfWeek)
-                {
-                    case DayOfWeek.Saturday:
-                        calculatedDay -= 1;
-                        break;
-                    case DayOfWeek.Sunday:
-                        calculatedDay -= 2;
-                        break;
-                }
+    return (daysOfMonthSet, dayHasNegativeOffset);
+}
 
-                var calculatedLastDayWithOffset = calculatedDay - lastWeekdayOffset;
-                // If the day has crossed to the prior month, reset to 1st.
-                if (calculatedLastDayWithOffset <= 0)
-                {
-                    calculatedLastDayWithOffset = 1;
-                }
+/// <summary>
+/// Calculates the nearest weekday for the last day of the month.
+/// </summary>
+/// <param name="currentDate">The current date.</param>
+/// <param name="lastDayOfMonthWithOffset">The last day of the month with the offset applied.</param>
+/// <returns>The calculated last day of the month, adjusted to the nearest weekday.</returns>
+private int CalculateNearestWeekdayForLastDay(DateTimeOffset currentDate, int lastDayOfMonthWithOffset)
+{
+    var checkDay = new DateTimeOffset(currentDate.Year, currentDate.Month, lastDayOfMonthWithOffset, currentDate.Hour, currentDate.Minute, currentDate.Second, currentDate.Millisecond, currentDate.Offset);
+    var calculatedDay = lastDayOfMonthWithOffset;
 
-                results.Add(calculatedLastDayWithOffset);
-            }
-            else
-            {
-                results.Add(lastDayOfMonthWithOffset);
-            }
-        }
-        else if (nearestWeekday) //AND not lastDay
-        {
-            var day = daysOfMonth.Min;
-            var tcal = new DateTimeOffset(dt.Year, dt.Month, day, 0, 0, 0, dt.Offset);
-            var lastDayOfMonth = GetLastDayOfMonth(dt.Month, dt.Year);
-            var dayOfWeek = tcal.DayOfWeek;
+    switch (checkDay.DayOfWeek)
+    {
+        case DayOfWeek.Saturday:
+            calculatedDay -= 1;
+            break;
+        case DayOfWeek.Sunday:
+            calculatedDay -= 2;
+            break;
+    }
 
-            // evict original date since it has a weekDayModifier
-            results.Remove(day);
+    var calculatedLastDayWithOffset = calculatedDay - lastWeekdayOffset;
 
-            switch (dayOfWeek)
-            {
-                case DayOfWeek.Saturday when day == 1:
-                    day += 2;
-                    break;
-                case DayOfWeek.Saturday:
-                    day -= 1;
-                    break;
-                case DayOfWeek.Sunday when day == lastDayOfMonth:
-                    day -= 2;
-                    break;
-                case DayOfWeek.Sunday:
-                    day += 1;
-                    break;
-            }
+    // If the day has crossed to the prior month, reset to 1st.
+    if (calculatedLastDayWithOffset <= 0)
+    {
+        calculatedLastDayWithOffset = 1;
+    }
 
-            results.Add(day);
-        }
+    return calculatedLastDayWithOffset;
+}
+
+/// <summary>
+/// Calculates the nearest weekday for the specified days of the month.
+/// </summary>
+/// <param name="currentDate">The current date.</param>
+/// <param name="daysOfMonthSet">The set of days of the month.</param>
+/// <returns>A tuple containing the updated set of days of the month and a flag indicating if any day has a negative offset.</returns>
+private (SortedSet<int> daysOfMonthSet, bool dayHasNegativeOffset) CalculateNearestWeekdayForDaysOfMonth(DateTimeOffset currentDate, SortedSet<int> daysOfMonthSet)
+{
+    int minDay = daysOfMonthSet.Min;
+    int endDayOfMonth = GetLastDayOfMonth(currentDate.Month, currentDate.Year);
 
-        return results;
+    DateTimeOffset firstDayOfMonth = new DateTimeOffset(currentDate.Year, currentDate.Month, minDay, 0, 0, 0, currentDate.Offset);
+    DayOfWeek dayOfWeek = firstDayOfMonth.DayOfWeek;
+
+    // Evict the original date if it is not a weekday
+    if (dayOfWeek is DayOfWeek.Saturday or DayOfWeek.Sunday)
+    {
+        daysOfMonthSet.Remove(minDay);
+    }
+
+    var (adjustedDay,dayHasNegativeOffset) = AdjustDayToNearestWeekday(minDay, dayOfWeek, endDayOfMonth);
+    daysOfMonthSet.Add(adjustedDay);
+
+    return (daysOfMonthSet, dayHasNegativeOffset);
+}
+
+/// <summary>
+/// Adjusts the day to the nearest weekday based on the specified day of the week and the last day of the month.
+/// </summary>
+/// <param name="day">The day to adjust.</param>
+/// <param name="dayOfWeek">The day of the week.</param>
+/// <param name="endDayOfMonth">The end day of the month.</param>
+/// <returns>The adjusted day and a flag to indicate adjust day has a negative offset</returns>
+private (int day, bool dayHasNegativeOffset) AdjustDayToNearestWeekday(int day, DayOfWeek dayOfWeek, int endDayOfMonth)
+{
+    var dayHasNegativeOffset = false;
+
+    switch (dayOfWeek)
+    {
+        case DayOfWeek.Saturday when day == 1:
+            day += 2;
+            break;
+        case DayOfWeek.Saturday:
+            day -= 1;
+            dayHasNegativeOffset = true;
+            break;
+        case DayOfWeek.Sunday when day == endDayOfMonth:
+            day -= 2;
+            dayHasNegativeOffset = true;
+            break;
+        case DayOfWeek.Sunday:
+            day += 1;
+            break;
     }
 
+    return (day, dayHasNegativeOffset);
+}
+
     private NextFireTimeCursor ProgressNextFireTimeDayOfMonth(DateTimeOffset d)
     {
         var day = d.Day;
-        var mon = d.Month;
+        var month = d.Month;
         var t = -1;
-        var tmon = mon;
+        var tmon = month;
 
         // get day by day of month rule
-        var daysOfMonthCalculated = CalculateDaysOfMonth(d);
-        if (daysOfMonthCalculated.TryGetMinValueStartingFrom(d.Day, out var min))
+        var (daysOfMonthCalculated, setIncludesDayBeforeStartDay) = CalculateDaysOfMonth(d);
+        if (daysOfMonthCalculated.TryGetMinValueStartingFrom(d, setIncludesDayBeforeStartDay, out var min))
         {
             t = day;
             day = min;
 
             // make sure we don't over-run a short month, such as february
-            var lastDay = GetLastDayOfMonth(mon, d.Year);
+            var lastDay = GetLastDayOfMonth(month, d.Year);
             if (day > lastDay)
             {
                 day = daysOfMonthCalculated.Min;
-                mon++;
+                month++;
             }
         }
         else
@@ -1571,28 +1621,28 @@ private NextFireTimeCursor ProgressNextFireTimeDayOfMonth(DateTimeOffset d)
                 day = daysOfMonth.Min; //if not then initial set of days uncalculated (to avoid issue with stale weekday in wrong month value)
             }
 
-            mon++;
+            month++;
         }
 
-        if (day != t || mon != tmon)
+        if (day != t || month != tmon)
         {
-            if (mon > 12)
+            if (month > 12)
             {
-                d = new DateTimeOffset(d.Year, 12, day, 0, 0, 0, d.Offset).AddMonths(mon - 12);
+                d = new DateTimeOffset(d.Year, 12, day, 0, 0, 0, d.Offset).AddMonths(month - 12);
             }
             else
             {
                 // This is to avoid a bug when moving from a month
                 // with 30 or 31 days to a month with less. Causes an invalid datetime to be instantiated.
-                // ex. 0 29 0 30 1 ? 2009 with clock set to 1/30/2009
-                var lDay = DateTime.DaysInMonth(d.Year, mon);
-                if (day <= lDay)
+                // ex. 0 29 0 30 1 ? 2009 with the clock set to 1/30/2009
+                var daysInMonth = DateTime.DaysInMonth(d.Year, month);
+                if (day <= daysInMonth)
                 {
-                    d = new DateTimeOffset(d.Year, mon, day, 0, 0, 0, d.Offset);
+                    d = new DateTimeOffset(d.Year, month, day, 0, 0, 0, d.Offset);
                 }
                 else
                 {
-                    d = new DateTimeOffset(d.Year, mon, lDay, 0, 0, 0, d.Offset).AddDays(day - lDay);
+                    d = new DateTimeOffset(d.Year, month, daysInMonth, 0, 0, 0, d.Offset).AddDays(day - daysInMonth);
                 }
             }
             return new NextFireTimeCursor(true, d);

src/Quartz/Util/SortedSetExtensions.cs

@@ -2,32 +2,41 @@ namespace Quartz.Util;
 
 internal static class SortedSetExtensions
 {
-    internal static bool TryGetMinValueStartingFrom(this SortedSet<int> set, int start, out int min)
+    internal static bool TryGetMinValueStartingFrom(this SortedSet<int> set, DateTimeOffset start, bool allowValueBeforeStartDay, out int minimumDay)
     {
-        min = set.Min;
+        minimumDay = set.Min;
+        var startDay = start.Day;
 
-        if (set.Contains(CronExpressionConstants.AllSpec) || set.Contains(start))
+        if (set.Contains(CronExpressionConstants.AllSpec) || set.Contains(startDay))
         {
-            min = start;
+            minimumDay = startDay;
             return true;
         }
 
-        if (set.Count == 0 || set.Max < start)
+        // In cases such as W modifier finding a match earlier than the month day.
+        // If the flag allowValueBeforeStartDay is set and the minimum value is less than the start day, return the minimum value 
+        if (allowValueBeforeStartDay && set.Min < startDay)
+        {
+            return true;
+        }
+
+        // If the set is empty or the maximum value is less than the start day, no suitable value is found
+        if (set.Count == 0 || set.Max < startDay)
         {
             return false;
         }
 
-        if (set.Min >= start)
+        // If the minimum value is greater than or equal to the start day, return the minimum value
+        if (set.Min >= startDay)
         {
-            // value is contained and would be returned from view
             return true;
         }
 
         // slow path
-        var view = set.GetViewBetween(start, int.MaxValue);
+        var view = set.GetViewBetween(startDay, int.MaxValue);
         if (view.Count > 0)
         {
-            min = view.Min;
+            minimumDay = view.Min;
             return true;
         }