Merge pull request #713 from cakephp/cleanup

Cleanup
cakephp · Apr 29, 2024 · 873025c · 873025c
2 parents bc37e9d + 6d90a6e
commit 873025c
Show file tree

Hide file tree

Showing 25 changed files with 148 additions and 229 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -150,10 +150,4 @@
       <code>self::VERBOSITY_*</code>
     </MoreSpecificReturnType>
   </file>
-  <file src="src/TableFinderTrait.php">
-    <PossiblyUndefinedArrayOffset>
-      <code>$split[0]</code>
-      <code>$splitted[0]</code>
-    </PossiblyUndefinedArrayOffset>
-  </file>
 </files>
diff --git a/src/Command/BakeMigrationSnapshotCommand.php b/src/Command/BakeMigrationSnapshotCommand.php
@@ -24,7 +24,7 @@
 use Cake\Datasource\ConnectionManager;
 use Cake\Event\Event;
 use Cake\Event\EventManager;
-use Migrations\TableFinderTrait;
+use Migrations\Util\TableFinder;
 use Migrations\Util\UtilTrait;
 
 /**
@@ -33,7 +33,6 @@
 class BakeMigrationSnapshotCommand extends BakeSimpleMigrationCommand
 {
     use SnapshotTrait;
-    use TableFinderTrait;
     use UtilTrait;
 
     /**
@@ -95,7 +94,8 @@ public function templateData(Arguments $arguments): array
             'require-table' => $arguments->getOption('require-table'),
             'plugin' => $this->plugin,
         ];
-        $tables = $this->getTablesToBake($collection, $options);
+        $finder = new TableFinder($this->connection);
+        $tables = $finder->getTablesToBake($collection, $options);
 
         sort($tables, SORT_NATURAL);
 

diff --git a/src/Command/DumpCommand.php b/src/Command/DumpCommand.php
@@ -21,7 +21,7 @@
 use Cake\Datasource\ConnectionManager;
 use Migrations\Config\ConfigInterface;
 use Migrations\Migration\ManagerFactory;
-use Migrations\TableFinderTrait;
+use Migrations\Util\TableFinder;
 
 /**
  * Dump command class.
@@ -30,8 +30,6 @@
  */
 class DumpCommand extends Command
 {
-    use TableFinderTrait;
-
     protected string $connection;
 
     /**
@@ -113,7 +111,7 @@ public function execute(Arguments $args, ConsoleIo $io): ?int
             'connection' => $args->getOption('connection'),
         ]);
         $config = $factory->createConfig();
-        $path = $config->getMigrationPaths()[0];
+        $path = $config->getMigrationPath();
         $connectionName = (string)$config->getConnection();
         $connection = ConnectionManager::get($connectionName);
         assert($connection instanceof Connection);
@@ -125,7 +123,8 @@ public function execute(Arguments $args, ConsoleIo $io): ?int
         ];
         // The connection property is used by the trait methods.
         $this->connection = $connectionName;
-        $tables = $this->getTablesToBake($collection, $options);
+        $finder = new TableFinder($connectionName);
+        $tables = $finder->getTablesToBake($collection, $options);
 
         $dump = [];
         if ($tables) {

diff --git a/src/Command/MarkMigratedCommand.php b/src/Command/MarkMigratedCommand.php
@@ -116,8 +116,7 @@ public function execute(Arguments $args, ConsoleIo $io): ?int
         ]);
         $manager = $factory->createManager($io);
         $config = $manager->getConfig();
-        $migrationPaths = $config->getMigrationPaths();
-        $path = array_pop($migrationPaths);
+        $path = $config->getMigrationPath();
 
         if ($this->invalidOnlyOrExclude($args)) {
             $io->err(

diff --git a/src/Command/MigrateCommand.php b/src/Command/MigrateCommand.php
@@ -138,7 +138,7 @@ protected function executeMigrations(Arguments $args, ConsoleIo $io): ?int
         }
         $io->out('<info>using connection</info> ' . (string)$args->getOption('connection'));
         $io->out('<info>using connection</info> ' . (string)$args->getOption('connection'));
-        $io->out('<info>using paths</info> ' . implode(', ', $config->getMigrationPaths()));
+        $io->out('<info>using paths</info> ' . $config->getMigrationPath());
         $io->out('<info>ordering by</info> ' . $versionOrder . ' time');
 
         if ($fake) {

diff --git a/src/Command/Phinx/Dump.php b/src/Command/Phinx/Dump.php
@@ -16,7 +16,7 @@
 use Cake\Database\Connection;
 use Cake\Datasource\ConnectionManager;
 use Migrations\ConfigurationTrait;
-use Migrations\TableFinderTrait;
+use Migrations\Util\TableFinder;
 use Phinx\Console\Command\AbstractCommand;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -31,7 +31,6 @@ class Dump extends AbstractCommand
 {
     use CommandTrait;
     use ConfigurationTrait;
-    use TableFinderTrait;
 
     /**
      * Output object.
@@ -96,7 +95,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             'require-table' => false,
             'plugin' => $this->getPlugin($input),
         ];
-        $tables = $this->getTablesToBake($collection, $options);
+        $finder = new TableFinder($connectionName);
+        $tables = $finder->getTablesToBake($collection, $options);
 
         $dump = [];
         if ($tables) {

diff --git a/src/Command/RollbackCommand.php b/src/Command/RollbackCommand.php
@@ -141,7 +141,7 @@ protected function executeMigrations(Arguments $args, ConsoleIo $io): ?int
 
         $versionOrder = $config->getVersionOrder();
         $io->out('<info>using connection</info> ' . (string)$args->getOption('connection'));
-        $io->out('<info>using paths</info> ' . implode(', ', $config->getMigrationPaths()));
+        $io->out('<info>using paths</info> ' . $config->getMigrationPath());
         $io->out('<info>ordering by</info> ' . $versionOrder . ' time');
 
         if ($dryRun) {

diff --git a/src/Command/SeedCommand.php b/src/Command/SeedCommand.php
@@ -115,7 +115,7 @@ protected function executeSeeds(Arguments $args, ConsoleIo $io): ?int
 
         $versionOrder = $config->getVersionOrder();
         $io->out('<info>using connection</info> ' . (string)$args->getOption('connection'));
-        $io->out('<info>using paths</info> ' . implode(', ', $config->getMigrationPaths()));
+        $io->out('<info>using paths</info> ' . $config->getMigrationPath());
         $io->out('<info>ordering by</info> ' . $versionOrder . ' time');
 
         $start = microtime(true);

diff --git a/src/Config/Config.php b/src/Config/Config.php
@@ -10,7 +10,6 @@
 
 use Closure;
 use InvalidArgumentException;
-use Psr\Container\ContainerInterface;
 use ReturnTypeWillChange;
 use UnexpectedValueException;
 
@@ -50,22 +49,26 @@ public function __construct(array $configArray)
      */
     public function getEnvironment(): ?array
     {
-        // TODO evolve this into connection only.
-        return $this->values['environment'] ?? null;
+        if (empty($this->values['environment'])) {
+            return null;
+        }
+        $config = (array)$this->values['environment'];
+        $config['version_order'] = $this->getVersionOrder();
+
+        return $config;
     }
 
     /**
      * @inheritDoc
      * @throws \UnexpectedValueException
      */
-    public function getMigrationPaths(): array
+    public function getMigrationPath(): string
     {
         if (!isset($this->values['paths']['migrations'])) {
             throw new UnexpectedValueException('Migrations path missing from config file');
         }
-
-        if (is_string($this->values['paths']['migrations'])) {
-            $this->values['paths']['migrations'] = [$this->values['paths']['migrations']];
+        if (is_array($this->values['paths']['migrations']) && isset($this->values['paths']['migrations'][0])) {
+            return $this->values['paths']['migrations'][0];
         }
 
         return $this->values['paths']['migrations'];
@@ -75,14 +78,13 @@ public function getMigrationPaths(): array
      * @inheritDoc
      * @throws \UnexpectedValueException
      */
-    public function getSeedPaths(): array
+    public function getSeedPath(): string
     {
         if (!isset($this->values['paths']['seeds'])) {
             throw new UnexpectedValueException('Seeds path missing from config file');
         }
-
-        if (is_string($this->values['paths']['seeds'])) {
-            $this->values['paths']['seeds'] = [$this->values['paths']['seeds']];
+        if (is_array($this->values['paths']['seeds']) && isset($this->values['paths']['seeds'][0])) {
+            return $this->values['paths']['seeds'][0];
         }
 
         return $this->values['paths']['seeds'];
@@ -93,6 +95,7 @@ public function getSeedPaths(): array
      */
     public function getMigrationBaseClassName(bool $dropNamespace = true): string
     {
+        /** @var string $className */
         $className = !isset($this->values['migration_base_class']) ? 'Phinx\Migration\AbstractMigration' : $this->values['migration_base_class'];
 
         return $dropNamespace ? (substr((string)strrchr($className, '\\'), 1) ?: $className) : $className;
@@ -103,6 +106,7 @@ public function getMigrationBaseClassName(bool $dropNamespace = true): string
      */
     public function getSeedBaseClassName(bool $dropNamespace = true): string
     {
+        /** @var string $className */
         $className = !isset($this->values['seed_base_class']) ? 'Phinx\Seed\AbstractSeed' : $this->values['seed_base_class'];
 
         return $dropNamespace ? substr((string)strrchr($className, '\\'), 1) : $className;
@@ -152,18 +156,6 @@ public function getTemplateStyle(): string
         return $this->values['templates']['style'] === self::TEMPLATE_STYLE_UP_DOWN ? self::TEMPLATE_STYLE_UP_DOWN : self::TEMPLATE_STYLE_CHANGE;
     }
 
-    /**
-     * @inheritDoc
-     */
-    public function getContainer(): ?ContainerInterface
-    {
-        if (!isset($this->values['container'])) {
-            return null;
-        }
-
-        return $this->values['container'];
-    }
-
     /**
      * @inheritdoc
      */

diff --git a/src/Config/ConfigInterface.php b/src/Config/ConfigInterface.php
@@ -9,7 +9,6 @@
 namespace Migrations\Config;
 
 use ArrayAccess;
-use Psr\Container\ContainerInterface;
 
 /**
  * Phinx configuration interface.
@@ -32,18 +31,18 @@ interface ConfigInterface extends ArrayAccess
     public function getEnvironment(): ?array;
 
     /**
-     * Gets the paths to search for migration files.
+     * Gets the path to search for migration files.
      *
-     * @return string[]
+     * @return string
      */
-    public function getMigrationPaths(): array;
+    public function getMigrationPath(): string;
 
     /**
-     * Gets the paths to search for seed files.
+     * Gets the path to search for seed files.
      *
-     * @return string[]
+     * @return string
      */
-    public function getSeedPaths(): array;
+    public function getSeedPath(): string;
 
     /**
      * Get the connection namee
@@ -73,13 +72,6 @@ public function getTemplateClass(): string|false;
      */
     public function getTemplateStyle(): string;
 
-    /**
-     * Get the user-provided container for instantiating seeds
-     *
-     * @return \Psr\Container\ContainerInterface|null
-     */
-    public function getContainer(): ?ContainerInterface;
-
     /**
      * Get the version order.
      *

diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php
@@ -134,8 +134,6 @@ public function setOptions(array $options): AdapterInterface
      */
     public function setConnection(Connection $connection): AdapterInterface
     {
-        // TODO how do PDO connection flags get set? Phinx used to
-        // turn on exception error mode, and I don't think Cake does that by default.
         $this->connection = $connection;
 
         // Create the schema table if it doesn't already exist

diff --git a/src/Db/Literal.php b/src/Db/Literal.php
@@ -8,7 +8,10 @@
 
 namespace Migrations\Db;
 
-// TODO replace/merge with cakephp/database
+/**
+ * Represent a value that should be used as a literal value when being
+ * interpolated into SQL commands.
+ */
 class Literal
 {
     /**

diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php
@@ -197,7 +197,7 @@ public function markMigrated(int|string|null $version = null, array $options = [
 
         $manager = $this->getManager($options);
         $config = $manager->getConfig();
-        $path = $config->getMigrationPaths()[0];
+        $path = $config->getMigrationPath();
 
         $versions = $manager->getVersionsToMark($args);
         $manager->markVersionsAsMigrated($path, $versions);

diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php
@@ -708,7 +708,7 @@ public function getEnvironment(): Environment
         $config = $this->getConfig();
         // create an environment instance and cache it
         $envOptions = $config->getEnvironment();
-        $envOptions['version_order'] = $config->getVersionOrder();
+        assert(is_array($envOptions));
 
         $environment = new Environment('default', $envOptions);
         $environment->setIo($this->getIo());
@@ -888,7 +888,7 @@ function ($phpFile) {
      */
     protected function getMigrationFiles(): array
     {
-        return Util::getFiles($this->getConfig()->getMigrationPaths());
+        return Util::getFiles($this->getConfig()->getMigrationPath());
     }
 
     /**
@@ -1040,7 +1040,7 @@ public function getSeeds(): array
      */
     protected function getSeedFiles(): array
     {
-        return Util::getFiles($this->getConfig()->getSeedPaths());
+        return Util::getFiles($this->getConfig()->getSeedPath());
     }
 
     /**

diff --git a/src/Migration/ManagerFactory.php b/src/Migration/ManagerFactory.php
@@ -110,7 +110,6 @@ public function createConfig(): ConfigInterface
 
         $configData = [
             'paths' => [
-                // TODO make paths a simple list.
                 'migrations' => $dir,
                 'seeds' => $dir,
             ],
@@ -125,7 +124,6 @@ public function createConfig(): ConfigInterface
                 'unsigned_primary_keys' => Configure::read('Migrations.unsigned_primary_keys'),
                 'column_null_default' => Configure::read('Migrations.column_null_default'),
             ],
-            // TODO do we want to support the DI container in migrations?
         ];
 
         return new Config($configData);

diff --git a/src/Migration/PhinxBackend.php b/src/Migration/PhinxBackend.php
@@ -146,9 +146,6 @@ public function getCommand(): string
      */
     public function status(array $options = []): array
     {
-        // TODO This class could become an interface that chooses between a phinx and builtin
-        // implementation. Having two implementations would be easier to cleanup
-        // than having all the logic in one class with branching
         $input = $this->getInput('Status', [], $options);
         $params = ['default', $input->getOption('format')];