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 +++++------ modules-available/statistics/install.inc.php | 11 +++++++++ 4 files changed, 43 insertions(+), 21 deletions(-) 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'); } diff --git a/modules-available/statistics/install.inc.php b/modules-available/statistics/install.inc.php index 090f0617..2bcb6b8d 100644 --- a/modules-available/statistics/install.inc.php +++ b/modules-available/statistics/install.inc.php @@ -196,6 +196,8 @@ if ($addTrigger) { if (Module::isAvailable('locations')) { if (tableExists('subnet')) { AutoLocation::rebuildAll(); + } else { + $res[] = UPDATE_RETRY; } } $res[] = UPDATE_DONE; @@ -205,6 +207,15 @@ $res[] = tableAddConstraint('machine_x_hw', 'hwid', 'statistic_hw', 'hwid', 'ON $res[] = tableAddConstraint('machine_x_hw', 'machineuuid', 'machine', 'machineuuid', 'ON DELETE CASCADE ON UPDATE CASCADE'); $res[] = tableAddConstraint('machine_x_hw_prop', 'machinehwid', 'machine_x_hw', 'machinehwid', 'ON DELETE CASCADE'); $res[] = tableAddConstraint('statistic_hw_prop', 'hwid', 'statistic_hw', 'hwid', 'ON DELETE CASCADE'); +if (Module::isAvailable('locations')) { + if (tableExists('location')) { + $res[] = tableAddConstraint('machine', 'fixedlocationid', 'location', 'locationid', + 'ON UPDATE CASCADE ON DELETE SET NULL'); + // No constraint for subnetlocationid -- needs recalc anyways (AutoLocation::rebuildAll()) + } else { + $res[] = UPDATE_RETRY; + } +} // 2017-11-27: Add state column if (!tableHasColumn('machine', 'state')) { -- cgit v1.2.3-55-g7522