From ec738218a10e807c8dda873494ab87fd174dafbc Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:00:04 +0100 Subject: [PATCH 1/6] Refactor Data Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 25 -------------------- psalm-baseline.xml | 13 ----------- src/Server/Status/Data.php | 48 +++++++++++++++++--------------------- 3 files changed, 21 insertions(+), 65 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3f450848f6c0..bcb8135c50e7 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -13745,11 +13745,6 @@ parameters: count: 1 path: src/Server/Select.php - - - message: "#^Cannot access offset 'Com_admin_commands' on mixed\\.$#" - count: 1 - path: src/Server/Status/Data.php - - message: "#^Cannot access offset 'doc' on mixed\\.$#" count: 4 @@ -13775,26 +13770,6 @@ parameters: count: 1 path: src/Server/Status/Data.php - - - message: "#^Parameter \\#2 \\$needle of function str_contains expects string, \\(int\\|string\\) given\\.$#" - count: 1 - path: src/Server/Status/Data.php - - - - message: "#^Property PhpMyAdmin\\\\Server\\\\Status\\\\Data\\:\\:\\$allocationMap \\(array\\) does not accept mixed\\.$#" - count: 1 - path: src/Server/Status/Data.php - - - - message: "#^Property PhpMyAdmin\\\\Server\\\\Status\\\\Data\\:\\:\\$sectionUsed \\(array\\) does not accept mixed\\.$#" - count: 1 - path: src/Server/Status/Data.php - - - - message: "#^Property PhpMyAdmin\\\\Server\\\\Status\\\\Data\\:\\:\\$usedQueries \\(array\\) does not accept mixed\\.$#" - count: 1 - path: src/Server/Status/Data.php - - message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#" count: 4 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e57f9dfbd189..5bdd5b99441d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -2899,7 +2899,6 @@ $linkName $linkUrl $sectionLinks - $sectionName $value @@ -10984,30 +10983,18 @@ - $filter $name - - - - - $sectionUsed[$section] - - $allocationMap[$name] $keyReadRequests $keyReads $keyWriteRequests $keyWrites - $section - allocationMap]]> - sectionUsed]]> - usedQueries]]> $usedQueries[$name] $value diff --git a/src/Server/Status/Data.php b/src/Server/Status/Data.php index 6adb58fb839d..17560b86fe2e 100644 --- a/src/Server/Status/Data.php +++ b/src/Server/Status/Data.php @@ -31,7 +31,7 @@ class Data /** @var mixed[] */ public array $status; - /** @var mixed[] */ + /** @var array */ public array $sections; /** @var mixed[] */ @@ -40,7 +40,7 @@ class Data /** @var mixed[] */ public array $usedQueries; - /** @var mixed[] */ + /** @var string[] */ public array $allocationMap; /** @var mixed[] */ @@ -48,7 +48,7 @@ class Data public bool $dbIsLocal; - /** @var mixed[] */ + /** @var true[] */ public array $sectionUsed; public bool $dataLoaded; @@ -74,7 +74,7 @@ public function __set(string $a, mixed $b): void /** * Gets the allocations for constructor * - * @return mixed[] + * @return array */ private function getAllocations(): array { @@ -128,7 +128,7 @@ private function getAllocations(): array /** * Gets the sections for constructor * - * @return mixed[] + * @return array */ private function getSections(): array { @@ -284,21 +284,25 @@ private function calculateValues(array $serverStatus, array $serverVariables): a /** * Sort variables into arrays * - * @param mixed[] $serverStatus contains results of SHOW GLOBAL STATUS - * @param mixed[] $allocations allocations for sections - * @param mixed[] $allocationMap map variables to their section - * @param mixed[] $sectionUsed is a section used? - * @param mixed[] $usedQueries used queries + * @param mixed[] $serverStatus contains results of SHOW GLOBAL STATUS + * @param array $allocations allocations for sections * - * @return mixed[] ($allocationMap, $sectionUsed, $used_queries) + * @return array{string[], true[], mixed[]} */ private function sortVariables( array $serverStatus, array $allocations, - array $allocationMap, - array $sectionUsed, - array $usedQueries, ): array { + // Variable to contain all com_ variables (query statistics) + $usedQueries = []; + + // Variable to map variable names to their respective section name + // (used for js category filtering) + $allocationMap = []; + + // Variable to mark used sections + $sectionUsed = []; + foreach ($serverStatus as $name => $value) { $sectionFound = false; foreach ($allocations as $filter => $section) { @@ -360,22 +364,12 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf // define some needful links/commands $links = $this->getLinks(); - // Variable to contain all com_ variables (query statistics) - $usedQueries = []; - - // Variable to map variable names to their respective section name - // (used for js category filtering) - $allocationMap = []; - - // Variable to mark used sections - $sectionUsed = []; - // sort vars into arrays [ $allocationMap, $sectionUsed, $usedQueries, - ] = $this->sortVariables($serverStatus, $allocations, $allocationMap, $sectionUsed, $usedQueries); + ] = $this->sortVariables($serverStatus, $allocations); // admin commands are not queries (e.g. they include COM_PING, // which is excluded from $server_status['Questions']) @@ -405,9 +399,9 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf /** * cleanup of some deprecated values * - * @param mixed[] $serverStatus status array to process + * @param (string|null)[] $serverStatus status array to process * - * @return mixed[] + * @return (string|null)[] */ public static function cleanDeprecated(array $serverStatus): array { From 8973a9dd2d5da769604e3951aeb1aaf0a61acf24 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:13:00 +0100 Subject: [PATCH 2/6] Replace function with a constant Signed-off-by: Kamil Tekiela --- src/Server/Status/Data.php | 120 ++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 68 deletions(-) diff --git a/src/Server/Status/Data.php b/src/Server/Status/Data.php index 17560b86fe2e..41b8b0be0499 100644 --- a/src/Server/Status/Data.php +++ b/src/Server/Status/Data.php @@ -28,6 +28,52 @@ */ class Data { + private const ALLOCATIONS = [ + // variable name => section + // variable names match when they begin with the given string + + 'Com_' => 'com', + 'Innodb_' => 'innodb', + 'Ndb_' => 'ndb', + 'Handler_' => 'handler', + 'Qcache_' => 'qcache', + 'Threads_' => 'threads', + 'Slow_launch_threads' => 'threads', + + 'Binlog_cache_' => 'binlog_cache', + 'Created_tmp_' => 'created_tmp', + 'Key_' => 'key', + + 'Delayed_' => 'delayed', + 'Not_flushed_delayed_rows' => 'delayed', + + 'Flush_commands' => 'query', + 'Last_query_cost' => 'query', + 'Slow_queries' => 'query', + 'Queries' => 'query', + 'Prepared_stmt_count' => 'query', + + 'Select_' => 'select', + 'Sort_' => 'sort', + + 'Open_tables' => 'table', + 'Opened_tables' => 'table', + 'Open_table_definitions' => 'table', + 'Opened_table_definitions' => 'table', + 'Table_locks_' => 'table', + + 'Rpl_status' => 'repl', + 'Slave_' => 'repl', + + 'Tc_' => 'tc', + + 'Ssl_' => 'ssl', + + 'Open_files' => 'files', + 'Open_streams' => 'files', + 'Opened_files' => 'files', + ]; + /** @var mixed[] */ public array $status; @@ -71,60 +117,6 @@ public function __set(string $a, mixed $b): void // Discard everything } - /** - * Gets the allocations for constructor - * - * @return array - */ - private function getAllocations(): array - { - return [ - // variable name => section - // variable names match when they begin with the given string - - 'Com_' => 'com', - 'Innodb_' => 'innodb', - 'Ndb_' => 'ndb', - 'Handler_' => 'handler', - 'Qcache_' => 'qcache', - 'Threads_' => 'threads', - 'Slow_launch_threads' => 'threads', - - 'Binlog_cache_' => 'binlog_cache', - 'Created_tmp_' => 'created_tmp', - 'Key_' => 'key', - - 'Delayed_' => 'delayed', - 'Not_flushed_delayed_rows' => 'delayed', - - 'Flush_commands' => 'query', - 'Last_query_cost' => 'query', - 'Slow_queries' => 'query', - 'Queries' => 'query', - 'Prepared_stmt_count' => 'query', - - 'Select_' => 'select', - 'Sort_' => 'sort', - - 'Open_tables' => 'table', - 'Opened_tables' => 'table', - 'Open_table_definitions' => 'table', - 'Opened_table_definitions' => 'table', - 'Table_locks_' => 'table', - - 'Rpl_status' => 'repl', - 'Slave_' => 'repl', - - 'Tc_' => 'tc', - - 'Ssl_' => 'ssl', - - 'Open_files' => 'files', - 'Open_streams' => 'files', - 'Opened_files' => 'files', - ]; - } - /** * Gets the sections for constructor * @@ -284,15 +276,12 @@ private function calculateValues(array $serverStatus, array $serverVariables): a /** * Sort variables into arrays * - * @param mixed[] $serverStatus contains results of SHOW GLOBAL STATUS - * @param array $allocations allocations for sections + * @param mixed[] $serverStatus contains results of SHOW GLOBAL STATUS * * @return array{string[], true[], mixed[]} */ - private function sortVariables( - array $serverStatus, - array $allocations, - ): array { + private function sortVariables(array $serverStatus): array + { // Variable to contain all com_ variables (query statistics) $usedQueries = []; @@ -305,7 +294,7 @@ private function sortVariables( foreach ($serverStatus as $name => $value) { $sectionFound = false; - foreach ($allocations as $filter => $section) { + foreach (self::ALLOCATIONS as $filter => $section) { if (! str_contains($name, $filter)) { continue; } @@ -356,11 +345,6 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf // calculate some values $serverStatus = $this->calculateValues($serverStatus, $serverVariables); - // split variables in sections - $allocations = $this->getAllocations(); - - $sections = $this->getSections(); - // define some needful links/commands $links = $this->getLinks(); @@ -369,7 +353,7 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf $allocationMap, $sectionUsed, $usedQueries, - ] = $this->sortVariables($serverStatus, $allocations); + ] = $this->sortVariables($serverStatus); // admin commands are not queries (e.g. they include COM_PING, // which is excluded from $server_status['Questions']) @@ -388,7 +372,7 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf } $this->status = $serverStatus; - $this->sections = $sections; + $this->sections = $this->getSections(); $this->variables = $serverVariables; $this->usedQueries = $usedQueries; $this->allocationMap = $allocationMap; From de970d22915e282542c2fe44319f20be7bfe3e81 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:40:57 +0100 Subject: [PATCH 3/6] Remove useless comments, mark final and readonly Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 23 ++++++++++---- src/Server/Status/Data.php | 65 ++++++++++++++------------------------ 2 files changed, 40 insertions(+), 48 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 5bdd5b99441d..f1f49d53994a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -11016,13 +11016,6 @@ - - __set - - - $a - $b - $dbi @@ -13370,6 +13363,10 @@ DatabaseInterface::getInstance() DatabaseInterface::getInstance() + + data->dataLoaded]]> + data->dataLoaded]]> + @@ -13434,6 +13431,10 @@ Config::getInstance() DatabaseInterface::getInstance() + + data->status]]> + data->usedQueries]]> + $totalQueries * $hourFactor data->status['Uptime']]]> @@ -13455,6 +13456,14 @@ DatabaseInterface::getInstance() DatabaseInterface::getInstance() + + status]]> + status]]> + status]]> + status]]> + status]]> + status]]> + diff --git a/src/Server/Status/Data.php b/src/Server/Status/Data.php index 41b8b0be0499..7cca825e805d 100644 --- a/src/Server/Status/Data.php +++ b/src/Server/Status/Data.php @@ -1,8 +1,4 @@ section + * variable names match when they begin with the given string + */ private const ALLOCATIONS = [ - // variable name => section - // variable names match when they begin with the given string - 'Com_' => 'com', 'Innodb_' => 'innodb', 'Ndb_' => 'ndb', @@ -74,32 +69,39 @@ class Data 'Opened_files' => 'files', ]; - /** @var mixed[] */ + /** + * @var mixed[] + * @readonly + * */ public array $status; /** @var array */ - public array $sections; + public readonly array $sections; /** @var mixed[] */ - public array $variables; + public readonly array $variables; - /** @var mixed[] */ + /** + * @var mixed[] + * @readonly + */ public array $usedQueries; /** @var string[] */ - public array $allocationMap; + public readonly array $allocationMap; /** @var mixed[] */ - public array $links; + public readonly array $links; - public bool $dbIsLocal; + public readonly bool $dbIsLocal; /** @var true[] */ - public array $sectionUsed; + public readonly array $sectionUsed; + /** @readonly */ public bool $dataLoaded; - private ReplicationInfo $replicationInfo; + private readonly ReplicationInfo $replicationInfo; public function getReplicationInfo(): ReplicationInfo { @@ -107,25 +109,13 @@ public function getReplicationInfo(): ReplicationInfo } /** - * An empty setter makes the above properties read-only - * - * @param string $a key - * @param mixed $b value - */ - public function __set(string $a, mixed $b): void - { - // Discard everything - } - - /** - * Gets the sections for constructor + * Returns the map of section => section name (description) * * @return array */ private function getSections(): array { return [ - // section => section name (description) 'com' => 'Com', 'query' => __('SQL query'), 'innodb' => 'InnoDB', @@ -359,17 +349,10 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf // which is excluded from $server_status['Questions']) unset($usedQueries['Com_admin_commands']); - // Set all class properties - $this->dbIsLocal = false; - // can be null if $cfg['ServerDefault'] = 0; $serverHostToLower = mb_strtolower($config->selectedServer['host']); - if ( - $serverHostToLower === 'localhost' + $this->dbIsLocal = $serverHostToLower === 'localhost' || $config->selectedServer['host'] === '127.0.0.1' - || $config->selectedServer['host'] === '::1' - ) { - $this->dbIsLocal = true; - } + || $config->selectedServer['host'] === '::1'; $this->status = $serverStatus; $this->sections = $this->getSections(); From 8be66c5d13e877bfa7a88be6a2a668ba899c7d3a Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:45:12 +0100 Subject: [PATCH 4/6] Remove $sectionFound Signed-off-by: Kamil Tekiela --- src/Server/Status/Data.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Server/Status/Data.php b/src/Server/Status/Data.php index 7cca825e805d..9cfbd29c6642 100644 --- a/src/Server/Status/Data.php +++ b/src/Server/Status/Data.php @@ -283,7 +283,6 @@ private function sortVariables(array $serverStatus): array $sectionUsed = []; foreach ($serverStatus as $name => $value) { - $sectionFound = false; foreach (self::ALLOCATIONS as $filter => $section) { if (! str_contains($name, $filter)) { continue; @@ -291,16 +290,11 @@ private function sortVariables(array $serverStatus): array $allocationMap[$name] = $section; $sectionUsed[$section] = true; - $sectionFound = true; if ($section === 'com' && $value > 0) { $usedQueries[$name] = $value; } - break; // Only exits inner loop - } - - if ($sectionFound) { - continue; + continue 2; } $allocationMap[$name] = 'other'; From 092af6e87847b224d790a0f4961a0f80ee3348de Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:54:56 +0100 Subject: [PATCH 5/6] Remove unnecessary comments Signed-off-by: Kamil Tekiela --- src/Server/Status/Data.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Server/Status/Data.php b/src/Server/Status/Data.php index 9cfbd29c6642..a6df004902a6 100644 --- a/src/Server/Status/Data.php +++ b/src/Server/Status/Data.php @@ -279,7 +279,6 @@ private function sortVariables(array $serverStatus): array // (used for js category filtering) $allocationMap = []; - // Variable to mark used sections $sectionUsed = []; foreach ($serverStatus as $name => $value) { @@ -309,7 +308,6 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf $this->replicationInfo = new ReplicationInfo($this->dbi); $this->replicationInfo->load($_POST['primary_connection'] ?? null); - // get status from server $serverStatusResult = $this->dbi->tryQuery('SHOW GLOBAL STATUS'); if ($serverStatusResult === false) { $serverStatus = []; @@ -323,16 +321,12 @@ public function __construct(private DatabaseInterface $dbi, private Config $conf // for some calculations we require also some server settings $serverVariables = $this->dbi->fetchResult('SHOW GLOBAL VARIABLES', 0, 1); - // cleanup of some deprecated values $serverStatus = self::cleanDeprecated($serverStatus); - // calculate some values $serverStatus = $this->calculateValues($serverStatus, $serverVariables); - // define some needful links/commands $links = $this->getLinks(); - // sort vars into arrays [ $allocationMap, $sectionUsed, From b715c8143f7f7a967c55a2cc4beacc4d73e468da Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 6 Feb 2024 16:55:34 +0100 Subject: [PATCH 6/6] Remove confusing continue Signed-off-by: Kamil Tekiela --- src/Server/Status/Processes.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Server/Status/Processes.php b/src/Server/Status/Processes.php index 7af4e5593012..88ad300ba194 100644 --- a/src/Server/Status/Processes.php +++ b/src/Server/Status/Processes.php @@ -133,11 +133,8 @@ private function getSortableColumnsForProcessList( } $columns[$columnKey]['has_full_query'] = true; - if (! $showFullSql) { - continue; - } - $columns[$columnKey]['is_full'] = true; + $columns[$columnKey]['is_full'] = $showFullSql; } return $columns;