From 9b5c8c31522c2a7d02114f5def5fd823c5aae018 Mon Sep 17 00:00:00 2001
From: Yonas Habteab <yonas.habteab@icinga.com>
Date: Mon, 9 Sep 2024 17:25:13 +0200
Subject: [PATCH] Type: Simplify sort by load dependencies algorithm

---
 lib/base/type.cpp | 56 ++++++++++++++++-------------------------------
 lib/base/type.hpp | 14 ++++++++++++
 2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/lib/base/type.cpp b/lib/base/type.cpp
index 70dd6333ee0..f6539b7c447 100644
--- a/lib/base/type.cpp
+++ b/lib/base/type.cpp
@@ -7,9 +7,7 @@
 #include "base/scriptglobal.hpp"
 #include "base/namespace.hpp"
 #include "base/objectlock.hpp"
-#include <algorithm>
 #include <functional>
-#include <unordered_map>
 
 using namespace icinga;
 
@@ -41,53 +39,37 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() {
 static std::vector<Type::Ptr> l_SortedByLoadDependencies;
 static Atomic l_SortingByLoadDependenciesDone (false);
 
-typedef std::unordered_map<Type*, bool> Visited; // https://stackoverflow.com/a/8942986
-
 INITIALIZE_ONCE_WITH_PRIORITY([] {
-	auto types (Type::GetAllTypes());
-
-	types.erase(std::remove_if(types.begin(), types.end(), [](auto& type) {
-		return !ConfigObject::TypeInstance->IsAssignableFrom(type);
-	}), types.end());
-
-	// Depth-first search
-	std::unordered_set<Type*> unsorted;
-	Visited visited;
-	std::vector<Type::Ptr> sorted;
-
-	for (auto type : types) {
-		unsorted.emplace(type.get());
-	}
+	std::unordered_set<Type*> visited;
 
-	std::function<void(Type*)> visit ([&visit, &unsorted, &visited, &sorted](Type* type) {
-		if (unsorted.find(type) == unsorted.end()) {
+	std::function<void(Type*)> visit;
+	// Please note that this callback does not detect any cyclic load dependencies,
+	// instead, it relies on the "sort_by_load_after" unit test to fail.
+	visit = ([&visit, &visited](Type* type) {
+		if (visited.find(type) != visited.end()) {
 			return;
 		}
+		visited.emplace(type);
 
-		bool& alreadyVisited (visited.at(type));
-		VERIFY(!alreadyVisited);
-		alreadyVisited = true;
-
-		for (auto dep : type->GetLoadDependencies()) {
-			visit(dep);
+		for (auto dependency : type->GetLoadDependencies()) {
+			visit(dependency);
 		}
 
-		unsorted.erase(type);
-		sorted.emplace_back(type);
+		// We have managed to reach the final/top node in this dependency graph,
+		// so let's place them in reverse order to their final place.
+		l_SortedByLoadDependencies.emplace_back(type);
 	});
 
-	while (!unsorted.empty()) {
-		for (auto& type : types) {
-			visited[type.get()] = false;
+	// Sort the types by their load_after dependencies in a Depth-First search manner.
+	for (const Type::Ptr& type : Type::GetAllTypes()) {
+		// Note that only those types that are assignable to the dynamic ConfigObject type can have "load_after"
+		// dependencies, otherwise they are just some Icinga 2 primitive types such as Number, String, etc. and
+		// we need to ignore them.
+		if (ConfigObject::TypeInstance->IsAssignableFrom(type)) {
+			visit(type.get());
 		}
-
-		visit(*unsorted.begin());
 	}
 
-	VERIFY(sorted.size() == types.size());
-	VERIFY(sorted[0]->GetLoadDependencies().empty());
-
-	std::swap(sorted, l_SortedByLoadDependencies);
 	l_SortingByLoadDependenciesDone.store(true);
 }, InitializePriority::SortTypes);
 
diff --git a/lib/base/type.hpp b/lib/base/type.hpp
index fa78f858ed0..ee8a9b37cd4 100644
--- a/lib/base/type.hpp
+++ b/lib/base/type.hpp
@@ -82,6 +82,20 @@ class Type : public Object
 	static void Register(const Type::Ptr& type);
 	static Type::Ptr GetByName(const String& name);
 	static std::vector<Type::Ptr> GetAllTypes();
+
+	/**
+	 * Returns a list of config types sorted by their "load_after" dependencies.
+	 *
+	 * All dependencies of a given type are listed at a lower index than that of the type itself. In other words,
+	 * if a `Service` type load depends on the `Host` and `ApiListener` types, the Host and ApiListener types are
+	 * guaranteed to appear first on the list. Nevertheless, the order of the Host and ApiListener types themselves
+	 * is arbitrary if the two types are not dependent.
+	 *
+	 * It should be noted that this method will fail fatally when used prior to the completion
+	 * of namespace initialization.
+	 *
+	 * @return std::vector<Type::Ptr>
+	 */
 	static const std::vector<Ptr>& GetConfigTypesSortedByLoadDependencies();
 
 	void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty) override;