Remove doSaveData() method

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
2023-12-28 23:37:06 +01:00 · 2023-12-28 23:37:06 +01:00 · b22dc9e4aa
commit b22dc9e4aa
parent 187d948866
5 changed files with 74 additions and 101 deletions
--- a/phpstan-baseline.neon
+++ b/phpstan-baseline.neon
@ -4926,12 +4926,12 @@ parameters:
 			path: src/Controllers/Table/IndexRenameController.php

 		-
-			message: "#^Parameter \\#1 \\$params of static method PhpMyAdmin\\\\Url\\:\\:getCommon\\(\\) expects array\\<string, bool\\|int\\|string\\>, mixed given\\.$#"
+			message: "#^Parameter \\#1 \\$oldIndexName of method PhpMyAdmin\\\\Table\\\\Indexes\\:\\:getSqlQueryForRename\\(\\) expects string, mixed given\\.$#"
 			count: 1
 			path: src/Controllers/Table/IndexRenameController.php

 		-
-			message: "#^Parameter \\#6 \\$oldIndexName of method PhpMyAdmin\\\\Table\\\\Indexes\\:\\:doSaveData\\(\\) expects string, mixed given\\.$#"
+			message: "#^Parameter \\#1 \\$params of static method PhpMyAdmin\\\\Url\\:\\:getCommon\\(\\) expects array\\<string, bool\\|int\\|string\\>, mixed given\\.$#"
 			count: 1
 			path: src/Controllers/Table/IndexRenameController.php

--- a/src/Controllers/Table/IndexRenameController.php
+++ b/src/Controllers/Table/IndexRenameController.php
@ -101,36 +101,37 @@ final class IndexRenameController extends AbstractController

        $previewSql = $request->hasBodyParam('preview_sql');

-        $sqlResult = $this->indexes->doSaveData(
+        $sqlQuery = $this->indexes->getSqlQueryForRename(
+            $oldIndexName,
            $index,
-            true,
            $databaseName->getName(),
            $tableName->getName(),
-            $previewSql,
-            $oldIndexName,
        );

        if ($previewSql) {
            $this->response->addJSON(
                'sql_data',
-                $this->template->render('preview_sql', ['query_data' => $sqlResult]),
+                $this->template->render('preview_sql', ['query_data' => $sqlQuery]),
            );

            return;
        }

-        if ($sqlResult instanceof Message) {
+        $logicError = $this->indexes->getError();
+        if ($logicError instanceof Message) {
            $this->response->setRequestStatus(false);
-            $this->response->addJSON('message', $sqlResult);
+            $this->response->addJSON('message', $logicError);

            return;
        }

+        $this->dbi->query($sqlQuery);
+
        $message = Message::success(__('Table %1$s has been altered successfully.'));
        $message->addParam($tableName->getName());
        $this->response->addJSON(
            'message',
-            Generator::getMessage($message, $sqlResult, 'success'),
+            Generator::getMessage($message, $sqlQuery, 'success'),
        );

        $indexes = Index::getFromTable($this->dbi, $tableName->getName(), $databaseName->getName());
--- a/src/Controllers/Table/IndexesController.php
+++ b/src/Controllers/Table/IndexesController.php
@ -104,25 +104,28 @@ class IndexesController extends AbstractController
        if (isset($_POST['do_save_data'])) {
            $previewSql = $request->hasBodyParam('preview_sql');

-            $sqlResult = $this->indexes->doSaveData($index, false, Current::$database, Current::$table, $previewSql);
-
-            if ($sqlResult instanceof Message) {
-                $this->response->setRequestStatus(false);
-                $this->response->addJSON('message', $sqlResult);
-
-                return;
-            }
+            $sqlQuery = $this->indexes->getSqlQueryForIndexCreateOrEdit(Current::$database, Current::$table, $index);

            // If there is a request for SQL previewing.
            if ($previewSql) {
                $this->response->addJSON(
                    'sql_data',
-                    $this->template->render('preview_sql', ['query_data' => $sqlResult]),
+                    $this->template->render('preview_sql', ['query_data' => $sqlQuery]),
                );

                return;
            }

+            $logicError = $this->indexes->getError();
+            if ($logicError instanceof Message) {
+                $this->response->setRequestStatus(false);
+                $this->response->addJSON('message', $logicError);
+
+                return;
+            }
+
+            $this->dbi->query($sqlQuery);
+
            if ($request->isAjax()) {
                $message = Message::success(
                    __('Table %1$s has been altered successfully.'),
@ -130,7 +133,7 @@ class IndexesController extends AbstractController
                $message->addParam(Current::$table);
                $this->response->addJSON(
                    'message',
-                    Generator::getMessage($message, $sqlResult, 'success'),
+                    Generator::getMessage($message, $sqlQuery, 'success'),
                );

                $indexes = Index::getFromTable($this->dbi, Current::$table, Current::$database);
--- a/src/Table/Indexes.php
+++ b/src/Table/Indexes.php
@ -22,60 +22,13 @@ final class Indexes
 {
    private Message|null $error = null;

-    public function __construct(
-        private DatabaseInterface $dbi,
-    ) {
+    public function __construct(private readonly DatabaseInterface $dbi)
+    {
    }

-    /**
-     * Process the data from the edit/create index form,
-     * run the query to build the new index
-     * and moves back to /table/sql
-     *
-     * @param Index $index      An Index instance.
-     * @param bool  $renameMode Rename the Index mode
-     */
-    public function doSaveData(
-        Index $index,
-        bool $renameMode,
-        string $db,
-        string $table,
-        bool $previewSql,
-        string $oldIndexName = '',
-    ): string|Message {
-        if ($renameMode && Compatibility::isCompatibleRenameIndex($this->dbi->getVersion())) {
-            if ($oldIndexName === 'PRIMARY') {
-                if ($index->getName() === '') {
-                    $index->setName('PRIMARY');
-                } elseif ($index->getName() !== 'PRIMARY') {
-                    $this->error = Message::error(
-                        __('The name of the primary key must be "PRIMARY"!'),
-                    );
-                }
-            }
-
-            $sqlQuery = QueryGenerator::getSqlQueryForIndexRename(
-                $db,
-                $table,
-                $oldIndexName,
-                $index->getName(),
-            );
-        } else {
-            $sqlQuery = $this->getSqlQueryForIndexCreateOrEdit($db, $table, $index);
-        }
-
-        // If there is a request for SQL previewing.
-        if ($previewSql) {
-            return $sqlQuery;
-        }
-
-        if ($this->error instanceof Message) {
-            return $this->error;
-        }
-
-        $this->dbi->query($sqlQuery);
-
-        return $sqlQuery;
+    public function getError(): Message|null
+    {
+        return $this->error;
    }

    /**
@ -191,6 +144,30 @@ final class Indexes
        return $sqlQuery;
    }

+    public function getSqlQueryForRename(string $oldIndexName, Index $index, string $db, string $table): string
+    {
+        if (! Compatibility::isCompatibleRenameIndex($this->dbi->getVersion())) {
+            return $this->getSqlQueryForIndexCreateOrEdit($db, $table, $index);
+        }
+
+        if ($oldIndexName === 'PRIMARY') {
+            if ($index->getName() === '') {
+                $index->setName('PRIMARY');
+            } elseif ($index->getName() !== 'PRIMARY') {
+                $this->error = Message::error(
+                    __('The name of the primary key must be "PRIMARY"!'),
+                );
+            }
+        }
+
+        return QueryGenerator::getSqlQueryForIndexRename(
+            $db,
+            $table,
+            $oldIndexName,
+            $index->getName(),
+        );
+    }
+
    public function executeAddIndexSql(string|DatabaseName $db, string $sql): Message
    {
        $this->dbi->selectDb($db);
--- a/tests/classes/Table/IndexesTest.php
+++ b/tests/classes/Table/IndexesTest.php
@ -31,40 +31,24 @@ class IndexesTest extends AbstractTestCase
            ->getMock();
    }

-    public function testDoSaveData(): void
+    public function testGetSqlQueryForRename(): void
    {
-        $sqlQuery = 'ALTER TABLE `db`.`table` DROP PRIMARY KEY, ADD PRIMARY KEY (`id`);';
+        $sqlQuery = 'ALTER TABLE `db`.`table` RENAME INDEX `0` TO `ABC`;';

-        $table = $this->getMockBuilder(Table::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $this->dbi->expects($this->any())->method('getTable')
-            ->willReturn($table);
+        $this->dbi->expects($this->any())->method('getVersion')
+            ->willReturn(50700);

-        $index = new Index();
-        $index->set([
-            'Key_name' => 'PRIMARY',
-            'columns' => [['Column_name' => 'id']],
-        ]);
+        $index = new Index(['Key_name' => 'ABC']);

        $indexes = new Indexes($this->dbi);

-        $_POST['old_index'] = 'PRIMARY';
-
-        // Preview SQL
-        $sqlResult = $indexes->doSaveData($index, false, Current::$database, Current::$table, true);
-        $this->assertIsString($sqlResult);
-        $this->assertStringContainsString($sqlQuery, $sqlResult);
-
-        // Alter success
-        $sqlResult = $indexes->doSaveData($index, false, Current::$database, Current::$table, false);
-        $this->assertIsString($sqlResult);
+        $sqlResult = $indexes->getSqlQueryForRename('0', $index, Current::$database, Current::$table);
        $this->assertStringContainsString($sqlQuery, $sqlResult);

        // Error message
        $index->setName('NOT PRIMARY'); // Cannot rename primary so the operation should fail
-        $sqlResult = $indexes->doSaveData($index, false, Current::$database, Current::$table, false);
-        $this->assertInstanceOf(Message::class, $sqlResult);
+        $indexes->getSqlQueryForRename('PRIMARY', $index, Current::$database, Current::$table);
+        $this->assertInstanceOf(Message::class, $indexes->getError());
    }

    public function testGetSqlQueryForIndexCreateOrEdit(): void
@ -78,15 +62,23 @@ class IndexesTest extends AbstractTestCase

        $db = 'pma_db';
        $table = 'pma_table';
-        $index = new Index();
+        $index = new Index([
+            'Key_name' => 'PRIMARY',
+            'columns' => [['Column_name' => 'id']],
+        ]);
+
+        $sqlQueryExpected = 'ALTER TABLE `pma_db`.`pma_table` DROP PRIMARY KEY, ADD PRIMARY KEY (`id`);';

        $_POST['old_index'] = 'PRIMARY';
-        $sql = $indexes->getSqlQueryForIndexCreateOrEdit($db, $table, $index);
-        $this->assertEquals('ALTER TABLE `pma_db`.`pma_table` DROP PRIMARY KEY, ADD UNIQUE;', $sql);
+        $this->assertEquals($sqlQueryExpected, $indexes->getSqlQueryForIndexCreateOrEdit($db, $table, $index));

        $_POST['old_index'] = [];
        $_POST['old_index']['Key_name'] = 'PRIMARY';
-        $sql = $indexes->getSqlQueryForIndexCreateOrEdit($db, $table, $index);
-        $this->assertEquals('ALTER TABLE `pma_db`.`pma_table` DROP PRIMARY KEY, ADD UNIQUE;', $sql);
+        $this->assertEquals($sqlQueryExpected, $indexes->getSqlQueryForIndexCreateOrEdit($db, $table, $index));
+
+        // Error message
+        $index->setName('NOT PRIMARY'); // Cannot rename primary so the operation should fail
+        $indexes->getSqlQueryForIndexCreateOrEdit($db, $table, $index);
+        $this->assertInstanceOf(Message::class, $indexes->getError());
    }
 }