From 01aaa90861621936bbf6e0c99aad4ff9cc7ce5a3 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 14:42:39 +0100 Subject: [PATCH 1/9] Remove $_SESSION['is_multi_query'] It's not clear what this was supposed to do, but it definitely shouldn't be done this way. Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 2 +- src/Controllers/Import/ImportController.php | 10 +--------- src/Sql.php | 1 - src/Table/Table.php | 4 ---- 4 files changed, 2 insertions(+), 15 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 96bcb5feec..ebc510099c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -15467,7 +15467,7 @@ parameters: - message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" - count: 20 + count: 19 path: src/Table/Table.php - diff --git a/src/Controllers/Import/ImportController.php b/src/Controllers/Import/ImportController.php index a32010ef4b..a981ccbed5 100644 --- a/src/Controllers/Import/ImportController.php +++ b/src/Controllers/Import/ImportController.php @@ -112,12 +112,6 @@ final class ImportController extends AbstractController // default values $GLOBALS['reload'] = false; - // Use to identify current cycle is executing - // a multiquery statement or stored routine - if (! isset($_SESSION['is_multi_query'])) { - $_SESSION['is_multi_query'] = false; - } - $GLOBALS['ajax_reload'] = []; $GLOBALS['import_text'] = ''; // Are we just executing plain query or sql file? @@ -678,9 +672,7 @@ final class ImportController extends AbstractController } if ($GLOBALS['go_sql']) { - if ($queriesToBeExecuted !== []) { - $_SESSION['is_multi_query'] = true; - } else { + if ($queriesToBeExecuted === []) { $queriesToBeExecuted = [$GLOBALS['sql_query']]; } diff --git a/src/Sql.php b/src/Sql.php index 103a1ef246..088949ac7d 100644 --- a/src/Sql.php +++ b/src/Sql.php @@ -1088,7 +1088,6 @@ class Sql ); } - $_SESSION['is_multi_query'] = false; $displayResultsObject->setProperties( $unlimNumRows, $this->dbi->getFieldsMeta($result), diff --git a/src/Table/Table.php b/src/Table/Table.php index 71800e54b5..439ed8d503 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -289,10 +289,6 @@ class Table implements Stringable bool $forceRead = false, bool $disableError = false, ): mixed { - if (! empty($_SESSION['is_multi_query'])) { - $disableError = true; - } - $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); // sometimes there is only one entry (ExactRows) so From 7373b58cd529da1cb327ee4c0782bb17f7386cae Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 15:00:25 +0100 Subject: [PATCH 2/9] Remove trigger_error from getStatusInfo() This is a custom undefined index error, which is a developer error, not a runtime error. It should be enforced in the code, and should not throw a user error. Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 7 +------ psalm-baseline.xml | 2 +- src/Table/Table.php | 36 +++++++++--------------------------- 3 files changed, 11 insertions(+), 34 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ebc510099c..86bb16ca15 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -15452,7 +15452,7 @@ parameters: - message: "#^Cannot access offset string on mixed\\.$#" - count: 6 + count: 7 path: src/Table/Table.php - @@ -15610,11 +15610,6 @@ parameters: count: 2 path: src/Table/Table.php - - - message: "#^Parameter \\#2 \\$array of function array_key_exists expects array, mixed given\\.$#" - count: 1 - path: src/Table/Table.php - - message: "#^Parameter \\#2 \\$array of function array_map expects array, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 90349ebde5..577706cbd8 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -11596,7 +11596,6 @@ $cachedResult - $cachedResult $column $column @@ -11645,6 +11644,7 @@ dbName][$this->name]]]> + $cachedResult[$info] diff --git a/src/Table/Table.php b/src/Table/Table.php index 439ed8d503..be4447e4dd 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -34,7 +34,6 @@ use PhpMyAdmin\Util; use Stringable; use function __; -use function array_key_exists; use function array_keys; use function array_map; use function array_merge; @@ -64,11 +63,8 @@ use function strtolower; use function strtoupper; use function substr; use function substr_compare; -use function trigger_error; use function trim; -use const E_USER_WARNING; - /** * Handles everything related to tables * @@ -277,9 +273,8 @@ class Table implements Stringable * Returns full table status info, or specific if $info provided * this info is collected from information_schema * - * @param string|null $info specific information to be fetched - * @param bool $forceRead read new rather than serving from cache - * @param bool $disableError if true, disables error message + * @param string|null $info specific information to be fetched + * @param bool $forceRead read new rather than serving from cache * * @todo DatabaseInterface::getTablesFull needs to be merged * somehow into this class or at least better documented @@ -287,7 +282,6 @@ class Table implements Stringable public function getStatusInfo( string|null $info = null, bool $forceRead = false, - bool $disableError = false, ): mixed { $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); @@ -309,18 +303,6 @@ class Table implements Stringable return $cachedResult; } - // array_key_exists allows for null values - if (! array_key_exists($info, $cachedResult)) { - if (! $disableError) { - trigger_error( - __('Unknown table status:') . ' ' . $info, - E_USER_WARNING, - ); - } - - return false; - } - return $cachedResult[$info]; } @@ -332,7 +314,7 @@ class Table implements Stringable */ public function getStorageEngine(): string { - $tableStorageEngine = $this->getStatusInfo('ENGINE', false, true); + $tableStorageEngine = $this->getStatusInfo('ENGINE', false); return strtoupper((string) $tableStorageEngine); } @@ -344,7 +326,7 @@ class Table implements Stringable */ public function getComment(): string { - $tableComment = $this->getStatusInfo('TABLE_COMMENT', false, true); + $tableComment = $this->getStatusInfo('TABLE_COMMENT', false); if ($tableComment === false) { return ''; } @@ -359,7 +341,7 @@ class Table implements Stringable */ public function getCollation(): string { - $tableCollation = $this->getStatusInfo('TABLE_COLLATION', false, true); + $tableCollation = $this->getStatusInfo('TABLE_COLLATION', false); if ($tableCollation === false) { return ''; } @@ -374,7 +356,7 @@ class Table implements Stringable */ public function getNumRows(string $showTableName): int { - $tableNumRowInfo = $this->getStatusInfo('TABLE_ROWS', false, true); + $tableNumRowInfo = $this->getStatusInfo('TABLE_ROWS', false); if ($tableNumRowInfo === false) { $tableNumRowInfo = $this->dbi->getTable($this->dbName, $showTableName) ->countRecords(true); @@ -390,7 +372,7 @@ class Table implements Stringable */ public function getRowFormat(): string { - $tableRowFormat = $this->getStatusInfo('ROW_FORMAT', false, true); + $tableRowFormat = $this->getStatusInfo('ROW_FORMAT', false); return is_string($tableRowFormat) ? $tableRowFormat : ''; } @@ -402,7 +384,7 @@ class Table implements Stringable */ public function getAutoIncrement(): string { - $tableAutoIncrement = $this->getStatusInfo('AUTO_INCREMENT', false, true); + $tableAutoIncrement = $this->getStatusInfo('AUTO_INCREMENT', false); return $tableAutoIncrement ?? ''; } @@ -414,7 +396,7 @@ class Table implements Stringable */ public function getCreateOptions(): array { - $tableOptions = $this->getStatusInfo('CREATE_OPTIONS', false, true); + $tableOptions = $this->getStatusInfo('CREATE_OPTIONS', false); $createOptionsTmp = empty($tableOptions) ? [] : explode(' ', $tableOptions); $createOptions = []; // export create options by its name as variables into global namespace From 62fac42bd8af90aee2c3395e53cfbd0000d84817 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 15:08:09 +0100 Subject: [PATCH 3/9] Improve code quality Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 5 ----- psalm-baseline.xml | 7 ++----- src/Controllers/Operations/TableController.php | 6 +++--- src/Plugins/Schema/Pdf/PdfRelationSchema.php | 6 +----- 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 86bb16ca15..fd14f74c7d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3370,11 +3370,6 @@ parameters: count: 2 path: src/Controllers/Operations/DatabaseController.php - - - message: "#^Cannot access offset 'Row_format' on mixed\\.$#" - count: 1 - path: src/Controllers/Operations/TableController.php - - message: "#^Cannot access offset 'back' on mixed\\.$#" count: 2 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 577706cbd8..4bca645d4f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -2400,15 +2400,12 @@ $tableAlters is_array($partitionNames) ? $partitionNames : [] - - - - $showTable - $showTable + $rowFormat + $rowFormat diff --git a/src/Controllers/Operations/TableController.php b/src/Controllers/Operations/TableController.php index 08b6464265..7ae9aef9dc 100644 --- a/src/Controllers/Operations/TableController.php +++ b/src/Controllers/Operations/TableController.php @@ -118,7 +118,7 @@ class TableController extends AbstractController */ $this->dbi->selectDb(Current::$database); - $showTable = $pmaTable->getStatusInfo(); + $rowFormat = $pmaTable->getStatusInfo('Row_format'); if ($pmaTable->isView()) { $tableIsAView = true; $tableStorageEngine = __('View'); @@ -327,7 +327,7 @@ class TableController extends AbstractController // a change, clear the cache $this->dbi->getCache()->clearTableCache(); $this->dbi->selectDb(Current::$database); - $showTable = $pmaTable->getStatusInfo(forceRead: true); + $rowFormat = $pmaTable->getStatusInfo('Row_format'); if ($pmaTable->isView()) { $tableIsAView = true; $tableStorageEngine = __('View'); @@ -501,7 +501,7 @@ class TableController extends AbstractController 'collations' => $collations, 'tbl_collation' => $tableCollation, 'row_formats' => $possibleRowFormats[$tableStorageEngine] ?? [], - 'row_format_current' => $showTable['Row_format'], + 'row_format_current' => $rowFormat, 'has_auto_increment' => $hasAutoIncrement, 'auto_increment' => $GLOBALS['auto_increment'], 'has_pack_keys' => $hasPackKeys, diff --git a/src/Plugins/Schema/Pdf/PdfRelationSchema.php b/src/Plugins/Schema/Pdf/PdfRelationSchema.php index 094fb74ebd..505143955c 100644 --- a/src/Plugins/Schema/Pdf/PdfRelationSchema.php +++ b/src/Plugins/Schema/Pdf/PdfRelationSchema.php @@ -537,11 +537,7 @@ class PdfRelationSchema extends ExportRelationSchema $mimeMap = $this->transformations->getMime($this->db->getName(), $table, true); } - /** - * Gets table information - */ - $showTable = $dbi->getTable($this->db->getName(), $table) - ->getStatusInfo(); + $showTable = $dbi->getTable($this->db->getName(), $table)->getStatusInfo(); $showComment = $showTable['Comment'] ?? ''; $createTime = isset($showTable['Create_time']) ? Util::localisedDate( From 6ab2b59d8df1c848c2770f8c0f04f875f240af70 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 17:31:54 +0100 Subject: [PATCH 4/9] Remove redundant arguments Signed-off-by: Kamil Tekiela --- src/Table/Table.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Table/Table.php b/src/Table/Table.php index be4447e4dd..3d470c5045 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -314,7 +314,7 @@ class Table implements Stringable */ public function getStorageEngine(): string { - $tableStorageEngine = $this->getStatusInfo('ENGINE', false); + $tableStorageEngine = $this->getStatusInfo('ENGINE'); return strtoupper((string) $tableStorageEngine); } @@ -326,7 +326,7 @@ class Table implements Stringable */ public function getComment(): string { - $tableComment = $this->getStatusInfo('TABLE_COMMENT', false); + $tableComment = $this->getStatusInfo('TABLE_COMMENT'); if ($tableComment === false) { return ''; } @@ -341,7 +341,7 @@ class Table implements Stringable */ public function getCollation(): string { - $tableCollation = $this->getStatusInfo('TABLE_COLLATION', false); + $tableCollation = $this->getStatusInfo('TABLE_COLLATION'); if ($tableCollation === false) { return ''; } @@ -356,7 +356,7 @@ class Table implements Stringable */ public function getNumRows(string $showTableName): int { - $tableNumRowInfo = $this->getStatusInfo('TABLE_ROWS', false); + $tableNumRowInfo = $this->getStatusInfo('TABLE_ROWS'); if ($tableNumRowInfo === false) { $tableNumRowInfo = $this->dbi->getTable($this->dbName, $showTableName) ->countRecords(true); @@ -372,7 +372,7 @@ class Table implements Stringable */ public function getRowFormat(): string { - $tableRowFormat = $this->getStatusInfo('ROW_FORMAT', false); + $tableRowFormat = $this->getStatusInfo('ROW_FORMAT'); return is_string($tableRowFormat) ? $tableRowFormat : ''; } @@ -384,7 +384,7 @@ class Table implements Stringable */ public function getAutoIncrement(): string { - $tableAutoIncrement = $this->getStatusInfo('AUTO_INCREMENT', false); + $tableAutoIncrement = $this->getStatusInfo('AUTO_INCREMENT'); return $tableAutoIncrement ?? ''; } @@ -396,7 +396,7 @@ class Table implements Stringable */ public function getCreateOptions(): array { - $tableOptions = $this->getStatusInfo('CREATE_OPTIONS', false); + $tableOptions = $this->getStatusInfo('CREATE_OPTIONS'); $createOptionsTmp = empty($tableOptions) ? [] : explode(' ', $tableOptions); $createOptions = []; // export create options by its name as variables into global namespace From 1b913bb1da6bde9ddfa6334d9f9c751de4a77cbf Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 17:47:33 +0100 Subject: [PATCH 5/9] Privatize StructureController's elemets This class cannot be made final because it's mocked, but it's never extended and should not be extended. Signed-off-by: Kamil Tekiela --- src/Controllers/Table/StructureController.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Controllers/Table/StructureController.php b/src/Controllers/Table/StructureController.php index 0e4279e89f..39f563b5d1 100644 --- a/src/Controllers/Table/StructureController.php +++ b/src/Controllers/Table/StructureController.php @@ -48,7 +48,7 @@ use function strtotime; */ class StructureController extends AbstractController { - protected readonly Table $tableObj; + private readonly Table $tableObj; public function __construct( ResponseRenderer $response, @@ -150,7 +150,7 @@ class StructureController extends AbstractController * @param (string|int)[] $columnsWithIndex Columns with index * @psalm-param non-empty-string $route */ - protected function displayStructure( + private function displayStructure( RelationParameters $relationParameters, array $columnsWithUniqueIndex, Index|null $primaryIndex, @@ -292,7 +292,7 @@ class StructureController extends AbstractController /** * Get HTML snippet for display table statistics */ - protected function getTableStats( + private function getTableStats( bool $isSystemSchema, bool $tableIsAView, string $tableStorageEngine, From 58226717f43e001f0bc70ab3ccc9224e89fda9e9 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 17:52:26 +0100 Subject: [PATCH 6/9] Use clearTableCache() instead of $forceRead Signed-off-by: Kamil Tekiela --- src/Controllers/Table/StructureController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Controllers/Table/StructureController.php b/src/Controllers/Table/StructureController.php index 39f563b5d1..c8712bc7bb 100644 --- a/src/Controllers/Table/StructureController.php +++ b/src/Controllers/Table/StructureController.php @@ -297,7 +297,9 @@ class StructureController extends AbstractController bool $tableIsAView, string $tableStorageEngine, ): string { - $showTable = $this->dbi->getTable(Current::$database, Current::$table)->getStatusInfo(forceRead: true); + // Clear the cache as some table information might have gotten changed due to the user action. + $this->dbi->getCache()->clearTableCache(); + $showTable = $this->tableObj->getStatusInfo(); $tableInfoNunRows = $this->tableObj->getNumRows($showTable['Name']); if (is_string($showTable)) { From 00617da9756de5c692404df08215d68753340b34 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 17:55:34 +0100 Subject: [PATCH 7/9] Remove $forceRead param Signed-off-by: Kamil Tekiela --- src/Table/Table.php | 11 ++++------- tests/classes/Table/TableTest.php | 12 ++++++------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/Table/Table.php b/src/Table/Table.php index 3d470c5045..749d2b377d 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -273,21 +273,18 @@ class Table implements Stringable * Returns full table status info, or specific if $info provided * this info is collected from information_schema * - * @param string|null $info specific information to be fetched - * @param bool $forceRead read new rather than serving from cache + * @param string|null $info specific information to be fetched * * @todo DatabaseInterface::getTablesFull needs to be merged * somehow into this class or at least better documented */ - public function getStatusInfo( - string|null $info = null, - bool $forceRead = false, - ): mixed { + public function getStatusInfo(string|null $info = null): mixed + { $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); // sometimes there is only one entry (ExactRows) so // we have to get the table's details - if ($cachedResult === null || $forceRead || count($cachedResult) === 1) { + if ($cachedResult === null || count($cachedResult) === 1) { $this->dbi->getTablesFull($this->dbName, $this->name); $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); } diff --git a/tests/classes/Table/TableTest.php b/tests/classes/Table/TableTest.php index d7915f63fe..3d4b81969a 100644 --- a/tests/classes/Table/TableTest.php +++ b/tests/classes/Table/TableTest.php @@ -1446,7 +1446,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = 'DBIDUMMY'; $tblStorageEngine = $dbi->getTable($targetDb, $targetTable)->getStorageEngine(); $this->assertEquals($expect, $tblStorageEngine); @@ -1462,7 +1462,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = 'Test comment for "table1" in \'pma_test\''; $showComment = $dbi->getTable($targetDb, $targetTable)->getComment(); $this->assertEquals($expect, $showComment); @@ -1478,7 +1478,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = 'utf8mb4_general_ci'; $tblCollation = $dbi->getTable($targetDb, $targetTable)->getCollation(); $this->assertEquals($expect, $tblCollation); @@ -1494,7 +1494,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = 'Redundant'; $rowFormat = $dbi->getTable($targetDb, $targetTable)->getRowFormat(); $this->assertEquals($expect, $rowFormat); @@ -1510,7 +1510,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = '5'; $autoIncrement = $dbi->getTable($targetDb, $targetTable)->getAutoIncrement(); $this->assertEquals($expect, $autoIncrement); @@ -1526,7 +1526,7 @@ class TableTest extends AbstractTestCase $extension = new DbiDummy(); $dbi = new DatabaseInterface($extension); $tblObject = new Table($targetTable, $targetDb, $dbi); - $tblObject->getStatusInfo(null, true); + $tblObject->getStatusInfo(null); $expect = ['pack_keys' => 'DEFAULT', 'row_format' => 'REDUNDANT']; $createOptions = $dbi->getTable($targetDb, $targetTable)->getCreateOptions(); $this->assertEquals($expect, $createOptions); From 344e3306cfb3a39c0f039f21944ac4744b03f9b7 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 18:16:28 +0100 Subject: [PATCH 8/9] Redesign getCachedTableContent() Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 6 +++--- psalm-baseline.xml | 5 ----- src/Query/Cache.php | 17 ++++++++++------- src/Table/Table.php | 12 ++++++------ 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index fd14f74c7d..5170f67082 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -15447,12 +15447,12 @@ parameters: - message: "#^Cannot access offset string on mixed\\.$#" - count: 7 + count: 6 path: src/Table/Table.php - message: "#^Cannot cast mixed to int\\.$#" - count: 4 + count: 2 path: src/Table/Table.php - @@ -15602,7 +15602,7 @@ parameters: - message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#" - count: 2 + count: 1 path: src/Table/Table.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4bca645d4f..b53c2d3ed0 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -11592,7 +11592,6 @@ - $cachedResult $column $column @@ -11641,7 +11640,6 @@ dbName][$this->name]]]> - $cachedResult[$info] @@ -11700,8 +11698,6 @@ $altered $altered - $cachedResult - $cachedResult $column $column $column @@ -11727,7 +11723,6 @@ $refDbName $ret[] $row - $rowCount $tableAutoIncrement $tableCollation $tableComment diff --git a/src/Query/Cache.php b/src/Query/Cache.php index 6d6373cfb4..2aaa81d1e7 100644 --- a/src/Query/Cache.php +++ b/src/Query/Cache.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace PhpMyAdmin\Query; -use PhpMyAdmin\Util; - /** * Handles caching results */ @@ -46,14 +44,19 @@ class Cache /** * Get a cached value from table cache. * - * @param (int|string)[] $contentPath Array of the name of the target value - * @param mixed $default Return value on cache miss + * @param T $key * - * @return mixed cached value or default + * @return (T is null ? (string|int|null)[] : (string|int|null))|null + * + * @template T of string|null */ - public function getCachedTableContent(array $contentPath, mixed $default = null): mixed + public function getCachedTableContent(string $db, string $table, string|null $key = null): array|string|int|null { - return Util::getValueByKey($this->tableCache, $contentPath, $default); + if ($key === null) { + return $this->tableCache[$db][$table] ?? null; + } + + return $this->tableCache[$db][$table][$key] ?? null; } public function clearTableCache(): void diff --git a/src/Table/Table.php b/src/Table/Table.php index 749d2b377d..921aa85ece 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -218,7 +218,7 @@ class Table implements Stringable // use cached data or load information with SHOW command if ( - $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]) != null + $this->dbi->getCache()->getCachedTableContent($this->dbName, $this->name) !== null || Config::getInstance()->selectedServer['DisableIS'] ) { $type = $this->getStatusInfo('TABLE_TYPE'); @@ -280,13 +280,13 @@ class Table implements Stringable */ public function getStatusInfo(string|null $info = null): mixed { - $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); + $cachedResult = $this->dbi->getCache()->getCachedTableContent($this->dbName, $this->name); // sometimes there is only one entry (ExactRows) so // we have to get the table's details if ($cachedResult === null || count($cachedResult) === 1) { $this->dbi->getTablesFull($this->dbName, $this->name); - $cachedResult = $this->dbi->getCache()->getCachedTableContent([$this->dbName, $this->name]); + $cachedResult = $this->dbi->getCache()->getCachedTableContent($this->dbName, $this->name); } if ($cachedResult === null) { @@ -664,7 +664,7 @@ class Table implements Stringable $isView = $this->isView(); $cache = $this->dbi->getCache(); - $exactRowsCached = $cache->getCachedTableContent([$this->dbName, $this->name, 'ExactRows']); + $exactRowsCached = $cache->getCachedTableContent($this->dbName, $this->name, 'ExactRows'); if ($exactRowsCached !== null) { return (int) $exactRowsCached; } @@ -672,11 +672,11 @@ class Table implements Stringable $rowCount = null; if (! $forceExact) { - if (($cache->getCachedTableContent([$this->dbName, $this->name, 'Rows']) === null) && ! $isView) { + if (($cache->getCachedTableContent($this->dbName, $this->name, 'Rows') === null) && ! $isView) { $this->dbi->getTablesFull($this->dbName, $this->name); } - $rowCount = $cache->getCachedTableContent([$this->dbName, $this->name, 'Rows']); + $rowCount = $cache->getCachedTableContent($this->dbName, $this->name, 'Rows'); } // for a VIEW, $row_count is always false at this point From 6e102ed0e287c41db896dff523013f3678778da8 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 27 Dec 2023 18:26:18 +0100 Subject: [PATCH 9/9] Add type hint for getStatusInfo() Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 121 ++++-------------- psalm-baseline.xml | 97 +++++--------- src/Controllers/Table/StructureController.php | 9 +- src/Table/Table.php | 39 ++---- 4 files changed, 74 insertions(+), 192 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5170f67082..c62501e44c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1590,11 +1590,6 @@ parameters: count: 1 path: src/Controllers/Database/DataDictionaryController.php - - - message: "#^Cannot cast mixed to string\\.$#" - count: 1 - path: src/Controllers/Database/DataDictionaryController.php - - message: "#^Cannot access offset 'dbName' on mixed\\.$#" count: 2 @@ -5890,59 +5885,29 @@ parameters: count: 2 path: src/Controllers/Table/Structure/SaveController.php + - + message: "#^Binary operation \"\\+\" between int\\|non\\-falsy\\-string and int\\|non\\-falsy\\-string results in an error\\.$#" + count: 4 + path: src/Controllers/Table/StructureController.php + + - + message: "#^Binary operation \"\\-\" between mixed and int\\|string results in an error\\.$#" + count: 1 + path: src/Controllers/Table/StructureController.php + + - + message: "#^Binary operation \"/\" between mixed and int\\|string\\|null results in an error\\.$#" + count: 1 + path: src/Controllers/Table/StructureController.php + - message: "#^Call to function in_array\\(\\) requires parameter \\#3 to be set\\.$#" count: 1 path: src/Controllers/Table/StructureController.php - - - message: "#^Cannot access offset 'Check_time' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Create_time' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Data_free' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Data_length' on mixed\\.$#" - count: 7 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Index_length' on mixed\\.$#" - count: 2 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Name' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Rows' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Type' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Cannot access offset 'Update_time' on mixed\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - message: "#^Cannot use array destructuring on array\\\\|null\\.$#" - count: 1 + count: 4 path: src/Controllers/Table/StructureController.php - @@ -5956,28 +5921,18 @@ parameters: path: src/Controllers/Table/StructureController.php - - message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, mixed given\\.$#" + message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, int\\|string given\\.$#" count: 3 path: src/Controllers/Table/StructureController.php - - message: "#^Parameter \\#1 \\$showTableName of method PhpMyAdmin\\\\Table\\\\Table\\:\\:getNumRows\\(\\) expects string, mixed given\\.$#" - count: 1 - path: src/Controllers/Table/StructureController.php - - - - message: "#^Parameter \\#1 \\$timestamp of static method PhpMyAdmin\\\\Util\\:\\:localisedDate\\(\\) expects int, \\(int\\|false\\) given\\.$#" + message: "#^Parameter \\#1 \\$timestamp of static method PhpMyAdmin\\\\Util\\:\\:localisedDate\\(\\) expects int, int\\|false given\\.$#" count: 3 path: src/Controllers/Table/StructureController.php - - - message: "#^Parameter \\#1 \\$value of static method PhpMyAdmin\\\\Util\\:\\:formatByteDown\\(\\) expects float\\|int\\|string\\|null, \\(array\\|float\\|int\\) given\\.$#" - count: 2 - path: src/Controllers/Table/StructureController.php - - message: "#^Parameter \\#1 \\$value of static method PhpMyAdmin\\\\Util\\:\\:formatByteDown\\(\\) expects float\\|int\\|string\\|null, mixed given\\.$#" - count: 3 + count: 4 path: src/Controllers/Table/StructureController.php - @@ -13015,26 +12970,6 @@ parameters: count: 10 path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - - message: "#^Cannot access offset 'Check_time' on mixed\\.$#" - count: 1 - path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - - - message: "#^Cannot access offset 'Comment' on mixed\\.$#" - count: 1 - path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - - - message: "#^Cannot access offset 'Create_time' on mixed\\.$#" - count: 1 - path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - - - message: "#^Cannot access offset 'Update_time' on mixed\\.$#" - count: 1 - path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - message: "#^Cannot access offset 'foreign_field' on mixed\\.$#" count: 1 @@ -13091,7 +13026,7 @@ parameters: path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, mixed given\\.$#" + message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, int\\|string given\\.$#" count: 3 path: src/Plugins/Schema/Pdf/PdfRelationSchema.php @@ -13101,7 +13036,7 @@ parameters: path: src/Plugins/Schema/Pdf/PdfRelationSchema.php - - message: "#^Parameter \\#1 \\$timestamp of static method PhpMyAdmin\\\\Util\\:\\:localisedDate\\(\\) expects int, \\(int\\|false\\) given\\.$#" + message: "#^Parameter \\#1 \\$timestamp of static method PhpMyAdmin\\\\Util\\:\\:localisedDate\\(\\) expects int, int\\|false given\\.$#" count: 3 path: src/Plugins/Schema/Pdf/PdfRelationSchema.php @@ -15452,31 +15387,31 @@ parameters: - message: "#^Cannot cast mixed to int\\.$#" - count: 2 + count: 1 path: src/Table/Table.php - message: "#^Cannot cast mixed to string\\.$#" - count: 3 + count: 2 path: src/Table/Table.php - message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" - count: 19 + count: 18 path: src/Table/Table.php - - message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getAutoIncrement\\(\\) should return string but returns mixed\\.$#" + message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getAutoIncrement\\(\\) should return string but returns int\\|string\\.$#" count: 1 path: src/Table/Table.php - - message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getCollation\\(\\) should return string but returns mixed\\.$#" + message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getCollation\\(\\) should return string but returns int\\|string\\.$#" count: 1 path: src/Table/Table.php - - message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getComment\\(\\) should return string but returns mixed\\.$#" + message: "#^Method PhpMyAdmin\\\\Table\\\\Table\\:\\:getComment\\(\\) should return string but returns int\\|string\\.$#" count: 1 path: src/Table/Table.php @@ -15627,7 +15562,7 @@ parameters: - message: "#^Parameter \\#2 \\$string of function explode expects string, mixed given\\.$#" - count: 3 + count: 2 path: src/Table/Table.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b53c2d3ed0..40a8921d00 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -2404,8 +2404,6 @@ - $rowFormat - $rowFormat @@ -4087,47 +4085,34 @@ - - - - - - - - - - - - - - - - - - - - - $attributes[$rownum] - $showTable icon]]> icon]]> - - - - - + + + + + + + + + + + + + + + + + + + + @@ -5529,7 +5514,6 @@ $query $rel $relationalDisplay - $tableCreateTime $value @@ -9806,9 +9790,6 @@ - - - $table $table $table @@ -9831,10 +9812,6 @@ - - - - pdf->customLinks['RT']['-']]]> pdf->customLinks['doc'][$foreigner['foreign_table']]]]> pdf->customLinks['doc'][$table]]]> @@ -9872,8 +9849,6 @@ $oneField $oneKey $rel - $showComment - $showTable $table $table $type @@ -9884,12 +9859,14 @@ - $showComment $table $table + + + @@ -11588,6 +11565,16 @@ + + + getStatusInfo('TABLE_COLLATION') ?? '']]> + getStatusInfo('TABLE_COMMENT') ?? '']]> + + + string + string + string + @@ -11627,7 +11614,6 @@ - $tableOptions uiprefs[$property]]]> uiprefs[$property]]]> $value @@ -11703,7 +11689,6 @@ $column $constraintName - $currCreateTime $eachCol $foreignDb $foreignDb @@ -11723,14 +11708,6 @@ $refDbName $ret[] $row - $tableAutoIncrement - $tableCollation - $tableComment - $tableNumRowInfo - $tableOptions - $tableRowFormat - $tableStorageEngine - $type $value $value $value @@ -11739,9 +11716,6 @@ mixed[] string string - string - string - string has @@ -11760,11 +11734,6 @@ options]]> - - - - - $tableComment errors)]]> messages)]]> json_decode($value, true) diff --git a/src/Controllers/Table/StructureController.php b/src/Controllers/Table/StructureController.php index c8712bc7bb..e08464a947 100644 --- a/src/Controllers/Table/StructureController.php +++ b/src/Controllers/Table/StructureController.php @@ -38,7 +38,6 @@ use stdClass; use function __; use function in_array; -use function is_string; use function str_contains; use function strtotime; @@ -300,11 +299,6 @@ class StructureController extends AbstractController // Clear the cache as some table information might have gotten changed due to the user action. $this->dbi->getCache()->clearTableCache(); $showTable = $this->tableObj->getStatusInfo(); - $tableInfoNunRows = $this->tableObj->getNumRows($showTable['Name']); - - if (is_string($showTable)) { - $showTable = []; - } if (empty($showTable['Data_length'])) { $showTable['Data_length'] = 0; @@ -353,7 +347,7 @@ class StructureController extends AbstractController $avgSize = ''; $avgUnit = ''; - if ($tableInfoNunRows > 0) { + if ($this->tableObj->getNumRows() > 0) { [$avgSize, $avgUnit] = Util::formatByteDown( ($showTable['Data_length'] + $showTable['Index_length']) @@ -393,7 +387,6 @@ class StructureController extends AbstractController 'db' => Current::$database, 'table' => Current::$table, 'showtable' => $showTable, - 'table_info_num_rows' => $tableInfoNunRows, 'tbl_is_view' => $tableIsAView, 'db_is_system_schema' => $isSystemSchema, 'tbl_storage_engine' => $tableStorageEngine, diff --git a/src/Table/Table.php b/src/Table/Table.php index 921aa85ece..b4acea2b30 100644 --- a/src/Table/Table.php +++ b/src/Table/Table.php @@ -273,12 +273,13 @@ class Table implements Stringable * Returns full table status info, or specific if $info provided * this info is collected from information_schema * - * @param string|null $info specific information to be fetched + * @param T $info specific information to be fetched * - * @todo DatabaseInterface::getTablesFull needs to be merged - * somehow into this class or at least better documented + * @return (T is null ? (string|int|null)[]|null : (string|int|null)) + * + * @template T of string|null */ - public function getStatusInfo(string|null $info = null): mixed + public function getStatusInfo(string|null $info = null): array|string|int|null { $cachedResult = $this->dbi->getCache()->getCachedTableContent($this->dbName, $this->name); @@ -293,7 +294,7 @@ class Table implements Stringable // happens when we enter the table creation dialog // or when we really did not get any status info, for example // when $table === 'TABLE_NAMES' after the user tried SHOW TABLES - return ''; + return null; } if ($info === null) { @@ -323,12 +324,7 @@ class Table implements Stringable */ public function getComment(): string { - $tableComment = $this->getStatusInfo('TABLE_COMMENT'); - if ($tableComment === false) { - return ''; - } - - return $tableComment; + return $this->getStatusInfo('TABLE_COMMENT') ?? ''; } /** @@ -338,12 +334,7 @@ class Table implements Stringable */ public function getCollation(): string { - $tableCollation = $this->getStatusInfo('TABLE_COLLATION'); - if ($tableCollation === false) { - return ''; - } - - return $tableCollation ?? ''; + return $this->getStatusInfo('TABLE_COLLATION') ?? ''; } /** @@ -351,15 +342,9 @@ class Table implements Stringable * * @return int Return no of rows info if it is not null for the selected table or return 0. */ - public function getNumRows(string $showTableName): int + public function getNumRows(): int { - $tableNumRowInfo = $this->getStatusInfo('TABLE_ROWS'); - if ($tableNumRowInfo === false) { - $tableNumRowInfo = $this->dbi->getTable($this->dbName, $showTableName) - ->countRecords(true); - } - - return (int) $tableNumRowInfo; + return (int) $this->getStatusInfo('TABLE_ROWS'); } /** @@ -394,7 +379,7 @@ class Table implements Stringable public function getCreateOptions(): array { $tableOptions = $this->getStatusInfo('CREATE_OPTIONS'); - $createOptionsTmp = empty($tableOptions) ? [] : explode(' ', $tableOptions); + $createOptionsTmp = is_string($tableOptions) && $tableOptions !== '' ? explode(' ', $tableOptions) : []; $createOptions = []; // export create options by its name as variables into global namespace // f.e. pack_keys=1 becomes available as $pack_keys with value of '1' @@ -1775,7 +1760,7 @@ class Table implements Stringable // we want to save the create time if the property is PROP_COLUMN_ORDER if (! $this->isView() && ($property == self::PROP_COLUMN_ORDER || $property == self::PROP_COLUMN_VISIB)) { $currCreateTime = $this->getStatusInfo('CREATE_TIME'); - if (! isset($tableCreateTime) || $tableCreateTime != $currCreateTime) { + if ($tableCreateTime === null || $tableCreateTime != $currCreateTime) { // there is no $table_create_time, or // supplied $table_create_time is older than current create time, // so don't save