From 1bdf3d4ce66910f07842c7be1043938bccf0a462 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 12 Jan 2022 17:07:42 +0100 Subject: [statistics] Make query builder a bit more OOP --- modules-available/statistics/api.inc.php | 32 ++--- .../statistics/hooks/config-tgz.inc.php | 2 +- .../statistics/inc/devicetype.inc.php | 7 -- .../statistics/inc/hardwareinfo.inc.php | 5 +- .../statistics/inc/hardwarequery.inc.php | 130 +++++++++------------ .../statistics/inc/hardwarequerycolumn.inc.php | 88 ++++++++++++++ modules-available/statistics/pages/hints.inc.php | 22 +++- modules-available/statistics/pages/machine.inc.php | 2 +- .../statistics/pages/projectors.inc.php | 2 +- 9 files changed, 175 insertions(+), 115 deletions(-) delete mode 100644 modules-available/statistics/inc/devicetype.inc.php create mode 100644 modules-available/statistics/inc/hardwarequerycolumn.inc.php diff --git a/modules-available/statistics/api.inc.php b/modules-available/statistics/api.inc.php index 431d4cd4..7aa6de34 100644 --- a/modules-available/statistics/api.inc.php +++ b/modules-available/statistics/api.inc.php @@ -1,26 +1,12 @@ addCompare(false, 'Memory Slot Occupied', '>=', true, 'Memory Slot Count'); - $x->addWhere(true, 'vendor', '=', '8086'); - $x->addGlobalColumn('device'); - $res = $x->query(); - foreach ($res as $row) { - error_log(json_encode($row)); - } - exit; - */ - $data = file_get_contents('/tmp/bla.json'); - Database::exec( - "UPDATE machine SET data = :data WHERE machineuuid = :uuid", - ['uuid' => $uuid, 'data' => $data]); - HardwareParser::parseMachine($uuid, json_decode($data, true)); - echo 'Kweries: ' . Database::getQueryCount(); + $q = new HardwareQuery(HardwareInfo::RAM_MODULE); + $speed = $q->addGlobalColumn('Speed'); + $q->addLocalColumn('Configured Speed')->addCondition('<', $speed); + $speed->addCondition('>', 2666); + $q->addMachineWhere('clientip', '>', 5); + echo $q->buildQuery(), "\n"; exit; } @@ -352,7 +338,7 @@ if ($type[0] === '~') { $hwid = $hwids[$screen['name']]; } else { $hwid = (int)Database::insertIgnore('statistic_hw', 'hwid', - array('hwtype' => DeviceType::SCREEN, 'hwname' => $screen['name'])); + array('hwtype' => HardwareInfo::SCREEN, 'hwname' => $screen['name'])); $hwids[$screen['name']] = $hwid; } // Now add new entries @@ -398,7 +384,7 @@ if ($type[0] === '~') { Database::exec("UPDATE machine_x_hw x, statistic_hw h SET x.disconnecttime = UNIX_TIMESTAMP() WHERE x.machineuuid = :uuid AND x.hwid = h.hwid AND h.hwtype = :type AND x.disconnecttime = 0", - array('uuid' => $uuid, 'type' => DeviceType::SCREEN)); + array('uuid' => $uuid, 'type' => HardwareInfo::SCREEN)); } else { // Some screens connected, make sure old entries get removed Database::exec("UPDATE machine_x_hw x, statistic_hw h @@ -407,7 +393,7 @@ if ($type[0] === '~') { AND x.machineuuid = :uuid", array( 'pairs' => $keepPair, 'uuid' => $uuid, - 'type' => DeviceType::SCREEN, + 'type' => HardwareInfo::SCREEN, )); } } diff --git a/modules-available/statistics/hooks/config-tgz.inc.php b/modules-available/statistics/hooks/config-tgz.inc.php index 0a43e115..b732fd7a 100644 --- a/modules-available/statistics/hooks/config-tgz.inc.php +++ b/modules-available/statistics/hooks/config-tgz.inc.php @@ -4,7 +4,7 @@ $res = Database::simpleQuery('SELECT h.hwname FROM statistic_hw h' . " INNER JOIN statistic_hw_prop p ON (h.hwid = p.hwid AND p.prop = :projector)" . " WHERE h.hwtype = :screen ORDER BY h.hwname ASC", array( 'projector' => 'projector', - 'screen' => DeviceType::SCREEN, + 'screen' => HardwareInfo::SCREEN, )); if ($res !== false) { // CHeck this in case we're running on old DB during update diff --git a/modules-available/statistics/inc/devicetype.inc.php b/modules-available/statistics/inc/devicetype.inc.php deleted file mode 100644 index a01ec310..00000000 --- a/modules-available/statistics/inc/devicetype.inc.php +++ /dev/null @@ -1,7 +0,0 @@ -addJoin(true, '@PASSTHROUGH', 'passthrough_group_x_location', 'groupid', + $hw->addForeignJoin(true, '@PASSTHROUGH', 'passthrough_group_x_location', 'groupid', 'locationid', $locations); $hw->addGlobalColumn('vendor'); $hw->addGlobalColumn('device'); @@ -77,7 +78,7 @@ class HardwareInfo foreach (array_keys($slots) as $slot) { //error_log('Querying slot ' . $slot); $hw = new HardwareQuery(self::PCI_DEVICE, $best['machineuuid'], true); - $hw->addWhere(false, 'slot', 'LIKE', $slot . '.%'); + $hw->addLocalColumn('slot')->addCondition('LIKE', $slot . '.%'); $hw->addGlobalColumn('vendor'); $hw->addGlobalColumn('device'); foreach ($hw->query() as $row) { diff --git a/modules-available/statistics/inc/hardwarequery.inc.php b/modules-available/statistics/inc/hardwarequery.inc.php index 24b27190..00a9bbc9 100644 --- a/modules-available/statistics/inc/hardwarequery.inc.php +++ b/modules-available/statistics/inc/hardwarequery.inc.php @@ -10,10 +10,10 @@ class HardwareQuery private $columns = []; /** - * @param string $type type form HardwareInfo - * @param ?string $uuid + * @param string $type hardware type form HardwareInfo + * @param ?string $uuid If set, only return data for specific client */ - public function __construct(string $type, $uuid = null, $connectedOnly = true) + public function __construct(string $type, string $uuid = null, $connectedOnly = true) { if ($connectedOnly) { $this->joins['mxhw_join'] = "INNER JOIN machine_x_hw mxhw ON (mxhw.hwid = shw.hwid AND mxhw.disconnecttime = 0)"; @@ -30,50 +30,33 @@ class HardwareQuery private function id(): string { - return 't' . (++$this->id); - } - - private function fillTableVars(bool $global, &$srcTable, &$table, &$column) - { - if ($global) { - $srcTable = 'shw'; - $table = 'statistic_hw_prop'; - $column = 'hwid'; - } else { - $srcTable = 'mxhw'; - $table = 'machine_x_hw_prop'; - $column = 'machinehwid'; - } + return 'b' . (++$this->id); } /** - * @param bool $global - * @param string $prop - * @param string $op - * @param string|string[] $value + * Add join of a virtual column (hw property) to an arbitrary table and column. + * @param bool $global Is the virtual column global or local to machine? + * @param string $prop Name of property/virtual column + * @param string $jTable Table to join on + * @param string $jColumn Column to join on + * @param string $condColumn optionally, another column from the joined table to match against $condVal + * @param string|array $condVal optionally, a literal, or array of literals, to match foreign column against + * @return void */ - public function addWhere(bool $global, string $prop, string $op, $value) - { - if (isset($this->columns[$prop])) - return; - $this->fillTableVars($global, $srcTable, $table, $column); - $tid = $this->id(); - $pid = $this->id(); - $vid = $this->id(); - $valueCol = ($op === '<' || $op === '>' || $op === '<=' || $op === '>=') ? 'numeric' : 'value'; - $this->joins[$prop] = "INNER JOIN $table $tid ON ($srcTable.$column = $tid.$column AND - $tid.prop = :$pid AND $tid.`$valueCol` $op (:$vid))"; - $this->args[$pid] = $prop; - $this->args[$vid] = $value; - $this->columns[$prop] = "$tid.`value` AS `$prop`"; - } - - public function addJoin(bool $global, string $prop, string $jTable, string $jColumn, string $condColumn = '', $condVal = null) + public function addForeignJoin(bool $global, string $prop, string $jTable, string $jColumn, string $condColumn = '', $condVal = null) { if (isset($this->columns[$jTable])) return; if (!isset($this->columns[$prop])) { - $this->fillTableVars($global, $srcTable, $table, $column); + if ($global) { + $srcTable = 'shw'; + $table = 'statistic_hw_prop'; + $column = 'hwid'; + } else { + $srcTable = 'mxhw'; + $table = 'machine_x_hw_prop'; + $column = 'machinehwid'; + } $tid = $this->id(); $pid = $this->id(); $this->joins[$prop] = "INNER JOIN $table $tid ON ($srcTable.$column = $tid.$column @@ -99,55 +82,33 @@ class HardwareQuery { if (isset($this->columns[$column])) return; - $valueCol = ($op === '<' || $op === '>' || $op === '<=' || $op === '>=') ? 'numeric' : 'value'; $vid = $this->id(); $this->joins['machine'] = 'INNER JOIN machine m USING (machineuuid)'; - $this->where[] = "m.$column $op :$vid"; + $this->where[] = "m.$column $op (:$vid)"; $this->args[$vid] = $value; $this->columns[$column] = "m.$column"; } - public function addCompare(bool $global1, string $prop1, string $op, string $global2, string $prop2) + public function addGlobalColumn(string $prop): HardwareQueryColumn { - $this->fillTableVars($global1, $srcTable1, $table1, $column1); - $this->fillTableVars($global2, $srcTable2, $table2, $column2); - $tid1 = $this->id(); - $pid1 = $this->id(); - $tid2 = $this->id(); - $pid2 = $this->id(); - $valueCol = ($op === '<' || $op === '>' || $op === '<=' || $op === '>=') ? 'numeric' : 'value'; - $this->joins[] = "INNER JOIN $table1 $tid1 ON ($srcTable1.$column1 = $tid1.$column1 AND - $tid1.prop = :$pid1)"; - $this->joins[] = "INNER JOIN $table2 $tid2 ON ($srcTable2.$column2 = $tid2.$column2 AND - $tid2.prop = :$pid2 AND $tid1.`$valueCol` $op $tid2.`$valueCol`)"; - $this->args[$pid1] = $prop1; - $this->args[$pid2] = $prop2; - $this->columns[$prop1] = "$tid1.`value` AS `$prop1`"; - $this->columns[$prop2] = "$tid2.`value` AS `$prop2`"; + return $this->addColumn(true, $prop); } - public function addGlobalColumn(string $prop) + public function addLocalColumn(string $prop): HardwareQueryColumn { - $this->addColumn(true, $prop); + return $this->addColumn(false, $prop); } - public function addLocalColumn(string $prop) + public function addColumn(bool $global, string $prop, string $alias = null): HardwareQueryColumn { - $this->addColumn(false, $prop); - } - - public function addColumn(bool $global, string $prop) - { - if (isset($this->columns[$prop])) - return; - $this->fillTableVars($global, $srcTable, $table, $column); - $tid = $this->id(); - $pid = $this->id(); - $this->joins[$prop] = "LEFT JOIN $table $tid ON ($srcTable.$column = $tid.$column AND $tid.prop = :$pid)"; - $this->args[$pid] = $prop; - $this->columns[$prop] = "$tid.`value` AS `$prop`"; + return $this->columns[] = new HardwareQueryColumn($global, $prop, $alias); } + /** + * Join the machine table and add the given column from it to the SELECT + * @param string $column + * @return void + */ public function addMachineColumn(string $column) { if (isset($this->columns[$column])) @@ -161,9 +122,27 @@ class HardwareQuery */ public function query(string $groupBy = '') { - $columns = $this->columns; + return Database::simpleQuery($this->buildQuery($groupBy), $this->args); + } + + /** + * Build query string + * @param string $groupBy Column to group by + */ + public function buildQuery(string $groupBy = ''): string + { + $groupConcat = !empty($groupBy) && $groupBy !== 'mxhw.machinehwid'; + $columns = []; + foreach ($this->columns as $column) { + if ($column instanceof HardwareQueryColumn) { + $column->generate($this->joins, $columns, $this->args, $groupConcat); + } else { + $columns[] = $column; + } + } $columns[] = 'mxhw.machineuuid'; $columns[] = 'shw.hwid'; + // TODO: Untangle this implicit magic if (empty($groupBy) || $groupBy === 'mxhw.machinehwid') { $columns[] = 'mxhw.disconnecttime'; } else { @@ -173,12 +152,11 @@ class HardwareQuery $columns[] = 'Count(*) AS group_count'; $groupBy = " GROUP BY $groupBy"; } - $query = 'SELECT ' . implode(', ', $columns) + return 'SELECT ' . implode(', ', $columns) . ' FROM statistic_hw shw ' . implode(' ', $this->joins) . ' WHERE ' . implode(' AND ', $this->where) . $groupBy; - return Database::simpleQuery($query, $this->args); } } \ No newline at end of file diff --git a/modules-available/statistics/inc/hardwarequerycolumn.inc.php b/modules-available/statistics/inc/hardwarequerycolumn.inc.php new file mode 100644 index 00000000..d073530b --- /dev/null +++ b/modules-available/statistics/inc/hardwarequerycolumn.inc.php @@ -0,0 +1,88 @@ +classId = ++self::$id; + $this->global = $global; + $this->tableAlias = self::getId(); + $this->virtualColumnName = $column; + $this->alias = $alias ?? $column; + } + + /** + * Add necessary conditions, joins, columns to final SQL arrays. To be called + * from HardwareQuery::buildQuery(). + * @param bool $groupConcat whether to add distinct GROUP_CONCAT to column. + */ + public function generate(array &$joins, array &$columns, array &$params, bool $groupConcat) + { + if ($this->global) { + $srcTable = 'shw'; + $table = 'statistic_hw_prop'; + $column = 'hwid'; + } else { + $srcTable = 'mxhw'; + $table = 'machine_x_hw_prop'; + $column = 'machinehwid'; + } + $tid = $this->tableAlias; + $pid = self::getId(); + $this->conditions[] = "$srcTable.$column = $tid.$column AND $tid.prop = :$pid"; + $params[$pid] = $this->virtualColumnName; // value of property column is our virtual column + // If we have just one condition, it's the join condition itself. Since we pretend we're just adding + // a column to the query, do a left join, so the "column" is NULL if the join doesn't match. + // If however any conditions were added to this class via the addCondition() method, do a regular + // INNER JOIN, so the result will be empty if the condition doesn't match. + $type = count($this->conditions) === 1 ? 'LEFT' : 'INNER'; + $joins[] = "$type JOIN $table $tid ON (" . implode(' AND ', $this->conditions) . ")"; + if ($groupConcat) { + $columns[] = "Group_Concat(DISTINCT $tid.`value` SEPARATOR ', ') AS `{$this->alias}`"; + } else { + $columns[] = "$tid.`value` AS `{$this->alias}`"; + } + $params += $this->params; + } + + /** + * @param string $op Operator (<>=, IN, LIKE) + * @param string|string[]|HardwareQueryColumn $other value to compare with. + * Can be a literal, an array (if opererator is IN), or another Column + * @return void + */ + public function addCondition(string $op, $other) + { + $valueCol = ($op === '<' || $op === '>' || $op === '<=' || $op === '>=') ? 'numeric' : 'value'; + if ($other instanceof HardwareQueryColumn) { + $cond = "{$this->tableAlias}.`$valueCol` $op {$other->tableAlias}.`$valueCol`"; + // Don't reference a column of a table that hasn't been joined yet + if ($this->classId > $other->classId) { + $this->conditions[] = $cond; + } else { + $other->conditions[] = $cond; + } + } else { + $pid = self::getId(); + $this->conditions[] = "{$this->tableAlias}.`$valueCol` $op (:$pid)"; + $this->params[$pid] = $other; + } + } + +} \ No newline at end of file diff --git a/modules-available/statistics/pages/hints.inc.php b/modules-available/statistics/pages/hints.inc.php index 278c0e26..67c42a18 100644 --- a/modules-available/statistics/pages/hints.inc.php +++ b/modules-available/statistics/pages/hints.inc.php @@ -19,6 +19,10 @@ class SubPage self::showUnusedSpace($locs); } + /** + * Machines that have less than 8GB of RAM. Highlight those + * that still have free memory slots. + */ private static function showMemoryUpgrade(array $locs) { $q = new HardwareQuery(HardwareInfo::MAINBOARD); @@ -30,7 +34,8 @@ class SubPage $q->addGlobalColumn('Memory Maximum Capacity'); $q->addMachineColumn('clientip'); $q->addMachineColumn('hostname'); - $q->addWhere(false, 'Memory Installed Capacity', '<', 8 * 1024 * 1024 * 1024); + $col = $q->addLocalColumn('Memory Installed Capacity'); + $col->addCondition('<', 8 * 1024 * 1024 * 1024); $list = []; foreach ($q->query() as $row) { if (HardwareParser::convertSize($row['Memory Installed Capacity'], 'M', false) @@ -47,6 +52,10 @@ class SubPage Render::addTemplate('hints-ram-upgrade', ['list' => $list]); } + /** + * Show machines where RAM modules are running slower + * than their design speed. + */ private static function showMemorySlow(array $locs) { $q = new HardwareQuery(HardwareInfo::RAM_MODULE); @@ -62,11 +71,16 @@ class SubPage $q->addLocalColumn('Serial Number'); $q->addMachineColumn('clientip'); $q->addMachineColumn('hostname'); - $q->addCompare(true, 'Speed', '>', false, 'Configured Memory Speed'); + $col = $q->addGlobalColumn('Speed'); + $col->addCondition('>', $q->addLocalColumn('Configured Memory Speed')); $list = $q->query()->fetchAll(); Render::addTemplate('hints-ram-underclocked', ['list' => $list]); } + /** + * Show machines that have unpartitioned space available, + * and no ID44 or ID45. + */ private static function showUnusedSpace(array $locs) { $id44 = $id45 = []; @@ -77,7 +91,7 @@ class SubPage } $q->addMachineColumn('clientip'); $q->addMachineColumn('hostname'); - $q->addWhere(false, 'unused', '>', 2000000000); // 2 GB + $q->addLocalColumn('unused')->addCondition('>', 2000000000); // 2 GB $q->addMachineWhere('id44mb', '<', 20000); // 20 GB foreach ($q->query()->fetchAll() as $row) { $row['unused_s'] = Util::readableFileSize($row['unused']); @@ -91,7 +105,7 @@ class SubPage } $q->addMachineColumn('clientip'); $q->addMachineColumn('hostname'); - $q->addWhere(false, 'unused', '>', 25000000000); // 25 GB + $q->addLocalColumn('unused')->addCondition('>', 25000000000); // 25 GB $q->addMachineWhere('id44mb', '>', 20000); // 20 GB $q->addMachineWhere('id45mb', '<', 20000); // 20 GB foreach ($q->query()->fetchAll() as $row) { diff --git a/modules-available/statistics/pages/machine.inc.php b/modules-available/statistics/pages/machine.inc.php index 019b0d8d..6ea0ce28 100644 --- a/modules-available/statistics/pages/machine.inc.php +++ b/modules-available/statistics/pages/machine.inc.php @@ -229,7 +229,7 @@ class SubPage . " LEFT JOIN machine_x_hw_prop p ON (m.machinehwid = p.machinehwid AND p.prop = 'resolution')" . " LEFT JOIN statistic_hw_prop q ON (m.hwid = q.hwid AND q.prop = 'projector')" . " WHERE m.machineuuid = :uuid", - array('screen' => DeviceType::SCREEN, 'uuid' => $uuid)); + array('screen' => HardwareInfo::SCREEN, 'uuid' => $uuid)); $client['screens'] = array(); $ports = array(); foreach ($res as $row) { diff --git a/modules-available/statistics/pages/projectors.inc.php b/modules-available/statistics/pages/projectors.inc.php index 97a21ebd..91abfe9c 100644 --- a/modules-available/statistics/pages/projectors.inc.php +++ b/modules-available/statistics/pages/projectors.inc.php @@ -49,7 +49,7 @@ class SubPage . " INNER JOIN statistic_hw_prop p ON (h.hwid = p.hwid AND p.prop = :projector)" . " WHERE h.hwtype = :screen ORDER BY h.hwname ASC", array( 'projector' => 'projector', - 'screen' => DeviceType::SCREEN, + 'screen' => HardwareInfo::SCREEN, )); $data = array( 'projectors' => $res->fetchAll() -- cgit v1.2.3-55-g7522