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

Cleanup #713

Merged
merged 8 commits into from
Apr 29, 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
6 changes: 0 additions & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
6 changes: 3 additions & 3 deletions src/Command/BakeMigrationSnapshotCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -33,7 +33,6 @@
class BakeMigrationSnapshotCommand extends BakeSimpleMigrationCommand
{
use SnapshotTrait;
use TableFinderTrait;
use UtilTrait;

/**
Expand Down Expand Up @@ -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);

Expand Down
9 changes: 4 additions & 5 deletions src/Command/DumpCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,8 +30,6 @@
*/
class DumpCommand extends Command
{
use TableFinderTrait;

protected string $connection;

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions src/Command/MarkMigratedCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/Command/MigrateCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions src/Command/Phinx/Dump.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,7 +31,6 @@ class Dump extends AbstractCommand
{
use CommandTrait;
use ConfigurationTrait;
use TableFinderTrait;

/**
* Output object.
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/Command/RollbackCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/Command/SeedCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 15 additions & 23 deletions src/Config/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

use Closure;
use InvalidArgumentException;
use Psr\Container\ContainerInterface;
use ReturnTypeWillChange;
use UnexpectedValueException;

Expand Down Expand Up @@ -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'];
Expand All @@ -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])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of this being an array if it's hard-coded to index 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to keep compatibility with all the tests that provide a list of paths.

return $this->values['paths']['seeds'][0];
}

return $this->values['paths']['seeds'];
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down
20 changes: 6 additions & 14 deletions src/Config/ConfigInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
namespace Migrations\Config;

use ArrayAccess;
use Psr\Container\ContainerInterface;

/**
* Phinx configuration interface.
Expand All @@ -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
Expand Down Expand Up @@ -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.
*
Expand Down
2 changes: 0 additions & 2 deletions src/Db/Adapter/PdoAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/Db/Literal.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
/**
Expand Down
2 changes: 1 addition & 1 deletion src/Migration/BuiltinBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/Migration/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -888,7 +888,7 @@ function ($phpFile) {
*/
protected function getMigrationFiles(): array
{
return Util::getFiles($this->getConfig()->getMigrationPaths());
return Util::getFiles($this->getConfig()->getMigrationPath());
}

/**
Expand Down Expand Up @@ -1040,7 +1040,7 @@ public function getSeeds(): array
*/
protected function getSeedFiles(): array
{
return Util::getFiles($this->getConfig()->getSeedPaths());
return Util::getFiles($this->getConfig()->getSeedPath());
}

/**
Expand Down
2 changes: 0 additions & 2 deletions src/Migration/ManagerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ public function createConfig(): ConfigInterface

$configData = [
'paths' => [
// TODO make paths a simple list.
'migrations' => $dir,
'seeds' => $dir,
],
Expand All @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/Migration/PhinxBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')];

Expand Down
Loading
Loading