Refactor Indexes::doSaveData()

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
This commit is contained in:
Kamil Tekiela 2023-12-28 15:47:01 +01:00
parent 18d91ef865
commit 4677dac58f
12 changed files with 142 additions and 106 deletions

View File

@ -193,7 +193,7 @@ return [
],
'table_indexes' => [
'class' => Indexes::class,
'arguments' => ['$response' => '@response', '$template' => '@template', '$dbi' => '@dbi'],
'arguments' => ['$dbi' => '@dbi'],
],
'table_maintenance' => ['class' => PhpMyAdmin\Table\Maintenance::class, 'arguments' => ['$dbi' => '@dbi']],
'table_search' => ['class' => Search::class, 'arguments' => ['$dbi' => '@dbi']],

View File

@ -4931,7 +4931,7 @@ parameters:
path: src/Controllers/Table/IndexRenameController.php
-
message: "#^Parameter \\#7 \\$oldIndexName of method PhpMyAdmin\\\\Table\\\\Indexes\\:\\:doSaveData\\(\\) expects string, mixed given\\.$#"
message: "#^Parameter \\#6 \\$oldIndexName of method PhpMyAdmin\\\\Table\\\\Indexes\\:\\:doSaveData\\(\\) expects string, mixed given\\.$#"
count: 1
path: src/Controllers/Table/IndexRenameController.php
@ -15367,7 +15367,7 @@ parameters:
-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 18
count: 19
path: src/Table/Table.php
-
@ -17910,11 +17910,6 @@ parameters:
count: 1
path: tests/classes/Table/IndexesTest.php
-
message: "#^Parameter \\#2 \\$haystack of method PHPUnit\\\\Framework\\\\Assert\\:\\:assertStringContainsString\\(\\) expects string, mixed given\\.$#"
count: 1
path: tests/classes/Table/IndexesTest.php
-
message: "#^Call to an undefined method PhpMyAdmin\\\\DatabaseInterface\\:\\:expects\\(\\)\\.$#"
count: 4

View File

@ -11787,9 +11787,6 @@
<PossiblyUnusedReturnValue>
<code>true|Message</code>
</PossiblyUnusedReturnValue>
<ReferenceConstraintViolation>
<code>return $sqlQuery;</code>
</ReferenceConstraintViolation>
<UnsupportedPropertyReferenceUsage>
<code><![CDATA[$this->uiprefs =& $_SESSION['tmpval']['table_uiprefs'][$serverId][$this->dbName][$this->name]]]></code>
</UnsupportedPropertyReferenceUsage>
@ -15256,6 +15253,9 @@
<MixedAssignment>
<code>$value</code>
</MixedAssignment>
<PossiblyUnusedMethod>
<code>clear</code>
</PossiblyUnusedMethod>
</file>
<file src="tests/classes/SystemDatabaseTest.php">
<DeprecatedMethod>
@ -15275,9 +15275,6 @@
<code>Config::getInstance()</code>
<code>DatabaseInterface::getInstance()</code>
</DeprecatedMethod>
<MixedArgument>
<code><![CDATA[$jsonArray['sql_data']]]></code>
</MixedArgument>
<MixedMethodCall>
<code>method</code>
<code>willReturn</code>

View File

@ -5,10 +5,12 @@ declare(strict_types=1);
namespace PhpMyAdmin\Controllers\Table;
use PhpMyAdmin\Config;
use PhpMyAdmin\Container\ContainerBuilder;
use PhpMyAdmin\Controllers\AbstractController;
use PhpMyAdmin\Current;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\DbTableExists;
use PhpMyAdmin\Html\Generator;
use PhpMyAdmin\Http\ServerRequest;
use PhpMyAdmin\Identifiers\DatabaseName;
use PhpMyAdmin\Identifiers\TableName;
@ -96,16 +98,63 @@ final class IndexRenameController extends AbstractController
if (isset($_POST['do_save_data'])) {
$oldIndexName = $request->getParsedBodyParam('old_index', '');
$this->indexes->doSaveData(
$request,
$previewSql = $request->hasBodyParam('preview_sql');
$sqlResult = $this->indexes->doSaveData(
$index,
true,
Current::$database,
Current::$table,
$request->hasBodyParam('preview_sql'),
$previewSql,
$oldIndexName,
);
// If there is a request for SQL previewing.
if ($previewSql) {
$this->response->addJSON(
'sql_data',
$this->template->render('preview_sql', ['query_data' => $sqlResult]),
);
return;
}
if ($sqlResult instanceof Message) {
$this->response->setRequestStatus(false);
$this->response->addJSON('message', $sqlResult);
return;
}
if ($request->isAjax()) {
$message = Message::success(
__('Table %1$s has been altered successfully.'),
);
$message->addParam(Current::$table);
$this->response->addJSON(
'message',
Generator::getMessage($message, $sqlResult, 'success'),
);
$indexes = Index::getFromTable($this->dbi, Current::$table, Current::$database);
$indexesDuplicates = Index::findDuplicates(Current::$table, Current::$database);
$this->response->addJSON(
'index_table',
$this->template->render('indexes', [
'url_params' => ['db' => Current::$database, 'table' => Current::$table],
'indexes' => $indexes,
'indexes_duplicates' => $indexesDuplicates,
]),
);
return;
}
/** @var StructureController $controller */
$controller = ContainerBuilder::getContainer()->get(StructureController::class);
$controller($request);
return;
}

View File

@ -5,10 +5,12 @@ declare(strict_types=1);
namespace PhpMyAdmin\Controllers\Table;
use PhpMyAdmin\Config;
use PhpMyAdmin\Container\ContainerBuilder;
use PhpMyAdmin\Controllers\AbstractController;
use PhpMyAdmin\Current;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\DbTableExists;
use PhpMyAdmin\Html\Generator;
use PhpMyAdmin\Http\ServerRequest;
use PhpMyAdmin\Identifiers\DatabaseName;
use PhpMyAdmin\Identifiers\TableName;
@ -100,14 +102,55 @@ class IndexesController extends AbstractController
}
if (isset($_POST['do_save_data'])) {
$this->indexes->doSaveData(
$request,
$index,
false,
Current::$database,
Current::$table,
$request->hasBodyParam('preview_sql'),
);
$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;
}
// If there is a request for SQL previewing.
if ($previewSql) {
$this->response->addJSON(
'sql_data',
$this->template->render('preview_sql', ['query_data' => $sqlResult]),
);
return;
}
if ($request->isAjax()) {
$message = Message::success(
__('Table %1$s has been altered successfully.'),
);
$message->addParam(Current::$table);
$this->response->addJSON(
'message',
Generator::getMessage($message, $sqlResult, 'success'),
);
$indexes = Index::getFromTable($this->dbi, Current::$table, Current::$database);
$indexesDuplicates = Index::findDuplicates(Current::$table, Current::$database);
$this->response->addJSON(
'index_table',
$this->template->render('indexes', [
'url_params' => ['db' => Current::$database, 'table' => Current::$table],
'indexes' => $indexes,
'indexes_duplicates' => $indexesDuplicates,
]),
);
return;
}
/** @var StructureController $controller */
$controller = ContainerBuilder::getContainer()->get(StructureController::class);
$controller($request);
return;
}

View File

@ -4,26 +4,18 @@ declare(strict_types=1);
namespace PhpMyAdmin\Table;
use PhpMyAdmin\Container\ContainerBuilder;
use PhpMyAdmin\Controllers\Table\StructureController;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Html\Generator;
use PhpMyAdmin\Http\ServerRequest;
use PhpMyAdmin\Identifiers\DatabaseName;
use PhpMyAdmin\Index;
use PhpMyAdmin\Message;
use PhpMyAdmin\Query\Compatibility;
use PhpMyAdmin\Query\Generator as QueryGenerator;
use PhpMyAdmin\ResponseRenderer;
use PhpMyAdmin\Template;
use function __;
final class Indexes
{
public function __construct(
protected ResponseRenderer $response,
protected Template $template,
private DatabaseInterface $dbi,
) {
}
@ -37,14 +29,13 @@ final class Indexes
* @param bool $renameMode Rename the Index mode
*/
public function doSaveData(
ServerRequest $request,
Index $index,
bool $renameMode,
string $db,
string $table,
bool $previewSql,
string $oldIndexName = '',
): void {
): string|Message {
$error = false;
if ($renameMode && Compatibility::isCompatibleRenameIndex($this->dbi->getVersion())) {
if ($oldIndexName === 'PRIMARY') {
@ -64,56 +55,21 @@ final class Indexes
$index->getName(),
);
} else {
$sqlQuery = $this->dbi->getTable($db, $table)
->getSqlQueryForIndexCreateOrEdit($index, $error);
$sqlQuery = $this->dbi->getTable($db, $table)->getSqlQueryForIndexCreateOrEdit($index, $error);
}
// If there is a request for SQL previewing.
if ($previewSql) {
$this->response->addJSON(
'sql_data',
$this->template->render('preview_sql', ['query_data' => $sqlQuery]),
);
return;
return $sqlQuery;
}
if ($error) {
$this->response->setRequestStatus(false);
$this->response->addJSON('message', $error);
return;
if ($error instanceof Message) {
return $error;
}
$this->dbi->query($sqlQuery);
if ($request->isAjax()) {
$message = Message::success(
__('Table %1$s has been altered successfully.'),
);
$message->addParam($table);
$this->response->addJSON(
'message',
Generator::getMessage($message, $sqlQuery, 'success'),
);
$indexes = Index::getFromTable($this->dbi, $table, $db);
$indexesDuplicates = Index::findDuplicates($table, $db);
$this->response->addJSON(
'index_table',
$this->template->render('indexes', [
'url_params' => ['db' => $db, 'table' => $table],
'indexes' => $indexes,
'indexes_duplicates' => $indexesDuplicates,
]),
);
return;
}
/** @var StructureController $controller */
$controller = ContainerBuilder::getContainer()->get(StructureController::class);
$controller($request);
return $sqlQuery;
}
public function executeAddIndexSql(string|DatabaseName $db, string $sql): Message

View File

@ -1878,10 +1878,10 @@ class Table implements Stringable
/**
* Function to get the sql query for index creation or edit
*
* @param Index $index current index
* @param bool $error whether error occurred or not
* @param Index $index current index
* @param Message|false $error whether error occurred or not
*/
public function getSqlQueryForIndexCreateOrEdit(Index $index, bool &$error): string
public function getSqlQueryForIndexCreateOrEdit(Index $index, Message|false &$error): string
{
// $sql_query is the one displayed in the query box
$sqlQuery = sprintf(
@ -1891,7 +1891,7 @@ class Table implements Stringable
);
// Drops the old index
if (! empty($_POST['old_index'])) {
if (isset($_POST['old_index'])) {
$oldIndex = is_array($_POST['old_index']) ? $_POST['old_index']['Key_name'] : $_POST['old_index'];
if ($oldIndex === 'PRIMARY') {
$sqlQuery .= ' DROP PRIMARY KEY,';

View File

@ -48,7 +48,7 @@ class IndexRenameControllerTest extends AbstractTestCase
$response,
$template,
$dbi,
new Indexes($response, $template, $dbi),
new Indexes($dbi),
new DbTableExists($dbi),
))($request);
$this->assertSame($expected, $response->getHTMLResult());

View File

@ -96,7 +96,7 @@ class IndexesControllerTest extends AbstractTestCase
$response,
$template,
$dbi,
new Indexes($response, $template, $dbi),
new Indexes($dbi),
new DbTableExists($dbi),
);

View File

@ -36,7 +36,7 @@ class SpatialControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new SpatialController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);
@ -64,7 +64,7 @@ class SpatialControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new SpatialController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);
@ -90,7 +90,7 @@ class SpatialControllerTest extends AbstractTestCase
$controllerStub->expects($this->never())->method('__invoke');
$response = new ResponseRenderer();
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new SpatialController($response, new Template(), $controllerStub, $indexes);
$controller($request);
@ -120,7 +120,7 @@ class SpatialControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new SpatialController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);

View File

@ -36,7 +36,7 @@ class UniqueControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new UniqueController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);
@ -64,7 +64,7 @@ class UniqueControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new UniqueController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);
@ -90,7 +90,7 @@ class UniqueControllerTest extends AbstractTestCase
$controllerStub->expects($this->never())->method('__invoke');
$response = new ResponseRenderer();
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new UniqueController($response, new Template(), $controllerStub, $indexes);
$controller($request);
@ -120,7 +120,7 @@ class UniqueControllerTest extends AbstractTestCase
$controllerStub = $this->createMock(StructureController::class);
$controllerStub->expects($this->once())->method('__invoke')->with($request);
$indexes = new Indexes(new ResponseRenderer(), new Template(), DatabaseInterface::getInstance());
$indexes = new Indexes(DatabaseInterface::getInstance());
$controller = new UniqueController(new ResponseRenderer(), new Template(), $controllerStub, $indexes);
$controller($request);

View File

@ -7,13 +7,10 @@ namespace PhpMyAdmin\Tests\Table;
use PhpMyAdmin\Config;
use PhpMyAdmin\Current;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Http\Factory\ServerRequestFactory;
use PhpMyAdmin\Index;
use PhpMyAdmin\Table\Indexes;
use PhpMyAdmin\Table\Table;
use PhpMyAdmin\Template;
use PhpMyAdmin\Tests\AbstractTestCase;
use PhpMyAdmin\Tests\Stubs\ResponseRenderer as ResponseStub;
use PHPUnit\Framework\Attributes\CoversClass;
#[CoversClass(Indexes::class)]
@ -65,25 +62,24 @@ class IndexesTest extends AbstractTestCase
$dbi->expects($this->any())->method('getTable')
->willReturn($table);
$response = new ResponseStub();
$index = new Index();
$indexes = new Indexes($response, new Template(), $dbi);
$request = ServerRequestFactory::create()->createServerRequest('GET', 'http://example.com/')
->withQueryParams(['ajax_request' => '1']);
$indexes = new Indexes($dbi);
// Preview SQL
$indexes->doSaveData($request, $index, false, Current::$database, Current::$table, true);
$jsonArray = $response->getJSONResult();
$this->assertArrayHasKey('sql_data', $jsonArray);
$this->assertStringContainsString($sqlQuery, $jsonArray['sql_data']);
$sqlResult = $indexes->doSaveData($index, false, Current::$database, Current::$table, true);
$this->assertIsString($sqlResult);
$this->assertStringContainsString($sqlQuery, $sqlResult);
// Alter success
$response->clear();
$indexes->doSaveData($request, $index, false, Current::$database, Current::$table, false);
$jsonArray = $response->getJSONResult();
$this->assertArrayHasKey('index_table', $jsonArray);
$this->assertArrayHasKey('message', $jsonArray);
$sqlResult = $indexes->doSaveData($index, false, Current::$database, Current::$table, false);
$this->assertIsString($sqlResult);
$this->assertStringContainsString($sqlQuery, $sqlResult);
// Error message
// Cannot be tested at the moment.
// $index->setName('PRIMARY'); // Cannot rename any index to primary so the operation should fail
// $indexes->doSaveData($index, false, Current::$database, Current::$table, false);
// $this->assertInstanceOf(Message::class, $sqlResult);
}
}