From 4d51d90809da34022991e4a8c17caeb28c31bde0 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Wed, 3 Apr 2019 18:10:28 +0200
Subject: [statistics/locations] Add locationid constraints; update on delete

We didn't update subnetlocationid when deleting a location,
leading to machines pointing to invalid locations.
---
 .../locations/inc/autolocation.inc.php             | 27 ++++++++++++++++------
 modules-available/locations/install.inc.php        | 12 +++++-----
 modules-available/locations/page.inc.php           | 14 +++++------
 3 files changed, 32 insertions(+), 21 deletions(-)

(limited to 'modules-available/locations')

diff --git a/modules-available/locations/inc/autolocation.inc.php b/modules-available/locations/inc/autolocation.inc.php
index cccfe9b1..61507ebf 100644
--- a/modules-available/locations/inc/autolocation.inc.php
+++ b/modules-available/locations/inc/autolocation.inc.php
@@ -3,11 +3,23 @@
 class AutoLocation
 {
 
-	public static function rebuildAll()
+	/**
+	 * Rebuild the assigned subnetlocationid for clients that currently map to
+	 * the given locationids (if given), or all clients, if locationid is false
+	 *
+	 * @param int[]|false $locations Locations to rebuild, or false for everything
+	 */
+	public static function rebuildAll($locations = false)
 	{
 		if (Module::get('statistics') === false)
 			return; // Nothing to do
-		$res = Database::simpleQuery("SELECT machineuuid, clientip FROM machine");
+		if ($locations === false) {
+			// All
+			$res = Database::simpleQuery("SELECT machineuuid, clientip FROM machine");
+		} else {
+			$res = Database::simpleQuery("SELECT machineuuid, clientip FROM machine
+					WHERE fixedlocationid IN(:lid) OR subnetlocationid IN(:lid)", ['lid' => $locations]);
+		}
 		$updates = array();
 		$nulls = array();
 		while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
@@ -22,13 +34,14 @@ class AutoLocation
 			}
 		}
 		if (!empty($nulls)) {
-			$qs = '?' . str_repeat(',?', count($nulls) - 1);
-			Database::exec("UPDATE machine SET subnetlocationid = NULL WHERE machineuuid IN ($qs)", $nulls);
+			Database::exec("UPDATE machine SET subnetlocationid = NULL WHERE machineuuid IN (:nulls)",
+				['nulls' => $nulls]);
 		}
 		foreach ($updates as $lid => $machines) {
-			$qs = '?' . str_repeat(',?', count($machines) - 1);
-			$lid = (int)$lid;
-			Database::exec("UPDATE machine SET subnetlocationid = $lid WHERE machineuuid IN ($qs)", $machines);
+			if (empty($machines))
+				continue;
+			Database::exec("UPDATE machine SET subnetlocationid = :lid WHERE machineuuid IN (:machines)",
+				['lid' => $lid, 'machines' => $machines]);
 		}
 	}
 
diff --git a/modules-available/locations/install.inc.php b/modules-available/locations/install.inc.php
index f833568d..d4e1b67b 100644
--- a/modules-available/locations/install.inc.php
+++ b/modules-available/locations/install.inc.php
@@ -30,10 +30,10 @@ $res[] = tableCreate('subnet', '
 	KEY `locationid` (`locationid`)
 ');
 
-// Create response for browser
-
-if (in_array(UPDATE_DONE, $res)) {
-	finalResponse(UPDATE_DONE, 'Tables created successfully');
-}
+$res[] = tableAddConstraint('subnet', 'locationid', 'location', 'locationid',
+	'ON UPDATE CASCADE ON DELETE CASCADE');
+$res[] = tableAddConstraint('setting_location', 'locationid', 'location', 'locationid',
+	'ON UPDATE CASCADE ON DELETE CASCADE');
 
-finalResponse(UPDATE_NOOP, 'Everything already up to date');
+// Create response for browser
+responseFromArray($res);
diff --git a/modules-available/locations/page.inc.php b/modules-available/locations/page.inc.php
index d20c6068..5b3d7ff0 100644
--- a/modules-available/locations/page.inc.php
+++ b/modules-available/locations/page.inc.php
@@ -144,22 +144,20 @@ class Page_Locations extends Page
 	private function deleteLocation($location)
 	{
 		$locationId = (int)$location['locationid'];
-		$ids = $locationId;
 		if (Request::post('recursive', false) === 'on') {
 			$rows = Location::queryLocations();
 			$rows = Location::buildTree($rows, $locationId);
-			$rows = Location::extractIds($rows);
-			if (!empty($rows)) {
-				$ids .= ',' . implode(',', $rows);
-			}
+			$ids = Location::extractIds($rows);
+		} else {
+			$ids = [$locationId];
 		}
-		$subs = Database::exec("DELETE FROM subnet WHERE locationid IN ($ids)");
-		$locs = Database::exec("DELETE FROM location WHERE locationid IN ($ids)");
+		$locs = Database::exec("DELETE FROM location WHERE locationid IN (:ids)", ['ids' => $ids]);
 		Database::exec('UPDATE location SET parentlocationid = :newparent WHERE parentlocationid = :oldparent', array(
 			'newparent' => $location['parentlocationid'],
 			'oldparent' => $location['locationid']
 		));
-		Message::addSuccess('location-deleted', $locs, $subs);
+		AutoLocation::rebuildAll($ids);
+		Message::addSuccess('location-deleted', $locs, implode(', ', $ids));
 		Util::redirect('?do=Locations');
 	}
 
-- 
cgit v1.2.3-55-g7522