From c45829b59ff2b564136900dce91c9b3a84a03534 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 33e666544a8..212ff8439cc 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 first 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 d2ce3a0ce7c..defc5e8894b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -188,6 +188,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 e1150be57eb..50cad3f28ce 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -691,4 +691,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()