From 9e2fb9af2b4e1a0ad58b9f210a19cb4b49a20a44 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 1 Jan 2025 15:47:28 +0000 Subject: [PATCH 1/3] Move queryAndGetNumRows to Node This function is an antipattern so let's limit it to Node class. Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 4 ++-- psalm-baseline.xml | 1 + src/Dbal/DatabaseInterface.php | 19 ------------------- src/Navigation/Nodes/Node.php | 20 ++++++++++++++++++-- src/Navigation/Nodes/NodeTable.php | 6 +++--- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a98fdba9b4..1835305037 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6702,7 +6702,7 @@ parameters: - message: '#^Only booleans are allowed in a negated boolean, PhpMyAdmin\\Dbal\\ResultInterface\|false given\.$#' identifier: booleanNot.exprNotBoolean - count: 3 + count: 2 path: src/Dbal/DatabaseInterface.php - @@ -10200,7 +10200,7 @@ parameters: Use dependency injection instead\.$# ''' identifier: staticMethod.deprecated - count: 7 + count: 8 path: src/Navigation/Nodes/Node.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 7ce1f1f1f5..ad64f16ea6 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6005,6 +6005,7 @@ + diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index 630456cd45..d94421e8a5 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1710,25 +1710,6 @@ class DatabaseInterface return $this->extension->getError($this->connections[$connectionType->value]); } - /** - * returns the number of rows returned by last query - * used with tryQuery as it accepts false - * - * @param string $query query to run - * - * @psalm-return int|numeric-string - */ - public function queryAndGetNumRows(string $query): string|int - { - $result = $this->tryQuery($query); - - if (! $result) { - return 0; - } - - return $result->numRows(); - } - /** * returns last inserted auto_increment id for given $link */ diff --git a/src/Navigation/Nodes/Node.php b/src/Navigation/Nodes/Node.php index 8e03d0c7db..3041f162ad 100644 --- a/src/Navigation/Nodes/Node.php +++ b/src/Navigation/Nodes/Node.php @@ -379,13 +379,13 @@ class Node $query = 'SHOW DATABASES '; $query .= $this->getWhereClause('Database', $searchClause); - return (int) $dbi->queryAndGetNumRows($query); + return $this->queryAndGetNumRows($query); } $retval = 0; foreach ($this->getDatabasesToSearch($userPrivileges, $searchClause) as $db) { $query = 'SHOW DATABASES LIKE ' . $dbi->quoteString($db); - $retval += (int) $dbi->queryAndGetNumRows($query); + $retval += $this->queryAndGetNumRows($query); } return $retval; @@ -831,4 +831,20 @@ class Node return $retval; } + + /** + * returns the number of rows returned by last query + * used with tryQuery as it accepts false + */ + protected function queryAndGetNumRows(string $query): int + { + $dbi = DatabaseInterface::getInstance(); + $result = $dbi->tryQuery($query); + + if ($result === false) { + return 0; + } + + return (int) $result->numRows(); + } } diff --git a/src/Navigation/Nodes/NodeTable.php b/src/Navigation/Nodes/NodeTable.php index 246baabe23..14632669d7 100644 --- a/src/Navigation/Nodes/NodeTable.php +++ b/src/Navigation/Nodes/NodeTable.php @@ -94,7 +94,7 @@ class NodeTable extends NodeDatabaseChild $db = Util::backquote($db); $table = Util::backquote($table); $query = 'SHOW COLUMNS FROM ' . $table . ' FROM ' . $db; - $retval = (int) $dbi->queryAndGetNumRows($query); + $retval = $this->queryAndGetNumRows($query); } break; @@ -102,7 +102,7 @@ class NodeTable extends NodeDatabaseChild $db = Util::backquote($db); $table = Util::backquote($table); $query = 'SHOW INDEXES FROM ' . $table . ' FROM ' . $db; - $retval = (int) $dbi->queryAndGetNumRows($query); + $retval = $this->queryAndGetNumRows($query); break; case 'triggers': if (! $this->config->selectedServer['DisableIS']) { @@ -116,7 +116,7 @@ class NodeTable extends NodeDatabaseChild } else { $db = Util::backquote($db); $query = 'SHOW TRIGGERS FROM ' . $db . ' WHERE `Table` = ' . $dbi->quoteString($table); - $retval = (int) $dbi->queryAndGetNumRows($query); + $retval = $this->queryAndGetNumRows($query); } break; From 326da8369f467a276e2bf2ab340df83bd00c3d9a Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 1 Jan 2025 19:53:50 +0000 Subject: [PATCH 2/3] Implement executeQuery Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 5 +++++ src/Dbal/DatabaseInterface.php | 9 +++++++++ src/Dbal/DbiExtension.php | 7 +++++++ src/Dbal/DbiMysqli.php | 18 ++++++++++++++++++ src/Server/Privileges.php | 16 +++++++--------- tests/unit/DatabaseInterfaceTest.php | 13 +++++++++++++ tests/unit/Server/PrivilegesTest.php | 10 +++------- tests/unit/Stubs/DbiDummy.php | 10 ++++++++++ 8 files changed, 72 insertions(+), 16 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index ad64f16ea6..145ccb251d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -4309,6 +4309,11 @@ ]]> + + + + + diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index d94421e8a5..498fc9c867 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1960,6 +1960,15 @@ class DatabaseInterface return $this->extension->prepare($this->connections[$connectionType->value], $query); } + /** @param list $params */ + public function executeQuery( + string $query, + array $params, + ConnectionType $connectionType = ConnectionType::User, + ): ResultInterface|null { + return $this->extension->executeQuery($this->connections[$connectionType->value], $query, $params); + } + public function getDatabaseList(): ListDatabase { if ($this->databaseList === null) { diff --git a/src/Dbal/DbiExtension.php b/src/Dbal/DbiExtension.php index 619f831b54..49287b1a6f 100644 --- a/src/Dbal/DbiExtension.php +++ b/src/Dbal/DbiExtension.php @@ -97,6 +97,13 @@ interface DbiExtension */ public function prepare(Connection $connection, string $query): Statement|null; + /** + * Execute a prepared statement and return the result. + * + * @param list $params + */ + public function executeQuery(Connection $connection, string $query, array $params): ResultInterface|null; + /** * Returns the number of warnings from the last query. */ diff --git a/src/Dbal/DbiMysqli.php b/src/Dbal/DbiMysqli.php index dd993d4b3b..1864577204 100644 --- a/src/Dbal/DbiMysqli.php +++ b/src/Dbal/DbiMysqli.php @@ -300,6 +300,24 @@ class DbiMysqli implements DbiExtension return new MysqliStatement($statement); } + /** + * Execute a prepared statement and return the result. + * + * @param list $params + */ + public function executeQuery(Connection $connection, string $query, array $params): MysqliResult|null + { + /** @var mysqli $mysqli */ + $mysqli = $connection->connection; + $result = $mysqli->execute_query($query, $params); + + if ($result === false) { + return null; + } + + return new MysqliResult($result); + } + /** * Returns the number of warnings from the last query. */ diff --git a/src/Server/Privileges.php b/src/Server/Privileges.php index 6f94a08294..e157c69cf2 100644 --- a/src/Server/Privileges.php +++ b/src/Server/Privileges.php @@ -1196,12 +1196,12 @@ class Privileges NOT (`Table_priv` = \'\' AND Column_priv = \'\') ORDER BY `User` ASC, `Host` ASC, `Db` ASC, `Table_priv` ASC; '; - $statement = $this->dbi->prepare($query); - if ($statement === null || ! $statement->execute([$db->getName(), $table->getName()])) { + $result = $this->dbi->executeQuery($query, [$db->getName(), $table->getName()]); + if ($result === null) { return []; } - return $statement->getResult()->fetchAllAssoc(); + return $result->fetchAllAssoc(); } /** @return array> */ @@ -3171,12 +3171,11 @@ class Privileges private function getUserPrivileges(string $user, string $host, bool $hasAccountLocking): array|null { $query = 'SELECT * FROM `mysql`.`user` WHERE `User` = ? AND `Host` = ?;'; - $statement = $this->dbi->prepare($query); - if ($statement === null || ! $statement->execute([$user, $host])) { + $result = $this->dbi->executeQuery($query, [$user, $host]); + if ($result === null) { return null; } - $result = $statement->getResult(); /** @var array|null $userPrivileges */ $userPrivileges = $result->fetchAssoc(); if ($userPrivileges === []) { @@ -3190,12 +3189,11 @@ class Privileges $userPrivileges['account_locked'] = 'N'; $query = 'SELECT * FROM `mysql`.`global_priv` WHERE `User` = ? AND `Host` = ?;'; - $statement = $this->dbi->prepare($query); - if ($statement === null || ! $statement->execute([$user, $host])) { + $result = $this->dbi->executeQuery($query, [$user, $host]); + if ($result === null) { return $userPrivileges; } - $result = $statement->getResult(); /** @var array|null $globalPrivileges */ $globalPrivileges = $result->fetchAssoc(); if ($globalPrivileges === []) { diff --git a/tests/unit/DatabaseInterfaceTest.php b/tests/unit/DatabaseInterfaceTest.php index 81da15ca99..271b326981 100644 --- a/tests/unit/DatabaseInterfaceTest.php +++ b/tests/unit/DatabaseInterfaceTest.php @@ -771,6 +771,19 @@ class DatabaseInterfaceTest extends AbstractTestCase self::assertSame($stmtStub, $stmt); } + public function testExecuteQuery(): void + { + $query = 'SELECT * FROM `mysql`.`user` WHERE `User` = ? AND `Host` = ?;'; + $resultStub = self::createStub(ResultInterface::class); + $dummyDbi = $this->createMock(DbiExtension::class); + $dummyDbi->expects(self::once())->method('executeQuery') + ->with(self::isType('object'), self::equalTo($query), self::equalTo(['root', 'localhost'])) + ->willReturn($resultStub); + $dbi = $this->createDatabaseInterface($dummyDbi); + $stmt = $dbi->executeQuery($query, ['root', 'localhost'], ConnectionType::ControlUser); + self::assertSame($resultStub, $stmt); + } + /** * Tests for setVersion method. * diff --git a/tests/unit/Server/PrivilegesTest.php b/tests/unit/Server/PrivilegesTest.php index 4762ff8efc..5ac3996cf1 100644 --- a/tests/unit/Server/PrivilegesTest.php +++ b/tests/unit/Server/PrivilegesTest.php @@ -12,7 +12,6 @@ use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\ConnectionType; use PhpMyAdmin\Dbal\DatabaseInterface; use PhpMyAdmin\Dbal\ResultInterface; -use PhpMyAdmin\Dbal\Statement; use PhpMyAdmin\Html\Generator; use PhpMyAdmin\Http\Factory\ServerRequestFactory; use PhpMyAdmin\Message; @@ -1896,18 +1895,15 @@ class PrivilegesTest extends AbstractTestCase public function testGetUserPrivileges(): void { $mysqliResultStub = $this->createMock(ResultInterface::class); - $mysqliStmtStub = $this->createMock(Statement::class); - $mysqliStmtStub->expects(self::exactly(2))->method('execute')->willReturn(true); - $mysqliStmtStub->expects(self::exactly(2))->method('getResult')->willReturn($mysqliResultStub); $dbi = $this->createMock(DatabaseInterface::class); $dbi->expects(self::once())->method('isMariaDB')->willReturn(true); $userQuery = 'SELECT * FROM `mysql`.`user` WHERE `User` = ? AND `Host` = ?;'; $globalPrivQuery = 'SELECT * FROM `mysql`.`global_priv` WHERE `User` = ? AND `Host` = ?;'; - $dbi->expects(self::exactly(2))->method('prepare')->willReturnMap([ - [$userQuery, ConnectionType::User, $mysqliStmtStub], - [$globalPrivQuery, ConnectionType::User, $mysqliStmtStub], + $dbi->expects(self::exactly(2))->method('executeQuery')->willReturnMap([ + [$userQuery, ['test.user', 'test.host'], ConnectionType::User, $mysqliResultStub], + [$globalPrivQuery,['test.user', 'test.host'], ConnectionType::User, $mysqliResultStub], ]); $mysqliResultStub->expects(self::exactly(2)) diff --git a/tests/unit/Stubs/DbiDummy.php b/tests/unit/Stubs/DbiDummy.php index ca334e25c0..deb391678e 100644 --- a/tests/unit/Stubs/DbiDummy.php +++ b/tests/unit/Stubs/DbiDummy.php @@ -319,6 +319,16 @@ class DbiDummy implements DbiExtension return null; } + /** + * Execute a prepared statement and return the result. + * + * @param list $params + */ + public function executeQuery(Connection $connection, string $query, array $params): ResultInterface|null + { + return null; + } + /** * Returns the number of warnings from the last query. */ From 04d39125051c25ad988be70ada8960a777eb9668 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 1 Jan 2025 19:56:37 +0000 Subject: [PATCH 3/3] Remove prepare method and Statement class Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 5 ---- src/Dbal/DatabaseInterface.php | 10 ------- src/Dbal/DbiExtension.php | 7 ----- src/Dbal/DbiMysqli.php | 17 ----------- src/Dbal/MysqliStatement.php | 39 ------------------------- src/Dbal/Statement.php | 20 ------------- tests/unit/DatabaseInterfaceTest.php | 14 --------- tests/unit/Dbal/MysqliStatementTest.php | 25 ---------------- tests/unit/Stubs/DbiDummy.php | 6 ---- 9 files changed, 143 deletions(-) delete mode 100644 src/Dbal/MysqliStatement.php delete mode 100644 src/Dbal/Statement.php delete mode 100644 tests/unit/Dbal/MysqliStatementTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 145ccb251d..ad64f16ea6 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -4309,11 +4309,6 @@ ]]> - - - - - diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index 498fc9c867..afaa88a777 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1950,16 +1950,6 @@ class DatabaseInterface $this->isPercona = stripos($this->versionComment, 'percona') !== false; } - /** - * Prepare an SQL statement for execution. - * - * @param string $query The query, as a string. - */ - public function prepare(string $query, ConnectionType $connectionType = ConnectionType::User): Statement|null - { - return $this->extension->prepare($this->connections[$connectionType->value], $query); - } - /** @param list $params */ public function executeQuery( string $query, diff --git a/src/Dbal/DbiExtension.php b/src/Dbal/DbiExtension.php index 49287b1a6f..2c7e4ad33b 100644 --- a/src/Dbal/DbiExtension.php +++ b/src/Dbal/DbiExtension.php @@ -90,13 +90,6 @@ interface DbiExtension */ public function escapeString(Connection $connection, string $string): string; - /** - * Prepare an SQL statement for execution. - * - * @param string $query The query, as a string. - */ - public function prepare(Connection $connection, string $query): Statement|null; - /** * Execute a prepared statement and return the result. * diff --git a/src/Dbal/DbiMysqli.php b/src/Dbal/DbiMysqli.php index 1864577204..f7a8946167 100644 --- a/src/Dbal/DbiMysqli.php +++ b/src/Dbal/DbiMysqli.php @@ -283,23 +283,6 @@ class DbiMysqli implements DbiExtension return $mysqli->real_escape_string($string); } - /** - * Prepare an SQL statement for execution. - * - * @param string $query The query, as a string. - */ - public function prepare(Connection $connection, string $query): Statement|null - { - /** @var mysqli $mysqli */ - $mysqli = $connection->connection; - $statement = $mysqli->prepare($query); - if ($statement === false) { - return null; - } - - return new MysqliStatement($statement); - } - /** * Execute a prepared statement and return the result. * diff --git a/src/Dbal/MysqliStatement.php b/src/Dbal/MysqliStatement.php deleted file mode 100644 index 1e8557c3c8..0000000000 --- a/src/Dbal/MysqliStatement.php +++ /dev/null @@ -1,39 +0,0 @@ - $params - */ - public function execute(array $params): bool - { - $paramCount = $this->statement->param_count; - if (count($params) !== $paramCount) { - return false; - } - - return $this->statement->execute($params); - } - - /** - * Gets a result set from a prepared statement. - */ - public function getResult(): ResultInterface - { - return new MysqliResult($this->statement->get_result()); - } -} diff --git a/src/Dbal/Statement.php b/src/Dbal/Statement.php deleted file mode 100644 index 721206bba5..0000000000 --- a/src/Dbal/Statement.php +++ /dev/null @@ -1,20 +0,0 @@ - $params - */ - public function execute(array $params): bool; - - /** - * Gets a result set from a prepared statement. - */ - public function getResult(): ResultInterface; -} diff --git a/tests/unit/DatabaseInterfaceTest.php b/tests/unit/DatabaseInterfaceTest.php index 271b326981..81b481295d 100644 --- a/tests/unit/DatabaseInterfaceTest.php +++ b/tests/unit/DatabaseInterfaceTest.php @@ -13,7 +13,6 @@ use PhpMyAdmin\Dbal\ConnectionType; use PhpMyAdmin\Dbal\DatabaseInterface; use PhpMyAdmin\Dbal\DbiExtension; use PhpMyAdmin\Dbal\ResultInterface; -use PhpMyAdmin\Dbal\Statement; use PhpMyAdmin\I18n\LanguageManager; use PhpMyAdmin\Index; use PhpMyAdmin\Query\Utilities; @@ -758,19 +757,6 @@ class DatabaseInterfaceTest extends AbstractTestCase $dummyDbi->assertAllQueriesConsumed(); } - public function testPrepare(): void - { - $query = 'SELECT * FROM `mysql`.`user` WHERE `User` = ? AND `Host` = ?;'; - $stmtStub = self::createStub(Statement::class); - $dummyDbi = $this->createMock(DbiExtension::class); - $dummyDbi->expects(self::once())->method('prepare') - ->with(self::isType('object'), self::equalTo($query)) - ->willReturn($stmtStub); - $dbi = $this->createDatabaseInterface($dummyDbi); - $stmt = $dbi->prepare($query, ConnectionType::ControlUser); - self::assertSame($stmtStub, $stmt); - } - public function testExecuteQuery(): void { $query = 'SELECT * FROM `mysql`.`user` WHERE `User` = ? AND `Host` = ?;'; diff --git a/tests/unit/Dbal/MysqliStatementTest.php b/tests/unit/Dbal/MysqliStatementTest.php deleted file mode 100644 index 7cc19b689d..0000000000 --- a/tests/unit/Dbal/MysqliStatementTest.php +++ /dev/null @@ -1,25 +0,0 @@ -expects(self::once())->method('get_result')->willReturn(false); - $statement = new MysqliStatement($mysqliStmt); - $result = $statement->getResult(); - self::assertInstanceOf(MysqliResult::class, $result); - } -} diff --git a/tests/unit/Stubs/DbiDummy.php b/tests/unit/Stubs/DbiDummy.php index deb391678e..d62157788c 100644 --- a/tests/unit/Stubs/DbiDummy.php +++ b/tests/unit/Stubs/DbiDummy.php @@ -15,7 +15,6 @@ use PhpMyAdmin\Config\Settings\Server; use PhpMyAdmin\Dbal\Connection; use PhpMyAdmin\Dbal\DbiExtension; use PhpMyAdmin\Dbal\ResultInterface; -use PhpMyAdmin\Dbal\Statement; use PhpMyAdmin\FieldMetadata; use PhpMyAdmin\Identifiers\DatabaseName; use PhpMyAdmin\Tests\FieldHelper; @@ -314,11 +313,6 @@ class DbiDummy implements DbiExtension $this->dummyQueries = []; } - public function prepare(Connection $connection, string $query): Statement|null - { - return null; - } - /** * Execute a prepared statement and return the result. *