From 878dbac85a9f684916e2d30cab4782e21a03e767 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Mon, 4 Jul 2022 15:42:13 +0200
Subject: [rebootcontrol/main] Add subkey column to property_list table

This makes it easier to reference to list entries that have non-trivial
data values, e.g. long json data.
---
 apis/cron.inc.php                                  | 10 +--
 inc/property.inc.php                               | 71 ++++++++++++++++++++--
 modules-available/main/install.inc.php             | 18 +++++-
 .../rebootcontrol/inc/rebootcontrol.inc.php        |  6 +-
 4 files changed, 91 insertions(+), 14 deletions(-)

diff --git a/apis/cron.inc.php b/apis/cron.inc.php
index 41370038..0b1e92a9 100644
--- a/apis/cron.inc.php
+++ b/apis/cron.inc.php
@@ -21,8 +21,8 @@ if (($report = Request::get('crashreport', false, 'string'))) {
 		exit(0);
 	}
 	$str = array();
-	foreach ($list as $item) {
-		Property::removeFromList(CRON_KEY_STATUS, $item);
+	foreach ($list as $subkey => $item) {
+		Property::removeFromListByKey(CRON_KEY_STATUS, $subkey);
 		$entry = explode('|', $item, 2);
 		if (count($entry) !== 2)
 			continue;
@@ -91,13 +91,13 @@ foreach (Hook::load('cron') as $hook) {
 		$runtime = (time() - $status['start']);
 		if ($runtime < 0) {
 			// Clock skew
-			Property::removeFromList(CRON_KEY_STATUS, $status['string']);
+			Property::removeFromListByVal(CRON_KEY_STATUS, $status['string']);
 		} elseif ($runtime < 900) {
 			// Allow up to 15 minutes for a job to complete before we complain...
 			continue;
 		} else {
 			// Consider job crashed
-			Property::removeFromList(CRON_KEY_STATUS, $status['string']);
+			Property::removeFromListByVal(CRON_KEY_STATUS, $status['string']);
 			EventLog::failure('Cronjob for module ' . $hook->moduleId . ' seems to be stuck or has crashed.');
 			continue;
 		}
@@ -114,5 +114,5 @@ foreach (Hook::load('cron') as $hook) {
 		// Logging
 		EventLog::failure('Cronjob for module ' . $hook->moduleId . ' has crashed. Check the php or web server error log.', $e->getMessage());
 	}
-	Property::removeFromList(CRON_KEY_STATUS, $value);
+	Property::removeFromListByVal(CRON_KEY_STATUS, $value);
 }
diff --git a/inc/property.inc.php b/inc/property.inc.php
index 96787f00..734c559e 100644
--- a/inc/property.inc.php
+++ b/inc/property.inc.php
@@ -68,31 +68,76 @@ class Property
 	 */
 	public static function getList(string $key): array
 	{
-		$res = Database::simpleQuery("SELECT dateline, value FROM property_list WHERE name = :key", compact('key'));
+		$res = Database::simpleQuery("SELECT subkey, dateline, value FROM property_list
+			WHERE `name` = :key", compact('key'));
 		$NOW = time();
-		$return = array();
+		$return = [];
 		foreach ($res as $row) {
 			if ($row['dateline'] != 0 && $row['dateline'] < $NOW)
 				continue;
-			$return[] = $row['value'];
+			$return[$row['subkey']] = $row['value'];
 		}
 		return $return;
 	}
 
+	/**
+	 * @param string $key
+	 * @param int $subkey
+	 * @return ?string entry from property list
+	 */
+	public static function getListEntry(string $key, int $subkey)
+	{
+		$row = Database::queryFirst("SELECT dateline, `value` FROM property_list
+			WHERE `name` = :key AND subkey = :subkey", ['key' => $key, 'subkey' => $subkey]);
+		if ($row === false || ($row['dateline'] != 0 && $row['dateline'] < time()))
+			return null;
+		return $row['value'];
+	}
+
 	/**
 	 * Add item to property list.
 	 *
 	 * @param string $key key of value to set
 	 * @param string $value the value to add for $key
 	 * @param int $maxAgeMinutes how long to keep this entry around at least, in minutes. 0 for infinite
+	 * @return int The auto generated sub-key
 	 */
-	public static function addToList(string $key, string $value, int $maxAgeMinutes = 0)
+	public static function addToList(string $key, string $value, int $maxAgeMinutes = 0): int
 	{
 		Database::exec("INSERT INTO property_list (name, value, dateline) VALUES (:key, :value, :dateline)", array(
 			'key' => $key,
 			'value' => $value,
 			'dateline' => ($maxAgeMinutes === 0 ? 0 : time() + ($maxAgeMinutes * 60))
 		));
+		return Database::lastInsertId();
+	}
+
+	/**
+	 * Update existing entry in property list.
+	 *
+	 * @param string $key key of list
+	 * @param int $subkey subkey of entry in list
+	 * @param string $value new value to set entry to
+	 * @param string|null $expectedValue if not null, the value will only be updated if it currently has this value
+	 * @param int $maxAgeMinutes the new lifetime of that entry
+	 * @return bool whether the entry existed and has been updated
+	 */
+	public static function updateListEntry(string $key, int $subkey, string $value, string $expectedValue = null,
+														int $maxAgeMinutes = 0): bool
+	{
+		$args = [
+			'name' => $key,
+			'subkey' => $subkey,
+			'newvalue' => $value,
+			'dateline' => ($maxAgeMinutes === 0 ? 0 : time() + ($maxAgeMinutes * 60)),
+		];
+		if ($expectedValue !== null) {
+			$args['oldvalue'] = $expectedValue;
+			return Database::exec("UPDATE property_list SET `value` = :newvalue, dateline = :dateline
+				WHERE `name` = :name AND subkey = :subkey AND `value` = :oldvalue", $args);
+		}
+		return Database::exec("UPDATE property_list SET `value` = :newvalue, dateline = :dateline
+				WHERE `name` = :name AND subkey = :subkey", $args);
 	}
 
 	/**
@@ -103,7 +148,7 @@ class Property
 	 * @param string $value item to remove
 	 * @return int number of items removed
 	 */
-	public static function removeFromList(string $key, string $value): int
+	public static function removeFromListByVal(string $key, string $value): int
 	{
 		return Database::exec("DELETE FROM property_list WHERE name = :key AND value = :value", array(
 			'key' => $key,
@@ -111,6 +156,22 @@ class Property
 		));
 	}
 
+	/**
+	 * Remove given item from property list. If the list contains this item
+	 * multiple times, they will all be removed.
+	 *
+	 * @param string $key Key of list
+	 * @param int $value item to remove
+	 * @return bool whether item was found and removed
+	 */
+	public static function removeFromListByKey(string $key, int $subkey): bool
+	{
+		return Database::exec("DELETE FROM property_list WHERE name = :key AND subkey = :subkey", array(
+			'key' => $key,
+			'subkey' => $subkey,
+		)) > 0;
+	}
+
 	/**
 	 * Delete entire list with given key.
 	 *
diff --git a/modules-available/main/install.inc.php b/modules-available/main/install.inc.php
index b7dd2c4d..a19a069d 100644
--- a/modules-available/main/install.inc.php
+++ b/modules-available/main/install.inc.php
@@ -28,10 +28,13 @@ $res[] = tableCreate('property', "
 
 $res[] = tableCreate('property_list', "
 	`name` varchar(50) NOT NULL,
+	`subkey` int(10) unsigned NOT NULL AUTO_INCREMENT,
 	`dateline` int(10) unsigned NOT NULL DEFAULT '0',
 	`value` text NOT NULL,
 	KEY (`name`),
-	KEY `dateline` (`dateline`)
+	KEY `dateline` (`dateline`),
+	ADD KEY (`subkey`),
+	UNIQUE KEY `compound` (`name`, `subkey`)
 ");
 
 $res[] = tableCreate('user', "
@@ -116,6 +119,19 @@ if (!tableHasColumn('user', 'serverid')) {
 	Database::exec("ALTER TABLE `user` ADD `serverid` int(10) unsigned NULL DEFAULT NULL");
 }
 
+//
+//
+if (!tableHasColumn('property_list', 'subkey')) {
+	$ret = Database::exec("ALTER TABLE property_list
+    	ADD COLUMN `subkey` int(10) unsigned NOT NULL AUTO_INCREMENT AFTER `name`,
+    	ADD KEY (`subkey`),
+    	ADD UNIQUE KEY `compound` (`name`, `subkey`)");
+	if ($ret === false) {
+		finalResponse(UPDATE_FAILED, 'Cannot add subkey to property_list: ' . Database::lastError());
+	}
+	$res[] = UPDATE_DONE;
+}
+
 // Make sure that if any users exist, one of the has UID=1, otherwise if the permission module is
 // used we'd lock out everyone
 $someUser = Database::queryFirst('SELECT userid FROM user ORDER BY userid ASC LIMIT 1');
diff --git a/modules-available/rebootcontrol/inc/rebootcontrol.inc.php b/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
index 36b2b14f..83513081 100644
--- a/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
+++ b/modules-available/rebootcontrol/inc/rebootcontrol.inc.php
@@ -105,10 +105,10 @@ class RebootControl
 		}
 		$list = Property::getList(RebootControl::KEY_TASKLIST);
 		$return = [];
-		foreach ($list as $entry) {
+		foreach ($list as $subkey => $entry) {
 			$p = json_decode($entry, true);
 			if (!is_array($p) || !isset($p['id'])) {
-				Property::removeFromList(RebootControl::KEY_TASKLIST, $entry);
+				Property::removeFromListByKey(RebootControl::KEY_TASKLIST, $subkey);
 				continue;
 			}
 			if (is_array($locations) && is_array($p['locations']) && array_diff($p['locations'], $locations) !== [])
@@ -131,7 +131,7 @@ class RebootControl
 				}
 			}
 			if (!$valid) {
-				Property::removeFromList(RebootControl::KEY_TASKLIST, $entry);
+				Property::removeFromListByKey(RebootControl::KEY_TASKLIST, $subkey);
 				continue;
 			}
 			$return[] = $p;
-- 
cgit v1.2.3-55-g7522