Commit b458675b authored by Andreas Brandl's avatar Andreas Brandl

Merge branch 'ab/versioned-migration-class' into 'master'

Introduce versioned GitLab migration class

See merge request gitlab-org/gitlab!68986
parents 3e46f22b 3af8ec8b
...@@ -40,9 +40,7 @@ It's recommended to create two separate migration script files. ...@@ -40,9 +40,7 @@ It's recommended to create two separate migration script files.
desired limit using `create_or_update_plan_limit` migration helper, such as: desired limit using `create_or_update_plan_limit` migration helper, such as:
```ruby ```ruby
class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2] class InsertProjectHooksPlanLimits < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
create_or_update_plan_limit('project_hooks', 'default', 0) create_or_update_plan_limit('project_hooks', 'default', 0)
create_or_update_plan_limit('project_hooks', 'free', 10) create_or_update_plan_limit('project_hooks', 'free', 10)
......
...@@ -95,9 +95,7 @@ renaming. For example ...@@ -95,9 +95,7 @@ renaming. For example
```ruby ```ruby
# A regular migration in db/migrate # A regular migration in db/migrate
class RenameUsersUpdatedAtToUpdatedAtTimestamp < ActiveRecord::Migration[4.2] class RenameUsersUpdatedAtToUpdatedAtTimestamp < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -125,9 +123,7 @@ We can perform this cleanup using ...@@ -125,9 +123,7 @@ We can perform this cleanup using
```ruby ```ruby
# A post-deployment migration in db/post_migrate # A post-deployment migration in db/post_migrate
class CleanupUsersUpdatedAtRename < ActiveRecord::Migration[4.2] class CleanupUsersUpdatedAtRename < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -174,9 +170,7 @@ as follows: ...@@ -174,9 +170,7 @@ as follows:
```ruby ```ruby
# A regular migration in db/migrate # A regular migration in db/migrate
class ChangeUsersUsernameStringToText < ActiveRecord::Migration[4.2] class ChangeUsersUsernameStringToText < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -195,9 +189,7 @@ Next we need to clean up our changes using a post-deployment migration: ...@@ -195,9 +189,7 @@ Next we need to clean up our changes using a post-deployment migration:
```ruby ```ruby
# A post-deployment migration in db/post_migrate # A post-deployment migration in db/post_migrate
class ChangeUsersUsernameStringToTextCleanup < ActiveRecord::Migration[4.2] class ChangeUsersUsernameStringToTextCleanup < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -245,9 +237,7 @@ the work / load over a longer time period, without slowing down deployments. ...@@ -245,9 +237,7 @@ the work / load over a longer time period, without slowing down deployments.
For example, to change the column type using a background migration: For example, to change the column type using a background migration:
```ruby ```ruby
class ExampleMigration < ActiveRecord::Migration[4.2] class ExampleMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
class Issue < ActiveRecord::Base class Issue < ActiveRecord::Base
...@@ -289,9 +279,7 @@ release) by a cleanup migration, which should steal from the queue and handle ...@@ -289,9 +279,7 @@ release) by a cleanup migration, which should steal from the queue and handle
any remaining rows. For example: any remaining rows. For example:
```ruby ```ruby
class MigrateRemainingIssuesClosedAt < ActiveRecord::Migration[4.2] class MigrateRemainingIssuesClosedAt < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
class Issue < ActiveRecord::Base class Issue < ActiveRecord::Base
......
...@@ -254,7 +254,7 @@ existing data. Since we're dealing with a lot of rows we'll schedule jobs in ...@@ -254,7 +254,7 @@ existing data. Since we're dealing with a lot of rows we'll schedule jobs in
batches instead of doing this one by one: batches instead of doing this one by one:
```ruby ```ruby
class ScheduleExtractServicesUrl < ActiveRecord::Migration[4.2] class ScheduleExtractServicesUrl < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -281,7 +281,7 @@ jobs and manually run on any un-migrated rows. Such a migration would look like ...@@ -281,7 +281,7 @@ jobs and manually run on any un-migrated rows. Such a migration would look like
this: this:
```ruby ```ruby
class ConsumeRemainingExtractServicesUrlJobs < ActiveRecord::Migration[4.2] class ConsumeRemainingExtractServicesUrlJobs < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -463,8 +463,6 @@ end ...@@ -463,8 +463,6 @@ end
```ruby ```ruby
# Post deployment migration # Post deployment migration
include Gitlab::Database::MigrationHelpers
MIGRATION = 'YourBackgroundMigrationName' MIGRATION = 'YourBackgroundMigrationName'
DELAY_INTERVAL = 2.minutes.to_i # can be different DELAY_INTERVAL = 2.minutes.to_i # can be different
BATCH_SIZE = 10_000 # can be different BATCH_SIZE = 10_000 # can be different
...@@ -494,8 +492,6 @@ You can reschedule pending migrations from the `background_migration_jobs` table ...@@ -494,8 +492,6 @@ You can reschedule pending migrations from the `background_migration_jobs` table
```ruby ```ruby
# Post deployment migration # Post deployment migration
include Gitlab::Database::MigrationHelpers
MIGRATION = 'YourBackgroundMigrationName' MIGRATION = 'YourBackgroundMigrationName'
DELAY_INTERVAL = 2.minutes DELAY_INTERVAL = 2.minutes
......
...@@ -38,7 +38,7 @@ Settings are not cascading by default. To define a cascading setting, take the f ...@@ -38,7 +38,7 @@ Settings are not cascading by default. To define a cascading setting, take the f
`application_settings`. `application_settings`.
```ruby ```ruby
class AddDelayedProjectRemovalCascadingSetting < ActiveRecord::Migration[6.0] class AddDelayedProjectRemovalCascadingSetting < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers::CascadingNamespaceSettings include Gitlab::Database::MigrationHelpers::CascadingNamespaceSettings
def up def up
......
...@@ -66,9 +66,7 @@ In the example above, you'd be still able to update records in the `emails` tabl ...@@ -66,9 +66,7 @@ In the example above, you'd be still able to update records in the `emails` tabl
Migration file for adding `NOT VALID` foreign key: Migration file for adding `NOT VALID` foreign key:
```ruby ```ruby
class AddNotValidForeignKeyToEmailsUser < ActiveRecord::Migration[5.2] class AddNotValidForeignKeyToEmailsUser < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
# safe to use: it requires short lock on the table since we don't validate the foreign key # safe to use: it requires short lock on the table since we don't validate the foreign key
add_foreign_key :emails, :users, on_delete: :cascade, validate: false add_foreign_key :emails, :users, on_delete: :cascade, validate: false
...@@ -92,9 +90,7 @@ In case the data volume is higher (>1000 records), it's better to create a backg ...@@ -92,9 +90,7 @@ In case the data volume is higher (>1000 records), it's better to create a backg
Example for cleaning up records in the `emails` table within a database migration: Example for cleaning up records in the `emails` table within a database migration:
```ruby ```ruby
class RemoveRecordsWithoutUserFromEmailsTable < ActiveRecord::Migration[5.2] class RemoveRecordsWithoutUserFromEmailsTable < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
class Email < ActiveRecord::Base class Email < ActiveRecord::Base
...@@ -126,9 +122,7 @@ Migration file for validating the foreign key: ...@@ -126,9 +122,7 @@ Migration file for validating the foreign key:
```ruby ```ruby
# frozen_string_literal: true # frozen_string_literal: true
class ValidateForeignKeyOnEmailUsers < ActiveRecord::Migration[5.2] class ValidateForeignKeyOnEmailUsers < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
validate_foreign_key :emails, :user_id validate_foreign_key :emails, :user_id
end end
......
...@@ -25,7 +25,7 @@ For example, consider a migration that creates a table with two `NOT NULL` colum ...@@ -25,7 +25,7 @@ For example, consider a migration that creates a table with two `NOT NULL` colum
`db/migrate/20200401000001_create_db_guides.rb`: `db/migrate/20200401000001_create_db_guides.rb`:
```ruby ```ruby
class CreateDbGuides < ActiveRecord::Migration[6.0] class CreateDbGuides < Gitlab::Database::Migration[1.0]
def change def change
create_table :db_guides do |t| create_table :db_guides do |t|
t.bigint :stars, default: 0, null: false t.bigint :stars, default: 0, null: false
...@@ -44,7 +44,7 @@ For example, consider a migration that adds a new `NOT NULL` column `active` to ...@@ -44,7 +44,7 @@ For example, consider a migration that adds a new `NOT NULL` column `active` to
`db/migrate/20200501000001_add_active_to_db_guides.rb`: `db/migrate/20200501000001_add_active_to_db_guides.rb`:
```ruby ```ruby
class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0] class AddExtendedTitleToSprints < Gitlab::Database::Migration[1.0]
def change def change
add_column :db_guides, :active, :boolean, default: true, null: false add_column :db_guides, :active, :boolean, default: true, null: false
end end
...@@ -111,9 +111,7 @@ with `validate: false` in a post-deployment migration, ...@@ -111,9 +111,7 @@ with `validate: false` in a post-deployment migration,
`db/post_migrate/20200501000001_add_not_null_constraint_to_epics_description.rb`: `db/post_migrate/20200501000001_add_not_null_constraint_to_epics_description.rb`:
```ruby ```ruby
class AddNotNullConstraintToEpicsDescription < ActiveRecord::Migration[6.0] class AddNotNullConstraintToEpicsDescription < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -144,9 +142,7 @@ so we are going to add a post-deployment migration for the 13.0 milestone (curre ...@@ -144,9 +142,7 @@ so we are going to add a post-deployment migration for the 13.0 milestone (curre
`db/post_migrate/20200501000002_cleanup_epics_with_null_description.rb`: `db/post_migrate/20200501000002_cleanup_epics_with_null_description.rb`:
```ruby ```ruby
class CleanupEpicsWithNullDescription < ActiveRecord::Migration[6.0] class CleanupEpicsWithNullDescription < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
# With BATCH_SIZE=1000 and epics.count=29500 on GitLab.com # With BATCH_SIZE=1000 and epics.count=29500 on GitLab.com
# - 30 iterations will be run # - 30 iterations will be run
# - each requires on average ~150ms # - each requires on average ~150ms
...@@ -184,9 +180,7 @@ migration helper in a final post-deployment migration, ...@@ -184,9 +180,7 @@ migration helper in a final post-deployment migration,
`db/post_migrate/20200601000001_validate_not_null_constraint_on_epics_description.rb`: `db/post_migrate/20200601000001_validate_not_null_constraint_on_epics_description.rb`:
```ruby ```ruby
class ValidateNotNullConstraintOnEpicsDescription < ActiveRecord::Migration[6.0] class ValidateNotNullConstraintOnEpicsDescription < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
......
...@@ -60,8 +60,6 @@ Consider the next release as "Release N.M". ...@@ -60,8 +60,6 @@ Consider the next release as "Release N.M".
Execute a standard migration (not a post-migration): Execute a standard migration (not a post-migration):
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
rename_table_safely(:issues, :tickets) rename_table_safely(:issues, :tickets)
end end
...@@ -96,8 +94,6 @@ At this point, we don't have applications using the old database table name in t ...@@ -96,8 +94,6 @@ At this point, we don't have applications using the old database table name in t
1. Remove the database view through a post-migration: 1. Remove the database view through a post-migration:
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
finalize_table_rename(:issues, :tickets) finalize_table_rename(:issues, :tickets)
end end
......
...@@ -47,9 +47,7 @@ For example, consider a migration that creates a table with two text columns, ...@@ -47,9 +47,7 @@ For example, consider a migration that creates a table with two text columns,
`db/migrate/20200401000001_create_db_guides.rb`: `db/migrate/20200401000001_create_db_guides.rb`:
```ruby ```ruby
class CreateDbGuides < ActiveRecord::Migration[6.0] class CreateDbGuides < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
create_table_with_constraints :db_guides do |t| create_table_with_constraints :db_guides do |t|
t.bigint :stars, default: 0, null: false t.bigint :stars, default: 0, null: false
...@@ -84,7 +82,7 @@ For example, consider a migration that adds a new text column `extended_title` t ...@@ -84,7 +82,7 @@ For example, consider a migration that adds a new text column `extended_title` t
`db/migrate/20200501000001_add_extended_title_to_sprints.rb`: `db/migrate/20200501000001_add_extended_title_to_sprints.rb`:
```ruby ```ruby
class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0] class AddExtendedTitleToSprints < Gitlab::Database::Migration[1.0]
# rubocop:disable Migration/AddLimitToTextColumns # rubocop:disable Migration/AddLimitToTextColumns
# limit is added in 20200501000002_add_text_limit_to_sprints_extended_title # limit is added in 20200501000002_add_text_limit_to_sprints_extended_title
...@@ -99,8 +97,7 @@ A second migration should follow the first one with a limit added to `extended_t ...@@ -99,8 +97,7 @@ A second migration should follow the first one with a limit added to `extended_t
`db/migrate/20200501000002_add_text_limit_to_sprints_extended_title.rb`: `db/migrate/20200501000002_add_text_limit_to_sprints_extended_title.rb`:
```ruby ```ruby
class AddTextLimitToSprintsExtendedTitle < ActiveRecord::Migration[6.0] class AddTextLimitToSprintsExtendedTitle < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -175,9 +172,7 @@ in a post-deployment migration, ...@@ -175,9 +172,7 @@ in a post-deployment migration,
`db/post_migrate/20200501000001_add_text_limit_migration.rb`: `db/post_migrate/20200501000001_add_text_limit_migration.rb`:
```ruby ```ruby
class AddTextLimitMigration < ActiveRecord::Migration[6.0] class AddTextLimitMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -208,9 +203,7 @@ to add a background migration for the 13.0 milestone (current), ...@@ -208,9 +203,7 @@ to add a background migration for the 13.0 milestone (current),
`db/post_migrate/20200501000002_schedule_cap_title_length_on_issues.rb`: `db/post_migrate/20200501000002_schedule_cap_title_length_on_issues.rb`:
```ruby ```ruby
class ScheduleCapTitleLengthOnIssues < ActiveRecord::Migration[6.0] class ScheduleCapTitleLengthOnIssues < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
# Info on how many records will be affected on GitLab.com # Info on how many records will be affected on GitLab.com
# time each batch needs to run on average, etc ... # time each batch needs to run on average, etc ...
BATCH_SIZE = 5000 BATCH_SIZE = 5000
...@@ -255,9 +248,7 @@ helper in a final post-deployment migration, ...@@ -255,9 +248,7 @@ helper in a final post-deployment migration,
`db/post_migrate/20200601000001_validate_text_limit_migration.rb`: `db/post_migrate/20200601000001_validate_text_limit_migration.rb`:
```ruby ```ruby
class ValidateTextLimitMigration < ActiveRecord::Migration[6.0] class ValidateTextLimitMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
......
...@@ -173,7 +173,7 @@ An example migration of partitioning the `audit_events` table by its ...@@ -173,7 +173,7 @@ An example migration of partitioning the `audit_events` table by its
`created_at` column would look like: `created_at` column would look like:
```ruby ```ruby
class PartitionAuditEvents < ActiveRecord::Migration[6.0] class PartitionAuditEvents < Gitlab::Database::Migration[1.0]
include Gitlab::Database::PartitioningMigrationHelpers include Gitlab::Database::PartitioningMigrationHelpers
def up def up
...@@ -200,7 +200,7 @@ into the partitioned copy. ...@@ -200,7 +200,7 @@ into the partitioned copy.
Continuing the above example, the migration would look like: Continuing the above example, the migration would look like:
```ruby ```ruby
class BackfillPartitionAuditEvents < ActiveRecord::Migration[6.0] class BackfillPartitionAuditEvents < Gitlab::Database::Migration[1.0]
include Gitlab::Database::PartitioningMigrationHelpers include Gitlab::Database::PartitioningMigrationHelpers
def up def up
...@@ -233,7 +233,7 @@ failed jobs. ...@@ -233,7 +233,7 @@ failed jobs.
Once again, continuing the example, this migration would look like: Once again, continuing the example, this migration would look like:
```ruby ```ruby
class CleanupPartitionedAuditEventsBackfill < ActiveRecord::Migration[6.0] class CleanupPartitionedAuditEventsBackfill < Gitlab::Database::Migration[1.0]
include Gitlab::Database::PartitioningMigrationHelpers include Gitlab::Database::PartitioningMigrationHelpers
def up def up
......
...@@ -70,7 +70,7 @@ graph LR ...@@ -70,7 +70,7 @@ graph LR
H -->|Yes| E[Regular migration] H -->|Yes| E[Regular migration]
H -->|No| I[Post-deploy migration<br/>+ feature flag] H -->|No| I[Post-deploy migration<br/>+ feature flag]
D -->|Yes| F[Post-deploy migration] D -->|Yes| F[Post-deploy migration]
D -->|No| G[Background migration] D -->|No| G[Background migration]
``` ```
...@@ -217,6 +217,39 @@ In case you need to insert, update, or delete a significant amount of data, you: ...@@ -217,6 +217,39 @@ In case you need to insert, update, or delete a significant amount of data, you:
- Must disable the single transaction with `disable_ddl_transaction!`. - Must disable the single transaction with `disable_ddl_transaction!`.
- Should consider doing it in a [Background Migration](background_migrations.md). - Should consider doing it in a [Background Migration](background_migrations.md).
## Migration helpers and versioning
Various helper methods are available for many common patterns in database migrations. Those
helpers can be found in `Gitlab::Database::MigrationHelpers` and related modules.
In order to allow changing a helper's behavior over time, we implement a versioning scheme
for migration helpers. This allows us to maintain the behavior of a helper for already
existing migrations but change the behavior for any new migrations.
For that purpose, all database migrations should inherit from `Gitlab::Database::Migration`,
which is a "versioned" class. For new migrations, the latest version should be used (which
can be looked up in `Gitlab::Database::Migration::MIGRATION_CLASSES`) to use the latest version
of migration helpers.
In this example, we use version 1.0 of the migration class:
```ruby
class TestMigration < Gitlab::Database::Migration[1.0]
def change
end
end
```
NOTE:
It is discouraged to include `Gitlab::Database::MigrationHelpers` directly into a
migration. Instead, the latest version of `Gitlab::Database::Migration` will expose the latest
version of migration helpers automatically.
NOTE:
Migration helpers and versioning are available starting from [14.3](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68986).
For merge requests targeting previous stable branches, the old format needs to be used and we continue
to inherit from `ActiveRecord::Migration[6.1]` instead of `Gitlab::Database::Migration[1.0]` for those.
## Retry mechanism when acquiring database locks ## Retry mechanism when acquiring database locks
When changing the database schema, we use helper methods to invoke DDL (Data Definition When changing the database schema, we use helper methods to invoke DDL (Data Definition
...@@ -256,8 +289,6 @@ lock allow the database to process other statements. ...@@ -256,8 +289,6 @@ lock allow the database to process other statements.
**Removing a column:** **Removing a column:**
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
with_lock_retries do with_lock_retries do
remove_column :users, :full_name remove_column :users, :full_name
...@@ -278,8 +309,6 @@ you should do as much as possible inside the transaction rather than trying to g ...@@ -278,8 +309,6 @@ you should do as much as possible inside the transaction rather than trying to g
Be careful about running long database statements within the block. The acquired locks are kept until the transaction (block) finishes and depending on the lock type, it might block other database operations. Be careful about running long database statements within the block. The acquired locks are kept until the transaction (block) finishes and depending on the lock type, it might block other database operations.
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
with_lock_retries do with_lock_retries do
add_column :users, :full_name, :string add_column :users, :full_name, :string
...@@ -298,8 +327,6 @@ end ...@@ -298,8 +327,6 @@ end
**Removing a foreign key:** **Removing a foreign key:**
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
with_lock_retries do with_lock_retries do
remove_foreign_key :issues, :projects remove_foreign_key :issues, :projects
...@@ -316,8 +343,6 @@ end ...@@ -316,8 +343,6 @@ end
**Changing default value for a column:** **Changing default value for a column:**
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
def up def up
with_lock_retries do with_lock_retries do
change_column_default :merge_requests, :lock_version, from: nil, to: 0 change_column_default :merge_requests, :lock_version, from: nil, to: 0
...@@ -387,8 +412,6 @@ We can use the `add_concurrent_foreign_key` method in this case, as this helper ...@@ -387,8 +412,6 @@ We can use the `add_concurrent_foreign_key` method in this case, as this helper
has the lock retries built into it. has the lock retries built into it.
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -405,8 +428,6 @@ end ...@@ -405,8 +428,6 @@ end
Adding foreign key to `users`: Adding foreign key to `users`:
```ruby ```ruby
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
def up def up
...@@ -498,11 +519,11 @@ by calling the method `disable_ddl_transaction!` in the body of your migration ...@@ -498,11 +519,11 @@ by calling the method `disable_ddl_transaction!` in the body of your migration
class like so: class like so:
```ruby ```ruby
class MyMigration < ActiveRecord::Migration[6.0] class MyMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
INDEX_NAME = 'index_name' INDEX_NAME = 'index_name'
def up def up
remove_concurrent_index :table_name, :column_name, name: INDEX_NAME remove_concurrent_index :table_name, :column_name, name: INDEX_NAME
end end
...@@ -549,7 +570,7 @@ by calling the method `disable_ddl_transaction!` in the body of your migration ...@@ -549,7 +570,7 @@ by calling the method `disable_ddl_transaction!` in the body of your migration
class like so: class like so:
```ruby ```ruby
class MyMigration < ActiveRecord::Migration[6.0] class MyMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers include Gitlab::Database::MigrationHelpers
disable_ddl_transaction! disable_ddl_transaction!
...@@ -594,7 +615,7 @@ The easiest way to test for existence of an index by name is to use the ...@@ -594,7 +615,7 @@ The easiest way to test for existence of an index by name is to use the
be used with a name option. For example: be used with a name option. For example:
```ruby ```ruby
class MyMigration < ActiveRecord::Migration[6.0] class MyMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers include Gitlab::Database::MigrationHelpers
INDEX_NAME = 'index_name' INDEX_NAME = 'index_name'
...@@ -631,7 +652,7 @@ Here's an example where we add a new column with a foreign key ...@@ -631,7 +652,7 @@ Here's an example where we add a new column with a foreign key
constraint. Note it includes `index: true` to create an index for it. constraint. Note it includes `index: true` to create an index for it.
```ruby ```ruby
class Migration < ActiveRecord::Migration[6.0] class Migration < Gitlab::Database::Migration[1.0]
def change def change
add_reference :model, :other_model, index: true, foreign_key: { on_delete: :cascade } add_reference :model, :other_model, index: true, foreign_key: { on_delete: :cascade }
...@@ -677,7 +698,7 @@ expensive and disruptive operation for larger tables, but in reality it's not. ...@@ -677,7 +698,7 @@ expensive and disruptive operation for larger tables, but in reality it's not.
Take the following migration as an example: Take the following migration as an example:
```ruby ```ruby
class DefaultRequestAccessGroups < ActiveRecord::Migration[5.2] class DefaultRequestAccessGroups < Gitlab::Database::Migration[1.0]
def change def change
change_column_default(:namespaces, :request_access_enabled, from: false, to: true) change_column_default(:namespaces, :request_access_enabled, from: false, to: true)
end end
...@@ -884,7 +905,7 @@ The Rails 5 natively supports `JSONB` (binary JSON) column type. ...@@ -884,7 +905,7 @@ The Rails 5 natively supports `JSONB` (binary JSON) column type.
Example migration adding this column: Example migration adding this column:
```ruby ```ruby
class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0] class AddOptionsToBuildMetadata < Gitlab::Database::Migration[1.0]
def change def change
add_column :ci_builds_metadata, :config_options, :jsonb add_column :ci_builds_metadata, :config_options, :jsonb
end end
...@@ -916,7 +937,7 @@ Do not store `attr_encrypted` attributes as `:text` in the database; use ...@@ -916,7 +937,7 @@ Do not store `attr_encrypted` attributes as `:text` in the database; use
efficient: efficient:
```ruby ```ruby
class AddSecretToSomething < ActiveRecord::Migration[5.0] class AddSecretToSomething < Gitlab::Database::Migration[1.0]
def change def change
add_column :something, :encrypted_secret, :binary add_column :something, :encrypted_secret, :binary
add_column :something, :encrypted_secret_iv, :binary add_column :something, :encrypted_secret_iv, :binary
...@@ -974,7 +995,7 @@ If you need more complex logic, you can define and use models local to a ...@@ -974,7 +995,7 @@ If you need more complex logic, you can define and use models local to a
migration. For example: migration. For example:
```ruby ```ruby
class MyMigration < ActiveRecord::Migration[6.0] class MyMigration < Gitlab::Database::Migration[1.0]
class Project < ActiveRecord::Base class Project < ActiveRecord::Base
self.table_name = 'projects' self.table_name = 'projects'
end end
...@@ -1073,7 +1094,7 @@ in a previous migration. ...@@ -1073,7 +1094,7 @@ in a previous migration.
It is important not to leave out the `User.reset_column_information` command, in order to ensure that the old schema is dropped from the cache and ActiveRecord loads the updated schema information. It is important not to leave out the `User.reset_column_information` command, in order to ensure that the old schema is dropped from the cache and ActiveRecord loads the updated schema information.
```ruby ```ruby
class AddAndSeedMyColumn < ActiveRecord::Migration[6.0] class AddAndSeedMyColumn < Gitlab::Database::Migration[1.0]
class User < ActiveRecord::Base class User < ActiveRecord::Base
self.table_name = 'users' self.table_name = 'users'
end end
......
...@@ -36,9 +36,7 @@ After you add or change an existing common metric, you must [re-run the import s ...@@ -36,9 +36,7 @@ After you add or change an existing common metric, you must [re-run the import s
Or, you can create a database migration: Or, you can create a database migration:
```ruby ```ruby
class ImportCommonMetrics < ActiveRecord::Migration[4.2] class ImportCommonMetrics < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
end end
......
...@@ -1002,9 +1002,7 @@ When renaming queues, use the `sidekiq_queue_migrate` helper migration method ...@@ -1002,9 +1002,7 @@ When renaming queues, use the `sidekiq_queue_migrate` helper migration method
in a **post-deployment migration**: in a **post-deployment migration**:
```ruby ```ruby
class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0] class MigrateTheRenamedSidekiqQueue < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
def up def up
sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name' sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name'
end end
......
...@@ -31,7 +31,7 @@ could result in loading unexpected code or associations which may cause unintend ...@@ -31,7 +31,7 @@ could result in loading unexpected code or associations which may cause unintend
side effects or failures during upgrades. side effects or failures during upgrades.
```ruby ```ruby
class SomeMigration < ActiveRecord::Migration[6.0] class SomeMigration < Gitlab::Database::Migration[1.0]
class Services < ActiveRecord::Base class Services < ActiveRecord::Base
self.table_name = 'services' self.table_name = 'services'
self.inheritance_column = :_type_disabled self.inheritance_column = :_type_disabled
...@@ -47,7 +47,7 @@ This ensures that the migration loads the columns for the migration in isolation ...@@ -47,7 +47,7 @@ This ensures that the migration loads the columns for the migration in isolation
and the helper disables STI by default. and the helper disables STI by default.
```ruby ```ruby
class EnqueueSomeBackgroundMigration < ActiveRecord::Migration[6.0] class EnqueueSomeBackgroundMigration < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! disable_ddl_transaction!
def up def up
......
...@@ -102,7 +102,7 @@ transaction. Transactions for migrations can be disabled using the following ...@@ -102,7 +102,7 @@ transaction. Transactions for migrations can be disabled using the following
pattern: pattern:
```ruby ```ruby
class MigrationName < ActiveRecord::Migration[4.2] class MigrationName < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! disable_ddl_transaction!
end end
``` ```
...@@ -110,7 +110,7 @@ end ...@@ -110,7 +110,7 @@ end
For example: For example:
```ruby ```ruby
class AddUsersLowerUsernameEmailIndexes < ActiveRecord::Migration[4.2] class AddUsersLowerUsernameEmailIndexes < Gitlab::Database::Migration[1.0]
disable_ddl_transaction! disable_ddl_transaction!
def up def up
......
...@@ -3,10 +3,7 @@ ...@@ -3,10 +3,7 @@
# See https://docs.gitlab.com/ee/development/migration_style_guide.html # See https://docs.gitlab.com/ee/development/migration_style_guide.html
# for more information on how to write migrations for GitLab. # for more information on how to write migrations for GitLab.
class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>]
# Uncomment the following include if you require helper functions:
# include Gitlab::Database::MigrationHelpers
# When using the methods "add_concurrent_index" or "remove_concurrent_index" # When using the methods "add_concurrent_index" or "remove_concurrent_index"
# you must disable the use of transactions # you must disable the use of transactions
# as these methods can not run in an existing transaction. # as these methods can not run in an existing transaction.
......
...@@ -3,10 +3,7 @@ ...@@ -3,10 +3,7 @@
# See https://docs.gitlab.com/ee/development/migration_style_guide.html # See https://docs.gitlab.com/ee/development/migration_style_guide.html
# for more information on how to write migrations for GitLab. # for more information on how to write migrations for GitLab.
class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>] class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>]
# Uncomment the following include if you require helper functions:
# include Gitlab::Database::MigrationHelpers
# When using the methods "add_concurrent_index" or "remove_concurrent_index" # When using the methods "add_concurrent_index" or "remove_concurrent_index"
# you must disable the use of transactions # you must disable the use of transactions
# as these methods can not run in an existing transaction. # as these methods can not run in an existing transaction.
...@@ -20,6 +17,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi ...@@ -20,6 +17,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi
# comments: # comments:
# disable_ddl_transaction! # disable_ddl_transaction!
def change def up
end
def down
end end
end end
# frozen_string_literal: true
module Gitlab
module Database
class Migration
# This implements a simple versioning scheme for migration helpers.
#
# We need to be able to version helpers, so we can change their behavior without
# altering the behavior of already existing migrations in incompatible ways.
#
# We can continue to change the behavior of helpers without bumping the version here,
# *if* the change is backwards-compatible.
#
# If not, we would typically override the helper method in a new MigrationHelpers::V[0-9]+
# class and create a new entry with a bumped version below.
#
# We use major version bumps to indicate significant changes and minor version bumps
# to indicate backwards-compatible or otherwise minor changes (e.g. a Rails version bump).
# However, this hasn't been strictly formalized yet.
MIGRATION_CLASSES = {
1.0 => Class.new(ActiveRecord::Migration[6.1]) do
include Gitlab::Database::MigrationHelpers::V2
end
}.freeze
def self.[](version)
MIGRATION_CLASSES[version] || raise(ArgumentError, "Unknown migration version: #{version}")
end
# The current version to be used in new migrations
def self.current_version
1.0
end
end
end
end
# frozen_string_literal: true
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
class VersionedMigrationClass < RuboCop::Cop::Cop
include MigrationHelpers
ENFORCED_SINCE = 2021_09_02_00_00_00
MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
MIGRATION_CLASS = 'Gitlab::Database::Migration'
def_node_search :includes_helpers?, <<~PATTERN
(send nil? :include
(const
(const
(const nil? :Gitlab) :Database) :MigrationHelpers))
PATTERN
def on_class(node)
return unless relevant_migration?(node)
add_offense(node, location: :expression, message: MSG_INHERIT) unless gitlab_migration_class?(node)
end
def on_send(node)
return unless relevant_migration?(node)
add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_helpers?(node)
end
private
def relevant_migration?(node)
in_migration?(node) && version(node) >= ENFORCED_SINCE
end
def gitlab_migration_class?(node)
superclass(node) == MIGRATION_CLASS
end
def superclass(class_node)
_, *others = class_node.descendants
others.find { |node| node.const_type? && node&.const_name != 'Types' }&.const_name
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::Migration do
describe '.[]' do
context 'version: 1.0' do
subject { described_class[1.0] }
it 'inherits from ActiveRecord::Migration[6.1]' do
expect(subject.superclass).to eq(ActiveRecord::Migration[6.1])
end
it 'includes migration helpers version 2' do
expect(subject.included_modules).to include(Gitlab::Database::MigrationHelpers::V2)
end
end
context 'unknown version' do
it 'raises an error' do
expect { described_class[0] }.to raise_error(ArgumentError, /Unknown migration version/)
end
end
end
describe '.current_version' do
it 'includes current ActiveRecord migration class' do
# This breaks upon Rails upgrade. In that case, we'll add a new version in Gitlab::Database::Migration::MIGRATION_CLASSES,
# bump .current_version and leave existing migrations and already defined versions of Gitlab::Database::Migration
# untouched.
expect(described_class[described_class.current_version].superclass).to eq(ActiveRecord::Migration::Current)
end
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require_relative '../../../../rubocop/cop/migration/versioned_migration_class'
RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass do
subject(:cop) { described_class.new }
let(:migration) do
<<~SOURCE
class TestMigration < Gitlab::Database::Migration[1.0]
def up
execute 'select 1'
end
def down
execute 'select 1'
end
end
SOURCE
end
shared_examples 'a disabled cop' do
it 'does not register any offenses' do
expect_no_offenses(migration)
end
end
context 'outside of a migration' do
it_behaves_like 'a disabled cop'
end
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
context 'in an old migration' do
before do
allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5)
end
it_behaves_like 'a disabled cop'
end
context 'that is recent' do
before do
allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE + 5)
end
it 'adds an offence if inheriting from ActiveRecord::Migration' do
expect_offense(<<~RUBY)
class MyMigration < ActiveRecord::Migration[6.1]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.
end
RUBY
end
it 'adds an offence if including Gitlab::Database::MigrationHelpers directly' do
expect_offense(<<~RUBY)
class MyMigration < Gitlab::Database::Migration[1.0]
include Gitlab::Database::MigrationHelpers
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.
end
RUBY
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment