From ec96f2252ecc51f079d5169851b1f2b7ea5a26ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Roland=20H=C3=A4der?= <roland@mxchange.org>
Date: Mon, 20 Jun 2022 03:10:02 +0200
Subject: [PATCH] Changes: - added type-hints - added some missing
 documentation

---
 src/Database/DBA.php      |  70 ++++++++++++------------
 src/Database/Database.php | 111 +++++++++++++++++++++++---------------
 2 files changed, 101 insertions(+), 80 deletions(-)

diff --git a/src/Database/DBA.php b/src/Database/DBA.php
index d62edf6ce1..d0a0bbe2f4 100644
--- a/src/Database/DBA.php
+++ b/src/Database/DBA.php
@@ -90,7 +90,7 @@ class DBA
 	 *
 	 * @return string
 	 */
-	public static function serverInfo()
+	public static function serverInfo(): string
 	{
 		return DI::dba()->serverInfo();
 	}
@@ -101,7 +101,7 @@ class DBA
 	 * @return string
 	 * @throws \Exception
 	 */
-	public static function databaseName()
+	public static function databaseName(): string
 	{
 		return DI::dba()->databaseName();
 	}
@@ -112,7 +112,7 @@ class DBA
 	 * @param string $str
 	 * @return string escaped string
 	 */
-	public static function escape($str)
+	public static function escape(string $str): string
 	{
 		return DI::dba()->escape($str);
 	}
@@ -122,7 +122,7 @@ class DBA
 	 *
 	 * @return boolean is the database connected?
 	 */
-	public static function connected()
+	public static function connected(): bool
 	{
 		return DI::dba()->connected();
 	}
@@ -138,7 +138,7 @@ class DBA
 	 * @param string $sql An SQL string without the values
 	 * @return string The input SQL string modified if necessary.
 	 */
-	public static function anyValueFallback($sql)
+	public static function anyValueFallback(string $sql): string
 	{
 		return DI::dba()->anyValueFallback($sql);
 	}
@@ -152,7 +152,7 @@ class DBA
 	 * @param string $sql An SQL string without the values
 	 * @return string The input SQL string modified if necessary.
 	 */
-	public static function cleanQuery($sql)
+	public static function cleanQuery(string $sql): string
 	{
 		$search = ["\t", "\n", "\r", "  "];
 		$replace = [' ', ' ', ' ', ' '];
@@ -169,7 +169,7 @@ class DBA
 	 * @param array $args Parameter array
 	 * @return array universalized parameter array
 	 */
-	public static function getParam($args)
+	public static function getParam(array $args): array
 	{
 		unset($args[0]);
 
@@ -192,7 +192,7 @@ class DBA
 	 * @return bool|object statement object or result object
 	 * @throws \Exception
 	 */
-	public static function p($sql)
+	public static function p(string $sql)
 	{
 		$params = self::getParam(func_get_args());
 
@@ -208,8 +208,8 @@ class DBA
 	 * @return boolean Was the query successfull? False is returned only if an error occurred
 	 * @throws \Exception
 	 */
-	public static function e($sql) {
-
+	public static function e(string $sql): bool
+	{
 		$params = self::getParam(func_get_args());
 
 		return DI::dba()->e($sql, $params);
@@ -220,11 +220,10 @@ class DBA
 	 *
 	 * @param string|array $table     Table name or array [schema => table]
 	 * @param array        $condition array of fields for condition
-	 *
 	 * @return boolean Are there rows for that condition?
 	 * @throws \Exception
 	 */
-	public static function exists($table, $condition)
+	public static function exists(string $table, array $condition): bool
 	{
 		return DI::dba()->exists($table, $condition);
 	}
@@ -238,7 +237,7 @@ class DBA
 	 * @return array first row of query
 	 * @throws \Exception
 	 */
-	public static function fetchFirst($sql)
+	public static function fetchFirst(string $sql)
 	{
 		$params = self::getParam(func_get_args());
 
@@ -250,7 +249,7 @@ class DBA
 	 *
 	 * @return int Number of rows
 	 */
-	public static function affectedRows()
+	public static function affectedRows(): int
 	{
 		return DI::dba()->affectedRows();
 	}
@@ -261,7 +260,7 @@ class DBA
 	 * @param object Statement object
 	 * @return int Number of columns
 	 */
-	public static function columnCount($stmt)
+	public static function columnCount($stmt): int
 	{
 		return DI::dba()->columnCount($stmt);
 	}
@@ -271,7 +270,7 @@ class DBA
 	 * @param PDOStatement|mysqli_result|mysqli_stmt Statement object
 	 * @return int Number of rows
 	 */
-	public static function numRows($stmt)
+	public static function numRows($stmt): int
 	{
 		return DI::dba()->numRows($stmt);
 	}
@@ -297,7 +296,7 @@ class DBA
 	 * @return boolean was the insert successful?
 	 * @throws \Exception
 	 */
-	public static function insert($table, array $param, int $duplicate_mode = Database::INSERT_DEFAULT)
+	public static function insert($table, array $param, int $duplicate_mode = Database::INSERT_DEFAULT): bool
 	{
 		return DI::dba()->insert($table, $param, $duplicate_mode);
 	}
@@ -312,7 +311,7 @@ class DBA
 	 * @return boolean was the insert successful?
 	 * @throws \Exception
 	 */
-	public static function replace($table, $param)
+	public static function replace($table, array $param): bool
 	{
 		return DI::dba()->replace($table, $param);
 	}
@@ -322,7 +321,7 @@ class DBA
 	 *
 	 * @return integer Last inserted id
 	 */
-	public static function lastInsertId()
+	public static function lastInsertId(): int
 	{
 		return DI::dba()->lastInsertId();
 	}
@@ -337,7 +336,7 @@ class DBA
 	 * @return boolean was the lock successful?
 	 * @throws \Exception
 	 */
-	public static function lock($table)
+	public static function lock($table): bool
 	{
 		return DI::dba()->lock($table);
 	}
@@ -348,7 +347,7 @@ class DBA
 	 * @return boolean was the unlock successful?
 	 * @throws \Exception
 	 */
-	public static function unlock()
+	public static function unlock(): bool
 	{
 		return DI::dba()->unlock();
 	}
@@ -358,7 +357,7 @@ class DBA
 	 *
 	 * @return boolean Was the command executed successfully?
 	 */
-	public static function transaction()
+	public static function transaction(): bool
 	{
 		return DI::dba()->transaction();
 	}
@@ -368,7 +367,7 @@ class DBA
 	 *
 	 * @return boolean Was the command executed successfully?
 	 */
-	public static function commit()
+	public static function commit(): bool
 	{
 		return DI::dba()->commit();
 	}
@@ -378,7 +377,7 @@ class DBA
 	 *
 	 * @return boolean Was the command executed successfully?
 	 */
-	public static function rollback()
+	public static function rollback(): bool
 	{
 		return DI::dba()->rollback();
 	}
@@ -392,7 +391,7 @@ class DBA
 	 * @return boolean was the delete successful?
 	 * @throws \Exception
 	 */
-	public static function delete($table, array $conditions, array $options = [])
+	public static function delete($table, array $conditions, array $options = []): bool
 	{
 		return DI::dba()->delete($table, $conditions, $options);
 	}
@@ -427,7 +426,7 @@ class DBA
 	 * @return boolean was the update successfull?
 	 * @throws \Exception
 	 */
-	public static function update($table, array $fields, array $condition, $old_fields = [], array $params = [])
+	public static function update($table, array $fields, array $condition, $old_fields = [], array $params = []): bool
 	{
 		return DI::dba()->update($table, $fields, $condition, $old_fields, $params);
 	}
@@ -528,7 +527,7 @@ class DBA
 	 * @param string|array $tables
 	 * @return string
 	 */
-	public static function buildTableString($tables)
+	public static function buildTableString($tables): string
 	{
 		if (is_string($tables)) {
 			$tables = [$tables];
@@ -553,7 +552,7 @@ class DBA
 	 * @param $identifier
 	 * @return string
 	 */
-	public static function quoteIdentifier($identifier)
+	public static function quoteIdentifier(string $identifier): string
 	{
 		return '`' . str_replace('`', '``', $identifier) . '`';
 	}
@@ -576,7 +575,7 @@ class DBA
 	 * @param array $condition
 	 * @return string
 	 */
-	public static function buildCondition(array &$condition = [])
+	public static function buildCondition(array &$condition = []): string
 	{
 		$condition = self::collapseCondition($condition);
 		
@@ -600,7 +599,7 @@ class DBA
 	 * @param array $condition
 	 * @return array
 	 */
-	public static function collapseCondition(array $condition)
+	public static function collapseCondition(array $condition): array
 	{
 		// Ensures an always true condition is returned
 		if (count($condition) < 1) {
@@ -675,7 +674,7 @@ class DBA
 	 * @return array A collapsed condition
 	 * @see DBA::collapseCondition() for the condition array formats
 	 */
-	public static function mergeConditions(array ...$conditions)
+	public static function mergeConditions(array ...$conditions): array
 	{
 		if (count($conditions) == 1) {
 			return current($conditions);
@@ -724,7 +723,7 @@ class DBA
 	 * @param array $params
 	 * @return string
 	 */
-	public static function buildParameter(array $params = [])
+	public static function buildParameter(array $params = []): string
 	{
 		$groupby_string = '';
 		if (!empty($params['group_by'])) {
@@ -771,7 +770,7 @@ class DBA
 	 *
 	 * @return array Data array
 	 */
-	public static function toArray($stmt, $do_close = true, int $count = 0): array
+	public static function toArray($stmt, bool $do_close = true, int $count = 0): array
 	{
 		return DI::dba()->toArray($stmt, $do_close, $count);
 	}
@@ -847,10 +846,9 @@ class DBA
 	 * Checks if $array is a filled array with at least one entry.
 	 *
 	 * @param mixed $array A filled array with at least one entry
-	 *
 	 * @return boolean Whether $array is a filled array or an object with rows
 	 */
-	public static function isResult($array)
+	public static function isResult($array): bool
 	{
 		return DI::dba()->isResult($array);
 	}
@@ -862,7 +860,7 @@ class DBA
 	 * @param boolean $add_quotation add quotation marks for string values
 	 * @return void
 	 */
-	public static function escapeArray(&$arr, $add_quotation = false)
+	public static function escapeArray(&$arr, bool $add_quotation = false)
 	{
 		DI::dba()->escapeArray($arr, $add_quotation);
 	}
diff --git a/src/Database/Database.php b/src/Database/Database.php
index 7edbdf2a2c..590c3d83d0 100644
--- a/src/Database/Database.php
+++ b/src/Database/Database.php
@@ -88,7 +88,12 @@ class Database
 		}
 	}
 
-	public function connect()
+	/**
+	 * Tries to connect to database
+	 *
+	 * @return bool Success
+	 */
+	public function connect(): bool
 	{
 		if (!is_null($this->connection) && $this->connected()) {
 			return $this->connected;
@@ -255,7 +260,7 @@ class Database
 	 *
 	 * @return string with either "pdo" or "mysqli"
 	 */
-	public function getDriver()
+	public function getDriver(): string
 	{
 		return $this->driver;
 	}
@@ -266,9 +271,9 @@ class Database
 	 * This function discriminate between the deprecated mysql API and the current
 	 * object-oriented mysqli API. Example of returned string: 5.5.46-0+deb8u1
 	 *
-	 * @return string
+	 * @return string Database server information
 	 */
-	public function serverInfo()
+	public function serverInfo(): string
 	{
 		if ($this->server_info == '') {
 			switch ($this->driver) {
@@ -286,10 +291,10 @@ class Database
 	/**
 	 * Returns the selected database name
 	 *
-	 * @return string
+	 * @return string Database name
 	 * @throws \Exception
 	 */
-	public function databaseName()
+	public function databaseName(): string
 	{
 		$ret  = $this->p("SELECT DATABASE() AS `db`");
 		$data = $this->toArray($ret);
@@ -300,10 +305,10 @@ class Database
 	 * Analyze a database query and log this if some conditions are met.
 	 *
 	 * @param string $query The database query that will be analyzed
-	 *
+	 * @return void
 	 * @throws \Exception
 	 */
-	private function logIndex($query)
+	private function logIndex(string $query)
 	{
 
 		if (!$this->configCache->get('system', 'db_log_index')) {
@@ -359,11 +364,10 @@ class Database
 	 * Removes every not allowlisted character from the identifier string
 	 *
 	 * @param string $identifier
-	 *
 	 * @return string sanitized identifier
 	 * @throws \Exception
 	 */
-	private function sanitizeIdentifier($identifier)
+	private function sanitizeIdentifier(string $identifier): string
 	{
 		return preg_replace('/[^A-Za-z0-9_\-]+/', '', $identifier);
 	}
@@ -383,11 +387,21 @@ class Database
 		}
 	}
 
-	public function isConnected()
+	/**
+	 * Returns connected flag
+	 *
+	 * @return bool Whether connection to database was success
+	 */
+	public function isConnected(): bool
 	{
 		return $this->connected;
 	}
 
+	/**
+	 * Checks connection status
+	 *
+	 * @return bool Whether connection to database was success
+	 */
 	public function connected()
 	{
 		$connected = false;
@@ -424,7 +438,7 @@ class Database
 	 *
 	 * @return string The input SQL string modified if necessary.
 	 */
-	public function anyValueFallback($sql)
+	public function anyValueFallback(string $sql): string
 	{
 		$server_info = $this->serverInfo();
 		if (version_compare($server_info, '5.7.5', '<') ||
@@ -442,7 +456,7 @@ class Database
 	 *
 	 * @return string The replaced SQL query
 	 */
-	private function replaceParameters($sql, $args)
+	private function replaceParameters(string $sql, array $args): string
 	{
 		$offset = 0;
 		foreach ($args as $param => $value) {
@@ -476,7 +490,7 @@ class Database
 	 * @return bool|object statement object or result object
 	 * @throws \Exception
 	 */
-	public function p($sql)
+	public function p(string $sql)
 	{
 
 		$this->profiler->startRecording('database');
@@ -741,8 +755,9 @@ class Database
 	 * @return boolean Was the query successfull? False is returned only if an error occurred
 	 * @throws \Exception
 	 */
-	public function e($sql)
+	public function e(string $sql): bool
 	{
+		$retval = false;
 
 		$this->profiler->startRecording('database_write');
 
@@ -810,7 +825,7 @@ class Database
 	 * @return boolean Are there rows for that condition?
 	 * @throws \Exception
 	 */
-	public function exists($table, $condition)
+	public function exists($table, array $condition): bool
 	{
 		if (empty($table)) {
 			return false;
@@ -850,10 +865,10 @@ class Database
 	 *
 	 * @param string $sql SQL statement
 	 *
-	 * @return array first row of query
+	 * @return array|bool first row of query or false on failure
 	 * @throws \Exception
 	 */
-	public function fetchFirst($sql)
+	public function fetchFirst(string $sql)
 	{
 		$params = DBA::getParam(func_get_args());
 
@@ -875,7 +890,7 @@ class Database
 	 *
 	 * @return int Number of rows
 	 */
-	public function affectedRows()
+	public function affectedRows(): int
 	{
 		return $this->affected_rows;
 	}
@@ -887,7 +902,7 @@ class Database
 	 *
 	 * @return int Number of columns
 	 */
-	public function columnCount($stmt)
+	public function columnCount($stmt): int
 	{
 		if (!is_object($stmt)) {
 			return 0;
@@ -908,7 +923,7 @@ class Database
 	 *
 	 * @return int Number of rows
 	 */
-	public function numRows($stmt)
+	public function numRows($stmt): int
 	{
 		if (!is_object($stmt)) {
 			return 0;
@@ -927,7 +942,7 @@ class Database
 	 *
 	 * @param bool|PDOStatement|mysqli_stmt $stmt statement object
 	 *
-	 * @return array|false current row
+	 * @return array|bool Current row or false on failure
 	 */
 	public function fetch($stmt)
 	{
@@ -994,7 +1009,7 @@ class Database
 	 * @return boolean was the insert successful?
 	 * @throws \Exception
 	 */
-	public function insert($table, array $param, int $duplicate_mode = self::INSERT_DEFAULT)
+	public function insert($table, array $param, int $duplicate_mode = self::INSERT_DEFAULT): bool
 	{
 		if (empty($table) || empty($param)) {
 			$this->logger->info('Table and fields have to be set');
@@ -1044,7 +1059,7 @@ class Database
 	 * @return boolean was the insert successful?
 	 * @throws \Exception
 	 */
-	public function replace($table, array $param)
+	public function replace($table, array $param): bool
 	{
 		if (empty($table) || empty($param)) {
 			$this->logger->info('Table and fields have to be set');
@@ -1069,7 +1084,7 @@ class Database
 	 *
 	 * @return integer Last inserted id
 	 */
-	public function lastInsertId()
+	public function lastInsertId(): int
 	{
 		switch ($this->driver) {
 			case self::PDO:
@@ -1092,7 +1107,7 @@ class Database
 	 * @return boolean was the lock successful?
 	 * @throws \Exception
 	 */
-	public function lock($table)
+	public function lock($table): bool
 	{
 		// See here: https://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html
 		if ($this->driver == self::PDO) {
@@ -1126,7 +1141,7 @@ class Database
 	 * @return boolean was the unlock successful?
 	 * @throws \Exception
 	 */
-	public function unlock()
+	public function unlock(): bool
 	{
 		// See here: https://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html
 		$this->performCommit();
@@ -1177,7 +1192,12 @@ class Database
 		return true;
 	}
 
-	protected function performCommit()
+	/**
+	 * Performs the commit
+	 *
+	 * @return boolean Was the command executed successfully?
+	 */
+	protected function performCommit(): bool
 	{
 		switch ($this->driver) {
 			case self::PDO:
@@ -1199,7 +1219,7 @@ class Database
 	 *
 	 * @return boolean Was the command executed successfully?
 	 */
-	public function commit()
+	public function commit(): bool
 	{
 		if (!$this->performCommit()) {
 			return false;
@@ -1213,7 +1233,7 @@ class Database
 	 *
 	 * @return boolean Was the command executed successfully?
 	 */
-	public function rollback()
+	public function rollback(): bool
 	{
 		$ret = false;
 
@@ -1230,6 +1250,7 @@ class Database
 				$ret = $this->connection->rollback();
 				break;
 		}
+
 		$this->in_transaction = false;
 		return $ret;
 	}
@@ -1243,7 +1264,7 @@ class Database
 	 * @return boolean was the delete successful?
 	 * @throws \Exception
 	 */
-	public function delete($table, array $conditions)
+	public function delete($table, array $conditions): bool
 	{
 		if (empty($table) || empty($conditions)) {
 			$this->logger->info('Table and conditions have to be set');
@@ -1288,8 +1309,9 @@ class Database
 	 *
 	 * @return boolean was the update successfull?
 	 * @throws \Exception
+	 * @todo Implement "bool $update_on_duplicate" to avoid mixed type for $old_fields
 	 */
-	public function update($table, $fields, $condition, $old_fields = [], $params = [])
+	public function update($table, array $fields, array $condition, $old_fields = [], array $params = [])
 	{
 		if (empty($table) || empty($fields) || empty($condition)) {
 			$this->logger->info('Table, fields and condition have to be set');
@@ -1354,7 +1376,7 @@ class Database
 	 * @throws \Exception
 	 * @see   $this->select
 	 */
-	public function selectFirst($table, array $fields = [], array $condition = [], $params = [])
+	public function selectFirst($table, array $fields = [], array $condition = [], array $params = [])
 	{
 		$params['limit'] = 1;
 		$result          = $this->select($table, $fields, $condition, $params);
@@ -1390,9 +1412,9 @@ class Database
 	 *
 	 * @param array $fields 
 	 * @param array $options 
-	 * @return array 
+	 * @return array Escaped fields
 	 */
-	private function escapeFields(array $fields, array $options)
+	private function escapeFields(array $fields, array $options): array
 	{
 		// In the case of a "GROUP BY" we have to add all the ORDER fields to the fieldlist.
 		// This needs to done to apply the "ANY_VALUE(...)" treatment from below to them.
@@ -1504,6 +1526,7 @@ class Database
 	 */
 	public function count($table, array $condition = [], array $params = [])
 	{
+		// @TODO Can we dump this to have ": int" as returned type-hint?
 		if (empty($table)) {
 			return false;
 		}
@@ -1569,7 +1592,8 @@ class Database
 	 * @param array  $fields
 	 * @return array casted fields
 	 */
-	public function castFields(string $table, array $fields) {
+	public function castFields(string $table, array $fields): array
+	{
 		// When there is no data, we don't need to do something
 		if (empty($fields)) {
 			return $fields;
@@ -1642,7 +1666,7 @@ class Database
 	 *
 	 * @return string Error message ('' if no error)
 	 */
-	public function errorMessage()
+	public function errorMessage(): string
 	{
 		return $this->error;
 	}
@@ -1729,7 +1753,7 @@ class Database
 	 * @param string $name
 	 * @return string content
 	 */
-	public function getVariable(string $name)
+	public function getVariable(string $name): string
 	{
 		$result = $this->fetchFirst("SHOW GLOBAL VARIABLES WHERE `Variable_name` = ?", $name);
 		return $result['Value'] ?? null;
@@ -1742,7 +1766,7 @@ class Database
 	 *
 	 * @return boolean Whether $array is a filled array or an object with rows
 	 */
-	public function isResult($array)
+	public function isResult($array): bool
 	{
 		// It could be a return value from an update statement
 		if (is_bool($array)) {
@@ -1762,10 +1786,9 @@ class Database
 	 * @param mixed   $value         Array value
 	 * @param string  $key           Array key
 	 * @param boolean $add_quotation add quotation marks for string values
-	 *
 	 * @return void
 	 */
-	private function escapeArrayCallback(&$value, $key, $add_quotation)
+	private function escapeArrayCallback(&$value, string $key, bool $add_quotation)
 	{
 		if (!$add_quotation) {
 			if (is_bool($value)) {
@@ -1790,10 +1813,9 @@ class Database
 	 *
 	 * @param mixed   $arr           Array with values to be escaped
 	 * @param boolean $add_quotation add quotation marks for string values
-	 *
 	 * @return void
 	 */
-	public function escapeArray(&$arr, $add_quotation = false)
+	public function escapeArray(&$arr, bool $add_quotation = false)
 	{
 		array_walk($arr, [$this, 'escapeArrayCallback'], $add_quotation);
 	}
@@ -1801,10 +1823,11 @@ class Database
 	/**
 	 * Replaces a string in the provided fields of the provided table
 	 *
-	 * @param string $table_name
+	 * @param string $table_name Table name
 	 * @param array  $fields List of field names in the provided table
 	 * @param string $search
 	 * @param string $replace
+	 * @return void
 	 * @throws \Exception
 	 */
 	public function replaceInTableFields(string $table_name, array $fields, string $search, string $replace)