Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get Environment tests passing with migrations engine #682

Merged
merged 4 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ parameters:
count: 1
path: src/Command/BakeMigrationSnapshotCommand.php

-
message: "#^Method Migrations\\\\Db\\\\Adapter\\\\AdapterFactory\\:\\:getAdapter\\(\\) should return Migrations\\\\Db\\\\Adapter\\\\AdapterInterface but returns object\\.$#"
count: 1
path: src/Db/Adapter/AdapterFactory.php

-
message: "#^Unsafe usage of new static\\(\\)\\.$#"
count: 1
Expand Down Expand Up @@ -75,16 +70,6 @@ parameters:
count: 2
path: src/Db/Adapter/SqlserverAdapter.php

-
message: "#^Parameter \\#1 \\$adapter of method Phinx\\\\Migration\\\\MigrationInterface\\:\\:setAdapter\\(\\) expects Phinx\\\\Db\\\\Adapter\\\\AdapterInterface, Migrations\\\\Db\\\\Adapter\\\\AdapterInterface given\\.$#"
count: 2
path: src/Migration/Environment.php

-
message: "#^Parameter \\#1 \\$adapter of method Phinx\\\\Seed\\\\SeedInterface\\:\\:setAdapter\\(\\) expects Phinx\\\\Db\\\\Adapter\\\\AdapterInterface, Migrations\\\\Db\\\\Adapter\\\\AdapterInterface given\\.$#"
count: 1
path: src/Migration/Environment.php

-
message: "#^Possibly invalid array key type Cake\\\\Database\\\\Schema\\\\TableSchemaInterface\\|string\\.$#"
count: 2
Expand Down
10 changes: 5 additions & 5 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
<code><![CDATA[isset($this->output)]]></code>
</RedundantPropertyInitializationCheck>
</file>
<file src="src/Db/Adapter/AdapterFactory.php">
<PropertyTypeCoercion>
<code><![CDATA[$this->adapters]]></code>
</PropertyTypeCoercion>
</file>
<file src="src/Db/Adapter/AdapterWrapper.php">
<DeprecatedMethod>
<code>getQueryBuilder</code>
Expand Down Expand Up @@ -69,11 +74,6 @@
</MissingParamType>
</file>
<file src="src/Migration/Environment.php">
<InvalidArgument>
<code><![CDATA[$this->getAdapter()]]></code>
<code><![CDATA[$this->getAdapter()]]></code>
<code><![CDATA[$this->getAdapter()]]></code>
</InvalidArgument>
<RedundantPropertyInitializationCheck>
<code><![CDATA[isset($this->adapter)]]></code>
</RedundantPropertyInitializationCheck>
Expand Down
44 changes: 18 additions & 26 deletions src/Db/Adapter/AdapterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace Migrations\Db\Adapter;

use Closure;
use RuntimeException;

/**
Expand Down Expand Up @@ -39,9 +40,9 @@ public static function instance(): static
/**
* Class map of database adapters, indexed by PDO::ATTR_DRIVER_NAME.
*
* @var array<string, \Migrations\Db\Adapter\AdapterInterface|string>
* @phpstan-var array<string, class-string<\Migrations\Db\Adapter\AdapterInterface>>
* @psalm-var array<string, class-string<\Migrations\Db\Adapter\AdapterInterface>>
* @var array<string, string|\Closure>
* @phpstan-var array<string, class-string<\Migrations\Db\Adapter\AdapterInterface>|\Closure>
* @psalm-var array<string, class-string<\Migrations\Db\Adapter\AdapterInterface>|\Closure>
*/
protected array $adapters = [
'mysql' => MysqlAdapter::class,
Expand All @@ -65,13 +66,15 @@ public static function instance(): static
* Register an adapter class with a given name.
*
* @param string $name Name
* @param string $class Class
* @param \Closure|string $class Class or factory method for the adapter.
* @throws \RuntimeException
* @return $this
*/
public function registerAdapter(string $name, string $class)
public function registerAdapter(string $name, Closure|string $class)
{
if (!is_subclass_of($class, AdapterInterface::class)) {
if (
!($class instanceof Closure || is_subclass_of($class, AdapterInterface::class))
) {
throw new RuntimeException(sprintf(
'Adapter class "%s" must implement Migrations\\Db\\Adapter\\AdapterInterface',
$class
Expand All @@ -83,37 +86,26 @@ public function registerAdapter(string $name, string $class)
}

/**
* Get an adapter class by name.
* Get an adapter instance by name.
*
* @param string $name Name
* @throws \RuntimeException
* @return string
* @phpstan-return class-string<\Migrations\Db\Adapter\AdapterInterface>
* @param array<string, mixed> $options Options
* @return \Migrations\Db\Adapter\AdapterInterface
*/
protected function getClass(string $name): object|string
public function getAdapter(string $name, array $options): AdapterInterface
{
if (empty($this->adapters[$name])) {
throw new RuntimeException(sprintf(
'Adapter "%s" has not been registered',
$name
));
}
$classOrFactory = $this->adapters[$name];
if ($classOrFactory instanceof Closure) {
return $classOrFactory($options);
}

return $this->adapters[$name];
}

/**
* Get an adapter instance by name.
*
* @param string $name Name
* @param array<string, mixed> $options Options
* @return \Migrations\Db\Adapter\AdapterInterface
*/
public function getAdapter(string $name, array $options): AdapterInterface
{
$class = $this->getClass($name);

return new $class($options);
return new $classOrFactory($options);
}

/**
Expand Down
52 changes: 31 additions & 21 deletions src/Migration/Environment.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

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;
Expand Down Expand Up @@ -78,10 +79,11 @@
$migration->setMigratingUp($direction === MigrationInterface::UP);

$startTime = time();
// Need to get a phinx interface adapter here. We will need to have a shim
// to bridge the interfaces. Changing the MigrationInterface is tricky
// because of the method names.
$migration->setAdapter($this->getAdapter());
// Use an adapter shim to bridge between the new migrations
// engine and the Phinx compatible interface
$adapter = $this->getAdapter();
$phinxShim = new PhinxAdapter($adapter);
$migration->setAdapter($phinxShim);

$migration->preFlightCheck();

Expand All @@ -90,24 +92,29 @@
}

// begin the transaction if the adapter supports it
if ($this->getAdapter()->hasTransactions()) {
$this->getAdapter()->beginTransaction();
if ($adapter->hasTransactions()) {
$adapter->beginTransaction();
}

if (!$fake) {
// Run the migration
if (method_exists($migration, MigrationInterface::CHANGE)) {
if ($direction === MigrationInterface::DOWN) {
// Create an instance of the ProxyAdapter so we can record all
// Create an instance of the RecordingAdapter so we can record all
// of the migration commands for reverse playback

/** @var \Phinx\Db\Adapter\ProxyAdapter $proxyAdapter */
$proxyAdapter = AdapterFactory::instance()
->getWrapper('proxy', $this->getAdapter());
$migration->setAdapter($proxyAdapter);
/** @var \Migrations\Db\Adapter\RecordingAdapter $recordAdapter */
$recordAdapter = AdapterFactory::instance()
->getWrapper('record', $adapter);

// Wrap the adapter with a phinx shim to maintain contain
$phinxAdapter = new PhinxAdapter($recordAdapter);
$migration->setAdapter($phinxAdapter);

$migration->{MigrationInterface::CHANGE}();
$proxyAdapter->executeInvertedCommands();
$migration->setAdapter($this->getAdapter());
$recordAdapter->executeInvertedCommands();

$migration->setAdapter(new PhinxAdapter($this->getAdapter()));
} else {
$migration->{MigrationInterface::CHANGE}();
}
Expand All @@ -117,11 +124,11 @@
}

// Record it in the database
$this->getAdapter()->migrated($migration, $direction, date('Y-m-d H:i:s', $startTime), date('Y-m-d H:i:s', time()));
$adapter->migrated($migration, $direction, date('Y-m-d H:i:s', $startTime), date('Y-m-d H:i:s', time()));

// commit the transaction if the adapter supports it
if ($this->getAdapter()->hasTransactions()) {
$this->getAdapter()->commitTransaction();
if ($adapter->hasTransactions()) {
$adapter->commitTransaction();
}

$migration->postFlightCheck();
Expand All @@ -135,14 +142,17 @@
*/
public function executeSeed(SeedInterface $seed): void
{
$seed->setAdapter($this->getAdapter());
$adapter = $this->getAdapter();
$phinxAdapter = new PhinxAdapter($adapter);

$seed->setAdapter($phinxAdapter);
if (method_exists($seed, SeedInterface::INIT)) {
$seed->{SeedInterface::INIT}();
}

// begin the transaction if the adapter supports it
if ($this->getAdapter()->hasTransactions()) {
$this->getAdapter()->beginTransaction();
if ($adapter->hasTransactions()) {
$adapter->beginTransaction();

Check warning on line 155 in src/Migration/Environment.php

View check run for this annotation

Codecov / codecov/patch

src/Migration/Environment.php#L155

Added line #L155 was not covered by tests
}

// Run the seeder
Expand All @@ -151,8 +161,8 @@
}

// commit the transaction if the adapter supports it
if ($this->getAdapter()->hasTransactions()) {
$this->getAdapter()->commitTransaction();
if ($adapter->hasTransactions()) {
$adapter->commitTransaction();

Check warning on line 165 in src/Migration/Environment.php

View check run for this annotation

Codecov / codecov/patch

src/Migration/Environment.php#L165

Added line #L165 was not covered by tests
}
}

Expand Down
16 changes: 8 additions & 8 deletions tests/TestCase/Db/Adapter/AdapterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Migrations\Test\Db\Adapter;

use Migrations\Db\Adapter\AdapterFactory;
use Migrations\Db\Adapter\PdoAdapter;
use PHPUnit\Framework\TestCase;
use ReflectionMethod;
use RuntimeException;
Expand All @@ -27,20 +28,19 @@ protected function tearDown(): void

public function testInstanceIsFactory()
{
$this->assertInstanceOf('Migrations\Db\Adapter\AdapterFactory', $this->factory);
$this->assertInstanceOf(AdapterFactory::class, $this->factory);
}

public function testRegisterAdapter()
{
// AdapterFactory::getClass is protected, work around it to avoid
// creating unnecessary instances and making the test more complex.
$method = new ReflectionMethod(get_class($this->factory), 'getClass');
$method->setAccessible(true);
$mock = $this->getMockForAbstractClass(PdoAdapter::class, [['foo' => 'bar']]);
$this->factory->registerAdapter('test', function (array $options) use ($mock) {
$this->assertEquals('value', $options['key']);

$adapter = $method->invoke($this->factory, 'mysql');
$this->factory->registerAdapter('test', $adapter);
return $mock;
});

$this->assertEquals($adapter, $method->invoke($this->factory, 'test'));
$this->assertEquals($mock, $this->factory->getAdapter('test', ['key' => 'value']));
}

public function testRegisterAdapterFailure()
Expand Down
Loading
Loading