Server: delegate mod management & config to ServerModConfiguration (#7131)

* Server: delegate mod management & config to ServerModConfiguration (rename it to ServerModManager)

* Use c++11 range based loops
* Add unittests + experimental/default mod as a test case to permit testing mod loading in future tests

src/CMakeLists.txt

@@ -379,10 +379,12 @@ add_subdirectory(script)
 add_subdirectory(unittest)
 add_subdirectory(util)
 add_subdirectory(irrlicht_changes)
+add_subdirectory(server)
 
 set(common_SRCS
 	${database_SRCS}
 	${mapgen_SRCS}
+	${server_SRCS}
 	ban.cpp
 	chat.cpp
 	clientiface.cpp

src/mods.cpp

@@ -322,20 +322,6 @@ void ModConfiguration::resolveDependencies()
 	m_unsatisfied_mods.assign(unsatisfied.begin(), unsatisfied.end());
 }
 
-ServerModConfiguration::ServerModConfiguration(const std::string &worldpath):
-	ModConfiguration(worldpath)
-{
-	SubgameSpec gamespec = findWorldSubgame(worldpath);
-
-	// Add all game mods and all world mods
-	addModsInPath(gamespec.gamemods_path);
-	addModsInPath(worldpath + DIR_DELIM + "worldmods");
-
-	// Load normal mods
-	std::string worldmt = worldpath + DIR_DELIM + "world.mt";
-	addModsFromConfig(worldmt, gamespec.addon_mods_paths);
-}
-
 #ifndef SERVER
 ClientModConfiguration::ClientModConfiguration(const std::string &path):
 	ModConfiguration(path)

src/mods.h

@@ -78,7 +78,7 @@ public:
 		return m_unsatisfied_mods.empty();
 	}
 
-	std::vector<ModSpec> getMods()
+	const std::vector<ModSpec> &getMods() const
 	{
 		return m_sorted_mods;
 	}
@@ -102,6 +102,13 @@ protected:
 	void addModsFromConfig(const std::string &settings_path, const std::set<std::string> &mods);
 
 	void checkConflictsAndDeps();
+protected:
+	// list of mods sorted such that they can be loaded in the
+	// given order with all dependencies being fullfilled. I.e.,
+	// every mod in this list has only dependencies on mods which
+	// appear earlier in the vector.
+	std::vector<ModSpec> m_sorted_mods;
+
 private:
 	// move mods from m_unsatisfied_mods to m_sorted_mods
 	// in an order that satisfies dependencies
@@ -112,12 +119,6 @@ private:
 	// only the ones with really unsatisfied dependencies.
 	std::vector<ModSpec> m_unsatisfied_mods;
 
-	// list of mods sorted such that they can be loaded in the
-	// given order with all dependencies being fullfilled. I.e.,
-	// every mod in this list has only dependencies on mods which
-	// appear earlier in the vector.
-	std::vector<ModSpec> m_sorted_mods;
-
 	// set of mod names for which an unresolved name conflict
 	// exists. A name conflict happens when two or more mods
 	// at the same level have the same name but different paths.
@@ -132,13 +133,6 @@ private:
 
 };
 
-class ServerModConfiguration: public ModConfiguration
-{
-public:
-	ServerModConfiguration(const std::string &worldpath);
-
-};
-
 #ifndef SERVER
 class ClientModConfiguration: public ModConfiguration
 {

src/server.cpp

@@ -58,6 +58,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/serialize.h"
 #include "util/thread.h"
 #include "defaultsettings.h"
+#include "server/mods.h"
 #include "util/base64.h"
 #include "util/sha1.h"
 #include "util/hex.h"
@@ -203,12 +204,12 @@ Server::Server(
 	std::string ban_path = m_path_world + DIR_DELIM "ipban.txt";
 	m_banmanager = new BanManager(ban_path);
 
-	ServerModConfiguration modconf(m_path_world);
+	m_modmgr = std::unique_ptr<ServerModManager>(new ServerModManager(
-	m_mods = modconf.getMods();
+		m_path_world));
-	std::vector<ModSpec> unsatisfied_mods = modconf.getUnsatisfiedMods();
+	std::vector<ModSpec> unsatisfied_mods = m_modmgr->getUnsatisfiedMods();
 	// complain about mods with unsatisfied dependencies
-	if (!modconf.isConsistent()) {
+	if (!m_modmgr->isConsistent()) {
-		modconf.printUnsatisfiedModsError();
+		m_modmgr->printUnsatisfiedModsError();
 	}
 
 	//lock environment
@@ -224,27 +225,9 @@ Server::Server(
 
 	m_script->loadMod(getBuiltinLuaPath() + DIR_DELIM "init.lua", BUILTIN_MOD_NAME);
 
-	// Print mods
+	m_mods = m_modmgr->getMods();
-	infostream << "Server: Loading mods: ";
+
-	for (std::vector<ModSpec>::const_iterator i = m_mods.begin();
+	m_modmgr->loadMods(m_script);
-			i != m_mods.end(); ++i) {
-		infostream << (*i).name << " ";
-	}
-	infostream << std::endl;
-	// Load and run "mod" scripts
-	for (std::vector<ModSpec>::const_iterator it = m_mods.begin();
-			it != m_mods.end(); ++it) {
-		const ModSpec &mod = *it;
-		if (!string_allowed(mod.name, MODNAME_ALLOWED_CHARS)) {
-			throw ModError("Error loading mod \"" + mod.name +
-				"\": Mod name does not follow naming conventions: "
-				"Only characters [a-z0-9_] are allowed.");
-		}
-		std::string script_path = mod.path + DIR_DELIM + "init.lua";
-		infostream << "  [" << padStringRight(mod.name, 12) << "] [\""
-				<< script_path << "\"]" << std::endl;
-		m_script->loadMod(script_path, mod.name);
-	}
 
 	// Read Textures and calculate sha1 sums
 	fillMediaCache();
@@ -580,7 +563,7 @@ void Server::AsyncRunStep(bool initial_step)
 					m_lag,
 					m_gamespec.id,
 					Mapgen::getMapgenName(m_emerge->mgparams->mgtype),
-					m_mods,
+					m_modmgr->getMods(),
 					m_dedicated);
 			counter = 0.01;
 		}
@@ -2237,13 +2220,7 @@ void Server::fillMediaCache()
 
 	// Collect all media file paths
 	std::vector<std::string> paths;
-	for (const ModSpec &mod : m_mods) {
+	m_modmgr->getModsMediaPaths(paths);
-		paths.push_back(mod.path + DIR_DELIM + "textures");
-		paths.push_back(mod.path + DIR_DELIM + "sounds");
-		paths.push_back(mod.path + DIR_DELIM + "media");
-		paths.push_back(mod.path + DIR_DELIM + "models");
-		paths.push_back(mod.path + DIR_DELIM + "locale");
-	}
 	fs::GetRecursiveDirs(paths, m_gamespec.path + DIR_DELIM + "textures");
 	fs::GetRecursiveDirs(paths, porting::path_user + DIR_DELIM + "textures" + DIR_DELIM + "server");
 
@@ -3326,22 +3303,19 @@ IWritableCraftDefManager *Server::getWritableCraftDefManager()
 	return m_craftdef;
 }
 
+const std::vector<ModSpec> & Server::getMods() const
+{
+	return m_modmgr->getMods();
+}
+
 const ModSpec *Server::getModSpec(const std::string &modname) const
 {
-	std::vector<ModSpec>::const_iterator it;
+	return m_modmgr->getModSpec(modname);
-	for (it = m_mods.begin(); it != m_mods.end(); ++it) {
-		const ModSpec &mod = *it;
-		if (mod.name == modname)
-			return &mod;
-	}
-	return NULL;
 }
 
 void Server::getModNames(std::vector<std::string> &modlist)
 {
-	std::vector<ModSpec>::iterator it;
+	m_modmgr->getModNames(modlist);
-	for (it = m_mods.begin(); it != m_mods.end(); ++it)
-		modlist.push_back(it->name);
 }
 
 std::string Server::getBuiltinLuaPath()

src/server.h

@@ -61,6 +61,7 @@ class ServerEnvironment;
 struct SimpleSoundSpec;
 struct CloudParams;
 class ServerThread;
+class ServerModManager;
 
 enum ClientDeletionReason {
 	CDR_LEAVE,
@@ -268,7 +269,7 @@ public:
 	NodeDefManager* getWritableNodeDefManager();
 	IWritableCraftDefManager* getWritableCraftDefManager();
 
-	virtual const std::vector<ModSpec> &getMods() const { return m_mods; }
+	virtual const std::vector<ModSpec> &getMods() const;
 	virtual const ModSpec* getModSpec(const std::string &modname) const;
 	void getModNames(std::vector<std::string> &modlist);
 	std::string getBuiltinLuaPath();
@@ -541,6 +542,7 @@ private:
 	EventManager *m_event;
 
 	// Mods
+	std::unique_ptr<ServerModManager> m_modmgr;
 	std::vector<ModSpec> m_mods;
 
 	/*

src/server/CMakeLists.txt

@@ -0,0 +1,3 @@
+set(server_SRCS
+	${CMAKE_CURRENT_SOURCE_DIR}/mods.cpp
+	PARENT_SCOPE)

src/server/mods.cpp

@@ -0,0 +1,100 @@
+/*
+Minetest
+Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#include "mods.h"
+#include "filesys.h"
+#include "log.h"
+#include "scripting_server.h"
+#include "subgame.h"
+
+/**
+ * Manage server mods
+ *
+ * All new calls to this class must be tested in test_servermodmanager.cpp
+ */
+
+/**
+ * Creates a ServerModManager which targets worldpath
+ * @param worldpath
+ */
+ServerModManager::ServerModManager(const std::string &worldpath) :
+		ModConfiguration(worldpath)
+{
+	SubgameSpec gamespec = findWorldSubgame(worldpath);
+
+	// Add all game mods and all world mods
+	addModsInPath(gamespec.gamemods_path);
+	addModsInPath(worldpath + DIR_DELIM + "worldmods");
+
+	// Load normal mods
+	std::string worldmt = worldpath + DIR_DELIM + "world.mt";
+	addModsFromConfig(worldmt, gamespec.addon_mods_paths);
+}
+
+// This function cannot be currenctly easily tested but it should be ASAP
+void ServerModManager::loadMods(ServerScripting *script)
+{
+	// Print mods
+	infostream << "Server: Loading mods: ";
+	for (const ModSpec &mod : m_sorted_mods) {
+		infostream << mod.name << " ";
+	}
+	infostream << std::endl;
+	// Load and run "mod" scripts
+	for (const ModSpec &mod : m_sorted_mods) {
+		if (!string_allowed(mod.name, MODNAME_ALLOWED_CHARS)) {
+			throw ModError("Error loading mod \"" + mod.name +
+					"\": Mod name does not follow naming "
+					"conventions: "
+					"Only characters [a-z0-9_] are allowed.");
+		}
+		std::string script_path = mod.path + DIR_DELIM + "init.lua";
+		infostream << "  [" << padStringRight(mod.name, 12) << "] [\""
+			   << script_path << "\"]" << std::endl;
+		script->loadMod(script_path, mod.name);
+	}
+}
+
+const ModSpec *ServerModManager::getModSpec(const std::string &modname) const
+{
+	std::vector<ModSpec>::const_iterator it;
+	for (it = m_sorted_mods.begin(); it != m_sorted_mods.end(); ++it) {
+		const ModSpec &mod = *it;
+		if (mod.name == modname)
+			return &mod;
+	}
+	return NULL;
+}
+
+void ServerModManager::getModNames(std::vector<std::string> &modlist) const
+{
+	for (const ModSpec &spec : m_sorted_mods)
+		modlist.push_back(spec.name);
+}
+
+void ServerModManager::getModsMediaPaths(std::vector<std::string> &paths) const
+{
+	for (const ModSpec &spec : m_sorted_mods) {
+		paths.push_back(spec.path + DIR_DELIM + "textures");
+		paths.push_back(spec.path + DIR_DELIM + "sounds");
+		paths.push_back(spec.path + DIR_DELIM + "media");
+		paths.push_back(spec.path + DIR_DELIM + "models");
+		paths.push_back(spec.path + DIR_DELIM + "locale");
+	}
+}

src/server/mods.h

@@ -0,0 +1,43 @@
+/*
+Minetest
+Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#pragma once
+
+#include "../mods.h"
+
+class ServerScripting;
+
+/**
+ * Manage server mods
+ *
+ * All new calls to this class must be tested in test_servermodmanager.cpp
+ */
+class ServerModManager : public ModConfiguration
+{
+public:
+	/**
+	 * Creates a ServerModManager which targets worldpath
+	 * @param worldpath
+	 */
+	ServerModManager(const std::string &worldpath);
+	void loadMods(ServerScripting *script);
+	const ModSpec *getModSpec(const std::string &modname) const;
+	void getModNames(std::vector<std::string> &modlist) const;
+	void getModsMediaPaths(std::vector<std::string> &paths) const;
+};

src/unittest/CMakeLists.txt

@@ -22,6 +22,7 @@ set (UNITTEST_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/test_serialization.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_settings.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_socket.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/test_servermodmanager.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_threading.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_utilities.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_voxelarea.cpp
@@ -33,3 +34,11 @@ set (UNITTEST_CLIENT_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/test_gameui.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/test_keycode.cpp
 	PARENT_SCOPE)
+
+set (TEST_WORLDDIR ${CMAKE_CURRENT_SOURCE_DIR}/test_world)
+set (TEST_SUBGAME_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../games/minimal)
+
+configure_file(
+	"${CMAKE_CURRENT_SOURCE_DIR}/test_config.h.in"
+	"${PROJECT_BINARY_DIR}/test_config.h"
+)

src/unittest/test_config.h.in

@@ -0,0 +1,6 @@
+// Filled in by the build system
+
+#pragma once
+
+#define TEST_WORLDDIR "@TEST_WORLDDIR@"
+#define TEST_SUBGAME_PATH "@TEST_SUBGAME_PATH@"

src/unittest/test_servermodmanager.cpp

@@ -0,0 +1,165 @@
+/*
+Minetest
+Copyright (C) 2018 nerzhul, Loic Blot <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#include "test.h"
+#include <algorithm>
+#include "server/mods.h"
+#include "test_config.h"
+
+class TestServerModManager : public TestBase
+{
+public:
+	TestServerModManager() { TestManager::registerTestModule(this); }
+	const char *getName() { return "TestServerModManager"; }
+
+	void runTests(IGameDef *gamedef);
+
+	void testCreation();
+	void testIsConsistent();
+	void testUnsatisfiedMods();
+	void testGetMods();
+	void testGetModsWrongDir();
+	void testGetModspec();
+	void testGetModNamesWrongDir();
+	void testGetModNames();
+	void testGetModMediaPathsWrongDir();
+	void testGetModMediaPaths();
+};
+
+static TestServerModManager g_test_instance;
+
+void TestServerModManager::runTests(IGameDef *gamedef)
+{
+	const char *saved_env_mt_subgame_path = getenv("MINETEST_SUBGAME_PATH");
+#ifdef WIN32
+	{
+		std::string subgame_path("MINETEST_SUBGAME_PATH=");
+		subgame_path.append(TEST_SUBGAME_PATH);
+		_putenv(subgame_path.c_str());
+	}
+#else
+	setenv("MINETEST_SUBGAME_PATH", TEST_SUBGAME_PATH, 1);
+#endif
+
+	TEST(testCreation);
+	TEST(testIsConsistent);
+	TEST(testGetModsWrongDir);
+	TEST(testUnsatisfiedMods);
+	TEST(testGetMods);
+	TEST(testGetModspec);
+	TEST(testGetModNamesWrongDir);
+	TEST(testGetModNames);
+	TEST(testGetModMediaPathsWrongDir);
+	TEST(testGetModMediaPaths);
+
+#ifdef WIN32
+	{
+		std::string subgame_path("MINETEST_SUBGAME_PATH=");
+		subgame_path.append(saved_env_mt_subgame_path);
+		_putenv(subgame_path.c_str());
+	}
+#else
+	setenv("MINETEST_SUBGAME_PATH", saved_env_mt_subgame_path, 1);
+#endif
+}
+
+void TestServerModManager::testCreation()
+{
+	ServerModManager sm(TEST_WORLDDIR);
+}
+
+void TestServerModManager::testGetModsWrongDir()
+{
+	// Test in non worlddir to ensure no mods are found
+	ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + "..");
+	UASSERTEQ(bool, sm.getMods().empty(), true);
+}
+
+void TestServerModManager::testUnsatisfiedMods()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	UASSERTEQ(bool, sm.getUnsatisfiedMods().empty(), true);
+}
+
+void TestServerModManager::testIsConsistent()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	UASSERTEQ(bool, sm.isConsistent(), true);
+}
+
+void TestServerModManager::testGetMods()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	const auto &mods = sm.getMods();
+	UASSERTEQ(bool, mods.empty(), false);
+
+	// Ensure we found default mod inside the test folder
+	bool default_found = false;
+	for (const auto &m : mods) {
+		if (m.name == "default")
+			default_found = true;
+
+		// Verify if paths are not empty
+		UASSERTEQ(bool, m.path.empty(), false);
+	}
+
+	UASSERTEQ(bool, default_found, true);
+}
+
+void TestServerModManager::testGetModspec()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	UASSERTEQ(const ModSpec *, sm.getModSpec("wrongmod"), NULL);
+	UASSERT(sm.getModSpec("default") != NULL);
+}
+
+void TestServerModManager::testGetModNamesWrongDir()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + "..");
+	std::vector<std::string> result;
+	sm.getModNames(result);
+	UASSERTEQ(bool, result.empty(), true);
+}
+
+void TestServerModManager::testGetModNames()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	std::vector<std::string> result;
+	sm.getModNames(result);
+	UASSERTEQ(bool, result.empty(), false);
+	UASSERT(std::find(result.begin(), result.end(), "default") != result.end());
+}
+
+void TestServerModManager::testGetModMediaPathsWrongDir()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + "..");
+	std::vector<std::string> result;
+	sm.getModsMediaPaths(result);
+	UASSERTEQ(bool, result.empty(), true);
+}
+
+void TestServerModManager::testGetModMediaPaths()
+{
+	ServerModManager sm(std::string(TEST_WORLDDIR));
+	std::vector<std::string> result;
+	sm.getModsMediaPaths(result);
+	UASSERTEQ(bool, result.empty(), false);
+	// We should have 5 folders for each mod (textures, media, locale, model, sounds)
+	UASSERTEQ(unsigned long, result.size() % 5, 0);
+}

src/unittest/test_world/world.mt

@@ -0,0 +1 @@
+gameid = minimal