From 120715b1420e5fca547b0b31e7b6628c51e2045a Mon Sep 17 00:00:00 2001 From: Satyam Date: Tue, 21 Apr 2026 15:01:03 +0530 Subject: [PATCH] Use request database/table names in CopyStructureController Stop relying on Current::$database for copy-structure SQL; use validated DatabaseName. Remove redundant selectDb from table copy controller and align unit tests with response handling and DBI expectations. Signed-off-by: Satyam --- .../Structure/CopyStructureController.php | 12 ++-- .../Structure/CopyStructureController.php | 4 +- .../Structure/CopyStructureControllerTest.php | 56 ++++++++------- .../Structure/CopyStructureControllerTest.php | 71 +++++++++---------- 4 files changed, 73 insertions(+), 70 deletions(-) diff --git a/src/Controllers/Database/Structure/CopyStructureController.php b/src/Controllers/Database/Structure/CopyStructureController.php index c6d8540b5e..b5b436b518 100644 --- a/src/Controllers/Database/Structure/CopyStructureController.php +++ b/src/Controllers/Database/Structure/CopyStructureController.php @@ -46,15 +46,15 @@ final readonly class CopyStructureController implements InvocableController return $this->response->response(); } - $this->dbi->selectDb(Current::$database); + $dbName = $databaseName->getName(); /** @var string[] $tableNames */ - $tableNames = $this->dbi->getTables(Current::$database); + $tableNames = $this->dbi->getTables($dbName); $baseTables = []; $views = []; foreach ($tableNames as $table) { - $object = $this->dbi->getTable(Current::$database, $table); + $object = $this->dbi->getTable($dbName, $table); if ($object->isView()) { $views[] = $table; } else { @@ -63,18 +63,18 @@ final readonly class CopyStructureController implements InvocableController } $segments = [ - sprintf('-- Database: %s', Current::$database), + sprintf('-- Database: %s', $dbName), ]; foreach ($baseTables as $table) { - $object = $this->dbi->getTable(Current::$database, $table); + $object = $this->dbi->getTable($dbName, $table); $segments[] = $object->showCreate(); } if ($views !== []) { $segments[] = '-- Views'; foreach ($views as $table) { - $object = $this->dbi->getTable(Current::$database, $table); + $object = $this->dbi->getTable($dbName, $table); $segments[] = $object->showCreate(); } } diff --git a/src/Controllers/Table/Structure/CopyStructureController.php b/src/Controllers/Table/Structure/CopyStructureController.php index fb3dd709b8..32dd0a2900 100644 --- a/src/Controllers/Table/Structure/CopyStructureController.php +++ b/src/Controllers/Table/Structure/CopyStructureController.php @@ -44,8 +44,6 @@ final readonly class CopyStructureController implements InvocableController return $this->response->response(); } - $this->dbi->selectDb(Current::$database); - $databaseName = DatabaseName::tryFrom($request->getParam('db')); if ($databaseName === null || ! $this->dbTableExists->selectDatabase($databaseName)) { $this->response->setRequestStatus(false); @@ -62,7 +60,7 @@ final readonly class CopyStructureController implements InvocableController return $this->response->response(); } - $object = $this->dbi->getTable(Current::$database, Current::$table); + $object = $this->dbi->getTable($databaseName->getName(), $tableName->getName()); $this->response->addJSON('sql', $object->showCreate()); return $this->response->response(); diff --git a/tests/unit/Controllers/Database/Structure/CopyStructureControllerTest.php b/tests/unit/Controllers/Database/Structure/CopyStructureControllerTest.php index c0a7444c13..e128fd5d4d 100644 --- a/tests/unit/Controllers/Database/Structure/CopyStructureControllerTest.php +++ b/tests/unit/Controllers/Database/Structure/CopyStructureControllerTest.php @@ -9,10 +9,13 @@ use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\DatabaseInterface; use PhpMyAdmin\DbTableExists; use PhpMyAdmin\Http\Factory\ServerRequestFactory; +use PhpMyAdmin\Http\Response; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Tests\Stubs\ResponseRenderer as ResponseStub; use PHPUnit\Framework\Attributes\CoversClass; +use function strpos; + #[CoversClass(CopyStructureController::class)] final class CopyStructureControllerTest extends AbstractTestCase { @@ -23,14 +26,16 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface(); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => '']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No databases selected', $response->getJSONResult()['message']); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + $message = (string) $responseRenderer->getJSONResult()['message']; + self::assertStringContainsString('No databases selected', $message); } public function testReturnErrorWhenDatabaseNameInvalid(): void @@ -40,15 +45,17 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface(); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); // empty 'db' param → DatabaseName::tryFrom returns null $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => '']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No databases selected', $response->getJSONResult()['message']); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + $message = (string) $responseRenderer->getJSONResult()['message']; + self::assertStringContainsString('No databases selected', $message); } public function testReturnsSqlForTablesOnly(): void @@ -58,8 +65,6 @@ final class CopyStructureControllerTest extends AbstractTestCase $createSql = "CREATE TABLE `orders` (\n `id` int(11) NOT NULL\n) ENGINE=InnoDB"; $dbiDummy = $this->createDbiDummy(); - // DbTableExists::selectDatabase() + $this->dbi->selectDb() both call selectDb - $dbiDummy->addSelectDb('test_db'); $dbiDummy->addSelectDb('test_db'); // getTables() call $dbiDummy->addResult( @@ -79,15 +84,16 @@ final class CopyStructureControllerTest extends AbstractTestCase // Pre-seed the TABLE_TYPE cache so isView() returns false without an extra query $dbi->getCache()->cacheTableValue('test_db', 'orders', 'TABLE_TYPE', 'BASE TABLE'); - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db']) ->withParsedBody(['db' => 'test_db']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertTrue($response->hasSuccessState()); - $sql = $response->getJSONResult()['sql']; + self::assertInstanceOf(Response::class, $response); + self::assertTrue($responseRenderer->hasSuccessState()); + $sql = (string) $responseRenderer->getJSONResult()['sql']; self::assertStringContainsString('-- Database: test_db', $sql); self::assertStringContainsString($createSql, $sql); self::assertStringNotContainsString('-- Views', $sql); @@ -101,11 +107,10 @@ final class CopyStructureControllerTest extends AbstractTestCase Current::$database = 'test_db'; $tableSql = "CREATE TABLE `products` (\n `id` int(11) NOT NULL\n) ENGINE=InnoDB"; - $viewSql = "CREATE VIEW `v_products` AS SELECT * FROM `products`"; + $viewSql = 'CREATE VIEW `v_products` AS SELECT * FROM `products`'; $dbiDummy = $this->createDbiDummy(); $dbiDummy->addSelectDb('test_db'); - $dbiDummy->addSelectDb('test_db'); $dbiDummy->addResult( 'SHOW TABLES FROM `test_db`;', [['products'], ['v_products']], @@ -127,15 +132,16 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi->getCache()->cacheTableValue('test_db', 'products', 'TABLE_TYPE', 'BASE TABLE'); $dbi->getCache()->cacheTableValue('test_db', 'v_products', 'TABLE_TYPE', 'VIEW'); - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db']) ->withParsedBody(['db' => 'test_db']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertTrue($response->hasSuccessState()); - $sql = $response->getJSONResult()['sql']; + self::assertInstanceOf(Response::class, $response); + self::assertTrue($responseRenderer->hasSuccessState()); + $sql = (string) $responseRenderer->getJSONResult()['sql']; self::assertStringContainsString('-- Database: test_db', $sql); self::assertStringContainsString($tableSql, $sql); self::assertStringContainsString('-- Views', $sql); @@ -156,21 +162,21 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbiDummy = $this->createDbiDummy(); $dbiDummy->addSelectDb('empty_db'); - $dbiDummy->addSelectDb('empty_db'); $dbiDummy->addResult('SHOW TABLES FROM `empty_db`;', []); $dbi = $this->createDatabaseInterface($dbiDummy); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'empty_db']) ->withParsedBody(['db' => 'empty_db']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertTrue($response->hasSuccessState()); - $sql = $response->getJSONResult()['sql']; + self::assertInstanceOf(Response::class, $response); + self::assertTrue($responseRenderer->hasSuccessState()); + $sql = (string) $responseRenderer->getJSONResult()['sql']; self::assertStringContainsString('-- Database: empty_db', $sql); self::assertStringNotContainsString('CREATE TABLE', $sql); self::assertStringNotContainsString('-- Views', $sql); diff --git a/tests/unit/Controllers/Table/Structure/CopyStructureControllerTest.php b/tests/unit/Controllers/Table/Structure/CopyStructureControllerTest.php index 7c72dbc90b..a582b0cf4d 100644 --- a/tests/unit/Controllers/Table/Structure/CopyStructureControllerTest.php +++ b/tests/unit/Controllers/Table/Structure/CopyStructureControllerTest.php @@ -9,6 +9,7 @@ use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\DatabaseInterface; use PhpMyAdmin\DbTableExists; use PhpMyAdmin\Http\Factory\ServerRequestFactory; +use PhpMyAdmin\Http\Response; use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Tests\Stubs\ResponseRenderer as ResponseStub; use PHPUnit\Framework\Attributes\CoversClass; @@ -24,14 +25,16 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface(); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => '', 'table' => 'orders']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No databases selected', $response->getJSONResult()['message']); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + $message = (string) $responseRenderer->getJSONResult()['message']; + self::assertStringContainsString('No databases selected', $message); } public function testReturnErrorWhenNoTableSet(): void @@ -42,14 +45,15 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface(); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db', 'table' => '']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No table selected', $response->getJSONResult()['message']); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + self::assertStringContainsString('No table selected', (string) $responseRenderer->getJSONResult()['message']); } public function testReturnErrorWhenDatabaseNameInvalid(): void @@ -57,23 +61,20 @@ final class CopyStructureControllerTest extends AbstractTestCase Current::$database = 'test_db'; Current::$table = 'orders'; - // Controller calls selectDb(Current::$database) before the DatabaseName check - $dbiDummy = $this->createDbiDummy(); - $dbiDummy->addSelectDb('test_db'); - $dbi = $this->createDatabaseInterface($dbiDummy); + $dbi = $this->createDatabaseInterface(); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); - // 'db' param is empty → DatabaseName::tryFrom returns null + $responseRenderer = new ResponseStub(); + // 'db' param is empty → DatabaseName::tryFrom returns null, no DB call made $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => '', 'table' => 'orders']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No databases selected', $response->getJSONResult()['message']); - - $dbiDummy->assertAllSelectsConsumed(); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + $message = (string) $responseRenderer->getJSONResult()['message']; + self::assertStringContainsString('No databases selected', $message); } public function testReturnsSqlForTable(): void @@ -84,9 +85,6 @@ final class CopyStructureControllerTest extends AbstractTestCase $createSql = "CREATE TABLE `orders` (\n `id` int(11) NOT NULL\n) ENGINE=InnoDB"; $dbiDummy = $this->createDbiDummy(); - // 1st: controller calls selectDb(Current::$database) - // 2nd: DbTableExists::selectDatabase() calls selectDb($databaseName) - $dbiDummy->addSelectDb('test_db'); $dbiDummy->addSelectDb('test_db'); // DbTableExists::hasTable issues SELECT 1 FROM `db`.`table` LIMIT 1 $dbiDummy->addResult('SELECT 1 FROM `test_db`.`orders` LIMIT 1;', [['1']]); @@ -100,15 +98,16 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface($dbiDummy); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db', 'table' => 'orders']) ->withParsedBody(['db' => 'test_db', 'table' => 'orders']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertTrue($response->hasSuccessState()); - self::assertSame($createSql, $response->getJSONResult()['sql']); + self::assertInstanceOf(Response::class, $response); + self::assertTrue($responseRenderer->hasSuccessState()); + self::assertSame($createSql, $responseRenderer->getJSONResult()['sql']); $dbiDummy->assertAllSelectsConsumed(); $dbiDummy->assertAllQueriesConsumed(); @@ -123,7 +122,6 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbiDummy = $this->createDbiDummy(); $dbiDummy->addSelectDb('test_db'); - $dbiDummy->addSelectDb('test_db'); $dbiDummy->addResult('SELECT 1 FROM `test_db`.`v_orders` LIMIT 1;', [['1']]); $dbiDummy->addResult( 'SHOW CREATE TABLE `test_db`.`v_orders`', @@ -134,15 +132,16 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbi = $this->createDatabaseInterface($dbiDummy); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db', 'table' => 'v_orders']) ->withParsedBody(['db' => 'test_db', 'table' => 'v_orders']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertTrue($response->hasSuccessState()); - self::assertSame($viewSql, $response->getJSONResult()['sql']); + self::assertInstanceOf(Response::class, $response); + self::assertTrue($responseRenderer->hasSuccessState()); + self::assertSame($viewSql, $responseRenderer->getJSONResult()['sql']); $dbiDummy->assertAllSelectsConsumed(); $dbiDummy->assertAllQueriesConsumed(); @@ -155,22 +154,22 @@ final class CopyStructureControllerTest extends AbstractTestCase $dbiDummy = $this->createDbiDummy(); $dbiDummy->addSelectDb('test_db'); - $dbiDummy->addSelectDb('test_db'); // hasTable SELECT fails (table not found) $dbiDummy->addResult('SELECT 1 FROM `test_db`.`ghost_table` LIMIT 1;', false); $dbi = $this->createDatabaseInterface($dbiDummy); DatabaseInterface::$instance = $dbi; - $response = new ResponseStub(); + $responseRenderer = new ResponseStub(); $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/') ->withQueryParams(['db' => 'test_db', 'table' => 'ghost_table']) ->withParsedBody(['db' => 'test_db', 'table' => 'ghost_table']); - (new CopyStructureController($response, $dbi, new DbTableExists($dbi)))($request); + $response = (new CopyStructureController($responseRenderer, $dbi, new DbTableExists($dbi)))($request); - self::assertFalse($response->hasSuccessState()); - self::assertStringContainsString('No table selected', $response->getJSONResult()['message']); + self::assertInstanceOf(Response::class, $response); + self::assertFalse($responseRenderer->hasSuccessState()); + self::assertStringContainsString('No table selected', (string) $responseRenderer->getJSONResult()['message']); $dbiDummy->assertAllSelectsConsumed(); $dbiDummy->assertAllQueriesConsumed();