Use Host constructor/destructor/OnAttributeChange instead of OnCommitted/OnRemoved for Host->services

Fixes #3612

lib/base/dynamicobject.cpp

@@ -469,6 +469,9 @@ void DynamicObject::FlushTx(void)
 void DynamicObject::OnAttributeChanged(const String&, const Value&)
 { }
 
+void DynamicObject::OnInitCompleted(void)
+{ }
+
 DynamicObject::Ptr DynamicObject::GetObject(const String& type, const String& name)
 {
 	DynamicType::Ptr dtype = DynamicType::GetByName(type);

lib/base/dynamicobject.h

@@ -118,7 +118,7 @@ public:
 	virtual void Start(void);
 
 	const AttributeMap& GetAttributes(void) const;
-	
+
 	static DynamicObject::Ptr GetObject(const String& type, const String& name);
 
 	static void DumpObjects(const String& filename);
@@ -132,6 +132,7 @@ public:
 
 protected:
 	virtual void OnAttributeChanged(const String& name, const Value& oldValue);
+	virtual void OnInitCompleted(void);
 
 private:
 	void InternalSetAttribute(const String& name, const Value& data, double tx, bool suppressEvent = false, bool allowEditConfig = false);
@@ -145,6 +146,8 @@ private:
 	static set<DynamicObject::Ptr> m_ModifiedObjects;
 
 	void InternalApplyUpdate(const Dictionary::Ptr& serializedUpdate, int allowedTypes, bool suppressEvents);
+
+	friend class DynamicType; /* for OnInitCompleted */
 };
 
 }

lib/base/dynamictype.cpp

@@ -93,6 +93,9 @@ DynamicObject::Ptr DynamicType::CreateObject(const Dictionary::Ptr& serializedUp
 	/* apply the object's non-config attributes */
 	obj->ApplyUpdate(serializedUpdate, Attribute_All & ~Attribute_Config);
 
+	/* notify the object that it's "ready" */
+	obj->OnInitCompleted();
+
 	return obj;
 }
 

lib/config/configitem.cpp

@@ -144,7 +144,33 @@ DynamicObject::Ptr ConfigItem::Commit(void)
 {
 	Logger::Write(LogDebug, "base", "Commit called for ConfigItem Type=" + GetType() + ", Name=" + GetName());
 
-	DynamicObject::Ptr dobj = m_DynamicObject.lock();
+	/* Make sure the type is valid. */
+	DynamicType::Ptr dtype = DynamicType::GetByName(GetType());
+
+	if (!dtype)
+		BOOST_THROW_EXCEPTION(runtime_error("Type '" + GetType() + "' does not exist."));
+
+	/* Try to find an existing item with the same type and name. */
+	pair<String, String> ikey = make_pair(GetType(), GetName());
+	ItemMap::iterator it = m_Items.find(ikey);
+
+	if (it != m_Items.end()) {
+		/* Unregister the old item from its parents. */
+		ConfigItem::Ptr oldItem = it->second;
+		oldItem->UnregisterFromParents();
+
+		/* Steal the old item's children. */
+		m_ChildObjects = oldItem->m_ChildObjects;
+	}
+
+	/* Register this item with its parents. */
+	BOOST_FOREACH(const String& parentName, m_Parents) {
+		ConfigItem::Ptr parent = GetObject(GetType(), parentName);
+		parent->RegisterChild(GetSelf());
+	}
+
+	/* Register this item. */
+	m_Items[ikey] = GetSelf();
 
 	Dictionary::Ptr properties = Link();
 
@@ -166,10 +192,8 @@ DynamicObject::Ptr ConfigItem::Commit(void)
 	update->Set("attrs", attrs);
 	update->Set("configTx", DynamicObject::GetCurrentTx());
 
-	DynamicType::Ptr dtype = DynamicType::GetByName(GetType());
-
-	if (!dtype)
-		BOOST_THROW_EXCEPTION(runtime_error("Type '" + GetType() + "' does not exist."));
+	/* Update or create the object and apply the configuration settings. */
+	DynamicObject::Ptr dobj = m_DynamicObject.lock();
 
 	if (!dobj)
 		dobj = dtype->GetObject(GetName());
@@ -186,25 +210,7 @@ DynamicObject::Ptr ConfigItem::Commit(void)
 	else
 		dobj->Register();
 
-	pair<String, String> ikey = make_pair(GetType(), GetName());
-	ItemMap::iterator it = m_Items.find(ikey);
-
-	/* unregister the old item from its parents */
-	if (it != m_Items.end()) {
-		ConfigItem::Ptr oldItem = it->second;
-		oldItem->UnregisterFromParents();
-
-		/* steal the old item's children */
-		m_ChildObjects = oldItem->m_ChildObjects;
-	}
-
-	/* register this item with its parents */
-	BOOST_FOREACH(const String& parentName, m_Parents) {
-		ConfigItem::Ptr parent = GetObject(GetType(), parentName);
-		parent->RegisterChild(GetSelf());
-	}
-
-	/* We need to make a copy of the child objects becauuse the
+	/* We need to make a copy of the child objects because the
 	 * OnParentCommitted() handler is going to update the list. */
 	set<ConfigItem::WeakPtr> children = m_ChildObjects;
 
@@ -218,8 +224,6 @@ DynamicObject::Ptr ConfigItem::Commit(void)
 		child->OnParentCommitted();
 	}
 
-	m_Items[ikey] = GetSelf();
-
 	OnCommitted(GetSelf());
 
 	return dobj;
@@ -327,7 +331,7 @@ ConfigItem::Ptr ConfigItem::GetObject(const String& type, const String& name)
 void ConfigItem::Dump(ostream& fp) const
 {
 	fp << "object \"" << m_Type << "\" \"" << m_Name << "\"";
-	
+
 	if (m_Parents.size() > 0) {
 		fp << " inherits";
 
@@ -341,7 +345,7 @@ void ConfigItem::Dump(ostream& fp) const
 			fp << " \"" << name << "\"";
 		}
 	}
-	
+
 	fp << " {" << "\n";
 	m_ExpressionList->Dump(fp, 1);
 	fp << "}" << "\n";

lib/icinga/host.cpp

@@ -30,31 +30,37 @@ static AttributeDescription hostAttributes[] = {
 	{ "acknowledgement", Attribute_Replicated },
 	{ "acknowledgement_expiry", Attribute_Replicated },
 	{ "downtimes", Attribute_Replicated },
-	{ "comments", Attribute_Replicated }
+	{ "comments", Attribute_Replicated },
+	{ "convenience_services", Attribute_Transient }
 };
 
 REGISTER_TYPE(Host, hostAttributes);
 
-bool Host::m_InitializerDone = false;
-
 Host::Host(const Dictionary::Ptr& properties)
 	: DynamicObject(properties)
+{ }
+
+void Host::OnInitCompleted(void)
 {
-	if (!m_InitializerDone) {
-		ConfigItem::OnCommitted.connect(boost::bind(&Host::ObjectCommittedHandler, _1));
-		ConfigItem::OnRemoved.connect(boost::bind(&Host::ObjectRemovedHandler, _1));
-
-		m_InitializerDone = true;
-	}
-
 	HostGroup::InvalidateMembersCache();
 	DowntimeProcessor::InvalidateDowntimeCache();
+
+	Event::Post(boost::bind(&Host::UpdateSlaveServices, this));
 }
 
 Host::~Host(void)
 {
 	HostGroup::InvalidateMembersCache();
 	DowntimeProcessor::InvalidateDowntimeCache();
+
+	Dictionary::Ptr services = Get("convenience_services");
+
+	if (services) {
+		ConfigItem::Ptr service;
+		BOOST_FOREACH(tie(tuples::ignore, service), services) {
+			service->Unregister();
+		}
+	}
 }
 
 String Host::GetAlias(void) const
@@ -199,30 +205,28 @@ static void CopyServiceAttributes(TDict serviceDesc, const ConfigItemBuilder::Pt
 		builder->AddExpression("checkers", OperatorSet, checkers);
 }
 
-void Host::ObjectCommittedHandler(const ConfigItem::Ptr& item)
+void Host::UpdateSlaveServices(void)
 {
-	if (item->GetType() != "Host")
+	ConfigItem::Ptr item = ConfigItem::GetObject("Host", GetName());
+
+	/* Don't create slave services unless we own this object
+	 * and it's not a template. */
+	if (!item || IsAbstract())
 		return;
 
-	/* ignore abstract host objects */
-	if (!Host::Exists(item->GetName()))
-		return;
-
-	Host::Ptr host = Host::GetByName(item->GetName());
-
-	Dictionary::Ptr oldServices = host->Get("convenience_services");
+	Dictionary::Ptr oldServices = Get("convenience_services");
 
 	Dictionary::Ptr newServices;
 	newServices = boost::make_shared<Dictionary>();
 
-	Dictionary::Ptr serviceDescs = host->Get("services");
+	Dictionary::Ptr serviceDescs = Get("services");
 
 	if (serviceDescs) {
 		String svcname;
 		Value svcdesc;
 		BOOST_FOREACH(tie(svcname, svcdesc), serviceDescs) {
 			stringstream namebuf;
-			namebuf << item->GetName() << "-" << svcname;
+			namebuf << GetName() << "-" << svcname;
 			String name = namebuf.str();
 
 			ConfigItemBuilder::Ptr builder = boost::make_shared<ConfigItemBuilder>(item->GetDebugInfo());
@@ -232,7 +236,7 @@ void Host::ObjectCommittedHandler(const ConfigItem::Ptr& item)
 			builder->AddExpression("alias", OperatorSet, svcname);
 			builder->AddExpression("short_name", OperatorSet, svcname);
 
-			CopyServiceAttributes(host, builder);
+			CopyServiceAttributes(this, builder);
 
 			if (svcdesc.IsScalar()) {
 				builder->AddParent(svcdesc);
@@ -268,28 +272,7 @@ void Host::ObjectCommittedHandler(const ConfigItem::Ptr& item)
 		}
 	}
 
-	host->Set("convenience_services", newServices);
-}
-
-void Host::ObjectRemovedHandler(const ConfigItem::Ptr& item)
-{
-	if (item->GetType() != "Host")
-		return;
-
-	DynamicObject::Ptr host = item->GetDynamicObject();
-
-	if (!host)
-		return;
-
-	Dictionary::Ptr services = host->Get("convenience_services");
-
-	if (!services)
-		return;
-
-	ConfigItem::Ptr service;
-	BOOST_FOREACH(tie(tuples::ignore, service), services) {
-		service->Unregister();
-	}
+	Set("convenience_services", newServices);
 }
 
 void Host::OnAttributeChanged(const String& name, const Value&)
@@ -298,6 +281,8 @@ void Host::OnAttributeChanged(const String& name, const Value&)
 		HostGroup::InvalidateMembersCache();
 	else if (name == "downtimes")
 		DowntimeProcessor::InvalidateDowntimeCache();
+	else if (name == "services")
+		UpdateSlaveServices();
 }
 
 set<Service::Ptr> Host::GetServices(void) const

lib/icinga/host.h

@@ -37,6 +37,7 @@ public:
 	typedef weak_ptr<Host> WeakPtr;
 
 	Host(const Dictionary::Ptr& properties);
+	void OnInitCompleted(void);
 	~Host(void);
 
 	static bool Exists(const String& name);
@@ -78,13 +79,10 @@ protected:
 	void OnAttributeChanged(const String& name, const Value& oldValue);
 
 private:
-	static bool m_InitializerDone;
-
 	static map<String, map<String, weak_ptr<Service> > > m_ServicesCache;
 	static bool m_ServicesCacheValid;
 
-	static void ObjectCommittedHandler(const ConfigItem::Ptr& item);
-	static void ObjectRemovedHandler(const ConfigItem::Ptr& item);
+	void UpdateSlaveServices(void);
 
 	static void ValidateServicesCache(void);
 };