Skip to content

Commit

Permalink
yiisoft#14543: Adjusted implementation of migration name length limit
Browse files Browse the repository at this point in the history
  • Loading branch information
cebe authored and samdark committed Oct 26, 2017
1 parent 336404e commit e311001
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 17 deletions.
2 changes: 1 addition & 1 deletion framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Yii Framework 2 Change Log
2.0.13 under development
------------------------

- Bug #14543: Throw exception when trying to create migration longer than 180 symbols (dmirogin)
- Bug #14543: Throw exception when trying to create migration longer than 180 symbols (dmirogin, cebe)
- Enh #14877: Disabled profiling on connection opening when profiling is disabled (njasm)
- Enh #14967: Added Armenian Translations (gevorgmansuryan)
- Enh #4479: Implemented REST filters (klimov-paul)
Expand Down
29 changes: 19 additions & 10 deletions framework/console/controllers/BaseMigrateController.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ abstract class BaseMigrateController extends Controller
*/
const BASE_MIGRATION = 'm000000_000000_base';

/**
* Maximum length of migration name
*/
const MAX_NAME_LENGTH = 180;

/**
* @var string the default command action.
Expand Down Expand Up @@ -189,18 +185,18 @@ public function actionUp($limit = 0)
}

foreach ($migrations as $migration) {
$nameLimit = $this->getMigrationNameLimit();
if ($nameLimit !== null && strlen($migration) > $nameLimit) {
$this->stdout("\nThe migration name '$migration' is too long. Its not possible to apply this migration.\n", Console::FG_RED);
return ExitCode::UNSPECIFIED_ERROR;
}
$this->stdout("\t$migration\n");
}
$this->stdout("\n");

$applied = 0;
if ($this->confirm('Apply the above ' . ($n === 1 ? 'migration' : 'migrations') . '?')) {
foreach ($migrations as $migration) {
if (strlen($migration) > static::MAX_NAME_LENGTH) {
$this->stdout("\nThe migration name is too long. The rest of the migrations are canceled.\n", Console::FG_RED);
return ExitCode::UNSPECIFIED_ERROR;
}

if (!$this->migrateUp($migration)) {
$this->stdout("\n$applied from $n " . ($applied === 1 ? 'migration was' : 'migrations were') . " applied.\n", Console::FG_RED);
$this->stdout("\nMigration failed. The rest of the migrations are canceled.\n", Console::FG_RED);
Expand Down Expand Up @@ -633,7 +629,8 @@ public function actionCreate($name)

list($namespace, $className) = $this->generateClassName($name);
// Abort if name is too long
if (strlen($className) > static::MAX_NAME_LENGTH) {
$nameLimit = $this->getMigrationNameLimit();
if ($nameLimit !== null && strlen($className) > $nameLimit) {
throw new Exception('The migration name is too long.');
}

Expand Down Expand Up @@ -949,6 +946,18 @@ protected function truncateDatabase()
throw new NotSupportedException('This command is not implemented in ' . get_class($this));
}

/**
* Return the maximum name length for a migration.
*
* Subclasses may override this method to define a limit.
* @return int|null the maximum name length for a migration or `null` if no limit applies.
* @since 2.0.13
*/
protected function getMigrationNameLimit()
{
return null;
}

/**
* Returns the migration history.
* @param int $limit the maximum number of records in the history to be returned. `null` for "no limit".
Expand Down
33 changes: 28 additions & 5 deletions framework/console/controllers/MigrateController.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@
*/
class MigrateController extends BaseMigrateController
{
/**
* Maximum length of a migration name.
* @since 2.0.13
*/
const MAX_NAME_LENGTH = 180;


/**
* @var string the name of the table for keeping applied migration information.
*/
Expand Down Expand Up @@ -167,10 +174,7 @@ public function optionAliases()
public function beforeAction($action)
{
if (parent::beforeAction($action)) {
if ($action->id !== 'create') {
$this->db = Instance::ensure($this->db, Connection::className());
}

$this->db = Instance::ensure($this->db, Connection::className());
return true;
}

Expand Down Expand Up @@ -258,7 +262,7 @@ protected function createMigrationHistoryTable()
$tableName = $this->db->schema->getRawTableName($this->migrationTable);
$this->stdout("Creating migration history table \"$tableName\"...", Console::FG_YELLOW);
$this->db->createCommand()->createTable($this->migrationTable, [
'version' => 'varchar(180) NOT NULL PRIMARY KEY',
'version' => 'varchar(' . static::MAX_NAME_LENGTH . ') NOT NULL PRIMARY KEY',
'apply_time' => 'integer',
])->execute();
$this->db->createCommand()->insert($this->migrationTable, [
Expand Down Expand Up @@ -317,6 +321,25 @@ protected function removeMigrationHistory($version)
])->execute();
}

private $_migrationNameLimit;

/**
* @inheritdoc
* @since 2.0.13
*/
protected function getMigrationNameLimit()
{
if ($this->_migrationNameLimit !== null) {
return $this->_migrationNameLimit;
}
$tableSchema = $this->db->schema ? $this->db->schema->getTableSchema($this->migrationTable, true) : null;
if ($tableSchema !== null) {
return $this->_migrationNameLimit = $tableSchema->columns['version']->size;
}

return static::MAX_NAME_LENGTH;
}

/**
* @inheritdoc
* @since 2.0.8
Expand Down
18 changes: 17 additions & 1 deletion tests/framework/console/controllers/MigrateControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,23 @@ public function testUpdatingLongNamedMigration()

$result = $this->runMigrateControllerAction('up');

$this->assertContains('The migration name is too long. The rest of the migrations are canceled.', $result);
$this->assertContains('The migration name', $result);
$this->assertContains('is too long. Its not possible to apply this migration.', $result);
}

public function testNamedMigrationWithCustomLimit()
{
Yii::$app->db->createCommand()->createTable('migration', [
'version' => 'varchar(255) NOT NULL PRIMARY KEY', // varchar(255) is longer than the default of 180
'apply_time' => 'integer',
])->execute();

$this->createMigration(str_repeat('a', 180));

$result = $this->runMigrateControllerAction('up');

$this->assertContains('1 migration was applied.', $result);
$this->assertContains('Migrated up successfully.', $result);
}

public function testCreateLongNamedMigration()
Expand Down

0 comments on commit e311001

Please sign in to comment.