Commit 2df0db22 authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Do not register offense for drop table in down method

Since we are not running `down` method within the deployment process,
there is no downtime risk of using `drop_table` DSL method or the SQL
literal `DROP TABLE` in `down` methods.
parent 5d7a7584
...@@ -17,8 +17,6 @@ class CreateMergeRequestAssigneesTable < ActiveRecord::Migration[5.0] ...@@ -17,8 +17,6 @@ class CreateMergeRequestAssigneesTable < ActiveRecord::Migration[5.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :merge_request_assignees drop_table :merge_request_assignees
# rubocop:enable Migration/DropTable
end end
end end
...@@ -15,8 +15,6 @@ class CreateMilestoneReleasesTable < ActiveRecord::Migration[5.2] ...@@ -15,8 +15,6 @@ class CreateMilestoneReleasesTable < ActiveRecord::Migration[5.2]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :milestone_releases drop_table :milestone_releases
# rubocop:enable Migration/DropTable
end end
end end
...@@ -24,8 +24,6 @@ class CreateDescriptionVersions < ActiveRecord::Migration[5.2] ...@@ -24,8 +24,6 @@ class CreateDescriptionVersions < ActiveRecord::Migration[5.2]
def down def down
remove_column :system_note_metadata, :description_version_id remove_column :system_note_metadata, :description_version_id
# rubocop:disable Migration/DropTable
drop_table :description_versions drop_table :description_versions
# rubocop:enable Migration/DropTable
end end
end end
...@@ -23,8 +23,6 @@ class AddGroupDeletionSchedules < ActiveRecord::Migration[5.2] ...@@ -23,8 +23,6 @@ class AddGroupDeletionSchedules < ActiveRecord::Migration[5.2]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :group_deletion_schedules drop_table :group_deletion_schedules
# rubocop:enable Migration/DropTable
end end
end end
...@@ -23,8 +23,6 @@ class CreateGitlabSubscriptionHistories < ActiveRecord::Migration[5.2] ...@@ -23,8 +23,6 @@ class CreateGitlabSubscriptionHistories < ActiveRecord::Migration[5.2]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :gitlab_subscription_histories drop_table :gitlab_subscription_histories
# rubocop:enable Migration/DropTable
end end
end end
...@@ -20,9 +20,7 @@ class AddCanonicalEmails < ActiveRecord::Migration[6.0] ...@@ -20,9 +20,7 @@ class AddCanonicalEmails < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table(:user_canonical_emails) drop_table(:user_canonical_emails)
# rubocop:enable Migration/DropTable
end end
end end
end end
...@@ -20,9 +20,7 @@ class CreateUserDetails < ActiveRecord::Migration[6.0] ...@@ -20,9 +20,7 @@ class CreateUserDetails < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table :user_details drop_table :user_details
# rubocop:enable Migration/DropTable
end end
end end
end end
...@@ -19,9 +19,7 @@ class CreateUserHighestRoles < ActiveRecord::Migration[6.0] ...@@ -19,9 +19,7 @@ class CreateUserHighestRoles < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table :user_highest_roles drop_table :user_highest_roles
# rubocop:enable Migration/DropTable
end end
end end
end end
...@@ -30,8 +30,6 @@ class CreateDiffNotePositions < ActiveRecord::Migration[6.0] ...@@ -30,8 +30,6 @@ class CreateDiffNotePositions < ActiveRecord::Migration[6.0]
# rubocop:enable Migration/AddLimitToTextColumns # rubocop:enable Migration/AddLimitToTextColumns
def down def down
# rubocop:disable Migration/DropTable
drop_table :diff_note_positions drop_table :diff_note_positions
# rubocop:enable Migration/DropTable
end end
end end
...@@ -25,9 +25,7 @@ class RecreateCiRef < ActiveRecord::Migration[6.0] ...@@ -25,9 +25,7 @@ class RecreateCiRef < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table :ci_refs drop_table :ci_refs
# rubocop:enable Migration/DropTable
create_table :ci_refs do |t| create_table :ci_refs do |t|
t.references :project, null: false, index: false, foreign_key: { on_delete: :cascade }, type: :integer t.references :project, null: false, index: false, foreign_key: { on_delete: :cascade }, type: :integer
......
...@@ -16,9 +16,7 @@ class AddProjectComplianceFrameworkSettingsTable < ActiveRecord::Migration[6.0] ...@@ -16,9 +16,7 @@ class AddProjectComplianceFrameworkSettingsTable < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table :project_compliance_framework_settings drop_table :project_compliance_framework_settings
# rubocop:enable Migration/DropTable
end end
end end
end end
...@@ -26,8 +26,6 @@ class CreatePartitionedForeignKeys < ActiveRecord::Migration[6.0] ...@@ -26,8 +26,6 @@ class CreatePartitionedForeignKeys < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :partitioned_foreign_keys drop_table :partitioned_foreign_keys
# rubocop:enable Migration/DropTable
end end
end end
...@@ -26,8 +26,6 @@ class CreateProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0] ...@@ -26,8 +26,6 @@ class CreateProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_source_storage_name') remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_source_storage_name')
remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_destination_storage_name') remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_destination_storage_name')
# rubocop:disable Migration/DropTable
drop_table :project_repository_storage_moves drop_table :project_repository_storage_moves
# rubocop:enable Migration/DropTable
end end
end end
...@@ -25,8 +25,6 @@ class CreateCiFreezePeriods < ActiveRecord::Migration[6.0] ...@@ -25,8 +25,6 @@ class CreateCiFreezePeriods < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :ci_freeze_periods drop_table :ci_freeze_periods
# rubocop:enable Migration/DropTable
end end
end end
...@@ -15,8 +15,6 @@ class CreateStatusPagePublishedIncidents < ActiveRecord::Migration[6.0] ...@@ -15,8 +15,6 @@ class CreateStatusPagePublishedIncidents < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :status_page_published_incidents drop_table :status_page_published_incidents
# rubocop:enable Migration/DropTable
end end
end end
...@@ -39,8 +39,6 @@ class CreateAlertManagementAlerts < ActiveRecord::Migration[6.0] ...@@ -39,8 +39,6 @@ class CreateAlertManagementAlerts < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :alert_management_alerts drop_table :alert_management_alerts
# rubocop:enable Migration/DropTable
end end
end end
...@@ -20,8 +20,6 @@ class AddGroupImportStatesTable < ActiveRecord::Migration[6.0] ...@@ -20,8 +20,6 @@ class AddGroupImportStatesTable < ActiveRecord::Migration[6.0]
# rubocop:enable Migration/AddLimitToTextColumns # rubocop:enable Migration/AddLimitToTextColumns
def down def down
# rubocop:disable Migration/DropTable
drop_table :group_import_states drop_table :group_import_states
# rubocop:enable Migration/DropTable
end end
end end
...@@ -20,8 +20,6 @@ class CreateMetricsUsersStarredDashboard < ActiveRecord::Migration[6.0] ...@@ -20,8 +20,6 @@ class CreateMetricsUsersStarredDashboard < ActiveRecord::Migration[6.0]
# rubocop: enable Migration/AddLimitToTextColumns # rubocop: enable Migration/AddLimitToTextColumns
def down def down
# rubocop:disable Migration/DropTable
drop_table :metrics_users_starred_dashboards drop_table :metrics_users_starred_dashboards
# rubocop:enable Migration/DropTable
end end
end end
...@@ -26,8 +26,6 @@ class CreateCiInstanceVariables < ActiveRecord::Migration[6.0] ...@@ -26,8 +26,6 @@ class CreateCiInstanceVariables < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :ci_instance_variables drop_table :ci_instance_variables
# rubocop:enable Migration/DropTable
end end
end end
...@@ -21,8 +21,6 @@ class CreateNugetDependencyLinkMetadata < ActiveRecord::Migration[6.0] ...@@ -21,8 +21,6 @@ class CreateNugetDependencyLinkMetadata < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :packages_nuget_dependency_link_metadata drop_table :packages_nuget_dependency_link_metadata
# rubocop:enable Migration/DropTable
end end
end end
...@@ -29,8 +29,6 @@ class CreatePackagesNugetMetadata < ActiveRecord::Migration[6.0] ...@@ -29,8 +29,6 @@ class CreatePackagesNugetMetadata < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :packages_nuget_metadata drop_table :packages_nuget_metadata
# rubocop:enable Migration/DropTable
end end
end end
...@@ -31,8 +31,6 @@ class CreateGroupDeployKeys < ActiveRecord::Migration[6.0] ...@@ -31,8 +31,6 @@ class CreateGroupDeployKeys < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :group_deploy_keys drop_table :group_deploy_keys
# rubocop:enable Migration/DropTable
end end
end end
...@@ -17,8 +17,6 @@ class CreateAlertManagementAlertAssignees < ActiveRecord::Migration[6.0] ...@@ -17,8 +17,6 @@ class CreateAlertManagementAlertAssignees < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :alert_management_alert_assignees drop_table :alert_management_alert_assignees
# rubocop:enable Migration/DropTable
end end
end end
...@@ -21,9 +21,7 @@ class CreateProjectSecuritySettings < ActiveRecord::Migration[6.0] ...@@ -21,9 +21,7 @@ class CreateProjectSecuritySettings < ActiveRecord::Migration[6.0]
def down def down
with_lock_retries do with_lock_retries do
# rubocop:disable Migration/DropTable
drop_table :project_security_settings drop_table :project_security_settings
# rubocop:enable Migration/DropTable
end end
end end
end end
...@@ -15,8 +15,6 @@ class CreateBoardUserPreferences < ActiveRecord::Migration[6.0] ...@@ -15,8 +15,6 @@ class CreateBoardUserPreferences < ActiveRecord::Migration[6.0]
end end
def down def down
# rubocop:disable Migration/DropTable
drop_table :board_user_preferences drop_table :board_user_preferences
# rubocop:enable Migration/DropTable
end end
end end
...@@ -52,9 +52,7 @@ class MigrateCiJobArtifactsToSeparateRegistry < ActiveRecord::Migration[4.2] ...@@ -52,9 +52,7 @@ class MigrateCiJobArtifactsToSeparateRegistry < ActiveRecord::Migration[4.2]
end end
def down def down
# rubocop:disable Migration/DropTable
tracking_db.drop_table :job_artifact_registry tracking_db.drop_table :job_artifact_registry
# rubocop:enable Migration/DropTable
execute('DROP TRIGGER IF EXISTS replicate_job_artifact_registry ON file_registry') execute('DROP TRIGGER IF EXISTS replicate_job_artifact_registry ON file_registry')
execute('DROP FUNCTION IF EXISTS replicate_job_artifact_registry()') execute('DROP FUNCTION IF EXISTS replicate_job_artifact_registry()')
......
...@@ -17,6 +17,7 @@ module RuboCop ...@@ -17,6 +17,7 @@ module RuboCop
def on_def(node) def on_def(node)
return unless in_deployment_migration?(node) return unless in_deployment_migration?(node)
return if down_method?(node)
node.each_descendant(:send) do |send_node| node.each_descendant(:send) do |send_node|
next unless offensible?(send_node) next unless offensible?(send_node)
...@@ -27,6 +28,10 @@ module RuboCop ...@@ -27,6 +28,10 @@ module RuboCop
private private
def down_method?(node)
node.method?(:down)
end
def offensible?(node) def offensible?(node)
drop_table?(node) || drop_table_in_execute?(node) drop_table?(node) || drop_table_in_execute?(node)
end end
......
...@@ -17,22 +17,72 @@ describe RuboCop::Cop::Migration::DropTable do ...@@ -17,22 +17,72 @@ describe RuboCop::Cop::Migration::DropTable do
allow(cop).to receive(:in_deployment_migration?).and_return(true) allow(cop).to receive(:in_deployment_migration?).and_return(true)
end end
context 'with drop_table DSL method' do
context 'when in down method' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN)
def down
drop_table :table
end
PATTERN
end
end
context 'when in up method' do
it 'registers an offense' do
expect_offense(<<~PATTERN)
def up
drop_table :table
^^^^^^^^^^ #{described_class::MSG}
end
PATTERN
end
end
context 'when in change method' do
it 'registers an offense' do it 'registers an offense' do
expect_offense(<<~PATTERN) expect_offense(<<~PATTERN)
def change def change
drop_table :table drop_table :table
^^^^^^^^^^ #{described_class::MSG} ^^^^^^^^^^ #{described_class::MSG}
end
PATTERN
end
end
end
add_column(:users, :username, :text) context 'with DROP TABLE SQL literal' do
it 'does not register an offense' do
expect_no_offenses(<<~PATTERN)
def down
execute "DROP TABLE table"
end
PATTERN
end
end
context 'when in up method' do
it 'registers an offense' do
expect_offense(<<~PATTERN)
def up
execute "DROP TABLE table" execute "DROP TABLE table"
^^^^^^^ #{described_class::MSG} ^^^^^^^ #{described_class::MSG}
end
PATTERN
end
end
execute "CREATE UNIQUE INDEX email_index ON users (email);" context 'when in change method' do
it 'registers an offense' do
expect_offense(<<~PATTERN)
def change
execute "DROP TABLE table"
^^^^^^^ #{described_class::MSG}
end end
PATTERN PATTERN
end end
end end
end
context 'when in post-deployment migration' do context 'when in post-deployment migration' do
before do before do
......
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