From d4d01f5c07314f5bc6489dc6ba010b945159919d Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sat, 28 Sep 2024 11:35:30 +0530 Subject: [PATCH 01/26] Create PR --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 1cd2090b..3bb79307 100644 --- a/README.md +++ b/README.md @@ -568,3 +568,4 @@ Professional support, consulting as well as software development services are av https://www.cebe.cc/en/contact Development of this library is sponsored by [cebe.:cloud: "Your Professional Deployment Platform"](https://cebe.cloud). + From 290b7be3ad8e1ce47ff83a5a88ace08ee872706e Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sat, 28 Sep 2024 13:15:37 +0530 Subject: [PATCH 02/26] Add a failing test --- .../index.php | 13 ++++++++ .../index.yml | 27 ++++++++++++++++ tests/unit/IssueFixTest.php | 32 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php create mode 100644 tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php new file mode 100644 index 00000000..8ec860a4 --- /dev/null +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php @@ -0,0 +1,13 @@ + '@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml', + 'generateUrls' => false, + 'generateModels' => true, + 'excludeModels' => [ + 'Error', + ], + 'generateControllers' => false, + 'generateMigrations' => true, + 'generateModelFaker' => true, // `generateModels` must be `true` in order to use `generateModelFaker` as `true` +]; diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml new file mode 100644 index 00000000..62612c44 --- /dev/null +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml @@ -0,0 +1,27 @@ +openapi: 3.0.3 + +info: + title: 'Create migration for column position change if a field position is changed in spec #58' + version: 1.0.0 + +components: + schemas: + Fruit: + type: object + properties: + id: + type: integer + name: + type: string + description: desc + # colour: + # type: string + description: + type: string + +paths: + '/': + get: + responses: + '200': + description: OK diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index b6c7abdb..341668ca 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -360,4 +360,36 @@ public function test158BugGiiapiGeneratedRulesEnumWithTrim() ]); $this->checkFiles($actualFiles, $expectedFiles); } + + // https://github.com/php-openapi/yii2-openapi/issues/58 + public function test58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + { + $this->deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); + $this->createTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); + + $testFile = Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php"); + $this->runGenerator($testFile); + // $actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [ + // 'recursive' => true, + // ]); + // $expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql"), [ + // 'recursive' => true, + // ]); + // $this->checkFiles($actualFiles, $expectedFiles); + $this->deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); + } + + private function createTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + { + Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ + 'id' => 'pk', + 'description' => 'text', + 'name' => 'varchar(255) COMMENT "some-comment"', + ])->execute(); + } + + private function deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + { + Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); + } } From 3718dc63519f4627d0b3569913b2191929346df3 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sat, 28 Sep 2024 13:15:47 +0530 Subject: [PATCH 03/26] Cleanup --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 3bb79307..1cd2090b 100644 --- a/README.md +++ b/README.md @@ -568,4 +568,3 @@ Professional support, consulting as well as software development services are av https://www.cebe.cc/en/contact Development of this library is sponsored by [cebe.:cloud: "Your Professional Deployment Platform"](https://cebe.cloud). - From 79bc270c2022adc2f49bd445e980bf8173ead558 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 3 Oct 2024 22:11:21 +0530 Subject: [PATCH 04/26] Fix issue: case: drop 2 column: its position in add column in down code in migration --- src/db/ColumnSchema.php | 7 ++++ src/lib/migrations/BaseMigrationBuilder.php | 39 +++++++++++++++---- src/lib/migrations/MigrationRecordBuilder.php | 16 ++++++-- src/lib/migrations/MysqlMigrationBuilder.php | 15 ++++++- .../index.yml | 14 +++---- tests/unit/IssueFixTest.php | 2 +- 6 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/db/ColumnSchema.php b/src/db/ColumnSchema.php index 71127a24..c6725b10 100644 --- a/src/db/ColumnSchema.php +++ b/src/db/ColumnSchema.php @@ -25,4 +25,11 @@ class ColumnSchema extends \yii\db\ColumnSchema * ``` */ public $xDbType; + + /** + * TODO + * Used only for MySQL/MariaDB + * @var string|null + */ + public ?string $position = null; } diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index d4c49c21..ff0c49fe 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -220,6 +220,9 @@ function (string $unknownColumn) { } else { $this->buildRelations(); } + +// $this->handleColumnsPositionsChanges($haveNames, $wantNames); + return $this->migration; } @@ -249,12 +252,12 @@ protected function buildColumnsDrop(array $columns):void { foreach ($columns as $column) { $tableName = $this->model->getTableAlias(); + $position = $this->findPosition($column, true); if ($column->isPrimaryKey && !$column->autoIncrement) { $pkName = 'pk_' . $this->model->tableName . '_' . $column->name; $this->migration->addDownCode($this->recordBuilder->addPrimaryKey($tableName, [$column->name], $pkName)) ->addUpCode($this->recordBuilder->dropPrimaryKey($tableName, [$column->name], $pkName)); } - $position = $this->findPosition($column, true); $this->migration->addDownCode($this->recordBuilder->addDbColumn($tableName, $column, $position)) ->addUpCode($this->recordBuilder->dropColumn($tableName, $column->name)); } @@ -514,6 +517,8 @@ public function isDefaultValueChanged( } /** + * TODO move this method to MysqlMigrationBuilder + * Only for MySQL and MariaDB * Given a column, compute its previous column name present in OpenAPI schema * @return ?string * `null` if column is added at last @@ -528,24 +533,26 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?stri if ($key > 0) { $prevColName = $columnNames[$key-1]; - if (!isset($columnNames[$key+1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not + if (!$forDrop && !isset($columnNames[$key+1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not return null; } - // in case of `down()` code of migration, putting 'after ' in add column statmenet is erroneous because may not exist. + // in case of `down()` code of migration, putting 'after ' in add column statement is erroneous because may not exist. // Example: From col a, b, c, d, if I drop c and d then their migration code will be generated like: // `up()` code // drop c // drop d // `down()` code - // add d after c (c does not exist! Error!) - // add c + // add d after c (c does not exist! Error!) (TODO check if c is present in newColumn) + // add c after b (can fix this issue) TODO if ($forDrop) { - return null; +// return null; // TODO this case can be fixed } - - return self::POS_AFTER . ' ' . $prevColName; + if (array_key_exists($prevColName, $this->newColumns)) { + return self::POS_AFTER . ' ' . $prevColName; + } + return null; // if no `$columnSchema` is found, previous column does not exist. This happens when 'after column' is not yet added in migration or added after currently undertaken column } elseif ($key === 0) { @@ -574,4 +581,20 @@ public function modifyDesiredInContextOfDesiredFromDb(ColumnSchema $desired, Col } $desired->dbType = $desiredFromDb->dbType; } + + // TODO + public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) + { + $indices = []; + if ($haveNames !== $wantNames) { + foreach ($wantNames as $key => $name) { + if ($name !== $haveNames[$key]) { + $indices[] = $key; + } + } + } + for ($i = 0; $i < count($indices)/2; $i++) { + $this->migration->addUpCode($this->recordBuilder->alterColumn()); + } + } } diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 98ee03b8..6794d6d0 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -112,7 +112,7 @@ public function addDbColumn(string $tableAlias, ColumnSchema $column, ?string $p /** * @throws \yii\base\InvalidConfigException */ - public function alterColumn(string $tableAlias, ColumnSchema $column):string + public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string # TODO { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true); @@ -130,7 +130,12 @@ public function alterColumn(string $tableAlias, ColumnSchema $column):string /** * @throws \yii\base\InvalidConfigException */ - public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $addUsing = false):string + public function alterColumnType( + string $tableAlias, + ColumnSchema $column, + bool $addUsing = false, + ?string $position = null + ):string # TODO { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, false, false, true, true); @@ -149,7 +154,12 @@ public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $ * This method is only used in Pgsql * @throws \yii\base\InvalidConfigException */ - public function alterColumnTypeFromDb(string $tableAlias, ColumnSchema $column, bool $addUsing = false) :string + public function alterColumnTypeFromDb( + string $tableAlias, + ColumnSchema $column, + bool $addUsing = false, + ?string $position = null + ) :string # TODO { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true); diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 42354df3..9501bd46 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -26,14 +26,15 @@ final class MysqlMigrationBuilder extends BaseMigrationBuilder protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desired, array $changed):void { $newColumn = clone $current; + $position = $this->findPosition($desired); foreach ($changed as $attr) { $newColumn->$attr = $desired->$attr; } if (static::isEnum($newColumn)) { $newColumn->dbType = 'enum'; // TODO this is concretely not correct } - $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn)) - ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current)); + $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn, $position)) + ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current, $position)); } protected function compareColumns(ColumnSchema $current, ColumnSchema $desired):array @@ -159,4 +160,14 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch $desired->size = $current->size; } } + + /** + * TODO + * Check if order/position of column is changed + * @return void + */ + public function checkOrder() + { + + } } diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml index 62612c44..1b2fe2ff 100644 --- a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml @@ -11,13 +11,13 @@ components: properties: id: type: integer - name: - type: string - description: desc - # colour: - # type: string - description: - type: string +# name: # TODO +# type: string +# description: desc +# colour: +# type: string +# description: +# type: string paths: '/': diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 341668ca..2da7008b 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -384,7 +384,7 @@ private function createTableFor58CreateMigrationForColumnPositionChangeIfAFieldP Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ 'id' => 'pk', 'description' => 'text', - 'name' => 'varchar(255) COMMENT "some-comment"', + 'name' => 'text', ])->execute(); } From d5c2c2800380b78e5d96fc920b3ef62fc997e018 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 4 Oct 2024 17:26:52 +0530 Subject: [PATCH 05/26] Handle column position in case of dropping a column (in down code -> add column) (for one or more columns) + Fix failing tests --- .../m200000_000004_change_table_v2_users.php | 2 +- .../m200000_000005_change_table_v2_comments.php | 2 +- .../m200000_000004_change_table_v2_users.php | 2 +- .../m200000_000005_change_table_v2_comments.php | 2 +- .../m200000_000000_change_table_column_name_changes.php | 2 +- .../m200000_000000_change_table_column_name_changes.php | 2 +- .../m200000_000001_change_table_newcolumns.php | 2 +- .../m200000_000001_change_table_newcolumns.php | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/specs/blog_v2/migrations_maria_db/m200000_000004_change_table_v2_users.php b/tests/specs/blog_v2/migrations_maria_db/m200000_000004_change_table_v2_users.php index bf1c82fd..1231f6db 100644 --- a/tests/specs/blog_v2/migrations_maria_db/m200000_000004_change_table_v2_users.php +++ b/tests/specs/blog_v2/migrations_maria_db/m200000_000004_change_table_v2_users.php @@ -25,7 +25,7 @@ public function down() $this->alterColumn('{{%v2_users}}', 'created_at', $this->timestamp()->null()->defaultExpression("current_timestamp()")); $this->alterColumn('{{%v2_users}}', 'role', $this->string(20)->null()->defaultValue('reader')); $this->alterColumn('{{%v2_users}}', 'email', $this->string(200)->notNull()); - $this->addColumn('{{%v2_users}}', 'username', $this->string(200)->notNull()); + $this->addColumn('{{%v2_users}}', 'username', $this->string(200)->notNull()->after('id')); $this->dropColumn('{{%v2_users}}', 'login'); } } diff --git a/tests/specs/blog_v2/migrations_maria_db/m200000_000005_change_table_v2_comments.php b/tests/specs/blog_v2/migrations_maria_db/m200000_000005_change_table_v2_comments.php index 75553729..ead3128c 100644 --- a/tests/specs/blog_v2/migrations_maria_db/m200000_000005_change_table_v2_comments.php +++ b/tests/specs/blog_v2/migrations_maria_db/m200000_000005_change_table_v2_comments.php @@ -25,7 +25,7 @@ public function down() $this->alterColumn('{{%v2_comments}}', 'created_at', $this->integer(11)->notNull()); $this->alterColumn('{{%v2_comments}}', 'meta_data', 'json NOT NULL DEFAULT \'[]\''); $this->alterColumn('{{%v2_comments}}', 'message', 'json NOT NULL DEFAULT \'[]\''); - $this->addColumn('{{%v2_comments}}', 'author_id', $this->integer(11)->notNull()); + $this->addColumn('{{%v2_comments}}', 'author_id', $this->integer(11)->notNull()->after('post_id')); $this->dropColumn('{{%v2_comments}}', 'user_id'); $this->addForeignKey('fk_v2_comments_author_id_v2_users_id', '{{%v2_comments}}', 'id', 'v2_users', 'author_id'); $this->addForeignKey('fk_v2_comments_post_id_v2_posts_uid', '{{%v2_comments}}', 'uid', 'v2_posts', 'post_id'); diff --git a/tests/specs/blog_v2/migrations_mysql_db/m200000_000004_change_table_v2_users.php b/tests/specs/blog_v2/migrations_mysql_db/m200000_000004_change_table_v2_users.php index b7a1f5e3..412d6ade 100644 --- a/tests/specs/blog_v2/migrations_mysql_db/m200000_000004_change_table_v2_users.php +++ b/tests/specs/blog_v2/migrations_mysql_db/m200000_000004_change_table_v2_users.php @@ -25,7 +25,7 @@ public function down() $this->alterColumn('{{%v2_users}}', 'created_at', $this->timestamp()->null()->defaultExpression("CURRENT_TIMESTAMP")); $this->alterColumn('{{%v2_users}}', 'role', $this->string(20)->null()->defaultValue('reader')); $this->alterColumn('{{%v2_users}}', 'email', $this->string(200)->notNull()); - $this->addColumn('{{%v2_users}}', 'username', $this->string(200)->notNull()); + $this->addColumn('{{%v2_users}}', 'username', $this->string(200)->notNull()->after('id')); $this->dropColumn('{{%v2_users}}', 'login'); } } diff --git a/tests/specs/blog_v2/migrations_mysql_db/m200000_000005_change_table_v2_comments.php b/tests/specs/blog_v2/migrations_mysql_db/m200000_000005_change_table_v2_comments.php index 5861542c..497550fc 100644 --- a/tests/specs/blog_v2/migrations_mysql_db/m200000_000005_change_table_v2_comments.php +++ b/tests/specs/blog_v2/migrations_mysql_db/m200000_000005_change_table_v2_comments.php @@ -25,7 +25,7 @@ public function down() $this->alterColumn('{{%v2_comments}}', 'created_at', $this->integer()->notNull()); $this->alterColumn('{{%v2_comments}}', 'meta_data', 'json NOT NULL'); $this->alterColumn('{{%v2_comments}}', 'message', 'json NOT NULL'); - $this->addColumn('{{%v2_comments}}', 'author_id', $this->integer()->notNull()); + $this->addColumn('{{%v2_comments}}', 'author_id', $this->integer()->notNull()->after('post_id')); $this->dropColumn('{{%v2_comments}}', 'user_id'); $this->addForeignKey('fk_v2_comments_author_id_v2_users_id', '{{%v2_comments}}', 'id', 'v2_users', 'author_id'); $this->addForeignKey('fk_v2_comments_post_id_v2_posts_uid', '{{%v2_comments}}', 'uid', 'v2_posts', 'post_id'); diff --git a/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php b/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php index 1a75dbd3..199c7fb1 100644 --- a/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php +++ b/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php @@ -13,7 +13,7 @@ public function up() public function down() { - $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()); + $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()->after('name')); $this->dropColumn('{{%column_name_changes}}', 'updated_at_2'); } } diff --git a/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php b/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php index 1a75dbd3..199c7fb1 100644 --- a/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php +++ b/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php @@ -13,7 +13,7 @@ public function up() public function down() { - $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()); + $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()->after('name')); $this->dropColumn('{{%column_name_changes}}', 'updated_at_2'); } } diff --git a/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php b/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php index 78f6b2e5..b8750ddd 100644 --- a/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php +++ b/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php @@ -14,7 +14,7 @@ public function up() public function down() { - $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL'); + $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL AFTER id'); $this->dropColumn('{{%newcolumns}}', 'new_column_x'); $this->dropColumn('{{%newcolumns}}', 'new_column'); } diff --git a/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php b/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php index 78f6b2e5..b8750ddd 100644 --- a/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php +++ b/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php @@ -14,7 +14,7 @@ public function up() public function down() { - $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL'); + $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL AFTER id'); $this->dropColumn('{{%newcolumns}}', 'new_column_x'); $this->dropColumn('{{%newcolumns}}', 'new_column'); } From 1516b127394fa730467786c70470f45d94404773 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 4 Oct 2024 17:27:10 +0530 Subject: [PATCH 06/26] Fix style --- src/generator/default/dbmodel.php | 8 ++++---- src/lib/migrations/MysqlMigrationBuilder.php | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/generator/default/dbmodel.php b/src/generator/default/dbmodel.php index 17fa4a84..a3d269b6 100644 --- a/src/generator/default/dbmodel.php +++ b/src/generator/default/dbmodel.php @@ -49,8 +49,8 @@ */ abstract class getClassName() ?> extends \yii\db\ActiveRecord { -getScenarios()): -foreach($scenarios as $scenario): ?> +getScenarios()): +foreach ($scenarios as $scenario): ?> /** * @@ -76,7 +76,7 @@ public static function tableName() { return getTableAlias()) ?>; } - + /** * Automatically generated scenarios from the model 'x-scenarios'. @@ -92,7 +92,7 @@ public function scenarios() $default = parent::scenarios()[self::SCENARIO_DEFAULT]; return [ - + self:: => $default, /** diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 9501bd46..ad6c96be 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -168,6 +168,5 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch */ public function checkOrder() { - } } From 3a716198fb3122698c61992a19f6e7488fa80e5c Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 4 Oct 2024 19:02:59 +0530 Subject: [PATCH 07/26] Add more tests --- tests/unit/IssueFixTest.php | 82 +++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 2da7008b..3fd39b92 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -392,4 +392,86 @@ private function deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldP { Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); } + + public function test58DeleteLastCol() + { + $deleteTable = function () { + Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); + }; + $createTable = function () { + Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ + 'id' => 'pk', + 'description' => 'text not null', + 'name' => 'text not null', + ])->execute(); + }; + $schema = << 'data://text/plain;base64,'.base64_encode($schema), + 'generateUrls' => false, + 'generateModels' => false, + 'generateControllers' => false, + 'generateMigrations' => true, + 'generateModelFaker' => false, + ]; + $tmpConfigFile = Yii::getAlias("@runtime")."/tmp-config.php"; + file_put_contents($tmpConfigFile, 'dropColumn('{{%fruits}}', 'name'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'name', $this->text()->notNull()->after('description')); + } +} + +PHP; + + foreach (['Mysql', 'Mariadb'] as $db) { + $this->{"changeDbTo$db"}(); + $deleteTable(); + $createTable(); + + $dbStr = str_replace('db', '', strtolower($db)); + $this->runGenerator($tmpConfigFile, $dbStr); + $this->runActualMigrations($dbStr, 1); + $actual = file_get_contents(Yii::getAlias('@app').'/migrations_'.$dbStr.'_db/m200000_000000_change_table_fruits.php'); + $this->assertSame($expected, $actual); + + $deleteTable(); + } + FileHelper::unlink($tmpConfigFile); + } } From dae990b0cd08b1f4ee3d0657601be2f8f541b2fc Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 4 Oct 2024 19:27:50 +0530 Subject: [PATCH 08/26] Add more tests 2 --- tests/unit/IssueFixTest.php | 286 ++++++++++++++++++++++++++++++++---- 1 file changed, 259 insertions(+), 27 deletions(-) diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 3fd39b92..e8b5d1c4 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -393,7 +393,7 @@ private function deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldP Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); } - public function test58DeleteLastCol() + private function for58($schema, $expected) { $deleteTable = function () { Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); @@ -401,10 +401,42 @@ public function test58DeleteLastCol() $createTable = function () { Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ 'id' => 'pk', - 'description' => 'text not null', 'name' => 'text not null', + 'description' => 'text not null', + 'colour' => 'text not null', + 'size' => 'text not null', ])->execute(); }; + + $config = [ + 'openApiPath' => 'data://text/plain;base64,'.base64_encode($schema), + 'generateUrls' => false, + 'generateModels' => false, + 'generateControllers' => false, + 'generateMigrations' => true, + 'generateModelFaker' => false, + ]; + $tmpConfigFile = Yii::getAlias("@runtime")."/tmp-config.php"; + file_put_contents($tmpConfigFile, '{"changeDbTo$db"}(); + $deleteTable(); + $createTable(); + + $dbStr = str_replace('db', '', strtolower($db)); + $this->runGenerator($tmpConfigFile, $dbStr); + $this->runActualMigrations($dbStr, 1); + $actual = file_get_contents(Yii::getAlias('@app').'/migrations_'.$dbStr.'_db/m200000_000000_change_table_fruits.php'); + $this->assertSame($expected, $actual); + + $deleteTable(); + } + FileHelper::unlink($tmpConfigFile); + } + + public function test58DeleteLastCol() + { $schema = << 'data://text/plain;base64,'.base64_encode($schema), - 'generateUrls' => false, - 'generateModels' => false, - 'generateControllers' => false, - 'generateMigrations' => true, - 'generateModelFaker' => false, - ]; - $tmpConfigFile = Yii::getAlias("@runtime")."/tmp-config.php"; - file_put_contents($tmpConfigFile, 'dropColumn('{{%fruits}}', 'name'); + $this->dropColumn('{{%fruits}}', 'size'); } public function down() { - $this->addColumn('{{%fruits}}', 'name', $this->text()->notNull()->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); } } PHP; - foreach (['Mysql', 'Mariadb'] as $db) { - $this->{"changeDbTo$db"}(); - $deleteTable(); - $createTable(); + $this->for58($schema, $expected); + } - $dbStr = str_replace('db', '', strtolower($db)); - $this->runGenerator($tmpConfigFile, $dbStr); - $this->runActualMigrations($dbStr, 1); - $actual = file_get_contents(Yii::getAlias('@app').'/migrations_'.$dbStr.'_db/m200000_000000_change_table_fruits.php'); - $this->assertSame($expected, $actual); - - $deleteTable(); - } - FileHelper::unlink($tmpConfigFile); + public function test58DeleteLast2ConsecutiveCol() + { + $schema = <<dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'size'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58DeleteAColInBetween() + { + $schema = <<dropColumn('{{%fruits}}', 'description'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'description', $this->text()->notNull()->after('name')); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58Delete2ConsecutiveColInBetween() + { + $schema = <<dropColumn('{{%fruits}}', 'description'); + $this->dropColumn('{{%fruits}}', 'colour'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()); + $this->addColumn('{{%fruits}}', 'description', $this->text()->notNull()->after('name')); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58Delete2NonConsecutiveColInBetween() + { + $schema = <<dropColumn('{{%fruits}}', 'name'); + $this->dropColumn('{{%fruits}}', 'colour'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + $this->addColumn('{{%fruits}}', 'name', $this->text()->notNull()->after('id')); + } +} + +PHP; + + $this->for58($schema, $expected); } } From 4f3efe94811470c593136e61a2b4fe94e512cd3a Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sat, 5 Oct 2024 20:19:39 +0530 Subject: [PATCH 09/26] Refactor --- src/lib/migrations/BaseMigrationBuilder.php | 70 +++---------------- src/lib/migrations/MysqlMigrationBuilder.php | 53 +++++++++++++- .../migrations/PostgresMigrationBuilder.php | 16 ++++- 3 files changed, 76 insertions(+), 63 deletions(-) diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index ff0c49fe..7a1b0a32 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -516,51 +516,6 @@ public function isDefaultValueChanged( return false; } - /** - * TODO move this method to MysqlMigrationBuilder - * Only for MySQL and MariaDB - * Given a column, compute its previous column name present in OpenAPI schema - * @return ?string - * `null` if column is added at last - * 'FIRST' if column is added at first position - * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' - */ - public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string - { - $columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns); - - $key = array_search($column->name, $columnNames); - if ($key > 0) { - $prevColName = $columnNames[$key-1]; - - if (!$forDrop && !isset($columnNames[$key+1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not - return null; - } - - // in case of `down()` code of migration, putting 'after ' in add column statement is erroneous because may not exist. - // Example: From col a, b, c, d, if I drop c and d then their migration code will be generated like: - // `up()` code - // drop c - // drop d - // `down()` code - // add d after c (c does not exist! Error!) (TODO check if c is present in newColumn) - // add c after b (can fix this issue) TODO - if ($forDrop) { -// return null; // TODO this case can be fixed - } - - if (array_key_exists($prevColName, $this->newColumns)) { - return self::POS_AFTER . ' ' . $prevColName; - } - return null; - - // if no `$columnSchema` is found, previous column does not exist. This happens when 'after column' is not yet added in migration or added after currently undertaken column - } elseif ($key === 0) { - return self::POS_FIRST; - } - - return null; - } public function modifyDesiredFromDbInContextOfDesired(ColumnSchema $desired, ColumnSchema $desiredFromDb): void { @@ -582,19 +537,14 @@ public function modifyDesiredInContextOfDesiredFromDb(ColumnSchema $desired, Col $desired->dbType = $desiredFromDb->dbType; } - // TODO - public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) - { - $indices = []; - if ($haveNames !== $wantNames) { - foreach ($wantNames as $key => $name) { - if ($name !== $haveNames[$key]) { - $indices[] = $key; - } - } - } - for ($i = 0; $i < count($indices)/2; $i++) { - $this->migration->addUpCode($this->recordBuilder->alterColumn()); - } - } + /** + * TODO docs + */ + abstract public function handleColumnsPositionsChanges(array $haveNames, array $wantNames); + + + /** + * TODO docs + */ + abstract public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string; } diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index ad6c96be..2027da14 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -14,9 +14,7 @@ use yii\db\ColumnSchema; use yii\db\IndexConstraint; use yii\db\Schema; -use \Yii; use yii\helpers\ArrayHelper; -use yii\helpers\VarDumper; final class MysqlMigrationBuilder extends BaseMigrationBuilder { @@ -169,4 +167,55 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch public function checkOrder() { } + + // TODO + public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) + { + $indices = []; + if ($haveNames !== $wantNames) { + foreach ($wantNames as $key => $name) { + if ($name !== $haveNames[$key]) { + $indices[] = $key; + } + } + } + for ($i = 0; $i < count($indices) / 2; $i++) { + $this->migration->addUpCode($this->recordBuilder->alterColumn()); + } + } + + + /** + * TODO move this method to MysqlMigrationBuilder + * Only for MySQL and MariaDB + * Given a column, compute its previous column name present in OpenAPI schema + * @return ?string + * `null` if column is added at last + * 'FIRST' if column is added at first position + * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' + */ + public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string + { + $columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns); + + $key = array_search($column->name, $columnNames); + if ($key > 0) { + $prevColName = $columnNames[$key - 1]; + + if (!$forDrop && !isset($columnNames[$key + 1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not + return null; + } + + if (array_key_exists($prevColName, $this->newColumns)) { + return self::POS_AFTER . ' ' . $prevColName; + } + return null; + + // if no `$columnSchema` is found, previous column does not exist. This happens when 'after column' is not yet added in migration or added after currently undertaken column + } elseif ($key === 0) { + return self::POS_FIRST; + } + + return null; + } } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index b8c9324d..9810cbf7 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -9,7 +9,6 @@ use cebe\yii2openapi\lib\items\DbIndex; use yii\db\ColumnSchema; -use yii\helpers\VarDumper; use yii\helpers\ArrayHelper; final class PostgresMigrationBuilder extends BaseMigrationBuilder @@ -248,4 +247,19 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch $desired->size = $current->size; } } + + /** + * {@inheritDoc} + */ + public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) + { + } + + + /** + * {@inheritDoc} + */ + public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string + { + } } From 53bbf083cb4c063e5f2c012e2cecfcf8ab310bc5 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sun, 6 Oct 2024 21:04:54 +0530 Subject: [PATCH 10/26] Implementation in progress + add more tests --- src/lib/migrations/BaseMigrationBuilder.php | 9 +- src/lib/migrations/MigrationRecordBuilder.php | 4 +- src/lib/migrations/MysqlMigrationBuilder.php | 81 +++-- .../index.yml | 11 +- tests/unit/IssueFixTest.php | 342 +++++++++++++++++- 5 files changed, 400 insertions(+), 47 deletions(-) diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index 7a1b0a32..6d7a413f 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -544,7 +544,14 @@ abstract public function handleColumnsPositionsChanges(array $haveNames, array $ /** - * TODO docs + * Only for MySQL and MariaDB + * Given a column, compute its previous column name present in OpenAPI schema + * @param ColumnSchema $column + * @param bool $forDrop + * @return ?string + * `null` if column is added at last + * 'FIRST' if column is added at first position + * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' */ abstract public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string; } diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 6794d6d0..0ce9bf1b 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -115,7 +115,7 @@ public function addDbColumn(string $tableAlias, ColumnSchema $column, ?string $p public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string # TODO { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { - $converter = $this->columnToCode($tableAlias, $column, true, false, true, true); + $converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position); return sprintf( ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW, $tableAlias, @@ -123,7 +123,7 @@ public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $p ColumnToCode::escapeQuotes($converter->getCode()) ); } - $converter = $this->columnToCode($tableAlias, $column, true); + $converter = $this->columnToCode($tableAlias, $column, true, false, false, false, $position); return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getCode(true)); } diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 2027da14..53b99ba4 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -23,16 +23,32 @@ final class MysqlMigrationBuilder extends BaseMigrationBuilder */ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desired, array $changed):void { + $positionCurrent = $positionDesired = null; + if (in_array('position', $changed, true)) { + $positionCurrent = $this->findPosition($current, true); + $positionDesired = $this->findPosition($desired); + $key = array_search('position', $changed, true); + if ($key !== false) { + unset($changed[$key]); + } + } $newColumn = clone $current; - $position = $this->findPosition($desired); +// $positionCurrent = $this->findPosition($desired, true); +// $positionDesired = $this->findPosition($desired); +// if ($positionCurrent === $positionDesired) { +// $positionCurrent = $positionDesired = null; +// } # else { +// $position = $positionDesired; +// $newColumn->position = $position; +// } foreach ($changed as $attr) { $newColumn->$attr = $desired->$attr; } if (static::isEnum($newColumn)) { $newColumn->dbType = 'enum'; // TODO this is concretely not correct } - $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn, $position)) - ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current, $position)); + $this->migration->addUpCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $newColumn, $positionDesired)) + ->addDownCode($this->recordBuilder->alterColumn($this->model->getTableAlias(), $current, $positionCurrent)); } protected function compareColumns(ColumnSchema $current, ColumnSchema $desired):array @@ -67,6 +83,14 @@ protected function compareColumns(ColumnSchema $current, ColumnSchema $desired): } } } + + $positionCurrent = $this->findPosition($desired, true); + $positionDesired = $this->findPosition($desired); + + if ($positionCurrent !== $positionDesired) { + $changedAttributes[] = 'position'; + } + return $changedAttributes; } @@ -168,31 +192,8 @@ public function checkOrder() { } - // TODO - public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) - { - $indices = []; - if ($haveNames !== $wantNames) { - foreach ($wantNames as $key => $name) { - if ($name !== $haveNames[$key]) { - $indices[] = $key; - } - } - } - for ($i = 0; $i < count($indices) / 2; $i++) { - $this->migration->addUpCode($this->recordBuilder->alterColumn()); - } - } - - /** - * TODO move this method to MysqlMigrationBuilder - * Only for MySQL and MariaDB - * Given a column, compute its previous column name present in OpenAPI schema - * @return ?string - * `null` if column is added at last - * 'FIRST' if column is added at first position - * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' + * {@inheritDoc} */ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string { @@ -218,4 +219,30 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?stri return null; } + + + // TODO + public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) + { + $indices = []; + if ($haveNames !== $wantNames) { + foreach ($wantNames as $key => $name) { + if ($name !== $haveNames[$key]) { + $indices[] = $key; + } + } + } + for ($i = 0; $i < count($indices) / 2; $i++) { + $this->migration->addUpCode($this->recordBuilder->alterColumn( + $this->model->getTableAlias(), + $this->newColumns[$wantNames[$indices[$i]]], + $this->findPosition($this->newColumns[$wantNames[$indices[$i]]]) + ))->addDownCode($this->recordBuilder->alterColumn( + $this->model->getTableAlias(), + $this->tableSchema->columns[$wantNames[$indices[$i]]], + $this->findPosition($this->tableSchema->columns[$wantNames[$indices[$i]]], true) + )); + } +// $this->migration->addUpCode($this->recordBuilder->dropTable($this->model->getTableAlias())); + } } diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml index 1b2fe2ff..3c9a1e63 100644 --- a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml @@ -11,13 +11,10 @@ components: properties: id: type: integer -# name: # TODO -# type: string -# description: desc -# colour: -# type: string -# description: -# type: string + name: + type: string + description: + type: string paths: '/': diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index e8b5d1c4..beae7d8b 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -362,10 +362,10 @@ public function test158BugGiiapiGeneratedRulesEnumWithTrim() } // https://github.com/php-openapi/yii2-openapi/issues/58 - public function test58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + public function test58CreateMigrationForColumnPositionChange() { - $this->deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); - $this->createTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); + $this->deleteTableFor58CreateMigrationForColumnPositionChange(); + $this->createTableFor58CreateMigrationForColumnPositionChange(); $testFile = Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php"); $this->runGenerator($testFile); @@ -376,10 +376,10 @@ public function test58CreateMigrationForColumnPositionChangeIfAFieldPositionIsCh // 'recursive' => true, // ]); // $this->checkFiles($actualFiles, $expectedFiles); - $this->deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec(); + $this->deleteTableFor58CreateMigrationForColumnPositionChange(); } - private function createTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + private function createTableFor58CreateMigrationForColumnPositionChange() { Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ 'id' => 'pk', @@ -388,7 +388,7 @@ private function createTableFor58CreateMigrationForColumnPositionChangeIfAFieldP ])->execute(); } - private function deleteTableFor58CreateMigrationForColumnPositionChangeIfAFieldPositionIsChangedInSpec() + private function deleteTableFor58CreateMigrationForColumnPositionChange() { Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); } @@ -409,15 +409,15 @@ private function for58($schema, $expected) }; $config = [ - 'openApiPath' => 'data://text/plain;base64,'.base64_encode($schema), + 'openApiPath' => 'data://text/plain;base64,' . base64_encode($schema), 'generateUrls' => false, 'generateModels' => false, 'generateControllers' => false, 'generateMigrations' => true, 'generateModelFaker' => false, ]; - $tmpConfigFile = Yii::getAlias("@runtime")."/tmp-config.php"; - file_put_contents($tmpConfigFile, '{"changeDbTo$db"}(); @@ -427,7 +427,7 @@ private function for58($schema, $expected) $dbStr = str_replace('db', '', strtolower($db)); $this->runGenerator($tmpConfigFile, $dbStr); $this->runActualMigrations($dbStr, 1); - $actual = file_get_contents(Yii::getAlias('@app').'/migrations_'.$dbStr.'_db/m200000_000000_change_table_fruits.php'); + $actual = file_get_contents(Yii::getAlias('@app') . '/migrations_' . $dbStr . '_db/m200000_000000_change_table_fruits.php'); $this->assertSame($expected, $actual); $deleteTable(); @@ -702,6 +702,328 @@ public function down() } } +PHP; + + $this->for58($schema, $expected); + } + + // ------------ + public function test58AddAColAtLastPos() + { + // default position is last so no `AFTER` needed + $schema = <<addColumn('{{%fruits}}', 'weight', $this->text()->notNull()); + } + + public function down() + { + $this->dropColumn('{{%fruits}}', 'weight'); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58Add2ConsecutiveColAtLastPos() + { + $schema = <<addColumn('{{%fruits}}', 'weight', $this->text()->notNull()->after('size')); + $this->addColumn('{{%fruits}}', 'location', $this->text()->notNull()); + } + + public function down() + { + $this->dropColumn('{{%fruits}}', 'location'); + $this->dropColumn('{{%fruits}}', 'weight'); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58AddAColInBetween() + { + $schema = <<addColumn('{{%fruits}}', 'weight', $this->text()->notNull()->after('description')); + } + + public function down() + { + $this->dropColumn('{{%fruits}}', 'weight'); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58Add2ConsecutiveColInBetween() + { + $schema = <<addColumn('{{%fruits}}', 'weight', $this->text()->notNull()->after('description')); + $this->addColumn('{{%fruits}}', 'location', $this->text()->notNull()->after('weight')); + } + + public function down() + { + $this->dropColumn('{{%fruits}}', 'location'); + $this->dropColumn('{{%fruits}}', 'weight'); + } +} + +PHP; + + $this->for58($schema, $expected); + } + + public function test58Add2NonConsecutiveColInBetween() + { + $schema = <<addColumn('{{%fruits}}', 'weight', $this->text()->notNull()->after('name')); + $this->addColumn('{{%fruits}}', 'location', $this->text()->notNull()->after('colour')); + } + + public function down() + { + $this->dropColumn('{{%fruits}}', 'location'); + $this->dropColumn('{{%fruits}}', 'weight'); + } +} + PHP; $this->for58($schema, $expected); From 832c102fd380245c434b7d34597bbeb9be938109 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 7 Oct 2024 18:11:16 +0530 Subject: [PATCH 11/26] Implement for many test cases --- src/db/ColumnSchema.php | 13 +++- src/lib/migrations/BaseMigrationBuilder.php | 1 + src/lib/migrations/MysqlMigrationBuilder.php | 73 ++++++++++++++++++- .../migrations/PostgresMigrationBuilder.php | 5 ++ 4 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/db/ColumnSchema.php b/src/db/ColumnSchema.php index c6725b10..6ce1cb3a 100644 --- a/src/db/ColumnSchema.php +++ b/src/db/ColumnSchema.php @@ -29,7 +29,16 @@ class ColumnSchema extends \yii\db\ColumnSchema /** * TODO * Used only for MySQL/MariaDB - * @var string|null + * @var array|null + * [ + * index => int + * after => ?string + * before => ?string + * ] + * If `before` is null then column is last */ - public ?string $position = null; + public ?array $fromPosition = null; + public ?array $toPosition = null; + + public bool $isPositionReallyChanged = false; } diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index 6d7a413f..0a0070e4 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -171,6 +171,7 @@ public function buildSecondary(?ManyToManyRelation $relation = null):MigrationMo { $this->migration = Yii::createObject(MigrationModel::class, [$this->model, false, $relation, []]); $this->newColumns = $relation->columnSchema ?? $this->model->attributesToColumnSchema(); + $this->setPositions(); $wantNames = array_keys($this->newColumns); $haveNames = $this->tableSchema->columnNames; $columnsForCreate = array_map( diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 53b99ba4..1ac1a7b6 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -84,10 +84,10 @@ protected function compareColumns(ColumnSchema $current, ColumnSchema $desired): } } - $positionCurrent = $this->findPosition($desired, true); - $positionDesired = $this->findPosition($desired); - - if ($positionCurrent !== $positionDesired) { +// $positionCurrent = $this->findPosition($desired, true); +// $positionDesired = $this->findPosition($desired); +// if ($positionCurrent !== $positionDesired) { + if ($desired->isPositionReallyChanged) { $changedAttributes[] = 'position'; } @@ -245,4 +245,69 @@ public function handleColumnsPositionsChanges(array $haveNames, array $wantNames } // $this->migration->addUpCode($this->recordBuilder->dropTable($this->model->getTableAlias())); } + + public function setPositions() + { + $i = 0; + $haveColumns = $this->tableSchema->columns; + $onlyColumnNames = array_keys($this->newColumns); + foreach ($this->newColumns as $columnName => $column) { + /** @var \cebe\yii2openapi\db\ColumnSchema $column */ + $column->toPosition = [ + 'index' => $i + 1, + 'after' => $i === 0 ? null : $onlyColumnNames[$i - 1], + 'before' => $i === (count($onlyColumnNames) - 1) ? null : $onlyColumnNames[$i + 1], + ]; + + $haveNamesOnlyColNames = array_keys($haveColumns); + if (isset($haveColumns[$columnName])) { + $index = array_search($columnName, $haveNamesOnlyColNames) + 1; + $column->fromPosition = [ + 'index' => $index, + 'after' => $haveNamesOnlyColNames[$index - 2] ?? null, + 'before' => $haveNamesOnlyColNames[$index] ?? null, + ]; + } + + $i++; + } + + $takenIndices = []; + foreach ($this->newColumns as $columnName => $column) { + /** @var \cebe\yii2openapi\db\ColumnSchema $column */ + if (!$column->fromPosition || !$column->toPosition) { + continue; + } + if ( + is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices)) + ) { + continue; + } + if ($column->fromPosition === $column->toPosition) { + continue; + } + + if ($column->fromPosition['after'] === $column->toPosition['after']) { + continue; + } + + if ($column->fromPosition['before'] === $column->toPosition['before']) { + continue; + } +// if (!in_array($column->fromPosition['after'], $onlyColumnNames)) { +// continue; +// } +// if (!in_array($column->fromPosition['before'], $onlyColumnNames)) { +// continue; +// } + + if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { + continue; + } + + + $column->isPositionReallyChanged = true; + $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; + } + } } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 9810cbf7..36a88c3e 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -260,6 +260,11 @@ public function handleColumnsPositionsChanges(array $haveNames, array $wantNames * {@inheritDoc} */ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string + { + return null; + } + + public function setPositions() { } } From b58bf2ef9fb6c5e8c82f4d8d5409e3200a3b5c37 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 7 Oct 2024 20:36:23 +0530 Subject: [PATCH 12/26] Resolve TODOs + add tests --- src/lib/migrations/MigrationRecordBuilder.php | 12 +- src/lib/migrations/MysqlMigrationBuilder.php | 8 -- .../index.php | 4 +- .../m200000_000000_change_table_fruits.php | 17 +++ tests/unit/IssueFixTest.php | 117 +++++++++++++++--- 5 files changed, 124 insertions(+), 34 deletions(-) create mode 100644 tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql/migrations_mysql_db/m200000_000000_change_table_fruits.php diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 0ce9bf1b..76107dfa 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -112,7 +112,7 @@ public function addDbColumn(string $tableAlias, ColumnSchema $column, ?string $p /** * @throws \yii\base\InvalidConfigException */ - public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string # TODO + public function alterColumn(string $tableAlias, ColumnSchema $column, ?string $position = null):string { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position); @@ -135,10 +135,10 @@ public function alterColumnType( ColumnSchema $column, bool $addUsing = false, ?string $position = null - ):string # TODO + ):string { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { - $converter = $this->columnToCode($tableAlias, $column, false, false, true, true); + $converter = $this->columnToCode($tableAlias, $column, false, false, true, true, $position); return sprintf( ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW, $tableAlias, @@ -146,7 +146,7 @@ public function alterColumnType( rtrim(ltrim($converter->getAlterExpression($addUsing), "'"), "'") ); } - $converter = $this->columnToCode($tableAlias, $column, false); + $converter = $this->columnToCode($tableAlias, $column, false, false, false, false, $position); return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing)); } @@ -162,7 +162,7 @@ public function alterColumnTypeFromDb( ) :string # TODO { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { - $converter = $this->columnToCode($tableAlias, $column, true, false, true, true); + $converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position); return sprintf( ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW, $tableAlias, @@ -170,7 +170,7 @@ public function alterColumnTypeFromDb( rtrim(ltrim($converter->getAlterExpression($addUsing), "'"), "'") ); } - $converter = $this->columnToCode($tableAlias, $column, true); + $converter = $this->columnToCode($tableAlias, $column, true, false, false, false, $position); return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing)); } diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 1ac1a7b6..3044af3c 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -294,18 +294,10 @@ public function setPositions() if ($column->fromPosition['before'] === $column->toPosition['before']) { continue; } -// if (!in_array($column->fromPosition['after'], $onlyColumnNames)) { -// continue; -// } -// if (!in_array($column->fromPosition['before'], $onlyColumnNames)) { -// continue; -// } if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { continue; } - - $column->isPositionReallyChanged = true; $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php index 8ec860a4..5c16da25 100644 --- a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php @@ -3,11 +3,11 @@ return [ 'openApiPath' => '@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.yml', 'generateUrls' => false, - 'generateModels' => true, + 'generateModels' => false, 'excludeModels' => [ 'Error', ], 'generateControllers' => false, 'generateMigrations' => true, - 'generateModelFaker' => true, // `generateModels` must be `true` in order to use `generateModelFaker` as `true` + 'generateModelFaker' => false, // `generateModels` must be `true` in order to use `generateModelFaker` as `true` ]; diff --git a/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql/migrations_mysql_db/m200000_000000_change_table_fruits.php b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql/migrations_mysql_db/m200000_000000_change_table_fruits.php new file mode 100644 index 00000000..90ab0c79 --- /dev/null +++ b/tests/specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql/migrations_mysql_db/m200000_000000_change_table_fruits.php @@ -0,0 +1,17 @@ +alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('id')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('description')); + } +} diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index beae7d8b..5ff0d39b 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -369,13 +369,14 @@ public function test58CreateMigrationForColumnPositionChange() $testFile = Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/index.php"); $this->runGenerator($testFile); - // $actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [ - // 'recursive' => true, - // ]); - // $expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql"), [ - // 'recursive' => true, - // ]); - // $this->checkFiles($actualFiles, $expectedFiles); + $actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [ + 'recursive' => true, + ]); + $expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/58_create_migration_for_column_position_change_if_a_field_position_is_changed_in_spec/mysql"), [ + 'recursive' => true, + ]); + $this->checkFiles($actualFiles, $expectedFiles); + $this->runActualMigrations('mysql', 1); $this->deleteTableFor58CreateMigrationForColumnPositionChange(); } @@ -393,19 +394,19 @@ private function deleteTableFor58CreateMigrationForColumnPositionChange() Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); } - private function for58($schema, $expected) + private function for58($schema, $expected, $columns = [ + 'id' => 'pk', + 'name' => 'text not null', + 'description' => 'text not null', + 'colour' => 'text not null', + 'size' => 'text not null', + ]) { $deleteTable = function () { Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); }; - $createTable = function () { - Yii::$app->db->createCommand()->createTable('{{%fruits}}', [ - 'id' => 'pk', - 'name' => 'text not null', - 'description' => 'text not null', - 'colour' => 'text not null', - 'size' => 'text not null', - ])->execute(); + $createTable = function () use ($columns) { + Yii::$app->db->createCommand()->createTable('{{%fruits}}', $columns)->execute(); }; $config = [ @@ -426,15 +427,16 @@ private function for58($schema, $expected) $dbStr = str_replace('db', '', strtolower($db)); $this->runGenerator($tmpConfigFile, $dbStr); - $this->runActualMigrations($dbStr, 1); $actual = file_get_contents(Yii::getAlias('@app') . '/migrations_' . $dbStr . '_db/m200000_000000_change_table_fruits.php'); $this->assertSame($expected, $actual); + $this->runActualMigrations($dbStr, 1); $deleteTable(); } FileHelper::unlink($tmpConfigFile); } + // ------------ Delete public function test58DeleteLastCol() { $schema = <<for58($schema, $expected); } - // ------------ + // ------------ Add public function test58AddAColAtLastPos() { // default position is last so no `AFTER` needed @@ -1028,4 +1030,83 @@ public function down() $this->for58($schema, $expected); } + + // ------------ Just move columns + public function test58MoveColumns() + { + $columns = [ + 'id' => 'pk', + 'name' => 'text null', + 'description' => 'text null', + 'colour' => 'text null', + 'size' => 'text null', + 'col_6' => 'text null', + 'col_7' => 'text null', + 'col_8' => 'text null', + 'col_9' => 'text null', + + ]; + + $schema = <<alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('id')); + $this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); + } } From 6fa477f292913c4cfeaf92be9eac9f35f4767101 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 7 Oct 2024 21:41:55 +0530 Subject: [PATCH 13/26] Fix bugs + fix failing tests + add more tests --- src/lib/migrations/MigrationRecordBuilder.php | 3 +- src/lib/migrations/MysqlMigrationBuilder.php | 57 ++++++++++++++++++- tests/unit/IssueFixTest.php | 35 +++++------- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 76107dfa..0f7a2ca7 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -135,8 +135,7 @@ public function alterColumnType( ColumnSchema $column, bool $addUsing = false, ?string $position = null - ):string - { + ):string { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, false, false, true, true, $position); return sprintf( diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 3044af3c..cc3008b4 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -278,6 +278,7 @@ public function setPositions() if (!$column->fromPosition || !$column->toPosition) { continue; } + if ( is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices)) ) { @@ -287,19 +288,69 @@ public function setPositions() continue; } - if ($column->fromPosition['after'] === $column->toPosition['after']) { + if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { // deleted column continue; } - if ($column->fromPosition['before'] === $column->toPosition['before']) { + if ($column->fromPosition['index'] === $column->toPosition['index']) { continue; } - if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { + if ($this->checkAfterPosition($column)) { + continue; + } + if ($this->checkBeforePosition($column)) { continue; } + +// if ($column->fromPosition['before'] === $column->toPosition['before'] + //// && $column->fromPosition['index'] !== $column->toPosition['index'] +// ) { +// continue; +// } + $column->isPositionReallyChanged = true; $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } } + + public function checkAfterPosition($column) + { + if ($column->fromPosition['after'] === $column->toPosition['after'] +// && $column->fromPosition['index'] !== $column->toPosition['index'] + ) { + $afterColName = $column->toPosition['after']; + $afterCol = $this->newColumns[$afterColName] ?? null; + if ($afterCol) { + if ($this->checkAfterPosition($afterCol)) { + return true; + } else { + return false; + } + } else { + return true; + } + } + return false; + } + + public function checkBeforePosition($column) + { + if ($column->fromPosition['before'] === $column->toPosition['before'] +// && $column->fromPosition['index'] !== $column->toPosition['index'] + ) { + $beforeColName = $column->toPosition['before']; + $beforeCol = $this->newColumns[$beforeColName] ?? null; + if ($beforeCol) { + if ($this->checkBeforePosition($beforeCol)) { + return true; + } else { + return false; + } + } else { + return true; + } + } + return false; + } } diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 5ff0d39b..b42d0779 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -400,7 +400,7 @@ private function for58($schema, $expected, $columns = [ 'description' => 'text not null', 'colour' => 'text not null', 'size' => 'text not null', - ]) + ], $dbs = ['Mysql', 'Mariadb']) { $deleteTable = function () { Yii::$app->db->createCommand('DROP TABLE IF EXISTS {{%fruits}}')->execute(); @@ -420,7 +420,7 @@ private function for58($schema, $expected, $columns = [ $tmpConfigFile = Yii::getAlias("@runtime") . "/tmp-config.php"; file_put_contents($tmpConfigFile, '{"changeDbTo$db"}(); $deleteTable(); $createTable(); @@ -1040,10 +1040,10 @@ public function test58MoveColumns() 'description' => 'text null', 'colour' => 'text null', 'size' => 'text null', - 'col_6' => 'text null', - 'col_7' => 'text null', - 'col_8' => 'text null', - 'col_9' => 'text null', +// 'col_6' => 'text null', +// 'col_7' => 'text null', +// 'col_8' => 'text null', +// 'col_9' => 'text null', ]; @@ -1060,22 +1060,13 @@ public function test58MoveColumns() id: type: integer colour: - type: string + type: string size: type: string name: type: string description: - type: string - col_6: - type: string - col_7: - type: string - col_8: - type: string - col_9: - type: string - + type: string paths: '/': get: @@ -1094,19 +1085,19 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration { public function up() { - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('id')); - $this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); } public function down() { - $this->alterColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); } } PHP; - $this->for58($schema, $expected, $columns); + $this->for58($schema, $expected, $columns, ['Mysql']); } } From a0090f73eddff2d267e69aead3370e45a2e6d0e6 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Tue, 8 Oct 2024 12:19:05 +0530 Subject: [PATCH 14/26] Fix bugs --- src/lib/migrations/MysqlMigrationBuilder.php | 55 ++++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index cc3008b4..dfe089fd 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -251,6 +251,7 @@ public function setPositions() $i = 0; $haveColumns = $this->tableSchema->columns; $onlyColumnNames = array_keys($this->newColumns); + $haveNamesOnlyColNames = array_keys($haveColumns); foreach ($this->newColumns as $columnName => $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ $column->toPosition = [ @@ -259,7 +260,6 @@ public function setPositions() 'before' => $i === (count($onlyColumnNames) - 1) ? null : $onlyColumnNames[$i + 1], ]; - $haveNamesOnlyColNames = array_keys($haveColumns); if (isset($haveColumns[$columnName])) { $index = array_search($columnName, $haveNamesOnlyColNames) + 1; $column->fromPosition = [ @@ -275,6 +275,35 @@ public function setPositions() $takenIndices = []; foreach ($this->newColumns as $columnName => $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ + + if (count($onlyColumnNames) !== count($haveNamesOnlyColNames)) { + // check if only new columns are added without any explicit position change + $onlyColumnNamesCopy = $onlyColumnNames; + $columnsForCreate = array_diff($onlyColumnNames, $haveNamesOnlyColNames); + foreach ($columnsForCreate as $key => $value) { + $at = array_search($value, $onlyColumnNames); + if (is_int($at)) { + unset($onlyColumnNamesCopy[$at]); + } + } + if ($onlyColumnNamesCopy === $onlyColumnNames) { + continue; + } + + $haveNamesOnlyColNamesCopy = $haveNamesOnlyColNames; + $columnsForDrop = array_diff($haveNamesOnlyColNames, $onlyColumnNames); + foreach ($columnsForDrop as $key => $value) { + $at = array_search($value, $haveNamesOnlyColNames); + if (is_int($at)) { + unset($haveNamesOnlyColNamesCopy[$at]); + } + } + if ($haveNamesOnlyColNamesCopy === $haveNamesOnlyColNames) { + continue; + } + } + + if (!$column->fromPosition || !$column->toPosition) { continue; } @@ -288,20 +317,26 @@ public function setPositions() continue; } - if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { // deleted column - continue; - } + // TODO this does not look correct +// if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { // after and before column are deleted +// continue; +// } + + // TODO this does not look correct +// if (!in_array($column->toPosition['after'], $haveNamesOnlyColNames) && !in_array($column->toPosition['before'], $haveNamesOnlyColNames)) { // after and before column are added just now +// continue; +// } if ($column->fromPosition['index'] === $column->toPosition['index']) { continue; } - if ($this->checkAfterPosition($column)) { - continue; - } - if ($this->checkBeforePosition($column)) { - continue; - } + // if ($this->checkAfterPosition($column)) { + // continue; + // } + // if ($this->checkBeforePosition($column)) { + // continue; + // } // if ($column->fromPosition['before'] === $column->toPosition['before'] //// && $column->fromPosition['index'] !== $column->toPosition['index'] From 3dcc87e21a261d4d3c553207b8645f336d9bb2c1 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Tue, 8 Oct 2024 13:33:19 +0530 Subject: [PATCH 15/26] Refactor --- src/lib/migrations/MysqlMigrationBuilder.php | 62 ++++---------------- 1 file changed, 12 insertions(+), 50 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index dfe089fd..8e738e2d 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -276,74 +276,38 @@ public function setPositions() foreach ($this->newColumns as $columnName => $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ + if (!$column->fromPosition || !$column->toPosition) { + continue; + } + if (count($onlyColumnNames) !== count($haveNamesOnlyColNames)) { // check if only new columns are added without any explicit position change - $onlyColumnNamesCopy = $onlyColumnNames; $columnsForCreate = array_diff($onlyColumnNames, $haveNamesOnlyColNames); - foreach ($columnsForCreate as $key => $value) { - $at = array_search($value, $onlyColumnNames); - if (is_int($at)) { - unset($onlyColumnNamesCopy[$at]); + if ($columnsForCreate) { + if ($haveNamesOnlyColNames === array_values(array_diff($onlyColumnNames, $columnsForCreate))) { + continue; } } - if ($onlyColumnNamesCopy === $onlyColumnNames) { - continue; - } - $haveNamesOnlyColNamesCopy = $haveNamesOnlyColNames; + // check if only existing columns are deleted without any explicit position change $columnsForDrop = array_diff($haveNamesOnlyColNames, $onlyColumnNames); - foreach ($columnsForDrop as $key => $value) { - $at = array_search($value, $haveNamesOnlyColNames); - if (is_int($at)) { - unset($haveNamesOnlyColNamesCopy[$at]); + if ($columnsForDrop) { + if ($onlyColumnNames === array_values(array_diff($haveNamesOnlyColNames, $columnsForDrop))) { + continue; } } - if ($haveNamesOnlyColNamesCopy === $haveNamesOnlyColNames) { - continue; - } - } - - - if (!$column->fromPosition || !$column->toPosition) { - continue; } - if ( - is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices)) - ) { + if (is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices))) { continue; } if ($column->fromPosition === $column->toPosition) { continue; } - - // TODO this does not look correct -// if (!in_array($column->fromPosition['after'], $onlyColumnNames) && !in_array($column->fromPosition['before'], $onlyColumnNames)) { // after and before column are deleted -// continue; -// } - - // TODO this does not look correct -// if (!in_array($column->toPosition['after'], $haveNamesOnlyColNames) && !in_array($column->toPosition['before'], $haveNamesOnlyColNames)) { // after and before column are added just now -// continue; -// } - if ($column->fromPosition['index'] === $column->toPosition['index']) { continue; } - // if ($this->checkAfterPosition($column)) { - // continue; - // } - // if ($this->checkBeforePosition($column)) { - // continue; - // } - -// if ($column->fromPosition['before'] === $column->toPosition['before'] - //// && $column->fromPosition['index'] !== $column->toPosition['index'] -// ) { -// continue; -// } - $column->isPositionReallyChanged = true; $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } @@ -352,7 +316,6 @@ public function setPositions() public function checkAfterPosition($column) { if ($column->fromPosition['after'] === $column->toPosition['after'] -// && $column->fromPosition['index'] !== $column->toPosition['index'] ) { $afterColName = $column->toPosition['after']; $afterCol = $this->newColumns[$afterColName] ?? null; @@ -372,7 +335,6 @@ public function checkAfterPosition($column) public function checkBeforePosition($column) { if ($column->fromPosition['before'] === $column->toPosition['before'] -// && $column->fromPosition['index'] !== $column->toPosition['index'] ) { $beforeColName = $column->toPosition['before']; $beforeCol = $this->newColumns[$beforeColName] ?? null; From 9ef528cceb03bc5ad89a356740442cd36e761a00 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Tue, 8 Oct 2024 20:50:39 +0530 Subject: [PATCH 16/26] Fix bugs + fix failing tests + add more tests + refactor --- src/lib/migrations/BaseMigrationBuilder.php | 2 +- src/lib/migrations/MysqlMigrationBuilder.php | 74 +++++++--- tests/unit/IssueFixTest.php | 134 ++++++++++++++++++- 3 files changed, 187 insertions(+), 23 deletions(-) diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index 0a0070e4..01905e0b 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -185,7 +185,7 @@ function (string $missingColumn) { function (string $unknownColumn) { return $this->tableSchema->columns[$unknownColumn]; }, - array_diff($haveNames, $wantNames) + array_reverse(array_diff($haveNames, $wantNames), true) ); $columnsForChange = array_intersect($wantNames, $haveNames); diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 8e738e2d..4080ee77 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -197,6 +197,42 @@ public function checkOrder() */ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string { +// if (!$forDrop) { +// $columnNames = array_keys($this->newColumns); +// $key = array_search($column->name, $columnNames); +// if (is_int($key)) { +// if ($key > 0) { +// $prevColName = $columnNames[$key - 1]; +// +// // if the perv col is last then no need for `'AFTER '` because last is the default position +// if (array_search($prevColName, $columnNames) === (count($columnNames) - 1)) { +// return null; +// } +// +// return self::POS_AFTER . ' ' . $prevColName; +// } elseif ($key === 0) { +// return self::POS_FIRST; +// } +// } +// +// } else { +// $columnNames = array_keys($this->tableSchema->columns); +// $key = array_search($column->name, $columnNames); +// if (is_int($key)) { +// if ($key > 0) { +// $prevColName = $columnNames[$key - 1]; +// +// if (array_search($prevColName, $columnNames) === count($columnNames) - 1) { +// return null; +// } +// +// return self::POS_AFTER . ' ' . $prevColName; +// } elseif ($key === 0) { +// return self::POS_FIRST; +// } +// } +// } +// return null; $columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns); $key = array_search($column->name, $columnNames); @@ -250,22 +286,22 @@ public function setPositions() { $i = 0; $haveColumns = $this->tableSchema->columns; - $onlyColumnNames = array_keys($this->newColumns); - $haveNamesOnlyColNames = array_keys($haveColumns); - foreach ($this->newColumns as $columnName => $column) { + $wantNames = array_keys($this->newColumns); + $haveNames = array_keys($haveColumns); + foreach ($this->newColumns as $name => $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ $column->toPosition = [ 'index' => $i + 1, - 'after' => $i === 0 ? null : $onlyColumnNames[$i - 1], - 'before' => $i === (count($onlyColumnNames) - 1) ? null : $onlyColumnNames[$i + 1], + 'after' => $i === 0 ? null : $wantNames[$i - 1], + 'before' => $i === (count($wantNames) - 1) ? null : $wantNames[$i + 1], ]; - if (isset($haveColumns[$columnName])) { - $index = array_search($columnName, $haveNamesOnlyColNames) + 1; + if (isset($haveColumns[$name])) { + $index = array_search($name, $haveNames) + 1; $column->fromPosition = [ 'index' => $index, - 'after' => $haveNamesOnlyColNames[$index - 2] ?? null, - 'before' => $haveNamesOnlyColNames[$index] ?? null, + 'after' => $haveNames[$index - 2] ?? null, + 'before' => $haveNames[$index] ?? null, ]; } @@ -273,28 +309,24 @@ public function setPositions() } $takenIndices = []; - foreach ($this->newColumns as $columnName => $column) { + foreach ($this->newColumns as $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ if (!$column->fromPosition || !$column->toPosition) { continue; } - if (count($onlyColumnNames) !== count($haveNamesOnlyColNames)) { + if (count($wantNames) !== count($haveNames)) { // check if only new columns are added without any explicit position change - $columnsForCreate = array_diff($onlyColumnNames, $haveNamesOnlyColNames); - if ($columnsForCreate) { - if ($haveNamesOnlyColNames === array_values(array_diff($onlyColumnNames, $columnsForCreate))) { - continue; - } + $namesForCreate = array_diff($wantNames, $haveNames); + if ($namesForCreate && $haveNames === array_values(array_diff($wantNames, $namesForCreate))) { + continue; } // check if only existing columns are deleted without any explicit position change - $columnsForDrop = array_diff($haveNamesOnlyColNames, $onlyColumnNames); - if ($columnsForDrop) { - if ($onlyColumnNames === array_values(array_diff($haveNamesOnlyColNames, $columnsForDrop))) { - continue; - } + $namesForDrop = array_diff($haveNames, $wantNames); + if ($namesForDrop && $wantNames === array_values(array_diff($haveNames, $namesForDrop))) { + continue; } } diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index b42d0779..2809ffcc 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -709,6 +709,65 @@ public function down() $this->for58($schema, $expected); } + public function test58DeleteLast4Col() + { + $columns = [ + 'id' => 'pk', + 'name' => 'text null', + 'description' => 'text null', + 'colour' => 'text null', + 'size' => 'text null', + 'col_6' => 'text null', + ]; + + $schema = <<alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns, ['Mysql', 'Mariadb']); + } + // ------------ Add public function test58AddAColAtLastPos() { @@ -1032,7 +1091,7 @@ public function down() } // ------------ Just move columns - public function test58MoveColumns() + public function test58MoveLast2Col2PosUp() { $columns = [ 'id' => 'pk', @@ -1100,4 +1159,77 @@ public function down() $this->for58($schema, $expected, $columns, ['Mysql']); } + + // ----------- + public function test58Move1Add1Del1Col() + { + $columns = [ + 'id' => 'pk', + 'name' => 'text null', + 'description' => 'text null', + 'colour' => 'text null', + 'size' => 'text null', +// 'col_6' => 'text null', +// 'col_7' => 'text null', +// 'col_8' => 'text null', +// 'col_9' => 'text null', + + ]; + + $schema = <<alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns, ['Mysql']); + } + + // add 1 and del 1 col at same position + // add 1 and del 1 col at different position } From 9310dc618c9aaf6f0812bf9af055a80c17ded988 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 10 Oct 2024 20:19:42 +0530 Subject: [PATCH 17/26] Fix failing tests --- src/lib/migrations/MysqlMigrationBuilder.php | 20 ++++----- tests/unit/IssueFixTest.php | 43 +++++++++++--------- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 4080ee77..c3e31585 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -316,18 +316,16 @@ public function setPositions() continue; } - if (count($wantNames) !== count($haveNames)) { - // check if only new columns are added without any explicit position change - $namesForCreate = array_diff($wantNames, $haveNames); - if ($namesForCreate && $haveNames === array_values(array_diff($wantNames, $namesForCreate))) { - continue; - } +// // check if only new columns are added without any explicit position change + $namesForCreate = array_diff($wantNames, $haveNames); + if ($namesForCreate && $haveNames === array_values(array_diff($wantNames, $namesForCreate))) { + continue; + } - // check if only existing columns are deleted without any explicit position change - $namesForDrop = array_diff($haveNames, $wantNames); - if ($namesForDrop && $wantNames === array_values(array_diff($haveNames, $namesForDrop))) { - continue; - } + // check if only existing columns are deleted without any explicit position change + $namesForDrop = array_diff($haveNames, $wantNames); + if ($namesForDrop && $wantNames === array_values(array_diff($haveNames, $namesForDrop))) { + continue; } if (is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices))) { diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 2809ffcc..3f3ff3be 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -530,14 +530,14 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration { public function up() { - $this->dropColumn('{{%fruits}}', 'colour'); $this->dropColumn('{{%fruits}}', 'size'); + $this->dropColumn('{{%fruits}}', 'colour'); } public function down() { - $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()); $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()); } } @@ -639,14 +639,14 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration { public function up() { - $this->dropColumn('{{%fruits}}', 'description'); $this->dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'description'); } public function down() { - $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()); $this->addColumn('{{%fruits}}', 'description', $this->text()->notNull()->after('name')); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()); } } @@ -693,14 +693,14 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration { public function up() { - $this->dropColumn('{{%fruits}}', 'name'); $this->dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'name'); } public function down() { - $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); $this->addColumn('{{%fruits}}', 'name', $this->text()->notNull()->after('id')); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); } } @@ -752,20 +752,24 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration { public function up() { - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->dropColumn('{{%fruits}}', 'col_6'); + $this->dropColumn('{{%fruits}}', 'size'); + $this->dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'description'); } public function down() { - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + $this->addColumn('{{%fruits}}', 'description', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'size', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'col_6', $this->text()->null()); } } PHP; - $this->for58($schema, $expected, $columns, ['Mysql', 'Mariadb']); + $this->for58($schema, $expected, $columns, ['Mysql']); } // ------------ Add @@ -1169,11 +1173,6 @@ public function test58Move1Add1Del1Col() 'description' => 'text null', 'colour' => 'text null', 'size' => 'text null', -// 'col_6' => 'text null', -// 'col_7' => 'text null', -// 'col_8' => 'text null', -// 'col_9' => 'text null', - ]; $schema = <<addColumn('{{%fruits}}', 'col_6', $this->text()->null()); + $this->dropColumn('{{%fruits}}', 'size'); $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'description', $this->text()->null()->after('name')); } public function down() { - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'description', $this->text()->null()->after('name')); + $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('id')); $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->dropColumn('{{%fruits}}', 'col_6'); } } From 617fdf5b447ac71b4fff1354fb8e3a1b0a33e1c1 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 10 Oct 2024 20:28:45 +0530 Subject: [PATCH 18/26] Fix other failing tests --- .../m200000_000003_change_table_dropfirsttwocols.php | 4 ++-- .../m200000_000003_change_table_dropfirsttwocols.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php b/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php index 89ef1937..9877ed34 100644 --- a/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php +++ b/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php @@ -7,13 +7,13 @@ class m200000_000003_change_table_dropfirsttwocols extends \yii\db\Migration { public function up() { - $this->dropColumn('{{%dropfirsttwocols}}', 'name'); $this->dropColumn('{{%dropfirsttwocols}}', 'address'); + $this->dropColumn('{{%dropfirsttwocols}}', 'name'); } public function down() { - $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()->defaultValue(null)); $this->addColumn('{{%dropfirsttwocols}}', 'name', $this->text()->null()->defaultValue(null)->first()); + $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()->defaultValue(null)); } } diff --git a/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php b/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php index aaa54835..c4b398f5 100644 --- a/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php +++ b/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php @@ -7,13 +7,13 @@ class m200000_000003_change_table_dropfirsttwocols extends \yii\db\Migration { public function up() { - $this->dropColumn('{{%dropfirsttwocols}}', 'name'); $this->dropColumn('{{%dropfirsttwocols}}', 'address'); + $this->dropColumn('{{%dropfirsttwocols}}', 'name'); } public function down() { - $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()); $this->addColumn('{{%dropfirsttwocols}}', 'name', $this->text()->null()->first()); + $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()); } } From 85aac9105134dfe593eb483a241f1ba96e7a48b1 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 11 Oct 2024 21:30:41 +0530 Subject: [PATCH 19/26] Fix bugs + fix failing tests --- src/lib/migrations/BaseMigrationBuilder.php | 2 +- src/lib/migrations/MysqlMigrationBuilder.php | 24 ++++++++++++++----- .../migrations/PostgresMigrationBuilder.php | 2 +- tests/unit/IssueFixTest.php | 6 ++--- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index 01905e0b..d65582b1 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -554,5 +554,5 @@ abstract public function handleColumnsPositionsChanges(array $haveNames, array $ * 'FIRST' if column is added at first position * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' */ - abstract public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string; + abstract public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $forAlter = false): ?string; } diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index c3e31585..ee276e40 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -25,8 +25,8 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir { $positionCurrent = $positionDesired = null; if (in_array('position', $changed, true)) { - $positionCurrent = $this->findPosition($current, true); - $positionDesired = $this->findPosition($desired); + $positionDesired = $this->findPosition($desired, false, true); + $positionCurrent = $this->findPosition($desired, true, true); $key = array_search('position', $changed, true); if ($key !== false) { unset($changed[$key]); @@ -195,8 +195,13 @@ public function checkOrder() /** * {@inheritDoc} */ - public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string + public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $forAlter = false): ?string { +// if ($column instanceof \cebe\yii2openapi\db\ColumnSchema) { +// if (!$column->isPositionReallyChanged) { +// return null; +// } +// } // if (!$forDrop) { // $columnNames = array_keys($this->newColumns); // $key = array_search($column->name, $columnNames); @@ -233,17 +238,24 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false): ?stri // } // } // return null; + $columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns); $key = array_search($column->name, $columnNames); if ($key > 0) { $prevColName = $columnNames[$key - 1]; - - if (!$forDrop && !isset($columnNames[$key + 1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not + if (($key === count($columnNames) - 1) && !$forAlter) { return null; } - if (array_key_exists($prevColName, $this->newColumns)) { +// if (!$forDrop && !isset($columnNames[$key + 1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not +// return null; +// } + + if (array_key_exists($prevColName, $forDrop ? $this->tableSchema->columns : $this->newColumns)) { + if (($prevColName === $columnNames[count($columnNames) - 1]) && !$forAlter) { + return null; + } return self::POS_AFTER . ' ' . $prevColName; } return null; diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 36a88c3e..7fbc6419 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -259,7 +259,7 @@ public function handleColumnsPositionsChanges(array $haveNames, array $wantNames /** * {@inheritDoc} */ - public function findPosition(ColumnSchema $column, bool $forDrop = false): ?string + public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $forAlter = false): ?string { return null; } diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 3f3ff3be..0e1c6027 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -483,7 +483,7 @@ public function up() public function down() { - $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()->after('colour')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()); } } @@ -646,7 +646,7 @@ public function up() public function down() { $this->addColumn('{{%fruits}}', 'description', $this->text()->notNull()->after('name')); - $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); } } @@ -1225,7 +1225,7 @@ public function down() $this->alterColumn('{{%fruits}}', 'description', $this->text()->null()->after('name')); $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('id')); $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); - $this->addColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->null()); $this->dropColumn('{{%fruits}}', 'col_6'); } } From 95db88f3b7674837ffcdb3198c22b3d3edd0d764 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sat, 12 Oct 2024 17:25:12 +0530 Subject: [PATCH 20/26] Fix bugs + fix failing tests 2 --- src/lib/migrations/MysqlMigrationBuilder.php | 25 ++++++++++++------- ...00000_change_table_column_name_changes.php | 2 +- ...00000_change_table_column_name_changes.php | 2 +- ...m200000_000001_change_table_newcolumns.php | 2 +- ...m200000_000001_change_table_newcolumns.php | 2 +- ...0_000003_change_table_dropfirsttwocols.php | 2 +- ...0_000003_change_table_dropfirsttwocols.php | 2 +- tests/unit/IssueFixTest.php | 11 +++----- 8 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index ee276e40..c03979df 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -28,9 +28,7 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir $positionDesired = $this->findPosition($desired, false, true); $positionCurrent = $this->findPosition($desired, true, true); $key = array_search('position', $changed, true); - if ($key !== false) { - unset($changed[$key]); - } + unset($changed[$key]); } $newColumn = clone $current; // $positionCurrent = $this->findPosition($desired, true); @@ -253,8 +251,12 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $ // } if (array_key_exists($prevColName, $forDrop ? $this->tableSchema->columns : $this->newColumns)) { - if (($prevColName === $columnNames[count($columnNames) - 1]) && !$forAlter) { - return null; + if ($forDrop && !$forAlter) { + // if the previous column is the last one in the want names then no need for AFTER + $cols = array_keys($this->newColumns); + if ($prevColName === array_pop($cols)) { + return null; + } } return self::POS_AFTER . ' ' . $prevColName; } @@ -328,15 +330,20 @@ public function setPositions() continue; } -// // check if only new columns are added without any explicit position change + // check if only new columns are added without any explicit position change $namesForCreate = array_diff($wantNames, $haveNames); - if ($namesForCreate && $haveNames === array_values(array_diff($wantNames, $namesForCreate))) { + $wantNamesWoNewCols = array_values(array_diff($wantNames, $namesForCreate)); + if ($namesForCreate && $haveNames === $wantNamesWoNewCols) { continue; } - // check if only existing columns are deleted without any explicit position change $namesForDrop = array_diff($haveNames, $wantNames); - if ($namesForDrop && $wantNames === array_values(array_diff($haveNames, $namesForDrop))) { + $haveNamesWoDropCols = array_values(array_diff($haveNames, $namesForDrop)); + if ($namesForDrop && $wantNames === $haveNamesWoDropCols) { + continue; + } + // check both above simultaneously + if ($namesForCreate && $namesForDrop && ($wantNamesWoNewCols === $haveNamesWoDropCols)) { continue; } diff --git a/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php b/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php index 199c7fb1..1a75dbd3 100644 --- a/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php +++ b/tests/specs/change_column_name/maria/app/migrations_maria_db/m200000_000000_change_table_column_name_changes.php @@ -13,7 +13,7 @@ public function up() public function down() { - $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()->after('name')); + $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()); $this->dropColumn('{{%column_name_changes}}', 'updated_at_2'); } } diff --git a/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php b/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php index 199c7fb1..1a75dbd3 100644 --- a/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php +++ b/tests/specs/change_column_name/mysql/app/migrations_mysql_db/m200000_000000_change_table_column_name_changes.php @@ -13,7 +13,7 @@ public function up() public function down() { - $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()->after('name')); + $this->addColumn('{{%column_name_changes}}', 'updated_at', $this->datetime()->notNull()); $this->dropColumn('{{%column_name_changes}}', 'updated_at_2'); } } diff --git a/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php b/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php index b8750ddd..78f6b2e5 100644 --- a/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php +++ b/tests/specs/enum/new_column/maria/app/migrations_maria_db/m200000_000001_change_table_newcolumns.php @@ -14,7 +14,7 @@ public function up() public function down() { - $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL AFTER id'); + $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL'); $this->dropColumn('{{%newcolumns}}', 'new_column_x'); $this->dropColumn('{{%newcolumns}}', 'new_column'); } diff --git a/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php b/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php index b8750ddd..78f6b2e5 100644 --- a/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php +++ b/tests/specs/enum/new_column/mysql/app/migrations_mysql_db/m200000_000001_change_table_newcolumns.php @@ -14,7 +14,7 @@ public function up() public function down() { - $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL AFTER id'); + $this->addColumn('{{%newcolumns}}', 'delete_col', 'enum("FOUR", "FIVE", "SIX") NULL DEFAULT NULL'); $this->dropColumn('{{%newcolumns}}', 'new_column_x'); $this->dropColumn('{{%newcolumns}}', 'new_column'); } diff --git a/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php b/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php index 9877ed34..b67baf29 100644 --- a/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php +++ b/tests/specs/new_column_position/maria/app/migrations_maria_db/m200000_000003_change_table_dropfirsttwocols.php @@ -14,6 +14,6 @@ public function up() public function down() { $this->addColumn('{{%dropfirsttwocols}}', 'name', $this->text()->null()->defaultValue(null)->first()); - $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()->defaultValue(null)); + $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()->defaultValue(null)->after('name')); } } diff --git a/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php b/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php index c4b398f5..83aa17eb 100644 --- a/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php +++ b/tests/specs/new_column_position/mysql/app/migrations_mysql_db/m200000_000003_change_table_dropfirsttwocols.php @@ -14,6 +14,6 @@ public function up() public function down() { $this->addColumn('{{%dropfirsttwocols}}', 'name', $this->text()->null()->first()); - $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()); + $this->addColumn('{{%dropfirsttwocols}}', 'address', $this->text()->null()->after('name')); } } diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 0e1c6027..138967b8 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -536,7 +536,7 @@ public function up() public function down() { - $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()->after('description')); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->notNull()); $this->addColumn('{{%fruits}}', 'size', $this->text()->notNull()); } } @@ -761,8 +761,8 @@ public function up() public function down() { $this->addColumn('{{%fruits}}', 'description', $this->text()->null()); - $this->addColumn('{{%fruits}}', 'colour', $this->text()->null()); - $this->addColumn('{{%fruits}}', 'size', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); $this->addColumn('{{%fruits}}', 'col_6', $this->text()->null()); } } @@ -1103,11 +1103,6 @@ public function test58MoveLast2Col2PosUp() 'description' => 'text null', 'colour' => 'text null', 'size' => 'text null', -// 'col_6' => 'text null', -// 'col_7' => 'text null', -// 'col_8' => 'text null', -// 'col_9' => 'text null', - ]; $schema = << Date: Sat, 12 Oct 2024 20:21:41 +0530 Subject: [PATCH 21/26] Change tests and add few more --- tests/unit/IssueFixTest.php | 214 ++++++++++++++++++++++++++++-------- 1 file changed, 171 insertions(+), 43 deletions(-) diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 138967b8..947cadea 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -713,11 +713,11 @@ public function test58DeleteLast4Col() { $columns = [ 'id' => 'pk', - 'name' => 'text null', - 'description' => 'text null', - 'colour' => 'text null', - 'size' => 'text null', - 'col_6' => 'text null', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + 'col_6' => 'bool null', ]; $schema = <<addColumn('{{%fruits}}', 'description', $this->text()->null()); - $this->addColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); - $this->addColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); - $this->addColumn('{{%fruits}}', 'col_6', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)); + $this->addColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + $this->addColumn('{{%fruits}}', 'col_6', $this->tinyInteger(1)->null()->defaultValue(null)); } } PHP; - $this->for58($schema, $expected, $columns, ['Mysql']); + $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + } + + public function test58DeleteFirst4Col() + { + $columns = [ + 'name' => 'boolean null', + 'description' => 'boolean null', + 'colour' => 'boolean null', + 'size' => 'boolean null', + 'col_6' => 'boolean null', + 'col_7' => 'boolean null', + ]; + + $schema = <<dropColumn('{{%fruits}}', 'size'); + $this->dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'description'); + $this->dropColumn('{{%fruits}}', 'name'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->first()); + $this->addColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); + $this->addColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns/*, ['Mysql']*/); } // ------------ Add @@ -1099,10 +1162,10 @@ public function test58MoveLast2Col2PosUp() { $columns = [ 'id' => 'pk', - 'name' => 'text null', - 'description' => 'text null', - 'colour' => 'text null', - 'size' => 'text null', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', ]; $schema = <<alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + $this->alterColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); } public function down() { - $this->alterColumn('{{%fruits}}', 'size', $this->text()->null()->after('colour')); - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); + $this->alterColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); } } PHP; - $this->for58($schema, $expected, $columns, ['Mysql']); + $this->for58($schema, $expected, $columns/*, ['Mysql']*/); } - // ----------- + // ----------- Miscellaneous public function test58Move1Add1Del1Col() { $columns = [ 'id' => 'pk', - 'name' => 'text null', - 'description' => 'text null', - 'colour' => 'text null', - 'size' => 'text null', + 'name' => 'boolean null', + 'description' => 'boolean null', + 'colour' => 'boolean null', + 'size' => 'boolean null', ]; $schema = <<addColumn('{{%fruits}}', 'col_6', $this->text()->null()); + $this->addColumn('{{%fruits}}', 'col_6', $this->boolean()->null()->defaultValue(null)); $this->dropColumn('{{%fruits}}', 'size'); - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('id')); - $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('colour')); - $this->alterColumn('{{%fruits}}', 'description', $this->text()->null()->after('name')); + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + $this->alterColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); } public function down() { - $this->alterColumn('{{%fruits}}', 'description', $this->text()->null()->after('name')); - $this->alterColumn('{{%fruits}}', 'name', $this->text()->null()->after('id')); - $this->alterColumn('{{%fruits}}', 'colour', $this->text()->null()->after('description')); - $this->addColumn('{{%fruits}}', 'size', $this->text()->null()); + $this->alterColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); + $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); + $this->addColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)); $this->dropColumn('{{%fruits}}', 'col_6'); } } PHP; - $this->for58($schema, $expected, $columns, ['Mysql']); + $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + } + + public function test58Add1Del1ColAtSamePosition() + { + $columns = [ + 'id' => 'pk', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + ]; + + $schema = <<addColumn('{{%fruits}}', 'description_new', $this->boolean()->null()->defaultValue(null)->after('name')); + $this->dropColumn('{{%fruits}}', 'description'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); + $this->dropColumn('{{%fruits}}', 'description_new'); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); } - // add 1 and del 1 col at same position + // TODO add tests cases: // add 1 and del 1 col at different position + // add 2 and del 1 col at different positions } From e54defe5aef10d1a8b37087b9edc47b9a9a9f408 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 14 Oct 2024 17:33:13 +0530 Subject: [PATCH 22/26] Fix bug --- src/lib/migrations/MysqlMigrationBuilder.php | 5 + tests/unit/IssueFixTest.php | 149 ++++++++++++++++++- 2 files changed, 146 insertions(+), 8 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index c03979df..8ba6a0fa 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -258,6 +258,11 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $ return null; } } + if ($forAlter && $forDrop) { + if (!array_key_exists($prevColName, $this->newColumns)) { + return null; + } + } return self::POS_AFTER . ' ' . $prevColName; } return null; diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index 947cadea..f1d2410c 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -430,7 +430,6 @@ private function for58($schema, $expected, $columns = [ $actual = file_get_contents(Yii::getAlias('@app') . '/migrations_' . $dbStr . '_db/m200000_000000_change_table_fruits.php'); $this->assertSame($expected, $actual); $this->runActualMigrations($dbStr, 1); - $deleteTable(); } FileHelper::unlink($tmpConfigFile); @@ -769,7 +768,7 @@ public function down() PHP; - $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + $this->for58($schema, $expected, $columns); } public function test58DeleteFirst4Col() @@ -832,7 +831,7 @@ public function down() PHP; - $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + $this->for58($schema, $expected, $columns); } // ------------ Add @@ -1219,7 +1218,7 @@ public function down() PHP; - $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + $this->for58($schema, $expected, $columns); } // ----------- Miscellaneous @@ -1290,7 +1289,7 @@ public function down() PHP; - $this->for58($schema, $expected, $columns/*, ['Mysql']*/); + $this->for58($schema, $expected, $columns); } public function test58Add1Del1ColAtSamePosition() @@ -1357,7 +1356,141 @@ public function down() $this->for58($schema, $expected, $columns); } - // TODO add tests cases: - // add 1 and del 1 col at different position - // add 2 and del 1 col at different positions + public function test58Add3Del2ColAtDiffPos() + { + $columns = [ + 'id' => 'pk', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + ]; + + $schema = <<addColumn('{{%fruits}}', 'col_6', $this->boolean()->null()->defaultValue(null)->after('id')); + $this->addColumn('{{%fruits}}', 'col_7', $this->boolean()->null()->defaultValue(null)->after('name')); + $this->addColumn('{{%fruits}}', 'col_8', $this->boolean()->null()->defaultValue(null)->after('col_7')); + $this->dropColumn('{{%fruits}}', 'colour'); + $this->dropColumn('{{%fruits}}', 'description'); + } + + public function down() + { + $this->addColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); + $this->addColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); + $this->dropColumn('{{%fruits}}', 'col_8'); + $this->dropColumn('{{%fruits}}', 'col_7'); + $this->dropColumn('{{%fruits}}', 'col_6'); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); + } + + // This test fails. See description of https://github.com/php-openapi/yii2-openapi/pull/59 +// public function test58Add3Del2Move3ColAtDiffPos() +// { +// $columns = [ +// 'id' => 'pk', +// 'name' => 'bool null', +// 'description' => 'bool null', +// 'colour' => 'bool null', +// 'size' => 'bool null', +// 'col_6' => 'bool null', +// ]; +// +// $schema = <<for58($schema, $expected, $columns); +// } } From ca2273150a7eee864af85a3315aa880887d20fad Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 14 Oct 2024 17:53:28 +0530 Subject: [PATCH 23/26] Add docs + refactor --- src/db/ColumnSchema.php | 14 +-- src/lib/migrations/BaseMigrationBuilder.php | 11 +-- src/lib/migrations/MysqlMigrationBuilder.php | 87 +------------------ .../migrations/PostgresMigrationBuilder.php | 10 +-- 4 files changed, 17 insertions(+), 105 deletions(-) diff --git a/src/db/ColumnSchema.php b/src/db/ColumnSchema.php index 6ce1cb3a..90b43f06 100644 --- a/src/db/ColumnSchema.php +++ b/src/db/ColumnSchema.php @@ -27,18 +27,22 @@ class ColumnSchema extends \yii\db\ColumnSchema public $xDbType; /** - * TODO * Used only for MySQL/MariaDB * @var array|null * [ - * index => int - * after => ?string - * before => ?string + * index => int # position: starts from 1 + * after => ?string # after column + * before => ?string # before column * ] * If `before` is null then column is last + * If `after` is null then column is first + * If both are null then table has only 1 column */ public ?array $fromPosition = null; public ?array $toPosition = null; - public bool $isPositionReallyChanged = false; + /** + * From `$this->fromPosition` and `$this->toPosition` we can check if the position is changed or not. This is done in `BaseMigrationBuilder::setColumnsPositions()` + */ + public bool $isPositionChanged = false; } diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index d65582b1..89b20ccd 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -171,7 +171,7 @@ public function buildSecondary(?ManyToManyRelation $relation = null):MigrationMo { $this->migration = Yii::createObject(MigrationModel::class, [$this->model, false, $relation, []]); $this->newColumns = $relation->columnSchema ?? $this->model->attributesToColumnSchema(); - $this->setPositions(); + $this->setColumnsPositions(); $wantNames = array_keys($this->newColumns); $haveNames = $this->tableSchema->columnNames; $columnsForCreate = array_map( @@ -538,21 +538,18 @@ public function modifyDesiredInContextOfDesiredFromDb(ColumnSchema $desired, Col $desired->dbType = $desiredFromDb->dbType; } - /** - * TODO docs - */ - abstract public function handleColumnsPositionsChanges(array $haveNames, array $wantNames); - - /** * Only for MySQL and MariaDB * Given a column, compute its previous column name present in OpenAPI schema * @param ColumnSchema $column * @param bool $forDrop + * @param bool $forAlter * @return ?string * `null` if column is added at last * 'FIRST' if column is added at first position * 'AFTER ' if column is added in between e.g. if 'email' is added after 'username' then 'AFTER username' */ abstract public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $forAlter = false): ?string; + + abstract public function setColumnsPositions(); } diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 8ba6a0fa..2d8c6928 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -85,7 +85,7 @@ protected function compareColumns(ColumnSchema $current, ColumnSchema $desired): // $positionCurrent = $this->findPosition($desired, true); // $positionDesired = $this->findPosition($desired); // if ($positionCurrent !== $positionDesired) { - if ($desired->isPositionReallyChanged) { + if ($desired->isPositionChanged) { $changedAttributes[] = 'position'; } @@ -181,62 +181,11 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch } } - /** - * TODO - * Check if order/position of column is changed - * @return void - */ - public function checkOrder() - { - } - /** * {@inheritDoc} */ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $forAlter = false): ?string { -// if ($column instanceof \cebe\yii2openapi\db\ColumnSchema) { -// if (!$column->isPositionReallyChanged) { -// return null; -// } -// } -// if (!$forDrop) { -// $columnNames = array_keys($this->newColumns); -// $key = array_search($column->name, $columnNames); -// if (is_int($key)) { -// if ($key > 0) { -// $prevColName = $columnNames[$key - 1]; -// -// // if the perv col is last then no need for `'AFTER '` because last is the default position -// if (array_search($prevColName, $columnNames) === (count($columnNames) - 1)) { -// return null; -// } -// -// return self::POS_AFTER . ' ' . $prevColName; -// } elseif ($key === 0) { -// return self::POS_FIRST; -// } -// } -// -// } else { -// $columnNames = array_keys($this->tableSchema->columns); -// $key = array_search($column->name, $columnNames); -// if (is_int($key)) { -// if ($key > 0) { -// $prevColName = $columnNames[$key - 1]; -// -// if (array_search($prevColName, $columnNames) === count($columnNames) - 1) { -// return null; -// } -// -// return self::POS_AFTER . ' ' . $prevColName; -// } elseif ($key === 0) { -// return self::POS_FIRST; -// } -// } -// } -// return null; - $columnNames = array_keys($forDrop ? $this->tableSchema->columns : $this->newColumns); $key = array_search($column->name, $columnNames); @@ -246,10 +195,6 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $ return null; } -// if (!$forDrop && !isset($columnNames[$key + 1])) { // if new col is added at last then no need to add 'AFTER' SQL part. This is checked as if next column is present or not -// return null; -// } - if (array_key_exists($prevColName, $forDrop ? $this->tableSchema->columns : $this->newColumns)) { if ($forDrop && !$forAlter) { // if the previous column is the last one in the want names then no need for AFTER @@ -275,33 +220,7 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $ return null; } - - // TODO - public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) - { - $indices = []; - if ($haveNames !== $wantNames) { - foreach ($wantNames as $key => $name) { - if ($name !== $haveNames[$key]) { - $indices[] = $key; - } - } - } - for ($i = 0; $i < count($indices) / 2; $i++) { - $this->migration->addUpCode($this->recordBuilder->alterColumn( - $this->model->getTableAlias(), - $this->newColumns[$wantNames[$indices[$i]]], - $this->findPosition($this->newColumns[$wantNames[$indices[$i]]]) - ))->addDownCode($this->recordBuilder->alterColumn( - $this->model->getTableAlias(), - $this->tableSchema->columns[$wantNames[$indices[$i]]], - $this->findPosition($this->tableSchema->columns[$wantNames[$indices[$i]]], true) - )); - } -// $this->migration->addUpCode($this->recordBuilder->dropTable($this->model->getTableAlias())); - } - - public function setPositions() + public function setColumnsPositions() { $i = 0; $haveColumns = $this->tableSchema->columns; @@ -362,7 +281,7 @@ public function setPositions() continue; } - $column->isPositionReallyChanged = true; + $column->isPositionChanged = true; $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 7fbc6419..82dcfc76 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -248,14 +248,6 @@ public function modifyDesiredInContextOfCurrent(ColumnSchema $current, ColumnSch } } - /** - * {@inheritDoc} - */ - public function handleColumnsPositionsChanges(array $haveNames, array $wantNames) - { - } - - /** * {@inheritDoc} */ @@ -264,7 +256,7 @@ public function findPosition(ColumnSchema $column, bool $forDrop = false, bool $ return null; } - public function setPositions() + public function setColumnsPositions() { } } From 815907e7eb5b411142d72be3061ff38f8f5a3fd9 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Mon, 14 Oct 2024 18:05:23 +0530 Subject: [PATCH 24/26] Refactor --- src/lib/migrations/BaseMigrationBuilder.php | 2 - src/lib/migrations/MigrationRecordBuilder.php | 2 +- src/lib/migrations/MysqlMigrationBuilder.php | 90 +++++-------------- 3 files changed, 23 insertions(+), 71 deletions(-) diff --git a/src/lib/migrations/BaseMigrationBuilder.php b/src/lib/migrations/BaseMigrationBuilder.php index 89b20ccd..1bd09e9b 100644 --- a/src/lib/migrations/BaseMigrationBuilder.php +++ b/src/lib/migrations/BaseMigrationBuilder.php @@ -222,8 +222,6 @@ function (string $unknownColumn) { $this->buildRelations(); } -// $this->handleColumnsPositionsChanges($haveNames, $wantNames); - return $this->migration; } diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 0f7a2ca7..d624893b 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -158,7 +158,7 @@ public function alterColumnTypeFromDb( ColumnSchema $column, bool $addUsing = false, ?string $position = null - ) :string # TODO + ) :string { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position); diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 2d8c6928..352daed5 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -31,14 +31,6 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir unset($changed[$key]); } $newColumn = clone $current; -// $positionCurrent = $this->findPosition($desired, true); -// $positionDesired = $this->findPosition($desired); -// if ($positionCurrent === $positionDesired) { -// $positionCurrent = $positionDesired = null; -// } # else { -// $position = $positionDesired; -// $newColumn->position = $position; -// } foreach ($changed as $attr) { $newColumn->$attr = $desired->$attr; } @@ -82,10 +74,7 @@ protected function compareColumns(ColumnSchema $current, ColumnSchema $desired): } } -// $positionCurrent = $this->findPosition($desired, true); -// $positionDesired = $this->findPosition($desired); -// if ($positionCurrent !== $positionDesired) { - if ($desired->isPositionChanged) { + if (property_exists($desired, 'isPositionChanged') && $desired->isPositionChanged) { $changedAttributes[] = 'position'; } @@ -226,6 +215,8 @@ public function setColumnsPositions() $haveColumns = $this->tableSchema->columns; $wantNames = array_keys($this->newColumns); $haveNames = array_keys($haveColumns); + + // Part 1/2 compute from and to position foreach ($this->newColumns as $name => $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ $column->toPosition = [ @@ -246,6 +237,25 @@ public function setColumnsPositions() $i++; } + // Part 2/2 compute is position is really changed + + // check if only new columns are added without any explicit position change + $namesForCreate = array_diff($wantNames, $haveNames); + $wantNamesWoNewCols = array_values(array_diff($wantNames, $namesForCreate)); + if ($namesForCreate && $haveNames === $wantNamesWoNewCols) { + return; + } + // check if only existing columns are deleted without any explicit position change + $namesForDrop = array_diff($haveNames, $wantNames); + $haveNamesWoDropCols = array_values(array_diff($haveNames, $namesForDrop)); + if ($namesForDrop && $wantNames === $haveNamesWoDropCols) { + return; + } + // check both above simultaneously + if ($namesForCreate && $namesForDrop && ($wantNamesWoNewCols === $haveNamesWoDropCols)) { + return; + } + $takenIndices = []; foreach ($this->newColumns as $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ @@ -253,24 +263,6 @@ public function setColumnsPositions() if (!$column->fromPosition || !$column->toPosition) { continue; } - - // check if only new columns are added without any explicit position change - $namesForCreate = array_diff($wantNames, $haveNames); - $wantNamesWoNewCols = array_values(array_diff($wantNames, $namesForCreate)); - if ($namesForCreate && $haveNames === $wantNamesWoNewCols) { - continue; - } - // check if only existing columns are deleted without any explicit position change - $namesForDrop = array_diff($haveNames, $wantNames); - $haveNamesWoDropCols = array_values(array_diff($haveNames, $namesForDrop)); - if ($namesForDrop && $wantNames === $haveNamesWoDropCols) { - continue; - } - // check both above simultaneously - if ($namesForCreate && $namesForDrop && ($wantNamesWoNewCols === $haveNamesWoDropCols)) { - continue; - } - if (is_int(array_search([$column->toPosition['index'], $column->fromPosition['index']], $takenIndices))) { continue; } @@ -285,42 +277,4 @@ public function setColumnsPositions() $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } } - - public function checkAfterPosition($column) - { - if ($column->fromPosition['after'] === $column->toPosition['after'] - ) { - $afterColName = $column->toPosition['after']; - $afterCol = $this->newColumns[$afterColName] ?? null; - if ($afterCol) { - if ($this->checkAfterPosition($afterCol)) { - return true; - } else { - return false; - } - } else { - return true; - } - } - return false; - } - - public function checkBeforePosition($column) - { - if ($column->fromPosition['before'] === $column->toPosition['before'] - ) { - $beforeColName = $column->toPosition['before']; - $beforeCol = $this->newColumns[$beforeColName] ?? null; - if ($beforeCol) { - if ($this->checkBeforePosition($beforeCol)) { - return true; - } else { - return false; - } - } else { - return true; - } - } - return false; - } } From edbd140e3e13a6a0a9b859a58c3f8fa0fb84e4b3 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Tue, 15 Oct 2024 18:42:01 +0530 Subject: [PATCH 25/26] Add more tests + refactor `setColumnsPositions()` to remove unwanted migrations --- src/lib/migrations/MigrationRecordBuilder.php | 3 +- src/lib/migrations/MysqlMigrationBuilder.php | 29 ++- tests/unit/IssueFixTest.php | 192 +++++++++++++++++- 3 files changed, 217 insertions(+), 7 deletions(-) diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index d624893b..73fdce7d 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -158,8 +158,7 @@ public function alterColumnTypeFromDb( ColumnSchema $column, bool $addUsing = false, ?string $position = null - ) :string - { + ) :string { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true, $position); return sprintf( diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 352daed5..778a042f 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -256,7 +256,7 @@ public function setColumnsPositions() return; } - $takenIndices = []; + $takenIndices = $redundantIndices = []; # $redundantIndices are the unwanted ones which are created by moving of one or more columns. Example: if a column is moved from 2nd to 8th position then we will consider only one column is moved ignoring index/position change(-1) of 4rd to 8th column (4->3, 5->4 ...). So migration for this unwanted indices changes won't be generated foreach ($this->newColumns as $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ @@ -275,6 +275,33 @@ public function setColumnsPositions() $column->isPositionChanged = true; $takenIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; + + // ------- + if (($column->fromPosition['before'] !== $column->toPosition['before']) && + ($column->fromPosition['after'] !== $column->toPosition['after']) + ) { + $redundantIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; + } + } + + foreach ($this->newColumns as $column) { + /** @var \cebe\yii2openapi\db\ColumnSchema $column */ + + if (!isset($column->toPosition['index'], $column->fromPosition['index'])) { + continue; + } + $condition = (abs($column->toPosition['index'] - $column->fromPosition['index']) === count($redundantIndices)); + if (($column->fromPosition['before'] === $column->toPosition['before']) + && $condition + ) { + $column->isPositionChanged = false; + continue; + } + if (($column->fromPosition['after'] === $column->toPosition['after']) + && $condition + ) { + $column->isPositionChanged = false; + } } } } diff --git a/tests/unit/IssueFixTest.php b/tests/unit/IssueFixTest.php index f1d2410c..1c745376 100644 --- a/tests/unit/IssueFixTest.php +++ b/tests/unit/IssueFixTest.php @@ -1273,14 +1273,10 @@ public function up() $this->addColumn('{{%fruits}}', 'col_6', $this->boolean()->null()->defaultValue(null)); $this->dropColumn('{{%fruits}}', 'size'); $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); - $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); - $this->alterColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); } public function down() { - $this->alterColumn('{{%fruits}}', 'description', $this->tinyInteger(1)->null()->defaultValue(null)->after('name')); - $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); $this->addColumn('{{%fruits}}', 'size', $this->tinyInteger(1)->null()->defaultValue(null)); $this->dropColumn('{{%fruits}}', 'col_6'); @@ -1493,4 +1489,192 @@ public function down() // // $this->for58($schema, $expected, $columns); // } + + public function test58MoveAColAndChangeItsDataType() + { + $columns = [ + 'id' => 'pk', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + ]; + + $schema = <<alterColumn('{{%fruits}}', 'colour', $this->integer()->null()->defaultValue(null)); + $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); + } + + public function test58MoveAColDownwards() + { + $columns = [ + 'id' => 'pk', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + ]; + + $schema = <<alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('colour')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'name', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); + } + + public function test58MoveAColUpwards() + { + $columns = [ + 'id' => 'pk', + 'name' => 'bool null', + 'description' => 'bool null', + 'colour' => 'bool null', + 'size' => 'bool null', + ]; + + $schema = <<alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('id')); + } + + public function down() + { + $this->alterColumn('{{%fruits}}', 'colour', $this->tinyInteger(1)->null()->defaultValue(null)->after('description')); + } +} + +PHP; + + $this->for58($schema, $expected, $columns); + } } From 7cb8ab86b71e5b133a0d75f04e9348a6d311e360 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Tue, 15 Oct 2024 18:46:39 +0530 Subject: [PATCH 26/26] Modify variable name --- src/lib/migrations/MysqlMigrationBuilder.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/migrations/MysqlMigrationBuilder.php b/src/lib/migrations/MysqlMigrationBuilder.php index 778a042f..b28040a0 100644 --- a/src/lib/migrations/MysqlMigrationBuilder.php +++ b/src/lib/migrations/MysqlMigrationBuilder.php @@ -256,7 +256,7 @@ public function setColumnsPositions() return; } - $takenIndices = $redundantIndices = []; # $redundantIndices are the unwanted ones which are created by moving of one or more columns. Example: if a column is moved from 2nd to 8th position then we will consider only one column is moved ignoring index/position change(-1) of 4rd to 8th column (4->3, 5->4 ...). So migration for this unwanted indices changes won't be generated + $takenIndices = $nonRedundantIndices = []; # $nonRedundantIndices are the wanted ones which are created by moving of one or more columns. Example: if a column is moved from 2nd to 8th position then we will consider only one column is moved ignoring index/position change(-1) of 4rd to 8th column (4->3, 5->4 ...). So migration for this unwanted indices changes won't be generated. `$takenIndices` might have redundant indices foreach ($this->newColumns as $column) { /** @var \cebe\yii2openapi\db\ColumnSchema $column */ @@ -280,7 +280,7 @@ public function setColumnsPositions() if (($column->fromPosition['before'] !== $column->toPosition['before']) && ($column->fromPosition['after'] !== $column->toPosition['after']) ) { - $redundantIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; + $nonRedundantIndices[] = [$column->fromPosition['index'], $column->toPosition['index']]; } } @@ -290,7 +290,7 @@ public function setColumnsPositions() if (!isset($column->toPosition['index'], $column->fromPosition['index'])) { continue; } - $condition = (abs($column->toPosition['index'] - $column->fromPosition['index']) === count($redundantIndices)); + $condition = (abs($column->toPosition['index'] - $column->fromPosition['index']) === count($nonRedundantIndices)); if (($column->fromPosition['before'] === $column->toPosition['before']) && $condition ) {