DB IDO: Fix the way notification_id is handled.

Fixes #5103
Fixes #5265

lib/db_ido/servicedbobject.cpp

@@ -52,8 +52,8 @@ void ServiceDbObject::StaticInitialize(void)
 	Service::OnCommentAdded.connect(boost::bind(&ServiceDbObject::AddCommentHistory, _1, _2));
 	Service::OnDowntimeAdded.connect(boost::bind(&ServiceDbObject::AddDowntimeHistory, _1, _2));
 	Service::OnAcknowledgementSet.connect(boost::bind(&ServiceDbObject::AddAcknowledgementHistory, _1, _2, _3, _4, _5));
-	Service::OnNotificationSentToUser.connect(bind(&ServiceDbObject::AddContactNotificationHistory, _1, _2, _3));
+
-	Service::OnNotificationSendStart.connect(bind(&ServiceDbObject::AddNotificationHistory, _1, _2, _3, _4, _5, _6, _7));
+	Service::OnNotificationSentToAllUsers.connect(bind(&ServiceDbObject::AddNotificationHistory, _1, _2, _3, _4, _5, _6, _7));
 
 	Service::OnStateChange.connect(boost::bind(&ServiceDbObject::AddStateChangeHistory, _1, _2, _3));
 
@@ -758,36 +758,6 @@ void ServiceDbObject::AddAcknowledgementHistory(const Service::Ptr& service, con
 }
 
 /* notifications */
-
-void ServiceDbObject::AddContactNotificationHistory(const Notification::Ptr& notification, const Service::Ptr& service, const User::Ptr& user)
-{
-	Host::Ptr host = service->GetHost();
-
-	Log(LogDebug, "db_ido", "add contact notification history for '" + service->GetName() + "'");
-
-	/* start and end happen at the same time */
-	double now = Utility::GetTime();
-	std::pair<unsigned long, unsigned long> time_bag = CompatUtility::ConvertTimestamp(now);
-
-	DbQuery query1;
-	query1.Table = "contactnotifications";
-	query1.Type = DbQueryInsert;
-	query1.Category = DbCatNotification;
-
-	Dictionary::Ptr fields1 = make_shared<Dictionary>();
-	fields1->Set("contact_object_id", user);
-	fields1->Set("start_time", DbValue::FromTimestamp(time_bag.first));
-	fields1->Set("start_time_usec", time_bag.second);
-	fields1->Set("end_time", DbValue::FromTimestamp(time_bag.first));
-	fields1->Set("end_time_usec", time_bag.second);
-
-	fields1->Set("notification_id", 0); /* DbConnection class fills in real ID */
-	fields1->Set("instance_id", 0); /* DbConnection class fills in real ID */
-
-	query1.Fields = fields1;
-	OnQuery(query1);
-}
-
 void ServiceDbObject::AddNotificationHistory(const Notification::Ptr& notification, const Service::Ptr& service, const std::set<User::Ptr>& users, NotificationType type,
     const CheckResult::Ptr& cr, const String& author, const String& text)
 {
@@ -834,6 +804,29 @@ void ServiceDbObject::AddNotificationHistory(const Notification::Ptr& notificati
 		query1.Fields = fields1;
 		OnQuery(query1);
 	}
+
+	DbQuery query2;
+	query2.Table = "contactnotifications";
+	query2.Type = DbQueryInsert;
+	query2.Category = DbCatNotification;
+
+	/* filtered users */
+	BOOST_FOREACH(const User::Ptr& user, users) {
+		Log(LogDebug, "db_ido", "add contact notification history for service '" + service->GetName() + "' and user '" + user->GetName() + "'.");
+
+		Dictionary::Ptr fields2 = make_shared<Dictionary>();
+		fields2->Set("contact_object_id", user);
+		fields2->Set("start_time", DbValue::FromTimestamp(time_bag.first));
+		fields2->Set("start_time_usec", time_bag.second);
+		fields2->Set("end_time", DbValue::FromTimestamp(time_bag.first));
+		fields2->Set("end_time_usec", time_bag.second);
+
+		fields2->Set("notification_id", 0); /* DbConnection class fills in real ID */
+		fields2->Set("instance_id", 0); /* DbConnection class fills in real ID */
+
+		query2.Fields = fields2;
+		OnQuery(query2);
+	}
 }
 
 /* statehistory */

lib/db_ido/servicedbobject.h

@@ -95,17 +95,21 @@ private:
 	static void RemoveDowntime(const Service::Ptr& service, const Downtime::Ptr& downtime);
 	static void TriggerDowntime(const Service::Ptr& service, const Downtime::Ptr& downtime);
 
-        /* History */
+        /* comment, downtime, acknowledgement history */
         static void AddCommentHistory(const Service::Ptr& service, const Comment::Ptr& comment);
 	static void AddDowntimeHistory(const Service::Ptr& service, const Downtime::Ptr& downtime);
         static void AddAcknowledgementHistory(const Service::Ptr& service, const String& author, const String& comment,
             AcknowledgementType type, double expiry);
-        static void AddContactNotificationHistory(const Notification::Ptr& notification, const Service::Ptr& service, const User::Ptr& user);
+
+        /* notification & contactnotification history */
 	static void AddNotificationHistory(const Notification::Ptr& notification, const Service::Ptr& service,
             const std::set<User::Ptr>& users, NotificationType type, const CheckResult::Ptr& cr, const String& author,
             const String& text);
+
+        /* statehistory */
 	static void AddStateChangeHistory(const Service::Ptr& service, const CheckResult::Ptr& cr, StateType type);
 
+        /* logentries */
 	static void AddCheckResultLogHistory(const Service::Ptr& service, const CheckResult::Ptr &cr);
         static void AddTriggerDowntimeLogHistory(const Service::Ptr& service, const Downtime::Ptr& downtime);
         static void AddRemoveDowntimeLogHistory(const Service::Ptr& service, const Downtime::Ptr& downtime);
@@ -114,6 +118,7 @@ private:
             const String& comment_text);
         static void AddFlappingLogHistory(const Service::Ptr& service, FlappingState flapping_state);
 
+        /* other history */
         static void AddFlappingHistory(const Service::Ptr& service, FlappingState flapping_state);
 	static void AddServiceCheckHistory(const Service::Ptr& service, const CheckResult::Ptr &cr);
         static void AddEventHandlerHistory(const Service::Ptr& service);

lib/icinga/notification.cpp

@@ -258,15 +258,23 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
 
 	Service::OnNotificationSendStart(GetSelf(), GetService(), allUsers, type, cr, author, text);
 
+	std::set<User::Ptr> allNotifiedUsers;
 	BOOST_FOREACH(const User::Ptr& user, allUsers) {
+		if (!CheckNotificationUserFilters(type, user, force))
+			continue;
+
 		Log(LogDebug, "icinga", "Sending notification for user '" + user->GetName() + "'");
 		Utility::QueueAsyncCallback(boost::bind(&Notification::ExecuteNotificationHelper, this, type, user, cr, force, author, text));
+
+		/* collect all notified users */
+		allNotifiedUsers.insert(user);
 	}
 
-	Service::OnNotificationSentToAllUsers(GetSelf(), GetService(), allUsers, type, cr, author, text);
+	/* used in db_ido for notification history */
+	Service::OnNotificationSentToAllUsers(GetSelf(), GetService(), allNotifiedUsers, type, cr, author, text);
 }
 
-void Notification::ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author, const String& text)
+bool Notification::CheckNotificationUserFilters(NotificationType type, const User::Ptr& user, bool force)
 {
 	ASSERT(!OwnsLock());
 
@@ -276,7 +284,7 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
 		if (tp && !tp->IsInside(Utility::GetTime())) {
 			Log(LogInformation, "icinga", "Not sending notifications for notification object '" +
 			    GetName() + " and user '" + user->GetName() + "': user not in timeperiod");
-			return;
+			return false;
 		}
 
 		unsigned long ftype = 1 << type;
@@ -284,7 +292,7 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
 		if (!(ftype & user->GetNotificationTypeFilter())) {
 			Log(LogInformation, "icinga", "Not sending notifications for notification object '" +
 			    GetName() + " and user '" + user->GetName() + "': type filter does not match");
-			return;
+			return false;
 		}
 
 		unsigned long fstate = 1 << GetService()->GetState();
@@ -292,10 +300,17 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
 		if (!(fstate & user->GetNotificationStateFilter())) {
 			Log(LogInformation, "icinga", "Not sending notifications for notification object '" +
 			    GetName() + " and user '" + user->GetName() + "': state filter does not match");
-			return;
+			return false;
 		}
 	}
 
+	return true;
+}
+
+void Notification::ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author, const String& text)
+{
+	ASSERT(!OwnsLock());
+
 	try {
 		NotificationCommand::Ptr command = GetNotificationCommand();
 
@@ -312,6 +327,7 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
 			SetLastNotification(Utility::GetTime());
 		}
 
+		/* required by compatlogger */
 		Service::OnNotificationSentToUser(GetSelf(), GetService(), user, type, cr, author, text, command->GetName());
 
 		Log(LogInformation, "icinga", "Completed sending notification for service '" + GetService()->GetName() + "'");

lib/icinga/notification.h

@@ -78,6 +78,8 @@ public:
 
 	void BeginExecuteNotification(NotificationType type, const CheckResult::Ptr& cr, bool force, const String& author = "", const String& text = "");
 
+	bool CheckNotificationUserFilters(NotificationType type, const User::Ptr& user, bool force);
+
 	static String NotificationTypeToString(NotificationType type);
 
 	virtual bool ResolveMacro(const String& macro, const CheckResult::Ptr& cr, String *result) const;