From ea1713081c0579430ea11e4e8361c09968cdc52b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 18 Jul 2020 15:23:47 -0300 Subject: [PATCH 1/2] Create Export\TemplateModel class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the database calls from the controller to the model. Signed-off-by: Maurício Meneghini Fauth --- .../Controllers/ExportTemplateController.php | 161 +++++++---------- libraries/classes/Display/Export.php | 57 ++---- libraries/classes/Export/Template.php | 70 ++++++++ libraries/classes/Export/TemplateModel.php | 170 ++++++++++++++++++ libraries/services.php | 6 + libraries/services_controllers.php | 2 +- phpstan-baseline.neon | 15 -- psalm-baseline.xml | 12 -- .../display/export/template_options.twig | 4 +- 9 files changed, 335 insertions(+), 162 deletions(-) create mode 100644 libraries/classes/Export/Template.php create mode 100644 libraries/classes/Export/TemplateModel.php diff --git a/libraries/classes/Controllers/ExportTemplateController.php b/libraries/classes/Controllers/ExportTemplateController.php index 497235914c..e7b3766a65 100644 --- a/libraries/classes/Controllers/ExportTemplateController.php +++ b/libraries/classes/Controllers/ExportTemplateController.php @@ -5,16 +5,18 @@ declare(strict_types=1); namespace PhpMyAdmin\Controllers; use PhpMyAdmin\DatabaseInterface; -use PhpMyAdmin\Display\Export; +use PhpMyAdmin\Export\Template as ExportTemplate; +use PhpMyAdmin\Export\TemplateModel; use PhpMyAdmin\Relation; use PhpMyAdmin\Response; use PhpMyAdmin\Template; -use PhpMyAdmin\Util; +use function is_array; +use function is_string; final class ExportTemplateController extends AbstractController { - /** @var Export */ - private $export; + /** @var TemplateModel */ + private $model; /** @var Relation */ private $relation; @@ -23,164 +25,139 @@ final class ExportTemplateController extends AbstractController * @param Response $response * @param DatabaseInterface $dbi */ - public function __construct($response, $dbi, Template $template, Export $export, Relation $relation) - { + public function __construct( + $response, + $dbi, + Template $template, + TemplateModel $model, + Relation $relation + ) { parent::__construct($response, $dbi, $template); - $this->export = $export; + $this->model = $model; $this->relation = $relation; } public function create(): void { + global $cfg; + $cfgRelation = $this->relation->getRelationsParam(); if (! $cfgRelation['exporttemplateswork']) { return; } - $templateTable = Util::backquote($cfgRelation['db']) . '.' - . Util::backquote($cfgRelation['export_templates']); - $user = $this->dbi->escapeString($GLOBALS['cfg']['Server']['user']); + $template = ExportTemplate::fromArray([ + 'username' => $cfg['Server']['user'], + 'exportType' => $_POST['exportType'], + 'name' => $_POST['templateName'], + 'data' => $_POST['templateData'], + ]); + $result = $this->model->create($cfgRelation['db'], $cfgRelation['export_templates'], $template); - $query = 'INSERT INTO ' . $templateTable . '(' - . ' `username`, `export_type`,' - . ' `template_name`, `template_data`' - . ') VALUES (' - . "'" . $user . "', " - . "'" . $this->dbi->escapeString($_POST['exportType']) - . "', '" . $this->dbi->escapeString($_POST['templateName']) - . "', '" . $this->dbi->escapeString($_POST['templateData']) - . "');"; - - $result = $this->relation->queryAsControlUser($query, false); - - if (! $result) { - $error = $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + if (is_string($result)) { $this->response->setRequestStatus(false); - $this->response->addJSON('message', $error); + $this->response->addJSON('message', $result); return; } - $this->response->setRequestStatus(true); - - $this->response->addJSON( - 'data', - $this->export->getOptionsForTemplates($_POST['exportType']) + $templates = $this->model->getAll( + $cfgRelation['db'], + $cfgRelation['export_templates'], + $template->getUsername(), + $template->getExportType() ); - $this->dbi->freeResult($result); + $this->response->setRequestStatus(true); + $this->response->addJSON( + 'data', + $this->template->render('display/export/template_options', [ + 'templates' => is_array($templates) ? $templates : [], + 'selected_template' => $_POST['template_id'] ?? null, + ]) + ); } public function delete(): void { + global $cfg; + $cfgRelation = $this->relation->getRelationsParam(); if (! $cfgRelation['exporttemplateswork']) { return; } - $id = ''; - if (isset($_POST['templateId'])) { - $id = $this->dbi->escapeString($_POST['templateId']); - } + $result = $this->model->delete( + $cfgRelation['db'], + $cfgRelation['export_templates'], + $cfg['Server']['user'], + (int) $_POST['templateId'] + ); - $templateTable = Util::backquote($cfgRelation['db']) . '.' - . Util::backquote($cfgRelation['export_templates']); - $user = $this->dbi->escapeString($GLOBALS['cfg']['Server']['user']); - - $query = 'DELETE FROM ' . $templateTable - . ' WHERE `id` = ' . $id . " AND `username` = '" . $user . "'"; - - $result = $this->relation->queryAsControlUser($query, false); - - if (! $result) { - $error = $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + if (is_string($result)) { $this->response->setRequestStatus(false); - $this->response->addJSON('message', $error); + $this->response->addJSON('message', $result); return; } $this->response->setRequestStatus(true); - - $this->dbi->freeResult($result); } public function load(): void { + global $cfg; + $cfgRelation = $this->relation->getRelationsParam(); if (! $cfgRelation['exporttemplateswork']) { return; } - $id = ''; - if (isset($_POST['templateId'])) { - $id = $this->dbi->escapeString($_POST['templateId']); - } + $template = $this->model->load( + $cfgRelation['db'], + $cfgRelation['export_templates'], + $cfg['Server']['user'], + (int) $_POST['templateId'] + ); - $templateTable = Util::backquote($cfgRelation['db']) . '.' - . Util::backquote($cfgRelation['export_templates']); - $user = $this->dbi->escapeString($GLOBALS['cfg']['Server']['user']); - - $query = 'SELECT `template_data` FROM ' . $templateTable - . ' WHERE `id` = ' . $id . " AND `username` = '" . $user . "'"; - - $result = $this->relation->queryAsControlUser($query, false); - - if (! $result) { - $error = $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + if (! $template instanceof ExportTemplate) { $this->response->setRequestStatus(false); - $this->response->addJSON('message', $error); + $this->response->addJSON('message', $template); return; } $this->response->setRequestStatus(true); - - $data = null; - while ($row = $this->dbi->fetchAssoc($result, DatabaseInterface::CONNECT_CONTROL)) { - $data = $row['template_data']; - } - $this->response->addJSON('data', $data); - - $this->dbi->freeResult($result); + $this->response->addJSON('data', $template->getData()); } public function update(): void { + global $cfg; + $cfgRelation = $this->relation->getRelationsParam(); if (! $cfgRelation['exporttemplateswork']) { return; } - $id = ''; - if (isset($_POST['templateId'])) { - $id = $this->dbi->escapeString($_POST['templateId']); - } + $template = ExportTemplate::fromArray([ + 'id' => (int) $_POST['templateId'], + 'username' => $cfg['Server']['user'], + 'data' => $_POST['templateData'], + ]); + $result = $this->model->update($cfgRelation['db'], $cfgRelation['export_templates'], $template); - $templateTable = Util::backquote($cfgRelation['db']) . '.' - . Util::backquote($cfgRelation['export_templates']); - $user = $this->dbi->escapeString($GLOBALS['cfg']['Server']['user']); - - $query = 'UPDATE ' . $templateTable . ' SET `template_data` = ' - . "'" . $this->dbi->escapeString($_POST['templateData']) . "'" - . ' WHERE `id` = ' . $id . " AND `username` = '" . $user . "'"; - - $result = $this->relation->queryAsControlUser($query, false); - - if (! $result) { - $error = $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + if (is_string($result)) { $this->response->setRequestStatus(false); - $this->response->addJSON('message', $error); + $this->response->addJSON('message', $result); return; } $this->response->setRequestStatus(true); - - $this->dbi->freeResult($result); } } diff --git a/libraries/classes/Display/Export.php b/libraries/classes/Display/Export.php index b6a9c2a4f9..c46f9ec1aa 100644 --- a/libraries/classes/Display/Export.php +++ b/libraries/classes/Display/Export.php @@ -8,8 +8,8 @@ declare(strict_types=1); namespace PhpMyAdmin\Display; use PhpMyAdmin\Core; -use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Encoding; +use PhpMyAdmin\Export\TemplateModel; use PhpMyAdmin\Html\MySQLDocumentation; use PhpMyAdmin\Message; use PhpMyAdmin\Plugins; @@ -27,6 +27,7 @@ use Twig\Error\SyntaxError; use function explode; use function function_exists; use function in_array; +use function is_array; use function mb_strpos; use function strlen; use function urldecode; @@ -42,10 +43,14 @@ class Export /** @var Template */ public $template; + /** @var TemplateModel */ + private $templateModel; + public function __construct() { $this->relation = new Relation($GLOBALS['dbi']); $this->template = new Template(); + $this->templateModel = new TemplateModel($GLOBALS['dbi']); } /** @@ -148,44 +153,6 @@ class Export ]); } - /** - * Returns HTML for the options in template dropdown - * - * @param string $exportType export type - server, database, or table - * - * @return string HTML for the options in teplate dropdown - */ - public function getOptionsForTemplates($exportType) - { - // Get the relation settings - $cfgRelation = $this->relation->getRelationsParam(); - - $query = 'SELECT `id`, `template_name` FROM ' - . Util::backquote($cfgRelation['db']) . '.' - . Util::backquote($cfgRelation['export_templates']) - . ' WHERE `username` = ' - . "'" . $GLOBALS['dbi']->escapeString($GLOBALS['cfg']['Server']['user']) - . "' AND `export_type` = '" . $GLOBALS['dbi']->escapeString($exportType) . "'" - . ' ORDER BY `template_name`;'; - - $result = $this->relation->queryAsControlUser($query); - - $templates = []; - if ($result !== false) { - while ($row = $GLOBALS['dbi']->fetchAssoc($result, DatabaseInterface::CONNECT_CONTROL)) { - $templates[] = [ - 'name' => $row['template_name'], - 'id' => $row['id'], - ]; - } - } - - return $this->template->render('display/export/template_options', [ - 'templates' => $templates, - 'selected_template' => ! empty($_POST['template_id']) ? $_POST['template_id'] : null, - ]); - } - /** * Prints Html For Export Options Method * @@ -705,8 +672,18 @@ class Export ]); if ($cfgRelation['exporttemplateswork']) { + $templates = $this->templateModel->getAll( + $cfgRelation['db'], + $cfgRelation['export_templates'], + $GLOBALS['cfg']['Server']['user'], + $exportType + ); + $html .= $this->template->render('display/export/template_loading', [ - 'options' => $this->getOptionsForTemplates($exportType), + 'options' => $this->template->render('display/export/template_options', [ + 'templates' => is_array($templates) ? $templates : [], + 'selected_template' => $_POST['template_id'] ?? null, + ]), ]); } diff --git a/libraries/classes/Export/Template.php b/libraries/classes/Export/Template.php new file mode 100644 index 0000000000..7de53fb5fe --- /dev/null +++ b/libraries/classes/Export/Template.php @@ -0,0 +1,70 @@ +id = $id; + $this->username = $username; + $this->exportType = $exportType; + $this->name = $name; + $this->data = $data; + } + + /** @param array $state */ + public static function fromArray(array $state): self + { + return new self( + $state['id'] ?? 0, + $state['username'], + $state['exportType'] ?? '', + $state['name'] ?? '', + $state['data'] + ); + } + + public function getId(): int + { + return $this->id; + } + + public function getUsername(): string + { + return $this->username; + } + + public function getExportType(): string + { + return $this->exportType; + } + + public function getName(): string + { + return $this->name; + } + + public function getData(): string + { + return $this->data; + } +} diff --git a/libraries/classes/Export/TemplateModel.php b/libraries/classes/Export/TemplateModel.php new file mode 100644 index 0000000000..d8463ef2c4 --- /dev/null +++ b/libraries/classes/Export/TemplateModel.php @@ -0,0 +1,170 @@ +dbi = $dbi; + } + + /** @return bool|string */ + public function create(string $db, string $table, Template $template) + { + $query = sprintf( + 'INSERT INTO %s.%s (`username`, `export_type`, `template_name`, `template_data`)' + . ' VALUES (\'%s\', \'%s\', \'%s\', \'%s\');', + Util::backquote($db), + Util::backquote($table), + $this->dbi->escapeString($template->getUsername()), + $this->dbi->escapeString($template->getExportType()), + $this->dbi->escapeString($template->getName()), + $this->dbi->escapeString($template->getData()) + ); + $result = $this->dbi->tryQuery( + $query, + DatabaseInterface::CONNECT_CONTROL, + 0, + false + ); + + if ($result === false) { + return $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + } + + return true; + } + + /** @return bool|string */ + public function delete(string $db, string $table, string $user, int $id) + { + $query = sprintf( + 'DELETE FROM %s.%s WHERE `id` = %s AND `username` = \'%s\';', + Util::backquote($db), + Util::backquote($table), + $id, + $this->dbi->escapeString($user) + ); + $result = $this->dbi->tryQuery( + $query, + DatabaseInterface::CONNECT_CONTROL, + 0, + false + ); + + if ($result === false) { + return $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + } + + return true; + } + + /** @return Template|string|bool */ + public function load(string $db, string $table, string $user, int $id) + { + $query = sprintf( + 'SELECT * FROM %s.%s WHERE `id` = %s AND `username` = \'%s\';', + Util::backquote($db), + Util::backquote($table), + $id, + $this->dbi->escapeString($user) + ); + $result = $this->dbi->tryQuery( + $query, + DatabaseInterface::CONNECT_CONTROL, + 0, + false + ); + + if ($result === false) { + return $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + } + + $data = []; + while ($row = $this->dbi->fetchAssoc($result)) { + $data = $row; + } + + $this->dbi->freeResult($result); + + return Template::fromArray([ + 'id' => (int) $data['id'], + 'username' => $data['username'], + 'exportType' => $data['export_type'], + 'name' => $data['template_name'], + 'data' => $data['template_data'], + ]); + } + + /** @return bool|string */ + public function update(string $db, string $table, Template $template) + { + $query = sprintf( + 'UPDATE %s.%s SET `template_data` = \'%s\' WHERE `id` = %s AND `username` = \'%s\';', + Util::backquote($db), + Util::backquote($table), + $this->dbi->escapeString($template->getData()), + $template->getId(), + $this->dbi->escapeString($template->getUsername()) + ); + $result = $this->dbi->tryQuery( + $query, + DatabaseInterface::CONNECT_CONTROL, + 0, + false + ); + + if ($result === false) { + return $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + } + + return true; + } + + /** @return Template[]|string|bool */ + public function getAll(string $db, string $table, string $user, string $exportType) + { + $query = sprintf( + 'SELECT * FROM %s.%s WHERE `username` = \'%s\' AND `export_type` = \'%s\' ORDER BY `template_name`;', + Util::backquote($db), + Util::backquote($table), + $this->dbi->escapeString($user), + $this->dbi->escapeString($exportType) + ); + $result = $this->dbi->tryQuery( + $query, + DatabaseInterface::CONNECT_CONTROL, + 0, + false + ); + + if ($result === false) { + return $this->dbi->getError(DatabaseInterface::CONNECT_CONTROL); + } + + $templates = []; + while ($row = $this->dbi->fetchAssoc($result)) { + $templates[] = Template::fromArray([ + 'id' => (int) $row['id'], + 'username' => $row['username'], + 'exportType' => $row['export_type'], + 'name' => $row['template_name'], + 'data' => $row['template_data'], + ]); + } + + $this->dbi->freeResult($result); + + return $templates; + } +} diff --git a/libraries/services.php b/libraries/services.php index e6265c3b4e..27c7492a18 100644 --- a/libraries/services.php +++ b/libraries/services.php @@ -97,6 +97,12 @@ return [ 'arguments' => ['@dbi'], ], + 'export_template_model' => + [ + 'class' => PhpMyAdmin\Export\TemplateModel::class, + 'arguments' => + ['@dbi'], + ], 'expression_language' => [ 'class' => Symfony\Component\ExpressionLanguage\ExpressionLanguage::class, diff --git a/libraries/services_controllers.php b/libraries/services_controllers.php index c9816e633e..854f42460e 100644 --- a/libraries/services_controllers.php +++ b/libraries/services_controllers.php @@ -299,7 +299,7 @@ return [ 'response' => '@response', 'dbi' => '@dbi', 'template' => '@template', - 'export' => '@display_export', + 'model' => '@export_template_model', 'relation' => '@relation', ], ], diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index dba53d8edc..cd81d43bd1 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -270,21 +270,6 @@ parameters: count: 1 path: libraries/classes/Controllers/ExportController.php - - - message: "#^Parameter \\#1 \\$result of method PhpMyAdmin\\\\DatabaseInterface\\:\\:freeResult\\(\\) expects object, resource\\|true given\\.$#" - count: 4 - path: libraries/classes/Controllers/ExportTemplateController.php - - - - message: "#^Method PhpMyAdmin\\\\DatabaseInterface\\:\\:fetchAssoc\\(\\) invoked with 2 parameters, 1 required\\.$#" - count: 1 - path: libraries/classes/Controllers/ExportTemplateController.php - - - - message: "#^Parameter \\#1 \\$result of method PhpMyAdmin\\\\DatabaseInterface\\:\\:fetchAssoc\\(\\) expects object, resource\\|true given\\.$#" - count: 1 - path: libraries/classes/Controllers/ExportTemplateController.php - - message: "#^Parameter \\#1 \\$error_message of method PhpMyAdmin\\\\Import\\:\\:stop\\(\\) expects PhpMyAdmin\\\\Message, PhpMyAdmin\\\\Message\\|null given\\.$#" count: 2 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index db4f822270..52655bf32e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -247,18 +247,6 @@ $dump_buffer - - - $result - $result - $result - $result - $result - - - fetchAssoc - - mb_strpos($_POST['value'], '(') diff --git a/templates/display/export/template_options.twig b/templates/display/export/template_options.twig index ddcd4f5b55..d698d63b17 100644 --- a/templates/display/export/template_options.twig +++ b/templates/display/export/template_options.twig @@ -1,7 +1,7 @@ {% for template in templates %} - {% endfor %} From baf4d090bd8fd404cca3c0a0ea238bb3e93c0653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Mon, 20 Jul 2020 15:41:08 -0300 Subject: [PATCH 2/2] Add ExportTemplateControllerTest test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../ExportTemplateControllerTest.php | 134 ++++++++++++++++++ test/classes/Stubs/DbiDummy.php | 30 ++++ 2 files changed, 164 insertions(+) create mode 100644 test/classes/Controllers/ExportTemplateControllerTest.php diff --git a/test/classes/Controllers/ExportTemplateControllerTest.php b/test/classes/Controllers/ExportTemplateControllerTest.php new file mode 100644 index 0000000000..fb3c44d1aa --- /dev/null +++ b/test/classes/Controllers/ExportTemplateControllerTest.php @@ -0,0 +1,134 @@ +setGlobalDbi(); + + $PMA_Config = new Config(); + $PMA_Config->enableBc(); + + $_SESSION = [' PMA_token ' => 'token']; + $GLOBALS['server'] = 1; + $GLOBALS['text_dir'] = 'ltr'; + $GLOBALS['PMA_PHP_SELF'] = ''; + + $_SESSION['relation'][$GLOBALS['server']] = [ + 'PMA_VERSION' => PMA_VERSION, + 'exporttemplateswork' => true, + 'db' => 'db', + 'export_templates' => 'table', + ]; + + $this->response = new Response(); + $this->template = new Template(); + + $this->controller = new ExportTemplateController( + $this->response, + $dbi, + $this->template, + new TemplateModel($dbi), + new Relation($dbi, $this->template) + ); + } + + public function testCreate(): void + { + global $cfg; + + $cfg['Server']['user'] = 'user'; + $_POST['exportType'] = 'type'; + $_POST['templateName'] = 'name'; + $_POST['templateData'] = 'data'; + + $this->controller->create(); + + $templates = [ + ExportTemplate::fromArray([ + 'id' => 1, + 'username' => 'user1', + 'exportType' => 'type1', + 'name' => 'name1', + 'data' => 'data1', + ]), + ExportTemplate::fromArray([ + 'id' => 2, + 'username' => 'user2', + 'exportType' => 'type2', + 'name' => 'name2', + 'data' => 'data2', + ]), + ]; + + $options = $this->template->render('display/export/template_options', [ + 'templates' => $templates, + 'selected_template' => null, + ]); + + $this->assertTrue($this->response->hasSuccessState()); + $this->assertEquals(['data' => $options], $this->response->getJSONResult()); + } + + public function testDelete(): void + { + global $cfg; + + $cfg['Server']['user'] = 'user'; + $_POST['templateId'] = '1'; + + $this->controller->delete(); + + $this->assertTrue($this->response->hasSuccessState()); + } + + public function testLoad(): void + { + global $cfg; + + $cfg['Server']['user'] = 'user'; + $_POST['templateId'] = '1'; + + $this->controller->load(); + + $this->assertTrue($this->response->hasSuccessState()); + $this->assertEquals(['data' => 'data1'], $this->response->getJSONResult()); + } + + public function testUpdate(): void + { + global $cfg; + + $cfg['Server']['user'] = 'user'; + $_POST['templateId'] = '1'; + $_POST['templateData'] = 'data'; + + $this->controller->update(); + + $this->assertTrue($this->response->hasSuccessState()); + } +} diff --git a/test/classes/Stubs/DbiDummy.php b/test/classes/Stubs/DbiDummy.php index 744561c2ab..1157fa8e99 100644 --- a/test/classes/Stubs/DbiDummy.php +++ b/test/classes/Stubs/DbiDummy.php @@ -1938,6 +1938,36 @@ class DbiDummy implements DbiExtension ['slow_query_log', 'OFF'], ], ], + [ + 'query' => 'INSERT INTO `db`.`table` (`username`, `export_type`, `template_name`, `template_data`)' + . ' VALUES (\'user\', \'type\', \'name\', \'data\');', + 'result' => [], + ], + [ + 'query' => 'SELECT * FROM `db`.`table` WHERE `username` = \'user\'' + . ' AND `export_type` = \'type\' ORDER BY `template_name`;', + 'columns' => ['id', 'username', 'export_type', 'template_name', 'template_data'], + 'result' => [ + ['1', 'user1', 'type1', 'name1', 'data1'], + ['2', 'user2', 'type2', 'name2', 'data2'], + ], + ], + [ + 'query' => 'DELETE FROM `db`.`table` WHERE `id` = 1 AND `username` = \'user\';', + 'result' => [], + ], + [ + 'query' => 'SELECT * FROM `db`.`table` WHERE `id` = 1 AND `username` = \'user\';', + 'columns' => ['id', 'username', 'export_type', 'template_name', 'template_data'], + 'result' => [ + ['1', 'user1', 'type1', 'name1', 'data1'], + ], + ], + [ + 'query' => 'UPDATE `db`.`table` SET `template_data` = \'data\'' + . ' WHERE `id` = 1 AND `username` = \'user\';', + 'result' => [], + ], ]; /** * Current database.