1 files changed, 96 insertions, 92 deletions

diff --git a/modules-available/sysconfig/inc/configtgz.inc.php b/modules-available/sysconfig/inc/configtgz.inc.php
index 98f29753..8ac87908 100644
--- a/modules-available/sysconfig/inc/configtgz.inc.php
+++ b/modules-available/sysconfig/inc/configtgz.inc.php
@@ -4,29 +4,28 @@ class ConfigTgz
 {
 	
 	private $configId = 0;
-	private $configTitle = false;
-	private $file = false;
+	private $configTitle = '';
+	private $file = '';
 	private $modules = array();
 
 	private function __construct()
 	{
-		;
 	}
 
-	public function id()
+	public function id(): int
 	{
 		return $this->configId;
 	}
 	
-	public function title()
+	public function title(): string
 	{
 		return $this->configTitle;
 	}
 	
-	public function areAllModulesUpToDate()
+	public function areAllModulesUpToDate(): bool
 	{
 		if (!$this->configId > 0)
-			Util::traceError('ConfigTgz::areAllModulesUpToDate called on un-inserted config.tgz!');
+			ErrorHandler::traceError('ConfigTgz::areAllModulesUpToDate called on un-inserted config.tgz!');
 		foreach ($this->modules as $module) {
 			if (!empty($module['filepath']) && file_exists($module['filepath'])) {
 				if ($module['status'] !== 'OK')
@@ -38,24 +37,29 @@ class ConfigTgz
 		return true;
 	}
 	
-	public function isActive()
+	public function isActive(): bool
 	{
 		return readlink(CONFIG_HTTP_DIR . '/default/config.tgz') === $this->file;
 	}
-	
-	public function getModuleIds()
+
+	/**
+	 * @return int[]
+	 */
+	public function getModuleIds(): array
 	{
-		$ret = array();
+		$ret = [];
 		foreach ($this->modules as $module) {
-			$ret[] = $module['moduleid'];
+			$ret[] = (int)$module['moduleid'];
 		}
 		return $ret;
 	}
-		
-	public function update($title, $moduleIds)
+
+	/**
+	 * @param string $title New title for module
+	 * @param int[] $moduleIds List of modules to include in this config
+	 */
+	public function update(string $title, array $moduleIds): void
 	{
-		if (!is_array($moduleIds))
-			return false;
 		if (!empty($title)) {
 			$this->configTitle = $title;
 		}
@@ -69,7 +73,7 @@ class ConfigTgz
 		// Delete old connections
 		Database::exec("DELETE FROM configtgz_x_module WHERE configid = :configid", array('configid' => $this->configId));
 		// Make connection
-		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
+		foreach ($res as $row) {
 			Database::exec("INSERT INTO configtgz_x_module (configid, moduleid) VALUES (:configid, :moduleid)", array(
 				'configid' => $this->configId,
 				'moduleid' => $row['moduleid']
@@ -83,20 +87,19 @@ class ConfigTgz
 			'status' => 'OUTDATED',
 			'now' => time(),
 		));
-		return true;
 	}
 	
 	/**
 	 * 
-	 * @param bool $deleteOnError
-	 * @param int $timeoutMs
-	 * @param string|null $parentTask parent task to order this rebuild after
+	 * @param bool $deleteOnError Delete this config in case of error?
+	 * @param int $timeoutMs max time to wait for completion
+	 * @param string|null $parentTask parent task to order this (re)build after
 	 * @return string|bool true=success, false=error, string=taskid, still running
 	 */
-	public function generate($deleteOnError = false, $timeoutMs = 0, $parentTask = null)
+	public function generate(bool $deleteOnError = false, int $timeoutMs = 0, ?string $parentTask = null)
 	{
-		if (!($this->configId > 0) || !is_array($this->modules) || $this->file === false)
-			Util::traceError ('configId <= 0 or modules not array in ConfigTgz::rebuild()');
+		if (!($this->configId > 0) || empty($this->file))
+			ErrorHandler::traceError('configId <= 0 or no file in ConfigTgz::rebuild()');
 		$files = array();
 		// Get all config modules for system config
 		foreach ($this->modules as $module) {
@@ -134,33 +137,34 @@ class ConfigTgz
 		return $task['id'];
 	}
 
-	public function delete()
+	public function delete(): bool
 	{
 		if ($this->configId === 0)
-			Util::traceError('ConfigTgz::delete called with invalid config id!');
+			ErrorHandler::traceError('ConfigTgz::delete called with invalid config id!');
 		$ret = Database::exec("DELETE FROM configtgz WHERE configid = :configid LIMIT 1",
 				['configid' => $this->configId], true);
 		if ($ret !== false) {
-			if ($this->file !== false)
+			if (!empty($this->file)) {
 				Taskmanager::submit('DeleteFile', array('file' => $this->file), true);
+			}
 			$this->configId = 0;
-			$this->modules = false;
-			$this->file = false;
+			$this->modules = [];
+			$this->file = '';
 		}
 		return $ret !== false;
 	}
 	
-	public function markOutdated()
+	public function markOutdated(): void
 	{
 		if ($this->configId === 0)
-			Util::traceError('ConfigTgz::markOutdated called with invalid config id!');
-		return $this->mark('OUTDATED');
+			ErrorHandler::traceError('ConfigTgz::markOutdated called with invalid config id!');
+		$this->mark('OUTDATED');
 	}
 	
-	private function markUpdated($task)
+	private function markUpdated(array $task): void
 	{
 		if ($this->configId === 0)
-			Util::traceError('ConfigTgz::markUpdated called with invalid config id!');
+			ErrorHandler::traceError('ConfigTgz::markUpdated called with invalid config id!');
 		if ($this->areAllModulesUpToDate()) {
 			if (empty($task['data']['warnings'])) {
 				$warnings = '';
@@ -170,7 +174,7 @@ class ConfigTgz
 				// Get mapping of moduleid to module name for prettier log display
 				$res = Database::simpleQuery('SELECT moduleid, title FROM configtgz_module');
 				$mods = [];
-				while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
+				foreach ($res as $row) {
 					$mods[$row['moduleid']] = $row['title'];
 				}
 				// Now extract module id from filename and if applicable, replace filename by module name
@@ -186,27 +190,28 @@ class ConfigTgz
 				'status' => 'OK',
 				'warnings' => $warnings,
 			]);
-			return 'OK';
+			return;
 		}
-		return $this->mark('OUTDATED');
+		$this->mark('OUTDATED');
 	}
 	
-	private function markFailed()
+	private function markFailed(): void
 	{
 		if ($this->configId === 0)
-			Util::traceError('ConfigTgz::markFailed called with invalid config id!');
-		if ($this->file === false || !file_exists($this->file))
-			return $this->mark('MISSING');
-		return $this->mark('OUTDATED');
+			ErrorHandler::traceError('ConfigTgz::markFailed called with invalid config id!');
+		if (empty($this->file) || !file_exists($this->file)) {
+			$this->mark('MISSING');
+		} else {
+			$this->mark('OUTDATED');
+		}
 	}
 	
-	private function mark($status)
+	private function mark($status): void
 	{
 		Database::exec("UPDATE configtgz SET status = :status WHERE configid = :configid LIMIT 1", [
 			'configid' => $this->configId,
 			'status' => $status,
 		]);
-		return $status;
 	}
 
 	/*
@@ -218,12 +223,12 @@ class ConfigTgz
 	 * @param string $destFile where to store final result
 	 * @return false|array taskmanager task
 	 */
-	private static function recompress($files, $destFile, $parentTask = null)
+	private static function recompress(array $files, string $destFile, $parentTask = null)
 	{
 		// Get stuff other modules want to inject
 		$handler = function($hook) {
 			include $hook->file;
-			return isset($file) ? $file : false;
+			return $file ?? false;
 		};
 		foreach (Hook::load('config-tgz') as $hook) {
 			$file = $handler($hook);
@@ -246,15 +251,15 @@ class ConfigTgz
 	 * on each one. This mostly makes sense to call if a global module
 	 * that is injected via a hook has changed.
 	 */
-	public static function rebuildAllConfigs()
+	public static function rebuildAllConfigs(): void
 	{
 		Database::exec("UPDATE configtgz SET status = :status", array(
 			'status' => 'OUTDATED'
 		));
 		$res = Database::simpleQuery("SELECT configid FROM configtgz");
-		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
+		foreach ($res as $row) {
 			$configTgz = self::get($row['configid']);
-			if ($configTgz !== false) {
+			if ($configTgz !== null) {
 				$configTgz->generate();
 			}
 		}
@@ -263,12 +268,10 @@ class ConfigTgz
 	/**
 	 * @param string $title Title of config
 	 * @param int[] $moduleIds Modules to include in config
-	 * @return false|ConfigTgz The module instance, false on error
+	 * @return ConfigTgz The module instance
 	 */
-	public static function insert($title, $moduleIds)
+	public static function insert(string $title, array $moduleIds): ConfigTgz
 	{
-		if (!is_array($moduleIds))
-			return false;
 		$instance = new ConfigTgz;
 		$instance->configTitle = $title;
 		// Create output file name (config.tgz)
@@ -290,7 +293,7 @@ class ConfigTgz
 		}
 		$res = Database::simpleQuery("SELECT moduleid, moduletype, filepath, status FROM configtgz_module WHERE moduleid IN ($idstr)");
 		// Make connection
-		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
+		foreach ($res as $row) {
 			Database::exec("INSERT INTO configtgz_x_module (configid, moduleid) VALUES (:configid, :moduleid)", array(
 				'configid' => $instance->configId,
 				'moduleid' => $row['moduleid']
@@ -300,53 +303,53 @@ class ConfigTgz
 		return $instance;
 	}
 
-	public static function get($configId)
+	/**
+	 * @param array{configid: int, title:  string, filepath: string} $row Input data, fields mandatory
+	 */
+	private static function instanceFromRow(array $row): ConfigTgz
 	{
-		$ret = Database::queryFirst("SELECT configid, title, filepath FROM configtgz WHERE configid = :configid", array(
-			'configid' => $configId
-		));
-		if ($ret === false)
-			return false;
 		$instance = new ConfigTgz;
-		$instance->configId = $ret['configid'];
-		$instance->configTitle = $ret['title'];
-		$instance->file = $ret['filepath'];
-		$ret = Database::simpleQuery("SELECT moduleid, moduletype, filepath, status FROM configtgz_x_module "
+		$instance->configId = $row['configid'];
+		$instance->configTitle = $row['title'];
+		$instance->file = $row['filepath'];
+		$innerRes = Database::simpleQuery("SELECT moduleid, moduletype, filepath, status FROM configtgz_x_module "
 			. " INNER JOIN configtgz_module USING (moduleid) "
 			. " WHERE configid = :configid", array('configid' => $instance->configId));
 		$instance->modules = array();
-		while ($row = $ret->fetch(PDO::FETCH_ASSOC)) {
-			$instance->modules[] = $row;
+		foreach ($innerRes as $innerRow) {
+			$instance->modules[] = $innerRow;
 		}
 		return $instance;
 	}
 
+	public static function get(int $configId): ?ConfigTgz
+	{
+		$ret = Database::queryFirst("SELECT configid, title, filepath FROM configtgz WHERE configid = :configid", array(
+			'configid' => $configId
+		));
+		if ($ret === false)
+			return null;
+		return self::instanceFromRow($ret);
+	}
+
 	/**
 	 * @param int $moduleId ID of config module
-	 * @return ConfigTgz[]|false
+	 * @return ConfigTgz[]
 	 */
-	public static function getAllForModule($moduleId)
+	public static function getAllForModule(int $moduleId): array
 	{
 		$res = Database::simpleQuery("SELECT configid, title, filepath FROM configtgz_x_module "
 			. " INNER JOIN configtgz USING (configid) "
 			. " WHERE moduleid = :moduleid", array(
 			'moduleid' => $moduleId
 		));
-		if ($res === false)
-			return false;
+		if ($res === false) {
+			EventLog::warning('ConfigTgz::getAllForModule failed: ' . Database::lastError());
+			return [];
+		}
 		$list = array();
-		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
-			$instance = new ConfigTgz;
-			$instance->configId = $row['configid'];
-			$instance->configTitle = $row['title'];
-			$instance->file = $row['filepath'];
-			$innerRes = Database::simpleQuery("SELECT moduleid, moduletype, filepath, status FROM configtgz_x_module "
-				. " INNER JOIN configtgz_module USING (moduleid) "
-				. " WHERE configid = :configid", array('configid' => $instance->configId));
-			$instance->modules = array();
-			while ($innerRow = $innerRes->fetch(PDO::FETCH_ASSOC)) {
-				$instance->modules[] = $innerRow;
-			}
+		foreach ($res as $row) {
+			$instance = self::instanceFromRow($row);
 			$list[] = $instance;
 		}
 		return $list;
@@ -356,31 +359,32 @@ class ConfigTgz
 	 * Called when (re)generating a config tgz failed, so we can
 	 * update the status in the DB and add a server log entry.
 	 *
-	 * @param array $task
 	 * @param array $args contains 'configid' and optionally 'deleteOnError'
 	 */
-	public static function generateFailed($task, $args)
+	public static function generateFailed(array $task, array $args): void
 	{
 		if (!isset($args['configid']) || !is_numeric($args['configid'])) {
 			EventLog::warning('Ignoring generateFailed event as it has no configid assigned.');
 			return;
 		}
 		$config = self::get($args['configid']);
-		if ($config === false) {
+		if ($config === null) {
 			EventLog::warning('generateFailed callback for config id ' . $args['configid'] . ', but no instance could be generated.');
 			return;
 		}
-		if (isset($task['data']['error']))
+		if (isset($task['data']['error'])) {
 			$error = $task['data']['error'];
-		elseif (isset($task['data']['messages']))
+		} elseif (isset($task['data']['messages'])) {
 			$error = $task['data']['messages'];
-		else
+		} else {
 			$error = '';
+		}
 		EventLog::failure("Generating config.tgz '" . $config->configTitle . "' failed.", $error);
-		if ($args['deleteOnError'])
+		if ($args['deleteOnError']) {
 			$config->delete();
-		else
+		} else {
 			$config->markFailed();
+		}
 	}
 
 	/**
@@ -389,14 +393,14 @@ class ConfigTgz
 	 * @param array $task the task object
 	 * @param array $args contains 'configid' and optionally 'deleteOnError'
 	 */
-	public static function generateSucceeded($task, $args)
+	public static function generateSucceeded(array $task, array $args): void
 	{
 		if (!isset($args['configid']) || !is_numeric($args['configid'])) {
 			EventLog::warning('Ignoring generateSucceeded event as it has no configid assigned.');
 			return;
 		}
 		$config = self::get($args['configid']);
-		if ($config === false) {
+		if ($config === null) {
 			EventLog::warning('generateSucceeded callback for config id ' . $args['configid'] . ', but no instance could be generated.');
 			return;
 		}