[statistics] Make query builder a bit more OOP

5 files changed, 138 insertions, 102 deletions

diff --git a/modules-available/statistics/api.inc.php b/modules-available/statistics/api.inc.php
index 431d4cd4..fc0b2afe 100644
--- a/modules-available/statistics/api.inc.php
+++ b/modules-available/statistics/api.inc.php
@@ -1,26 +1,12 @@
 <?php
 
 if (Request::any('action') === 'test' && isLocalExecution()) {
-	$uuid = Request::any('uuid', '', 'string');
-	/*
-	error_log(HardwareInfo::getKclModifications());
-	exit;
-	$x = new HardwareQuery(HardwareInfo::PCI_DEVICE);
-	//$x->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;
 }
 
diff --git a/modules-available/statistics/inc/hardwareinfo.inc.php b/modules-available/statistics/inc/hardwareinfo.inc.php
index df33a5f6..9ccf88be 100644
--- a/modules-available/statistics/inc/hardwareinfo.inc.php
+++ b/modules-available/statistics/inc/hardwareinfo.inc.php
@@ -44,7 +44,7 @@ class HardwareInfo
 			return '';
 		$hw = new HardwareQuery(self::PCI_DEVICE, $best['machineuuid'], true);
 		// TODO: Get list of enabled pass through groups for this client's location
-		$hw->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 +77,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..525e05e8 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,26 @@ 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
+	{
+		$columns = [];
+		foreach ($this->columns as $column) {
+			if ($column instanceof HardwareQueryColumn) {
+				$column->generate($this->joins, $columns, $this->args);
+			} 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 +151,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..71793008
--- /dev/null
+++ b/modules-available/statistics/inc/hardwarequerycolumn.inc.php
@@ -0,0 +1,71 @@
+<?php
+
+class HardwareQueryColumn
+{
+	/** @var int For unique table names in join */
+	private static $id = 0;
+
+	private $global;
+	private $tableAlias;
+	private $virtualColumnName;
+	private $alias;
+	private $conditions = [];
+	private $params = [];
+
+	private static function getId(): string
+	{
+		return 't' . ++self::$id;
+	}
+
+	public function __construct(bool $global, string $column, string $alias = null)
+	{
+		$this->global = $global;
+		$this->tableAlias = self::getId();
+		$this->virtualColumnName = $column;
+		$this->alias = $alias ?? $column;
+	}
+
+	public function generate(array &$joins, array &$columns, array &$params)
+	{
+		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) . ")";
+		$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) {
+			$this->conditions[] = "{$this->tableAlias}.`$valueCol` $op {$other->tableAlias}.`$valueCol`";
+		} 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..d2d8c982 100644
--- a/modules-available/statistics/pages/hints.inc.php
+++ b/modules-available/statistics/pages/hints.inc.php
@@ -30,7 +30,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)
@@ -62,7 +63,8 @@ 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]);
 	}
@@ -77,7 +79,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 +93,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) {