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

@@ -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.

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

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