From 3247e839576181484247b3fd8a338ecb61a7bac3 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 2 Aug 2024 16:27:15 +0200 Subject: [PATCH] Timeperiods: fix off by one when calculating n-th last weekday of the month A day specification like "monday -1" refers to the last Monday of the month. However, there was an off by one if the first day of the next month is the same day of the week, i.e. a Monday in this example. LegacyTimePeriod::FindNthWeekday() picks a day to start the search for the day in question. When given a negative n to search for the n-th last day, it wrongly used the first day of the following month as the start and counted it as if it was within the current month. This resulted in a 1/7 chance that the result was one week too late. This is fixed by using the last day of the current month instead. --- lib/icinga/legacytimeperiod.cpp | 9 ++++-- test/CMakeLists.txt | 1 + test/icinga-legacytimeperiod.cpp | 52 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lib/icinga/legacytimeperiod.cpp b/lib/icinga/legacytimeperiod.cpp index 33e666544..f9893f53b 100644 --- a/lib/icinga/legacytimeperiod.cpp +++ b/lib/icinga/legacytimeperiod.cpp @@ -62,18 +62,21 @@ void LegacyTimePeriod::FindNthWeekday(int wday, int n, tm *reference) if (n > 0) { dir = 1; + + /* Postitive days are relative to the last day of the month. */ + t.tm_mday = 1; } else { n *= -1; dir = -1; - /* Negative days are relative to the next month. */ + /* Negative days are relative to the last day of the month which is + * what mktime() normalizes the 0th day of the next month to. */ t.tm_mon++; + t.tm_mday = 0; } ASSERT(n > 0); - t.tm_mday = 1; - for (;;) { // Always operate on 00:00:00 with automatic DST detection, otherwise days could // be skipped or counted twice if +-24 hours is not on the next or previous day. diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e181da581..57d02d09d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -189,6 +189,7 @@ add_boost_test(base icinga_legacytimeperiod/advanced icinga_legacytimeperiod/dst icinga_legacytimeperiod/dst_isinside + icinga_legacytimeperiod/find_nth_weekday icinga_perfdata/empty icinga_perfdata/simple icinga_perfdata/quotes diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp index d513f79ec..f0a1a21f4 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -682,4 +682,56 @@ BOOST_AUTO_TEST_CASE(dst_isinside) } } +BOOST_AUTO_TEST_CASE(find_nth_weekday) { + auto run = [](const std::string &refDay, int wday, int n, const std::string &expectedDay) { + tm expected = make_tm(expectedDay + " 00:00:00"); + + tm t = make_tm(refDay + " 00:00:00"); + LegacyTimePeriod::FindNthWeekday(wday, n, &t); + + BOOST_CHECK_MESSAGE(mktime(&expected) == mktime(&t), + "[ref=" << refDay << ", wday=" << wday << ", n=" << n << "] " + "expected: " << pretty_time(expected) << ", " + "got: " << pretty_time(t)); + }; + + /* March 2019 + * Mo Tu We Th Fr Sa Su + * 1 2 3 + * 4 5 6 7 8 9 10 + * 11 12 13 14 15 16 17 + * 18 19 20 21 22 23 24 + * 25 26 27 28 29 30 31 + */ + + // Use every day of the month as reference day, all must give the same result for that month. + for (int i = 1; i <= 31; ++i) { + std::stringstream refDayStream; + refDayStream << "2019-03-" << std::setw(2) << std::setfill('0') << i; + std::string refDay = refDayStream.str(); + + const int monday = 1; // 4 ocurrences in March 2019 + run(refDay, monday, 1, "2019-03-04"); + run(refDay, monday, 2, "2019-03-11"); + run(refDay, monday, 3, "2019-03-18"); + run(refDay, monday, 4, "2019-03-25"); + run(refDay, monday, -1, "2019-03-25"); + run(refDay, monday, -2, "2019-03-18"); + run(refDay, monday, -3, "2019-03-11"); + run(refDay, monday, -4, "2019-03-04"); + + const int friday = 5; // 5 ocurrences in March 2019 + run(refDay, friday, 1, "2019-03-01"); + run(refDay, friday, 2, "2019-03-08"); + run(refDay, friday, 3, "2019-03-15"); + run(refDay, friday, 4, "2019-03-22"); + run(refDay, friday, 5, "2019-03-29"); + run(refDay, friday, -1, "2019-03-29"); + run(refDay, friday, -2, "2019-03-22"); + run(refDay, friday, -3, "2019-03-15"); + run(refDay, friday, -4, "2019-03-08"); + run(refDay, friday, -5, "2019-03-01"); + } +} + BOOST_AUTO_TEST_SUITE_END()