diff --git a/src/Command/StatusCommand.php b/src/Command/StatusCommand.php index d3a3231d..95924ed6 100644 --- a/src/Command/StatusCommand.php +++ b/src/Command/StatusCommand.php @@ -137,6 +137,7 @@ protected function getConfig(Arguments $args): Config 'host' => $connectionConfig['host'], 'name' => $connectionConfig['database'], 'migration_table' => $table, + 'connection' => $connectionName, ]; $configData = [ @@ -147,7 +148,6 @@ protected function getConfig(Arguments $args): Config 'file' => $templatePath . 'Phinx/create.php.template', ], 'migration_base_class' => 'Migrations\AbstractMigration', - 'connection' => ConnectionManager::get($connectionName), 'environment' => $adapterConfig, // TODO do we want to support the DI container in migrations? ]; diff --git a/src/Config/Config.php b/src/Config/Config.php index b4d045ff..6c475ad7 100644 --- a/src/Config/Config.php +++ b/src/Config/Config.php @@ -108,6 +108,14 @@ public function getSeedBaseClassName(bool $dropNamespace = true): string return $dropNamespace ? substr((string)strrchr($className, '\\'), 1) : $className; } + /** + * @inheritdoc + */ + public function getConnection(): string|false + { + return $this->values['connection'] ?? false; + } + /** * @inheritdoc */ diff --git a/src/Config/ConfigInterface.php b/src/Config/ConfigInterface.php index 7e43adec..46824edb 100644 --- a/src/Config/ConfigInterface.php +++ b/src/Config/ConfigInterface.php @@ -42,6 +42,13 @@ public function getMigrationPaths(): array; */ public function getSeedPaths(): array; + /** + * Get the connection namee + * + * @return string|false + */ + public function getConnection(): string|false; + /** * Get the template file name. * diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index a6ac82d8..e680ac11 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -77,21 +77,10 @@ public function setOptions(array $options): AdapterInterface { $this->options = $options; - if (isset($options['default_migration_table'])) { - trigger_error('The default_migration_table setting for adapter has been deprecated since 0.13.0. Use `migration_table` instead.', E_USER_DEPRECATED); - if (!isset($options['migration_table'])) { - $options['migration_table'] = $options['default_migration_table']; - } - } - if (isset($options['migration_table'])) { $this->setSchemaTableName($options['migration_table']); } - if (isset($options['data_domain'])) { - $this->setDataDomain($options['data_domain']); - } - return $this; } @@ -198,82 +187,6 @@ public function setSchemaTableName(string $schemaTableName) return $this; } - /** - * Gets the data domain. - * - * @return array - */ - public function getDataDomain(): array - { - return $this->dataDomain; - } - - /** - * Sets the data domain. - * - * @param array $dataDomain Array for the data domain - * @return $this - */ - public function setDataDomain(array $dataDomain) - { - $this->dataDomain = []; - - // Iterate over data domain field definitions and perform initial and - // simple normalization. We make sure the definition as a base 'type' - // and it is compatible with the base Phinx types. - foreach ($dataDomain as $type => $options) { - if (!isset($options['type'])) { - throw new InvalidArgumentException(sprintf( - 'You must specify a type for data domain type "%s".', - $type - )); - } - - // Replace type if it's the name of a Phinx constant - if (defined('static::' . $options['type'])) { - $options['type'] = constant('static::' . $options['type']); - } - - if (!in_array($options['type'], $this->getColumnTypes(), true)) { - throw new InvalidArgumentException(sprintf( - 'An invalid column type "%s" was specified for data domain type "%s".', - $options['type'], - $type - )); - } - - $internal_type = $options['type']; - unset($options['type']); - - // Do a simple replacement for the 'length' / 'limit' option and - // detect hinting values for 'limit'. - if (isset($options['length'])) { - $options['limit'] = $options['length']; - unset($options['length']); - } - - if (isset($options['limit']) && !is_numeric($options['limit'])) { - if (!defined('static::' . $options['limit'])) { - throw new InvalidArgumentException(sprintf( - 'An invalid limit value "%s" was specified for data domain type "%s".', - $options['limit'], - $type - )); - } - - $options['limit'] = constant('static::' . $options['limit']); - } - - // Save the data domain types in a more suitable format - $this->dataDomain[$type] = [ - 'type' => $internal_type, - 'options' => $options, - ]; - } - - return $this; - } - /** * @inheritdoc */ @@ -281,14 +194,7 @@ public function getColumnForType(string $columnName, string $type, array $option { $column = new Column(); $column->setName($columnName); - - if (array_key_exists($type, $this->getDataDomain())) { - $column->setType($this->dataDomain[$type]['type']); - $column->setOptions($this->dataDomain[$type]['options']); - } else { - $column->setType($type); - } - + $column->setType($type); $column->setOptions($options); return $column; diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index 55926f61..539eb720 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -122,7 +122,17 @@ public function setOptions(array $options): AdapterInterface parent::setOptions($options); if (isset($options['connection'])) { - $this->setConnection($options['connection']); + // TODO Change migrations adapters to use the Cake + // Database API instead of PDO. + $driver = $options['connection']->getDriver(); + + // TODO this is gross and needs to be replaced + $reflect = new ReflectionProperty($driver, 'pdo'); + $reflect->setAccessible(true); + $pdo = $reflect->getValue($driver); + + assert($pdo instanceof PDO, 'Need a PDO connection'); + $this->setConnection($pdo); } return $this; diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php index bdb108d3..31a8fbb9 100644 --- a/src/Migration/Environment.php +++ b/src/Migration/Environment.php @@ -8,10 +8,10 @@ namespace Migrations\Migration; +use Cake\Datasource\ConnectionManager; use Migrations\Db\Adapter\AdapterFactory; use Migrations\Db\Adapter\AdapterInterface; use Migrations\Db\Adapter\PhinxAdapter; -use PDO; use Phinx\Migration\MigrationInterface; use Phinx\Seed\SeedInterface; use RuntimeException; @@ -340,21 +340,24 @@ public function getAdapter(): AdapterInterface } $options = $this->getOptions(); - if (isset($options['connection'])) { - if (!($options['connection'] instanceof PDO)) { - throw new RuntimeException('The specified connection is not a PDO instance'); - } - - $options['connection']->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - $options['adapter'] = $options['connection']->getAttribute(PDO::ATTR_DRIVER_NAME); - } - if (!isset($options['adapter'])) { - throw new RuntimeException('No adapter was specified for environment: ' . $this->getName()); + if (!isset($options['connection'])) { + throw new RuntimeException('No connection defined'); } + // TODO Start here again. Goal is to have Connection go into + // AdapterFactory to choose the adapter. + // Adapters should use Connection API instead of PDO. + $connection = ConnectionManager::get($options['connection']); + + // Get the driver classname as those are aligned with adapter names. + $driver = $connection->getDriver(); + $driverClass = get_class($driver); + $driverName = strtolower(substr($driverClass, (int)strrpos($driverClass, '\\') + 1)); + + $options['connection'] = $connection; $factory = AdapterFactory::instance(); $adapter = $factory - ->getAdapter($options['adapter'], $options); + ->getAdapter($driverName, $options); // Automatically time the executed commands $adapter = $factory->getWrapper('timed', $adapter); @@ -375,14 +378,6 @@ public function getAdapter(): AdapterInterface if ($output) { $adapter->setOutput($output); } - - // TODO remove this, cake connections don't do prefixes. - // Use the TablePrefixAdapter if table prefix/suffixes are in use - if ($adapter->hasOption('table_prefix') || $adapter->hasOption('table_suffix')) { - $adapter = AdapterFactory::instance() - ->getWrapper('prefix', $adapter); - } - $this->setAdapter($adapter); return $adapter; diff --git a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php index 57b376e0..ec2a76eb 100644 --- a/tests/TestCase/Db/Adapter/MysqlAdapterTest.php +++ b/tests/TestCase/Db/Adapter/MysqlAdapterTest.php @@ -736,53 +736,6 @@ public function testAddColumnWithDefaultLiteral() $this->assertTrue($rows[1]['Default'] === 'CURRENT_TIMESTAMP' || $rows[1]['Default'] === 'current_timestamp()'); } - public function testAddColumnWithCustomType() - { - $this->adapter->setDataDomain([ - 'custom1' => [ - 'type' => 'enum', - 'null' => true, - 'values' => 'a,b,c', - ], - 'custom2' => [ - 'type' => 'enum', - 'null' => true, - 'values' => ['a', 'b', 'c'], - ], - ]); - - (new Table('table1', [], $this->adapter)) - ->addColumn('custom1', 'custom1') - ->addColumn('custom2', 'custom2') - ->addColumn('custom_ext', 'custom2', [ - 'null' => false, - 'values' => ['d', 'e', 'f'], - ]) - ->save(); - - $this->assertTrue($this->adapter->hasTable('table1')); - - $columns = $this->adapter->getColumns('table1'); - - $this->assertArrayHasKey(1, $columns); - $this->assertArrayHasKey(2, $columns); - $this->assertArrayHasKey(3, $columns); - - foreach ([1, 2] as $index) { - $column = $this->adapter->getColumns('table1')[$index]; - $this->assertSame("custom{$index}", $column->getName()); - $this->assertSame('enum', $column->getType()); - $this->assertSame(['a', 'b', 'c'], $column->getValues()); - $this->assertTrue($column->getNull()); - } - - $column = $this->adapter->getColumns('table1')[3]; - $this->assertSame('custom_ext', $column->getName()); - $this->assertSame('enum', $column->getType()); - $this->assertSame(['d', 'e', 'f'], $column->getValues()); - $this->assertFalse($column->getNull()); - } - public function testAddColumnFirst() { $table = new Table('table1', [], $this->adapter); diff --git a/tests/TestCase/Db/Adapter/PdoAdapterTest.php b/tests/TestCase/Db/Adapter/PdoAdapterTest.php index 60eba49a..ad8113e2 100644 --- a/tests/TestCase/Db/Adapter/PdoAdapterTest.php +++ b/tests/TestCase/Db/Adapter/PdoAdapterTest.php @@ -33,14 +33,6 @@ public function testOptions() $this->assertEquals('bar', $options['foo']); } - public function testOptionsSetConnection() - { - $connection = $this->getMockBuilder(PDO::class)->disableOriginalConstructor()->getMock(); - $this->adapter->setOptions(['connection' => $connection]); - - $this->assertSame($connection, $this->adapter->getConnection()); - } - public function testOptionsSetSchemaTableName() { $this->assertEquals('phinxlog', $this->adapter->getSchemaTableName()); diff --git a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php index abe2da83..e52d85b4 100644 --- a/tests/TestCase/Db/Adapter/PostgresAdapterTest.php +++ b/tests/TestCase/Db/Adapter/PostgresAdapterTest.php @@ -880,39 +880,6 @@ public function testAddColumnArrayType($column_name, $column_type) $this->assertTrue($table->hasColumn($column_name)); } - public function testAddColumnWithCustomType() - { - $this->adapter->setDataDomain([ - 'custom' => [ - 'type' => 'inet', - 'null' => true, - ], - ]); - - (new Table('table1', [], $this->adapter)) - ->addColumn('custom', 'custom') - ->addColumn('custom_ext', 'custom', [ - 'null' => false, - ]) - ->save(); - - $this->assertTrue($this->adapter->hasTable('table1')); - - $columns = $this->adapter->getColumns('table1'); - $this->assertArrayHasKey(1, $columns); - $this->assertArrayHasKey(2, $columns); - - $column = $this->adapter->getColumns('table1')[1]; - $this->assertSame('custom', $column->getName()); - $this->assertSame('inet', $column->getType()); - $this->assertTrue($column->getNull()); - - $column = $this->adapter->getColumns('table1')[2]; - $this->assertSame('custom_ext', $column->getName()); - $this->assertSame('inet', $column->getType()); - $this->assertFalse($column->getNull()); - } - public function testRenameColumn() { $table = new Table('t', [], $this->adapter); diff --git a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php index 0b3efbdf..705f26ef 100644 --- a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php +++ b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php @@ -511,43 +511,6 @@ public function testAddColumnWithDefaultEmptyString() $this->assertEquals("''", $rows[1]['dflt_value']); } - public function testAddColumnWithCustomType() - { - $this->adapter->setDataDomain([ - 'custom' => [ - 'type' => 'string', - 'null' => true, - 'limit' => 15, - ], - ]); - - (new Table('table1', [], $this->adapter)) - ->addColumn('custom', 'custom') - ->addColumn('custom_ext', 'custom', [ - 'null' => false, - 'limit' => 30, - ]) - ->save(); - - $this->assertTrue($this->adapter->hasTable('table1')); - - $columns = $this->adapter->getColumns('table1'); - $this->assertArrayHasKey(1, $columns); - $this->assertArrayHasKey(2, $columns); - - $column = $this->adapter->getColumns('table1')[1]; - $this->assertSame('custom', $column->getName()); - $this->assertSame('string', $column->getType()); - $this->assertSame(15, $column->getLimit()); - $this->assertTrue($column->getNull()); - - $column = $this->adapter->getColumns('table1')[2]; - $this->assertSame('custom_ext', $column->getName()); - $this->assertSame('string', $column->getType()); - $this->assertSame(30, $column->getLimit()); - $this->assertFalse($column->getNull()); - } - public static function irregularCreateTableProvider() { return [ diff --git a/tests/TestCase/Db/Adapter/SqlserverAdapterTest.php b/tests/TestCase/Db/Adapter/SqlserverAdapterTest.php index d32d41f6..13e46485 100644 --- a/tests/TestCase/Db/Adapter/SqlserverAdapterTest.php +++ b/tests/TestCase/Db/Adapter/SqlserverAdapterTest.php @@ -508,39 +508,6 @@ public function testAddColumnWithDefaultBool() } } - public function testAddColumnWithCustomType() - { - $this->adapter->setDataDomain([ - 'custom' => [ - 'type' => 'geometry', - 'null' => true, - ], - ]); - - (new Table('table1', [], $this->adapter)) - ->addColumn('custom', 'custom') - ->addColumn('custom_ext', 'custom', [ - 'null' => false, - ]) - ->save(); - - $this->assertTrue($this->adapter->hasTable('table1')); - - $columns = $this->adapter->getColumns('table1'); - $this->assertArrayHasKey('custom', $columns); - $this->assertArrayHasKey('custom_ext', $columns); - - $column = $this->adapter->getColumns('table1')['custom']; - $this->assertSame('custom', $column->getName()); - $this->assertSame('geometry', (string)$column->getType()); - $this->assertTrue($column->getNull()); - - $column = $this->adapter->getColumns('table1')['custom_ext']; - $this->assertSame('custom_ext', $column->getName()); - $this->assertSame('geometry', (string)$column->getType()); - $this->assertFalse($column->getNull()); - } - public function testRenameColumn() { $table = new Table('t', [], $this->adapter); diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php index fdfac5b8..d9678226 100644 --- a/tests/TestCase/Migration/EnvironmentTest.php +++ b/tests/TestCase/Migration/EnvironmentTest.php @@ -3,6 +3,8 @@ namespace Test\Phinx\Migration; +use Cake\Datasource\ConnectionManager; +use Migrations\Db\Adapter\AdapterWrapper; use Migrations\Db\Adapter\PdoAdapter; use Migrations\Migration\Environment; use Phinx\Migration\AbstractMigration; @@ -10,7 +12,6 @@ use Phinx\Seed\AbstractSeed; use PHPUnit\Framework\TestCase; use RuntimeException; -use stdClass; class EnvironmentTest extends TestCase { @@ -48,7 +49,7 @@ public function testInvalidAdapter() $this->environment->setOptions(['adapter' => 'fakeadapter']); $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Adapter "fakeadapter" has not been registered'); + $this->expectExceptionMessage('No connection defined'); $this->environment->getAdapter(); } @@ -60,16 +61,30 @@ public function testNoAdapter() $this->environment->getAdapter(); } - public function testGetAdapterWithBadExistingPdoInstance() + public function testGetAdapterWithBadConnectionName() { - $this->environment->setOptions(['connection' => new stdClass()]); + $this->environment->setOptions(['connection' => 'lolnope']); $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('The specified connection is not a PDO instance'); + $this->expectExceptionMessage('The datasource configuration `lolnope` was not found'); $this->environment->getAdapter(); } + public function testGetAdapter() + { + /** @var array $config */ + $config = ConnectionManager::getConfig('test'); + $environment = new Environment('default', [ + 'connection' => 'test', + 'name' => $config['database'], + 'migration_table' => 'phinxlog', + ]); + $adapter = $environment->getAdapter(); + $this->assertNotEmpty($adapter); + $this->assertInstanceOf(AdapterWrapper::class, $adapter); + } + public function testSchemaName() { $this->assertEquals('phinxlog', $this->environment->getSchemaTableName()); diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index cffdbdf5..0ba2c991 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -77,12 +77,14 @@ public static function getConfigArray(): array /** @var array $dbConfig */ $dbConfig = ConnectionManager::getConfig('test'); $config = [ + 'connection' => 'test', + 'migration_table' => 'phinxlog', + // TODO all of these should go away. 'adapter' => $dbConfig['scheme'], - 'user' => $dbConfig['username'], - 'pass' => $dbConfig['password'], + 'user' => $dbConfig['username'] ?? null, + 'pass' => $dbConfig['password'] ?? null, 'host' => $dbConfig['host'], 'name' => $dbConfig['database'], - 'migration_table' => 'phinxlog', ]; return [ @@ -124,6 +126,8 @@ protected function prepareEnvironment(array $paths = []): AdapterInterface $connectionConfig = ConnectionManager::getConfig('test'); $adapter = $connectionConfig['scheme'] ?? null; $adapterConfig = [ + 'connection' => 'test', + // TODO all of this should go away 'adapter' => $adapter, 'user' => $connectionConfig['username'], 'pass' => $connectionConfig['password'], @@ -2253,6 +2257,7 @@ public function testGettingOutputObject(): void public function testReversibleMigrationsWorkAsExpected(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); $adapter = $this->prepareEnvironment([ 'migrations' => ROOT . '/config/Reversiblemigrations', ]); @@ -2296,6 +2301,7 @@ public function testReversibleMigrationsWorkAsExpected(): void public function testReversibleMigrationWithIndexConflict(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); if ($this->getDriverType() !== 'mysql') { $this->markTestSkipped('Test requires mysql connection'); } @@ -2336,6 +2342,7 @@ public function testReversibleMigrationWithIndexConflict(): void public function testReversibleMigrationWithFKConflictOnTableDrop(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); if ($this->getDriverType() !== 'mysql') { $this->markTestSkipped('Test requires mysql'); } @@ -2381,6 +2388,7 @@ public function testReversibleMigrationWithFKConflictOnTableDrop(): void public function testBreakpointsTogglingOperateAsExpected(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); if ($this->getDriverType() !== 'mysql') { $this->markTestSkipped('Test requires mysql'); } @@ -2550,6 +2558,7 @@ public function testBreakpointsTogglingOperateAsExpected(): void public function testBreakpointWithInvalidVersion(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); if ($this->getDriverType() !== 'mysql') { $this->markTestSkipped('test requires mysql'); } @@ -2704,6 +2713,7 @@ public function testInvalidVersionBreakpoint(): void public function testMigrationWillNotBeExecuted(): void { + $this->markTestIncomplete('Need to finish updating adapters to use Connection'); if ($this->getDriverType() !== 'mysql') { $this->markTestSkipped('Test requires mysql'); } @@ -2714,13 +2724,6 @@ public function testMigrationWillNotBeExecuted(): void $configArray['paths']['migrations'] = ROOT . '/config/ShouldExecute/'; $config = new Config($configArray); - // ensure the database is empty - $dbName = ConnectionManager::getConfig('test')['database'] ?? null; - $this->assertNotEmpty($dbName); - $adapter->dropDatabase($dbName); - $adapter->createDatabase($dbName); - $adapter->disconnect(); - // Run the migration with shouldExecute returning false: the table should not be created $this->manager->setConfig($config); $this->manager->migrate(20201207205056);