Merge pull request phpmyadmin#18953 from kamil-tekiela/Types-in-Datab…

…aseController Small refactoring of database controller in Server
liviuconcioiu · Feb 1, 2024 · 9b7a7a3 · 9b7a7a3
2 parents c7fd4df + bc8cd6b
commit 9b7a7a3
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 103 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -3446,12 +3446,7 @@ parameters:
 			path: src/Controllers/Server/DatabasesController.php
 
 		-
-			message: "#^Cannot access offset 'raw' on mixed\\.$#"
-			count: 2
-			path: src/Controllers/Server/DatabasesController.php
-
-		-
-			message: "#^Cannot access offset \\(int\\|string\\) on mixed\\.$#"
+			message: "#^Cannot access offset string on mixed\\.$#"
 			count: 2
 			path: src/Controllers/Server/DatabasesController.php
 
@@ -3467,7 +3462,7 @@ parameters:
 
 		-
 			message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
-			count: 2
+			count: 1
 			path: src/Controllers/Server/DatabasesController.php
 
 		-
@@ -3485,11 +3480,6 @@ parameters:
 			count: 1
 			path: src/Controllers/Server/DatabasesController.php
 
-		-
-			message: "#^Parameter \\#1 \\$sortBy of method PhpMyAdmin\\\\Controllers\\\\Server\\\\DatabasesController\\:\\:setSortDetails\\(\\) expects string\\|null, mixed given\\.$#"
-			count: 1
-			path: src/Controllers/Server/DatabasesController.php
-
 		-
 			message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#"
 			count: 2
@@ -3500,11 +3490,6 @@ parameters:
 			count: 4
 			path: src/Controllers/Server/DatabasesController.php
 
-		-
-			message: "#^Parameter \\#2 \\$sortOrder of method PhpMyAdmin\\\\Controllers\\\\Server\\\\DatabasesController\\:\\:setSortDetails\\(\\) expects string\\|null, mixed given\\.$#"
-			count: 1
-			path: src/Controllers/Server/DatabasesController.php
-
 		-
 			message: "#^Parameter \\#3 \\$name of static method PhpMyAdmin\\\\Charsets\\:\\:findCollationByName\\(\\) expects string, mixed given\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -2700,12 +2700,7 @@
       <code><![CDATA[$database['SCHEMA_NAME']]]></code>
       <code><![CDATA[$database['SCHEMA_NAME']]]></code>
       <code><![CDATA[$database['SCHEMA_NAME']]]></code>
-      <code><![CDATA[$totalStatistics[$key]['raw']]]></code>
     </MixedArrayAccess>
-    <MixedArrayAssignment>
-      <code><![CDATA[$statistics[$key]['raw']]]></code>
-      <code><![CDATA[$totalStatistics[$key]['raw']]]></code>
-    </MixedArrayAssignment>
     <MixedArrayOffset>
       <code><![CDATA[$databases[$database['SCHEMA_NAME']]]]></code>
       <code><![CDATA[$databases[$database['SCHEMA_NAME']]]]></code>
@@ -2717,22 +2712,7 @@
       <code>$key</code>
       <code>$key</code>
       <code>$key</code>
-      <code><![CDATA[$totalStatistics[$key]['raw']]]></code>
     </MixedAssignment>
-    <MixedOperand>
-      <code><![CDATA[$totalStatistics[$key]['raw']]]></code>
-    </MixedOperand>
-    <PossiblyInvalidArgument>
-      <code><![CDATA[$params['sort_by']]]></code>
-      <code><![CDATA[$params['sort_order']]]></code>
-    </PossiblyInvalidArgument>
-    <RiskyCast>
-      <code><![CDATA[$params['pos']]]></code>
-    </RiskyCast>
-    <RiskyTruthyFalsyComparison>
-      <code><![CDATA[empty($params['pos'])]]></code>
-      <code><![CDATA[empty($params['statistics'])]]></code>
-    </RiskyTruthyFalsyComparison>
   </file>
   <file src="src/Controllers/Server/EnginesController.php">
     <PossiblyUnusedParam>

diff --git a/src/Controllers/Server/DatabasesController.php b/src/Controllers/Server/DatabasesController.php
@@ -20,14 +20,14 @@
 use PhpMyAdmin\Url;
 use PhpMyAdmin\UserPrivileges;
 use PhpMyAdmin\Util;
+use Webmozart\Assert\Assert;
 
 use function __;
 use function array_keys;
 use function array_search;
 use function count;
-use function in_array;
-use function mb_strtolower;
 use function str_contains;
+use function strtolower;
 
 /**
  * Handles viewing and creating and deleting databases
@@ -49,6 +49,17 @@ class DatabasesController extends AbstractController
     /** @var bool whether to show database statistics */
     private bool $hasStatistics = false;
 
+    private const SORT_BY_ALLOWED_LIST = [
+        'SCHEMA_NAME',
+        'DEFAULT_COLLATION_NAME',
+        'SCHEMA_TABLES',
+        'SCHEMA_TABLE_ROWS',
+        'SCHEMA_DATA_LENGTH',
+        'SCHEMA_INDEX_LENGTH',
+        'SCHEMA_LENGTH',
+        'SCHEMA_DATA_FREE',
+    ];
+
     public function __construct(
         ResponseRenderer $response,
         Template $template,
@@ -64,12 +75,16 @@ public function __invoke(ServerRequest $request): void
     {
         $GLOBALS['errorUrl'] ??= null;
 
-        $params = [
-            'statistics' => $_REQUEST['statistics'] ?? null,
-            'pos' => $_REQUEST['pos'] ?? null,
-            'sort_by' => $_REQUEST['sort_by'] ?? null,
-            'sort_order' => $_REQUEST['sort_order'] ?? null,
-        ];
+        $this->hasStatistics = ! empty($request->getParam('statistics'));
+        $position = (int) $request->getParam('pos');
+
+        $sortBy = $request->getParam('sort_by', '');
+        Assert::string($sortBy);
+        $this->sortBy = self::SORT_BY_ALLOWED_LIST[array_search($sortBy, self::SORT_BY_ALLOWED_LIST, true)];
+
+        $sortOrder = $request->getParam('sort_order', '');
+        Assert::string($sortOrder);
+        $this->sortOrder = strtolower($sortOrder) !== 'desc' ? 'asc' : 'desc';
 
         $this->addScriptFiles(['server/databases.js']);
         $GLOBALS['errorUrl'] = Url::getFromRoute('/');
@@ -84,10 +99,6 @@ public function __invoke(ServerRequest $request): void
         $primaryInfo = $replicationInfo->getPrimaryInfo();
         $replicaInfo = $replicationInfo->getReplicaInfo();
 
-        $this->setSortDetails($params['sort_by'], $params['sort_order']);
-        $this->hasStatistics = ! empty($params['statistics']);
-        $position = ! empty($params['pos']) ? (int) $params['pos'] : 0;
-
         /**
          * Gets the databases list
          */
@@ -159,46 +170,11 @@ public function __invoke(ServerRequest $request): void
         ]);
     }
 
-    /**
-     * Extracts parameters sort order and sort by
-     *
-     * @param string|null $sortBy    sort by
-     * @param string|null $sortOrder sort order
-     */
-    private function setSortDetails(string|null $sortBy, string|null $sortOrder): void
-    {
-        if ($sortBy === null || $sortBy === '') {
-            $this->sortBy = 'SCHEMA_NAME';
-        } else {
-            $sortByAllowList = [
-                'SCHEMA_NAME',
-                'DEFAULT_COLLATION_NAME',
-                'SCHEMA_TABLES',
-                'SCHEMA_TABLE_ROWS',
-                'SCHEMA_DATA_LENGTH',
-                'SCHEMA_INDEX_LENGTH',
-                'SCHEMA_LENGTH',
-                'SCHEMA_DATA_FREE',
-            ];
-            $this->sortBy = 'SCHEMA_NAME';
-            if (in_array($sortBy, $sortByAllowList, true)) {
-                $this->sortBy = $sortBy;
-            }
-        }
-
-        $this->sortOrder = 'asc';
-        if (! isset($sortOrder) || mb_strtolower($sortOrder) !== 'desc') {
-            return;
-        }
-
-        $this->sortOrder = 'desc';
-    }
-
     /**
      * @param mixed[] $primaryInfo
      * @param mixed[] $replicaInfo
      *
-     * @return mixed[]
+     * @return array{databases:mixed[], total_statistics:mixed[]}
      */
     private function getDatabases(array $primaryInfo, array $replicaInfo): array
     {
@@ -280,7 +256,7 @@ private function getDatabases(array $primaryInfo, array $replicaInfo): array
     /**
      * Prepares the statistics columns
      *
-     * @return mixed[]
+     * @return array<string, array{title: string, format: 'number'|'byte', raw: int}>
      */
     private function getStatisticsColumns(): array
     {

diff --git a/src/Replication/ReplicationInfo.php b/src/Replication/ReplicationInfo.php
@@ -125,7 +125,7 @@ private function setDefaultPrimaryConnection(string $connection): void
     /**
      * @param mixed[] $status
      *
-     * @return mixed[]
+     * @return string[]
      */
     private function fill(array $status, string $key): array
     {
@@ -138,11 +138,7 @@ private function fill(array $status, string $key): array
 
     private function setPrimaryInfo(): void
     {
-        $this->primaryInfo = ['status' => false];
-
-        if ($this->primaryStatus !== []) {
-            $this->primaryInfo['status'] = true;
-        }
+        $this->primaryInfo = ['status' => $this->primaryStatus !== []];
 
         if (! $this->primaryInfo['status']) {
             return;
@@ -160,11 +156,7 @@ public function getPrimaryInfo(): array
 
     private function setReplicaInfo(): void
     {
-        $this->replicaInfo = ['status' => false];
-
-        if ($this->replicaStatus !== []) {
-            $this->replicaInfo['status'] = true;
-        }
+        $this->replicaInfo = ['status' => $this->replicaStatus !== []];
 
         if (! $this->replicaInfo['status']) {
             return;

diff --git a/tests/classes/Controllers/Server/DatabasesControllerTest.php b/tests/classes/Controllers/Server/DatabasesControllerTest.php
@@ -8,7 +8,7 @@
 use PhpMyAdmin\Controllers\Server\DatabasesController;
 use PhpMyAdmin\Current;
 use PhpMyAdmin\DatabaseInterface;
-use PhpMyAdmin\Http\ServerRequest;
+use PhpMyAdmin\Http\Factory\ServerRequestFactory;
 use PhpMyAdmin\Template;
 use PhpMyAdmin\Tests\AbstractTestCase;
 use PhpMyAdmin\Tests\Stubs\DbiDummy;
@@ -56,7 +56,14 @@ public function testIndexAction(): void
             ['SCHEMA_NAME'],
         );
         $this->dummyDbi->addSelectDb('mysql');
-        $controller(self::createStub(ServerRequest::class));
+        $request = ServerRequestFactory::create()->createServerRequest('GET', 'http://example.com/')
+            ->withQueryParams([
+                'statistics' => '',
+                'pos' => '',
+                'sort_by' => '',
+                'sort_order' => '',
+            ]);
+        $controller($request);
         $this->dummyDbi->assertAllSelectsConsumed();
         $actual = $response->getHTMLResult();
 
@@ -82,12 +89,16 @@ public function testIndexAction(): void
 
         $config->settings['ShowCreateDb'] = true;
         UserPrivileges::$isCreateDatabase = true;
-        $_REQUEST['statistics'] = '1';
-        $_REQUEST['sort_by'] = 'SCHEMA_TABLES';
-        $_REQUEST['sort_order'] = 'desc';
 
+        $request = ServerRequestFactory::create()->createServerRequest('GET', 'http://example.com/')
+            ->withQueryParams([
+                'statistics' => '1',
+                'pos' => '',
+                'sort_by' => 'SCHEMA_TABLES',
+                'sort_order' => 'desc',
+            ]);
         $this->dummyDbi->addSelectDb('mysql');
-        $controller(self::createStub(ServerRequest::class));
+        $controller($request);
         $this->dummyDbi->assertAllSelectsConsumed();
         $actual = $response->getHTMLResult();