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.
This commit is contained in:
Julian Brost 2024-08-02 16:27:15 +02:00
parent 0463607050
commit c45829b59f
3 changed files with 59 additions and 3 deletions

View File

@ -62,18 +62,21 @@ void LegacyTimePeriod::FindNthWeekday(int wday, int n, tm *reference)
if (n > 0) { if (n > 0) {
dir = 1; dir = 1;
/* Postitive days are relative to the first day of the month. */
t.tm_mday = 1;
} else { } else {
n *= -1; n *= -1;
dir = -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_mon++;
t.tm_mday = 0;
} }
ASSERT(n > 0); ASSERT(n > 0);
t.tm_mday = 1;
for (;;) { for (;;) {
// Always operate on 00:00:00 with automatic DST detection, otherwise days could // 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. // be skipped or counted twice if +-24 hours is not on the next or previous day.

View File

@ -188,6 +188,7 @@ add_boost_test(base
icinga_legacytimeperiod/advanced icinga_legacytimeperiod/advanced
icinga_legacytimeperiod/dst icinga_legacytimeperiod/dst
icinga_legacytimeperiod/dst_isinside icinga_legacytimeperiod/dst_isinside
icinga_legacytimeperiod/find_nth_weekday
icinga_perfdata/empty icinga_perfdata/empty
icinga_perfdata/simple icinga_perfdata/simple
icinga_perfdata/quotes icinga_perfdata/quotes

View File

@ -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() BOOST_AUTO_TEST_SUITE_END()