Skip to content

Commit

Permalink
Some refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@neeckeloo @mikeSimonson
neeckeloo authored and mikeSimonson committed Jun 28, 2015
1 parent 8ac9787 commit 2daede7
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 42 deletions.
12 changes: 8 additions & 4 deletions lib/Doctrine/DBAL/Migrations/Configuration/Configuration.php
View file
Expand Up @@ -70,7 +70,7 @@ class Configuration
private $outputWriter;

/**
* The migration finder implementation -- used to load migrations from a
* The migration finder implementation -- used to load migrations from a
* directory.
*
* @var MigrationFinderInterface
Expand Down Expand Up @@ -108,8 +108,9 @@ class Configuration
/**
* Construct a migration configuration object.
*
* @param Connection $connection A Connection instance
* @param OutputWriter $outputWriter A OutputWriter instance
* @param Connection $connection A Connection instance
* @param OutputWriter $outputWriter A OutputWriter instance
* @param MigrationFinderInterface $finder Migration files finder
*/
public function __construct(Connection $connection, OutputWriter $outputWriter = null, MigrationFinderInterface $finder = null)
{
Expand Down Expand Up @@ -387,7 +388,10 @@ public function hasVersionMigrated(Version $version)
{
$this->createMigrationTable();

$version = $this->connection->fetchColumn("SELECT version FROM " . $this->migrationsTableName . " WHERE version = ?", array($version->getVersion()));
$version = $this->connection->fetchColumn(
"SELECT version FROM " . $this->migrationsTableName . " WHERE version = ?",
array($version->getVersion())
);

return $version !== false;
}
Expand Down
38 changes: 23 additions & 15 deletions lib/Doctrine/DBAL/Migrations/Tools/Console/Command/MigrateCommand.php
View file
Expand Up @@ -98,9 +98,18 @@ public function execute(InputInterface $input, OutputInterface $output)
}

if (!empty($executedUnavailableMigrations)) {
$output->writeln(sprintf('<error>WARNING! You have %s previously executed migrations in the database that are not registered migrations.</error>', count($executedUnavailableMigrations)));
$output->writeln(sprintf(
'<error>WARNING! You have %s previously executed migrations'
. ' in the database that are not registered migrations.</error>',
count($executedUnavailableMigrations)
));

foreach ($executedUnavailableMigrations as $executedUnavailableMigration) {
$output->writeln(' <comment>>></comment> ' . $configuration->getDateTime($executedUnavailableMigration) . ' (<comment>' . $executedUnavailableMigration . '</comment>)');
$output->writeln(sprintf(
' <comment>>></comment> %s (<comment>%s</comment>)',
$configuration->getDateTime($executedUnavailableMigration),
$executedUnavailableMigration
));
}

$question = 'Are you sure you wish to continue? (y/n)';
Expand All @@ -119,9 +128,10 @@ public function execute(InputInterface $input, OutputInterface $output)

// warn the user if no dry run and interaction is on
if (! $dryRun) {
$question = 'WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)';
if (! $this->canExecute($question, $input, $output))
{
$question = 'WARNING! You are about to execute a database migration'
. ' that could result in schema changes and data lost.'
. ' Are you sure you wish to continue? (y/n)';
if (! $this->canExecute($question, $input, $output)) {
$output->writeln('<error>Migration cancelled!</error>');

return 1;
Expand All @@ -137,27 +147,22 @@ public function execute(InputInterface $input, OutputInterface $output)
}

/**
* @param $question
* @param string $question
* @param InputInterface $input
* @param OutputInterface $output
* @return bool
*/
private function canExecute($question, InputInterface $input, OutputInterface $output)
{
if ($input->isInteractive()) {
if (! $this->askConfirmation($question, $input, $output)) {

return false;
}

return true;
if ($input->isInteractive() && ! $this->askConfirmation($question, $input, $output)) {
return false;
}

return true;
}

/**
* @param $versionAlias
* @param string $versionAlias
* @param OutputInterface $output
* @param Configuration $configuration
* @return bool|string
Expand All @@ -175,7 +180,10 @@ private function getVersionNameFromAlias($versionAlias, OutputInterface $output,
return false;
}

$output->writeln('<error>Unknown version: ' . $output->getFormatter()->escape($versionAlias) . '</error>');
$output->writeln(sprintf(
'<error>Unknown version: %s</error>',
$output->getFormatter()->escape($versionAlias)
));
return false;
}

Expand Down
28 changes: 19 additions & 9 deletions lib/Doctrine/DBAL/Migrations/Version.php
View file
Expand Up @@ -147,13 +147,19 @@ public function isMigrated()
public function markMigrated()
{
$this->configuration->createMigrationTable();
$this->connection->executeQuery("INSERT INTO " . $this->configuration->getMigrationsTableName() . " (version) VALUES (?)", array($this->version));
$this->connection->insert(
$this->configuration->getMigrationsTableName(),
array('version' => $this->version)
);
}

public function markNotMigrated()
{
$this->configuration->createMigrationTable();
$this->connection->executeQuery("DELETE FROM " . $this->configuration->getMigrationsTableName() . " WHERE version = ?", array($this->version));
$this->connection->delete(
$this->configuration->getMigrationsTableName(),
array('version' => $this->version)
);
}

/**
Expand All @@ -176,8 +182,9 @@ public function addSql($sql, array $params = array(), array $types = array())
} else {
$this->sql[] = $sql;
if (!empty($params)) {
$this->params[count($this->sql) - 1] = $params;
$this->types[count($this->sql) - 1] = $types ?: array();
$index = count($this->sql) - 1;
$this->params[$index] = $params;
$this->types[$index] = $types;
}
}
}
Expand Down Expand Up @@ -230,7 +237,7 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false)
$this->sql = array();

$transaction = $this->migration->isTransactional();
if($transaction){
if ($transaction) {
//only start transaction if in transactional mode
$this->connection->beginTransaction();
}
Expand Down Expand Up @@ -270,7 +277,10 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false)
$this->outputQueryTime($queryStart, $timeAllQueries);
}
} else {
$this->outputWriter->write(sprintf('<error>Migration %s was executed but did not result in any SQL statements.</error>', $this->version));
$this->outputWriter->write(sprintf(
'<error>Migration %s was executed but did not result in any SQL statements.</error>',
$this->version
));
}

if ($direction === 'up') {
Expand All @@ -296,7 +306,7 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false)
$this->outputWriter->write(sprintf("\n <info>--</info> reverted (%ss)", $this->time));
}

if($transaction){
if ($transaction) {
//commit only if running in transactional mode
$this->connection->commit();
}
Expand All @@ -305,7 +315,7 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false)

return $this->sql;
} catch (SkipMigrationException $e) {
if($transaction){
if ($transaction) {
//only rollback transaction if in transactional mode
$this->connection->rollback();
}
Expand All @@ -331,7 +341,7 @@ public function execute($direction, $dryRun = false, $timeAllQueries = false)
$this->version, $this->getExecutionState(), $e->getMessage()
));

if($transaction){
if ($transaction) {
//only rollback transaction if in transactional mode
$this->connection->rollback();
}
Expand Down
59 changes: 55 additions & 4 deletions tests/Doctrine/DBAL/Migrations/Tests/ConfigurationTest.php
View file
Expand Up @@ -19,7 +19,10 @@ public function testValidateMigrationsNamespaceRequired()
{
$config = new Configuration($this->getSqliteConnection());

$this->setExpectedException("Doctrine\DBAL\Migrations\MigrationException", "Migrations namespace must be configured in order to use Doctrine migrations.");
$this->setExpectedException(
"Doctrine\DBAL\Migrations\MigrationException",
"Migrations namespace must be configured in order to use Doctrine migrations."
);
$config->validate();
}

Expand All @@ -28,7 +31,10 @@ public function testValidateMigrationsDirectoryRequired()
$config = new Configuration($this->getSqliteConnection());
$config->setMigrationsNamespace("DoctrineMigrations\\");

$this->setExpectedException("Doctrine\DBAL\Migrations\MigrationException", "Migrations directory must be configured in order to use Doctrine migrations.");
$this->setExpectedException(
"Doctrine\DBAL\Migrations\MigrationException",
"Migrations directory must be configured in order to use Doctrine migrations."
);
$config->validate();
}

Expand Down Expand Up @@ -72,7 +78,10 @@ public function testGetUnknownVersion()
{
$config = $this->getSqliteConfiguration();

$this->setExpectedException('Doctrine\DBAL\Migrations\MigrationException', 'Could not find migration version 1234');
$this->setExpectedException(
'Doctrine\DBAL\Migrations\MigrationException',
'Could not find migration version 1234'
);
$config->getVersion(1234);
}

Expand Down Expand Up @@ -113,7 +122,10 @@ public function testRegisterDuplicateVersion()

$config->registerMigration(1234, 'Doctrine\DBAL\Migrations\Tests\Stub\Version1Test');

$this->setExpectedException('Doctrine\DBAL\Migrations\MigrationException', 'Migration version 1234 already registered with class Doctrine\DBAL\Migrations\Version');
$this->setExpectedException(
'Doctrine\DBAL\Migrations\MigrationException',
'Migration version 1234 already registered with class Doctrine\DBAL\Migrations\Version'
);
$config->registerMigration(1234, 'Doctrine\DBAL\Migrations\Tests\Stub\Version1Test');
}

Expand Down Expand Up @@ -202,3 +214,42 @@ public function versionProvider()
];
}
}

class ConfigMigration extends \Doctrine\DBAL\Migrations\AbstractMigration
{
public function down(Schema $schema)
{

}

public function up(Schema $schema)
{

}
}

class Config2Migration extends \Doctrine\DBAL\Migrations\AbstractMigration
{
public function down(Schema $schema)
{

}

public function up(Schema $schema)
{

}
}

class Config3Migration extends \Doctrine\DBAL\Migrations\AbstractMigration
{
public function down(Schema $schema)
{

}

public function up(Schema $schema)
{

}
}
7 changes: 5 additions & 2 deletions tests/Doctrine/DBAL/Migrations/Tests/MigrationTest.php
View file
Expand Up @@ -43,7 +43,10 @@ public function testMigrateToUnknownVersionThrowsException()
{
$migration = new Migration($this->config);

$this->setExpectedException('Doctrine\DBAL\Migrations\MigrationException', 'Could not find migration version 1234');
$this->setExpectedException(
'Doctrine\DBAL\Migrations\MigrationException',
'Could not find migration version 1234'
);
$migration->migrate('1234');
}

Expand All @@ -55,7 +58,7 @@ public function testMigrateWithNoMigrationsThrowsException()
{
$migration = new Migration($this->config);

$sql = $migration->migrate();
$migration->migrate();
}

/**
Expand Down
9 changes: 5 additions & 4 deletions tests/Doctrine/DBAL/Migrations/Tests/Tools/MigrationsVersionTest.php
View file
Expand Up @@ -7,7 +7,8 @@ class MigrationsVersionTest extends \PHPUnit_Framework_TestCase {

private $MigrationVersionClass = 'Doctrine\DBAL\Migrations\MigrationsVersion';

public function testVersionNumber() {
public function testVersionNumber()
{
$class = new \ReflectionClass($this->MigrationVersionClass);
$property = $class->getProperty('version');
$property->setAccessible(true);
Expand All @@ -17,13 +18,13 @@ public function testVersionNumber() {
$this->assertEquals($versionNumber, MigrationsVersion::VERSION());
}

public function testIsACustomPharBuild(){
public function testIsACustomPharBuild()
{
$class = new \ReflectionClass($this->MigrationVersionClass);
$method = $class->getMethod('isACustomPharBuild');
$method->setAccessible(true);

$this->assertFalse($method->invokeArgs(new MigrationsVersion(), array('@git-version@')), 'This is not a custom phar build.');
$this->assertTrue($method->invokeArgs(new MigrationsVersion(), array('v1.0.0-alpha3-125435')), 'This has been replaced by box and is thus a phar build.');
}

}
}
17 changes: 13 additions & 4 deletions tests/Doctrine/DBAL/Migrations/Tests/VersionTest.php
View file
Expand Up @@ -36,8 +36,13 @@ class VersionTest extends MigrationTestCase
*/
public function testCreateVersion()
{
$version = new Version(new Configuration($this->getSqliteConnection()), $versionName = '003',
'Doctrine\DBAL\Migrations\Tests\VersionTest_Migration');
$versionName = '003';
$version = new Version(
new Configuration($this->getSqliteConnection()),
$versionName,
'Doctrine\DBAL\Migrations\Tests\VersionTest_Migration'
);

$this->assertEquals($versionName, $version->getVersion());
}

Expand All @@ -48,8 +53,12 @@ public function testCreateVersionWithCustomName()
{
$versionName = '003';
$versionDescription = 'My super migration';
$version = new Version(new Configuration($this->getSqliteConnection()), $versionName,
'Doctrine\DBAL\Migrations\Tests\VersionTest_MigrationDescription');
$version = new Version(
new Configuration($this->getSqliteConnection()),
$versionName,
'Doctrine\DBAL\Migrations\Tests\VersionTest_MigrationDescription'
);

$this->assertEquals($versionName, $version->getVersion());
$this->assertEquals($versionDescription, $version->getMigration()->getDescription());
}
Expand Down

0 comments on commit 2daede7

Please sign in to comment.