[sysconfig] Display duplicate files for generated system config

Implements #3670

6 files changed, 69 insertions, 23 deletions

diff --git a/inc/taskmanagercallback.inc.php b/inc/taskmanagercallback.inc.php
index 21bece38..5f153baa 100644
--- a/inc/taskmanagercallback.inc.php
+++ b/inc/taskmanagercallback.inc.php
@@ -159,7 +159,7 @@ class TaskmanagerCallback
 		if (Taskmanager::isFailed($task)) {
 			ConfigTgz::generateFailed($task, $args);
 		} else {
-			ConfigTgz::generateSucceeded($args);
+			ConfigTgz::generateSucceeded($task, $args);
 		}
 	}
 	
diff --git a/modules-available/sysconfig/addmodule_custommodule.inc.php b/modules-available/sysconfig/addmodule_custommodule.inc.php
index 86b98206..0c3c299d 100644
--- a/modules-available/sysconfig/addmodule_custommodule.inc.php
+++ b/modules-available/sysconfig/addmodule_custommodule.inc.php
@@ -120,7 +120,7 @@ class CustomModule_CompressModule extends AddModule_Base
 		$destFile = tempnam(sys_get_temp_dir(), 'bwlp-') . '.tgz';
 		Taskmanager::submit('RecompressArchive', array(
 			'id' => $this->taskId,
-			'inputFiles' => array($tempfile),
+			'inputFiles' => [$tempfile => false],
 			'outputFile' => $destFile
 		), true);
 		$status = Taskmanager::waitComplete($this->taskId, 5000);
diff --git a/modules-available/sysconfig/inc/configtgz.inc.php b/modules-available/sysconfig/inc/configtgz.inc.php
index c1df2c93..ff9e306d 100644
--- a/modules-available/sysconfig/inc/configtgz.inc.php
+++ b/modules-available/sysconfig/inc/configtgz.inc.php
@@ -88,9 +88,7 @@ class ConfigTgz
 	 * 
 	 * @param bool $deleteOnError
 	 * @param int $timeoutMs
-	 * @return string - OK (success)
-	 *		- OUTDATED (updating failed, but old version still exists)
-	 *		- MISSING (failed and no old version available)
+	 * @return string|bool true=success, false=error, string=taskid, still running
 	 */
 	public function generate($deleteOnError = false, $timeoutMs = 0)
 	{
@@ -100,18 +98,20 @@ class ConfigTgz
 		// Get all config modules for system config
 		foreach ($this->modules as $module) {
 			if (!empty($module['filepath']) && file_exists($module['filepath'])) {
-				$files[] = $module['filepath'];
+				// Dupcheck only for custom modules for now
+				$files[$module['filepath']] = ($module['moduletype'] === 'CustomModule');
 			}
 		}
 
 		$task = self::recompress($files, $this->file);
 
 		// Wait for completion
-		if ($timeoutMs > 0 && !Taskmanager::isFailed($task) && !Taskmanager::isFinished($task))
+		if ($timeoutMs > 0 && !Taskmanager::isFailed($task) && !Taskmanager::isFinished($task)) {
 			$task = Taskmanager::waitComplete($task, $timeoutMs);
-		if ($task === true || (isset($task['statusCode']) && $task['statusCode'] === Taskmanager::TASK_FINISHED)) {
+		}
+		if (Taskmanager::isFinished($task)) {
 			// Success!
-			$this->markUpdated();
+			$this->markUpdated($task);
 			return true;
 		}
 		if (!is_array($task) || !isset($task['id']) || Taskmanager::isFailed($task)) {
@@ -154,12 +154,37 @@ class ConfigTgz
 		return $this->mark('OUTDATED');
 	}
 	
-	private function markUpdated()
+	private function markUpdated($task)
 	{
 		if ($this->configId === 0)
 			Util::traceError('ConfigTgz::markUpdated called with invalid config id!');
-		if ($this->areAllModulesUpToDate())
-			return $this->mark('OK');
+		if ($this->areAllModulesUpToDate()) {
+			if (empty($task['data']['warnings'])) {
+				$warnings = '';
+			} else {
+				// There have been warnings while generating the combined archive
+				// Most likely duplicate file entries.
+				// 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)) {
+					$mods[$row['moduleid']] = $row['title'];
+				}
+				// Now extract module id from filename and if applicable, replace filename by module name
+				$warnings = preg_replace_callback('#/opt/openslx/configs/modules/(\w+)_id-(\d+)__.*$#m', function ($m) use ($mods) {
+					if (!isset($mods[$m[2]]))
+						return $m[0];
+					return $mods[$m[2]] . ' (' . $m[1] . '#' . $m[2] . ')';
+				}, $task['data']['warnings']);
+			}
+			Database::exec("UPDATE configtgz SET status = :status, warnings = :warnings
+					WHERE configid = :configid LIMIT 1", [
+				'configid' => $this->configId,
+				'status' => 'OK',
+				'warnings' => $warnings,
+			]);
+			return 'OK';
+		}
 		return $this->mark('OUTDATED');
 	}
 	
@@ -174,10 +199,10 @@ class ConfigTgz
 	
 	private function mark($status)
 	{
-		Database::exec("UPDATE configtgz SET status = :status WHERE configid = :configid LIMIT 1", array(
+		Database::exec("UPDATE configtgz SET status = :status WHERE configid = :configid LIMIT 1", [
 			'configid' => $this->configId,
-			'status' => $status
-		));
+			'status' => $status,
+		]);
 		return $status;
 	}
 
@@ -186,7 +211,7 @@ class ConfigTgz
 	 */
 
 	/**
-	 * @param string[] $files source files to include
+	 * @param bool[] $files source files to include key = file, value = dupCheck
 	 * @param string $destFile where to store final result
 	 * @return false|array taskmanager task
 	 */
@@ -200,7 +225,7 @@ class ConfigTgz
 		foreach (Hook::load('config-tgz') as $hook) {
 			$file = $handler($hook);
 			if ($file !== false) {
-				$files[] = $file;
+				$files[$file] = true;
 			}
 		}
 
@@ -356,9 +381,10 @@ class ConfigTgz
 	/**
 	 * (Re)generating a config tgz succeeded. Update db entry.
 	 *
+	 * @param array $task the task object
 	 * @param array $args contains 'configid' and optionally 'deleteOnError'
 	 */
-	public static function generateSucceeded($args)
+	public static function generateSucceeded($task, $args)
 	{
 		if (!isset($args['configid']) || !is_numeric($args['configid'])) {
 			EventLog::warning('Ignoring generateSucceeded event as it has no configid assigned.');
@@ -369,7 +395,7 @@ class ConfigTgz
 			EventLog::warning('generateSucceeded callback for config id ' . $args['configid'] . ', but no instance could be generated.');
 			return;
 		}
-		$config->markUpdated();
+		$config->markUpdated($task);
 	}
-	
+
 }
diff --git a/modules-available/sysconfig/install.inc.php b/modules-available/sysconfig/install.inc.php
index f7ff4405..ace5361b 100644
--- a/modules-available/sysconfig/install.inc.php
+++ b/modules-available/sysconfig/install.inc.php
@@ -7,6 +7,7 @@ $update[] = tableCreate('configtgz', "
 	`title` varchar(200) NOT NULL,
 	`filepath` varchar(255) NOT NULL,
 	`status` enum('OK','OUTDATED','MISSING') NOT NULL DEFAULT 'MISSING',
+	`warnings` TEXT NOT NULL DEFAULT NULL,
 	`dateline` int(10) unsigned NOT NULL DEFAULT '0',
 	PRIMARY KEY (`configid`)
 ");
@@ -110,6 +111,14 @@ if (!tableHasColumn('configtgz', 'dateline')) {
 // 2020-01-16: Change contents column type
 Database::exec("ALTER TABLE `configtgz_module` CHANGE `contents` `contents` LONGBLOB NOT NULL");
 
+// 2020-11-02: Add warnings column
+if (!tableHasColumn('configtgz', 'warnings')) {
+	if (Database::exec("ALTER TABLE `configtgz` ADD `warnings` TEXT NULL DEFAULT NULL") === false) {
+		finalResponse(UPDATE_FAILED, 'Could not add warnings to configtgz: ' . Database::lastError());
+	}
+	$update[] = UPDATE_DONE;
+}
+
 // ----- rebuild configs ------
 // TEMPORARY HACK; Rebuild configs.. move somewhere else?
 Module::isAvailable('sysconfig');
diff --git a/modules-available/sysconfig/page.inc.php b/modules-available/sysconfig/page.inc.php
index 64162294..ede80aa7 100644
--- a/modules-available/sysconfig/page.inc.php
+++ b/modules-available/sysconfig/page.inc.php
@@ -174,12 +174,12 @@ class Page_SysConfig extends Page
 	private function listConfigs()
 	{
 		// Configs
-		$res = Database::simpleQuery("SELECT c.configid, c.title, c.filepath, c.status, c.dateline,
+		$res = Database::simpleQuery("SELECT c.configid, c.title, c.filepath, c.status, c.dateline, c.warnings,
 				GROUP_CONCAT(DISTINCT cl.locationid) AS loclist, GROUP_CONCAT(DISTINCT cxm.moduleid) AS modlist
 			FROM configtgz c
 			LEFT JOIN configtgz_x_module cxm USING (configid)
 			LEFT JOIN configtgz_location cl ON (c.configid = cl.configid)
-			GROUP BY configid
+			GROUP BY configid, title
 			ORDER BY title ASC");
 		$configs = array();
 		if ($this->currentLoc !== 0) {
@@ -208,6 +208,7 @@ class Page_SysConfig extends Page
 				$this->haveOverriddenLocations = true;
 			}
 			$configs[] = array(
+				'warnings' => $row['warnings'],
 				'configid' => $row['configid'],
 				'config' => $row['title'],
 				'modlist' => $row['modlist'],
diff --git a/modules-available/sysconfig/templates/list-configs.html b/modules-available/sysconfig/templates/list-configs.html
index 83feeaa6..b5367f8b 100644
--- a/modules-available/sysconfig/templates/list-configs.html
+++ b/modules-available/sysconfig/templates/list-configs.html
@@ -22,9 +22,19 @@
 					{{#configs}}
 					<tr>
 						<td data-id="{{configid}}" data-modlist="{{modlist}}" class="confrow slx-pointer" width="100%" title="{{dateline_s}}">
-							<table class="slx-ellipsis"><tr><td>{{config}}</td></tr></table>
+							<table class="slx-ellipsis"><tr><td>
+								{{#warnings}}
+								<button type="button" class="btn btn-xs btn-default" data-confirm="#confirm-mod-{{configid}}">
+									<span class="glyphicon glyphicon-exclamation-sign text-danger"></span>
+								</button>
+								{{/warnings}}
+								{{config}}
+							</td></tr></table>
 						</td>
 						<td>
+							{{#warnings}}
+							<pre id="confirm-mod-{{configid}}" class="hidden">{{.}}</pre>
+							{{/warnings}}
 							{{^current}}
 							<button class="btn btn-primary btn-xs" name="activate" value="{{configid}}" {{perms.config.assign.disabled}}>
 								<span class="glyphicon glyphicon-flag"></span>