Merge pull request #10613 from Icinga/clear-suppr-notif-after-tp-resume

Clear suppressed notifications only after the TimePeriod is resumed

Author: yhabteab

lib/notification/notificationcomponent.cpp

@@ -83,37 +83,34 @@ void FireSuppressedNotifications(const Notification::Ptr& notification)
 	int subtract = 0;
 	auto checkable (notification->GetCheckable());
 
-	for (auto type : {NotificationProblem, NotificationRecovery, NotificationFlappingStart, NotificationFlappingEnd}) {
-		if ((suppressedTypes & type) && !checkable->NotificationReasonApplies(type)) {
-			subtract |= type;
-			suppressedTypes &= ~type;
-		}
-	}
+	auto tp (notification->GetPeriod());
 
-	if (suppressedTypes) {
-		auto tp (notification->GetPeriod());
+	if ((!tp || tp->IsInside(Utility::GetTime())) && !checkable->IsLikelyToBeCheckedSoon()) {
+		for (auto type : {NotificationProblem, NotificationRecovery, NotificationFlappingStart, NotificationFlappingEnd}) {
+			if (!(suppressedTypes & type) || checkable->NotificationReasonSuppressed(type)) {
+				continue;
+			}
 
-		if ((!tp || tp->IsInside(Utility::GetTime())) && !checkable->IsLikelyToBeCheckedSoon()) {
-			for (auto type : {NotificationProblem, NotificationRecovery, NotificationFlappingStart, NotificationFlappingEnd}) {
-				if (!(suppressedTypes & type) || checkable->NotificationReasonSuppressed(type))
-					continue;
+			if (!checkable->NotificationReasonApplies(type)) {
+				subtract |= type;
+				continue;
+			}
 
-				auto notificationName (notification->GetName());
+			auto notificationName (notification->GetName());
 
-				Log(LogNotice, "NotificationComponent")
-					<< "Attempting to re-send previously suppressed notification '" << notificationName << "'.";
+			Log(LogNotice, "NotificationComponent")
+				<< "Attempting to re-send previously suppressed notification '" << notificationName << "'.";
 
-				subtract |= type;
-				SubtractSuppressedNotificationTypes(notification, subtract);
-				subtract = 0;
-
-				try {
-					notification->BeginExecuteNotification(type, checkable->GetLastCheckResult(), false, false);
-				} catch (const std::exception& ex) {
-					Log(LogWarning, "NotificationComponent")
-						<< "Exception occurred during notification for object '"
-						<< notificationName << "': " << DiagnosticInformation(ex, false);
-				}
+			subtract |= type;
+			SubtractSuppressedNotificationTypes(notification, subtract);
+			subtract = 0;
+
+			try {
+				notification->BeginExecuteNotification(type, checkable->GetLastCheckResult(), false, false);
+			} catch (const std::exception& ex) {
+				Log(LogWarning, "NotificationComponent")
+					<< "Exception occurred during notification for object '"
+					<< notificationName << "': " << DiagnosticInformation(ex, false);
 			}
 		}
 	}

test/CMakeLists.txt

@@ -131,6 +131,13 @@ set(base_test_SOURCES
   $<TARGET_OBJECTS:methods>
 )
 
+if(ICINGA2_WITH_NOTIFICATION)
+  list(APPEND base_test_SOURCES
+    notification-notificationcomponent.cpp
+    $<TARGET_OBJECTS:notification>
+  )
+endif()
+
 if(ICINGA2_UNITY_BUILD)
   mkunity_target(base test base_test_SOURCES)
 endif()

test/base-testloggerfixture.hpp

@@ -52,6 +52,13 @@ class TestLogger : public Logger
 		return ret;
 	}
 
+	void Clear()
+	{
+		std::lock_guard lock(m_Mutex);
+		m_Expects.clear();
+		m_LogEntries.clear();
+	}
+
 private:
 	void ProcessLogEntry(const LogEntry& entry) override
 	{
@@ -119,6 +126,8 @@ struct TestLoggerFixture
 		return testLogger->ExpectLogPattern(pattern, timeout);
 	}
 
+	void ClearTestLogger() const { testLogger->Clear(); }
+
 	TestLogger::Ptr testLogger = new TestLogger;
 };