Move common code into getWhereInColumns()

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
phpmyadmin · May 25, 2023 · 2c611d0 · 2c611d0
1 parent e08ba58
commit 2c611d0
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 52 deletions.
diff --git a/libraries/classes/Database/CentralColumns.php b/libraries/classes/Database/CentralColumns.php
@@ -15,6 +15,7 @@
 use function __;
 use function array_column;
 use function array_diff;
+use function array_map;
 use function array_unique;
 use function array_values;
 use function bin2hex;
@@ -164,23 +165,25 @@ public function getCount(string $db): int
     /**
      * return the existing columns in central list among the given list of columns
      *
-     * @param string $db        the selected database
-     * @param string $cols      comma separated list of given columns
-     * @param bool   $allFields set if need all the fields of existing columns,
-     *                          otherwise only column_name is returned
+     * @param string   $db        the selected database
+     * @param string[] $cols      list of given columns
+     * @param bool     $allFields set if need all the fields of existing columns,
+     *                            otherwise only column_name is returned
      *
      * @return mixed[] list of columns in central columns among given set of columns
      */
     private function findExistingColNames(
         string $db,
-        string $cols,
+        array $cols,
         bool $allFields = false,
     ): array {
         $cfgCentralColumns = $this->getParams();
         if (! is_array($cfgCentralColumns)) {
             return [];
         }
 
+        $cols = $this->getWhereInColumns($cols);
+
         $pmadb = $cfgCentralColumns['db'];
         $this->dbi->selectDb($pmadb, Connection::TYPE_CONTROL);
         $centralListTable = $cfgCentralColumns['table'];
@@ -278,19 +281,19 @@ public function syncUniqueColumns(
         $centralListTable = $cfgCentralColumns['table'];
         $this->dbi->selectDb($db);
         $existingCols = [];
-        $cols = '';
+        $cols = [];
         $insQuery = [];
         $fields = [];
         $message = true;
         if ($isTable) {
             foreach ($fieldSelect as $table) {
                 $fields[$table] = $this->dbi->getColumns($db, $table, true);
                 foreach (array_column($fields[$table], 'Field') as $field) {
-                    $cols .= $this->dbi->quoteString($field, Connection::TYPE_CONTROL) . ',';
+                    $cols[] = $field;
                 }
             }
 
-            $hasList = $this->findExistingColNames($db, trim($cols, ','));
+            $hasList = $this->findExistingColNames($db, $cols);
             foreach ($fieldSelect as $table) {
                 foreach ($fields[$table] as $def) {
                     $field = $def['Field'];
@@ -307,11 +310,7 @@ public function syncUniqueColumns(
                 $table = $_POST['table'];
             }
 
-            foreach ($fieldSelect as $column) {
-                $cols .= $this->dbi->quoteString($column, Connection::TYPE_CONTROL) . ',';
-            }
-
-            $hasList = $this->findExistingColNames($db, trim($cols, ','));
+            $hasList = $this->findExistingColNames($db, $fieldSelect);
             foreach ($fieldSelect as $column) {
                 if (! in_array($column, $hasList)) {
                     $hasList[] = $column;
@@ -385,15 +384,14 @@ public function deleteColumnsFromList(
         $colNotExist = [];
         $fields = [];
         if ($isTable) {
-            $cols = '';
+            $cols = [];
             foreach ($fieldSelect as $table) {
                 $fields[$table] = $this->dbi->getColumnNames($database, $table);
                 foreach ($fields[$table] as $colSelect) {
-                    $cols .= $this->dbi->quoteString($colSelect, Connection::TYPE_CONTROL) . ',';
+                    $cols[] = $colSelect;
                 }
             }
 
-            $cols = trim($cols, ',');
             $hasList = $this->findExistingColNames($database, $cols);
             foreach ($fieldSelect as $table) {
                 foreach ($fields[$table] as $column) {
@@ -405,12 +403,7 @@ public function deleteColumnsFromList(
                 }
             }
         } else {
-            $cols = '';
-            foreach ($fieldSelect as $colSelect) {
-                $cols .= $this->dbi->quoteString($colSelect, Connection::TYPE_CONTROL) . ',';
-            }
-
-            $cols = trim($cols, ',');
+            $cols = $fieldSelect;
             $hasList = $this->findExistingColNames($database, $cols);
             foreach ($fieldSelect as $column) {
                 if (in_array($column, $hasList)) {
@@ -435,6 +428,7 @@ public function deleteColumnsFromList(
 
         $this->dbi->selectDb($pmadb, Connection::TYPE_CONTROL);
 
+        $cols = $this->getWhereInColumns($cols);
         $query = 'DELETE FROM ' . Util::backquote($centralListTable) . ' WHERE db_name = '
             . $this->dbi->quoteString($database, Connection::TYPE_CONTROL) . ' AND col_name IN (' . $cols . ');';
 
@@ -544,14 +538,8 @@ public function getFromTable(
 
         $this->dbi->selectDb($db);
         $fields = $this->dbi->getColumnNames($db, $table);
-        $cols = '';
-        foreach ($fields as $colSelect) {
-            $cols .= $this->dbi->quoteString($colSelect, Connection::TYPE_CONTROL) . ',';
-        }
 
-        $cols = trim($cols, ',');
-
-        return $this->findExistingColNames($db, $cols, $allFields);
+        return $this->findExistingColNames($db, $fields, $allFields);
     }
 
     /**
@@ -745,16 +733,10 @@ public function getListRaw(string $db, string $table): array
         } else {
             $this->dbi->selectDb($db);
             $columns = $this->dbi->getColumnNames($db, $table);
-            $cols = '';
-            foreach ($columns as $colSelect) {
-                $cols .= $this->dbi->quoteString($colSelect, Connection::TYPE_CONTROL) . ',';
-            }
-
-            $cols = trim($cols, ',');
             $query = 'SELECT * FROM ' . Util::backquote($centralTable) . ' '
                 . 'WHERE db_name = ' . $this->dbi->quoteString($db, Connection::TYPE_CONTROL);
-            if ($cols) {
-                $query .= ' AND col_name NOT IN (' . $cols . ')';
+            if ($columns !== []) {
+                $query .= ' AND col_name NOT IN (' . $this->getWhereInColumns($columns) . ')';
             }
 
             $query .= ';';
@@ -809,12 +791,7 @@ private function handleColumnExtra(array &$columnsList): void
     public function getHtmlForEditingPage(array $selectedFld, string $selectedDb): string
     {
         $html = '';
-        $selectedFldSafe = [];
-        foreach ($selectedFld as $key) {
-            $selectedFldSafe[] = $this->dbi->quoteString($key, Connection::TYPE_CONTROL);
-        }
-
-        $listDetailCols = $this->findExistingColNames($selectedDb, implode(',', $selectedFldSafe), true);
+        $listDetailCols = $this->findExistingColNames($selectedDb, $selectedFld, true);
         $rowNum = 0;
         foreach ($listDetailCols as $row) {
             $tableHtmlRow = $this->getHtmlForEditTableRow($row, $rowNum);
@@ -969,4 +946,13 @@ public function getTemplateVariablesForMain(
             'charsets' => $charsetsList,
         ];
     }
+
+    /** @param string[] $columns */
+    private function getWhereInColumns(array $columns): string
+    {
+        return implode(',', array_map(
+            fn (string $string): string => $this->dbi->quoteString($string, Connection::TYPE_CONTROL),
+            $columns,
+        ));
+    }
 }
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -2745,6 +2745,11 @@ parameters:
 			count: 1
 			path: libraries/classes/Database/CentralColumns.php
 
+		-
+			message: "#^Parameter \\#1 \\$columns of method PhpMyAdmin\\\\Database\\\\CentralColumns\\:\\:getWhereInColumns\\(\\) expects array\\<string\\>, array given\\.$#"
+			count: 1
+			path: libraries/classes/Database/CentralColumns.php
+
 		-
 			message: "#^Parameter \\#1 \\$dbname of method PhpMyAdmin\\\\DatabaseInterface\\:\\:selectDb\\(\\) expects PhpMyAdmin\\\\Dbal\\\\DatabaseName\\|string, mixed given\\.$#"
 			count: 8
@@ -2762,7 +2767,7 @@ parameters:
 
 		-
 			message: "#^Parameter \\#1 \\$str of method PhpMyAdmin\\\\DatabaseInterface\\:\\:quoteString\\(\\) expects string, mixed given\\.$#"
-			count: 4
+			count: 1
 			path: libraries/classes/Database/CentralColumns.php
 
 		-
@@ -2785,6 +2790,11 @@ parameters:
 			count: 1
 			path: libraries/classes/Database/CentralColumns.php
 
+		-
+			message: "#^Parameter \\#2 \\$cols of method PhpMyAdmin\\\\Database\\\\CentralColumns\\:\\:findExistingColNames\\(\\) expects array\\<string\\>, array given\\.$#"
+			count: 3
+			path: libraries/classes/Database/CentralColumns.php
+
 		-
 			message: "#^Parameter \\#2 \\$def of method PhpMyAdmin\\\\Database\\\\CentralColumns\\:\\:getInsertQuery\\(\\) expects array, array\\<string, string\\|null\\>\\|null given\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -4680,18 +4680,15 @@
       <code>$centralTable</code>
       <code><![CDATA[$cfgCentralColumns['db']]]></code>
       <code><![CDATA[$cfgCentralColumns['db']]]></code>
-      <code>$colSelect</code>
       <code>$collation</code>
       <code>$column</code>
-      <code>$column</code>
       <code>$columnDefault[$i]</code>
       <code>$columnExtra[$i]</code>
       <code><![CDATA[$column['col_name']]]></code>
       <code><![CDATA[$column['col_name']]]></code>
       <code><![CDATA[$def['Type']]]></code>
       <code>$default</code>
       <code><![CDATA[$extractedColumnSpec['attribute']]]></code>
-      <code>$key</code>
       <code><![CDATA[$meta['DefaultValue']]]></code>
       <code><![CDATA[$meta['DefaultValue']]]></code>
       <code><![CDATA[$params['db']]]></code>
@@ -4717,6 +4714,10 @@
       <code>$type</code>
     </MixedArgument>
     <MixedArgumentTypeCoercion>
+      <code>$cols</code>
+      <code>$cols</code>
+      <code>$fieldSelect</code>
+      <code>$selectedFld</code>
       <code>[$extra, $attribute]</code>
     </MixedArgumentTypeCoercion>
     <MixedArrayAccess>
@@ -4769,12 +4770,10 @@
       <code>$centralListTable</code>
       <code>$centralTable</code>
       <code>$centralTable</code>
-      <code>$colSelect</code>
       <code>$collation</code>
       <code>$column</code>
       <code>$column</code>
       <code>$column</code>
-      <code>$column</code>
       <code>$columnDefault</code>
       <code>$columnDefault[$i]</code>
       <code>$columnExtra[$i]</code>
@@ -4783,7 +4782,6 @@
       <code>$defaultValues[$rowNum]</code>
       <code>$extra</code>
       <code>$hasList[]</code>
-      <code>$key</code>
       <code>$length</code>
       <code><![CDATA[$meta['DefaultValue']]]></code>
       <code>$pmadb</code>

diff --git a/test/classes/Database/CentralColumnsTest.php b/test/classes/Database/CentralColumnsTest.php
@@ -393,7 +393,7 @@ public function testGetHtmlForEditingPage(): void
             $this->centralColumns,
             CentralColumns::class,
             'findExistingColNames',
-            ['phpmyadmin', "'col1','col2'", true],
+            ['phpmyadmin', ['col1', 'col2'], true],
         );
         $this->assertStringContainsString(
             $this->callFunction(
@@ -480,7 +480,7 @@ public function testFindExistingColNames(): void
                 $this->centralColumns,
                 CentralColumns::class,
                 'findExistingColNames',
-                ['phpmyadmin', "'col1'", true],
+                ['phpmyadmin', ['col1'], true],
             ),
         );
     }