Merge pull request #682 from cakephp/environment-adapter

Get Environment tests passing with migrations engine
cakephp · Jan 26, 2024 · 1a3682e · 1a3682e
2 parents 424224e + ac088ce
commit 1a3682e
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 139 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -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
@@ -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

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -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>
@@ -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>

diff --git a/src/Db/Adapter/AdapterFactory.php b/src/Db/Adapter/AdapterFactory.php
@@ -8,6 +8,7 @@
 
 namespace Migrations\Db\Adapter;
 
+use Closure;
 use RuntimeException;
 
 /**
@@ -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,
@@ -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
@@ -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);
     }
 
     /**

diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php
@@ -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;
@@ -78,10 +79,11 @@ public function executeMigration(MigrationInterface $migration, string $directio
         $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();
 
@@ -90,24 +92,29 @@ public function executeMigration(MigrationInterface $migration, string $directio
         }
 
         // 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}();
                 }
@@ -117,11 +124,11 @@ public function executeMigration(MigrationInterface $migration, string $directio
         }
 
         // 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();
@@ -135,14 +142,17 @@ public function executeMigration(MigrationInterface $migration, string $directio
      */
     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();
         }
 
         // Run the seeder
@@ -151,8 +161,8 @@ public function executeSeed(SeedInterface $seed): void
         }
 
         // commit the transaction if the adapter supports it
-        if ($this->getAdapter()->hasTransactions()) {
-            $this->getAdapter()->commitTransaction();
+        if ($adapter->hasTransactions()) {
+            $adapter->commitTransaction();
         }
     }
 

diff --git a/tests/TestCase/Db/Adapter/AdapterFactoryTest.php b/tests/TestCase/Db/Adapter/AdapterFactoryTest.php
@@ -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;
@@ -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()