Make sure comment/downtime legacy IDs are always unique (even in a cluster context).

lib/icinga/commentprocessor.cpp

@@ -47,10 +47,6 @@ String CommentProcessor::AddComment(const DynamicObject::Ptr& owner,
 	if (!comments)
 		comments = boost::make_shared<Dictionary>();
 
-	int legacy_id = m_NextCommentID;
-	m_NextCommentID++;
-	comment->Set("legacy_id", legacy_id);
-
 	String id = Utility::NewUUID();
 	comments->Set(id, comment);
 	owner->Set("comments", comments);
@@ -80,7 +76,12 @@ void CommentProcessor::RemoveComment(const String& id)
 
 String CommentProcessor::GetIDFromLegacyID(int id)
 {
-	return Convert::ToString(id);
+	map<int, String>::iterator it = m_LegacyCommentCache.find(id);
+
+	if (it == m_LegacyCommentCache.end())
+		throw_exception(invalid_argument("Invalid legacy comment ID specified."));
+
+	return it->second;
 }
 
 DynamicObject::Ptr CommentProcessor::GetOwnerByCommentID(const String& id)
@@ -124,15 +125,29 @@ void CommentProcessor::AddCommentsToCache(const DynamicObject::Ptr& owner)
 	String id;
 	Dictionary::Ptr comment;
 	BOOST_FOREACH(tie(id, comment), comments) {
-		if (comment->Contains("legacy_id")) {
+		int legacy_id;
-			int legacy_id = comment->Get("legacy_id");
 
-			m_LegacyCommentCache[legacy_id] = id;
+		if (!comment->Contains("legacy_id")) {
-
+			legacy_id = m_NextCommentID;
-			if (legacy_id > m_NextCommentID)
+			m_NextCommentID++;
-				m_NextCommentID = legacy_id;
+			comment->Set("legacy_id", legacy_id);
+		} else {
+			legacy_id = comment->Get("legacy_id");
 		}
 
+		if (legacy_id >= m_NextCommentID)
+			m_NextCommentID = legacy_id + 1;
+
+		if (m_LegacyCommentCache.find(legacy_id) != m_LegacyCommentCache.end()) {
+			/* The legacy_id is already in use by another comment;
+			 * this shouldn't usually happen - assign it a new ID */
+
+			legacy_id = m_NextCommentID;
+			m_NextCommentID++;
+			comment->Set("legacy_id", legacy_id);
+		}
+
+		m_LegacyCommentCache[legacy_id] = id;
 		m_CommentCache[id] = owner;
 	}
 }

lib/icinga/commentprocessor.h

@@ -53,6 +53,7 @@ public:
 	static Dictionary::Ptr GetCommentByID(const String& id);
 
 	static void InvalidateCommentCache(void);
+	static void ValidateCommentCache(void);
 
 private:
 	static int m_NextCommentID;
@@ -64,7 +65,6 @@ private:
 	CommentProcessor(void);
 
 	static void AddCommentsToCache(const DynamicObject::Ptr& owner);
-	static void ValidateCommentCache(void);
 };
 
 }

lib/icinga/downtimeprocessor.cpp

@@ -52,10 +52,6 @@ String DowntimeProcessor::AddDowntime(const DynamicObject::Ptr& owner,
 	if (!downtimes)
 		downtimes = boost::make_shared<Dictionary>();
 
-	int legacy_id = m_NextDowntimeID;
-	m_NextDowntimeID++;
-	downtime->Set("legacy_id", legacy_id);
-
 	String id = Utility::NewUUID();
 	downtimes->Set(id, downtime);
 	owner->Set("downtimes", downtimes);
@@ -77,7 +73,12 @@ void DowntimeProcessor::RemoveDowntime(const String& id)
 
 String DowntimeProcessor::GetIDFromLegacyID(int id)
 {
-	return Convert::ToString(id);
+	map<int, String>::iterator it = m_LegacyDowntimeCache.find(id);
+
+	if (it == m_LegacyDowntimeCache.end())
+		throw_exception(invalid_argument("Invalid legacy downtime ID specified."));
+
+	return it->second;
 }
 
 DynamicObject::Ptr DowntimeProcessor::GetOwnerByDowntimeID(const String& id)
@@ -140,15 +141,28 @@ void DowntimeProcessor::AddDowntimesToCache(const DynamicObject::Ptr& owner)
 	String id;
 	Dictionary::Ptr downtime;
 	BOOST_FOREACH(tie(id, downtime), downtimes) {
-		if (downtime->Contains("legacy_id")) {
+		int legacy_id;
-			int legacy_id = downtime->Get("legacy_id");
 
-			m_LegacyDowntimeCache[legacy_id] = id;
+		if (!downtime->Contains("legacy_id")) {
-
+			legacy_id = m_NextDowntimeID;
-			if (legacy_id > m_NextDowntimeID)
+			m_NextDowntimeID++;
-				m_NextDowntimeID = legacy_id;
+			downtime->Set("legacy_id", legacy_id);
+		} else {
+			legacy_id = downtime->Get("legacy_id");
 		}
 
+		if (legacy_id >= m_NextDowntimeID)
+			m_NextDowntimeID = legacy_id + 1;
+
+		if (m_LegacyDowntimeCache.find(legacy_id) != m_LegacyDowntimeCache.end()) {
+			/* The legacy_id is already in use by another downtime;
+			 * this shouldn't usually happen - assign it a new ID. */
+			legacy_id = m_NextDowntimeID;
+			m_NextDowntimeID++;
+			downtime->Set("legacy_id", legacy_id);
+		}
+
+		m_LegacyDowntimeCache[legacy_id] = id;
 		m_DowntimeCache[id] = owner;
 	}
 }

lib/icinga/downtimeprocessor.h

@@ -47,6 +47,7 @@ public:
 	static bool IsDowntimeActive(const Dictionary::Ptr& downtime);
 
 	static void InvalidateDowntimeCache(void);
+	static void ValidateDowntimeCache(void);
 
 private:
 	static int m_NextDowntimeID;
@@ -58,7 +59,6 @@ private:
 	DowntimeProcessor(void);
 
 	static void AddDowntimesToCache(const DynamicObject::Ptr& owner);
-	static void ValidateDowntimeCache(void);
 };
 
 }

lib/icinga/host.cpp

@@ -120,11 +120,15 @@ Dictionary::Ptr Host::GetMacros(void) const
 
 Dictionary::Ptr Host::GetDowntimes(void) const
 {
+	DowntimeProcessor::ValidateDowntimeCache();
+
 	return Get("downtimes");
 }
 
 Dictionary::Ptr Host::GetComments(void) const
 {
+	CommentProcessor::ValidateCommentCache();
+
 	return Get("comments");
 }
 

lib/icinga/service.cpp

@@ -120,11 +120,15 @@ Dictionary::Ptr Service::GetMacros(void) const
 
 Dictionary::Ptr Service::GetDowntimes(void) const
 {
+	DowntimeProcessor::ValidateDowntimeCache();
+
 	return Get("downtimes");
 }
 
 Dictionary::Ptr Service::GetComments(void) const
 {
+	CommentProcessor::ValidateCommentCache();
+
 	return Get("comments");
 }