Commit 1036f80c authored by Alper Akgun's avatar Alper Akgun

Merge branch 'pl-cop-allow-fk-with-lock-retries' into 'master'

RuboCop: Allow `add_foreign_key` within `with_lock_retries`

See merge request gitlab-org/gitlab!44152
parents ee979be5 66157b0b
......@@ -7,7 +7,7 @@ class RequirementsAddProjectFk < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key(:requirements, :projects, column: :project_id, on_delete: :cascade) # rubocop: disable Migration/AddConcurrentForeignKey
add_foreign_key(:requirements, :projects, column: :project_id, on_delete: :cascade)
end
end
......
......@@ -7,7 +7,7 @@ class RequirementsAddAuthorFk < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key(:requirements, :users, column: :author_id, on_delete: :nullify) # rubocop: disable Migration/AddConcurrentForeignKey
add_foreign_key(:requirements, :users, column: :author_id, on_delete: :nullify)
end
end
......
......@@ -7,7 +7,7 @@ class AddCiSourcesProjectPipelineForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_sources_projects, :ci_pipelines, column: :pipeline_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddCiSourcesProjectSourceProjectForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :ci_sources_projects, :projects, column: :source_project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_sources_projects, :projects, column: :source_project_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddVulnerabilityExportProjectForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :vulnerability_exports, :projects, column: :project_id, on_delete: :cascade, index: false # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :vulnerability_exports, :projects, column: :project_id, on_delete: :cascade, index: false
end
end
......
......@@ -7,7 +7,7 @@ class AddVulnerabilityExportUserForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :vulnerability_exports, :users, column: :author_id, on_delete: :cascade, index: false # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :vulnerability_exports, :users, column: :author_id, on_delete: :cascade, index: false
end
end
......
......@@ -7,7 +7,7 @@ class AddProjectsFkToJiraImportsTable < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :jira_imports, :projects, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :jira_imports, :projects, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddUsersFkToJiraImportsTable < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :jira_imports, :users, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :jira_imports, :users, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddLabelsFkToJiraImportsTable < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :jira_imports, :labels, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :jira_imports, :labels, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddLockedByUserIdForeignKeyToTerraformState < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :terraform_states, :users, column: :locked_by_user_id # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :terraform_states, :users, column: :locked_by_user_id
end
end
......
......@@ -7,7 +7,7 @@ class AddUserIdForeignKeyToResourceStateEvents < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :resource_state_events, :users, column: :user_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_state_events, :users, column: :user_id, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddIssueIdForeignKeyToResourceStateEvents < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :resource_state_events, :issues, column: :issue_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_state_events, :issues, column: :issue_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddMergeRequestIdForeignKeyToResourceStateEvents < ActiveRecord::Migration
def up
with_lock_retries do
add_foreign_key :resource_state_events, :merge_requests, column: :merge_request_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_state_events, :merge_requests, column: :merge_request_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddFkToProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :project_repository_storage_moves, :projects, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :project_repository_storage_moves, :projects, on_delete: :cascade
end
end
......
......@@ -9,7 +9,7 @@ class AddForeignKeyFromWebauthnRegistrationsToUsers < ActiveRecord::Migration[6.
def up
with_lock_retries do
add_foreign_key :webauthn_registrations, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :webauthn_registrations, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToEpicIdOnResourceStateEvents < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :resource_state_events, :epics, column: :epic_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_state_events, :epics, column: :epic_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddGroupWikiRepositoriesShardIdForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :group_wiki_repositories, :shards, on_delete: :restrict # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :group_wiki_repositories, :shards, on_delete: :restrict
end
end
......
......@@ -7,7 +7,7 @@ class AddGroupWikiRepositoriesGroupIdForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :group_wiki_repositories, :namespaces, column: :group_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :group_wiki_repositories, :namespaces, column: :group_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddAuthorForeignKeyToTestReports < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :requirements_management_test_reports, :users, column: :author_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :requirements_management_test_reports, :users, column: :author_id, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddPipelineForeignKeyToTestReports < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :requirements_management_test_reports, :ci_pipelines, column: :pipeline_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :requirements_management_test_reports, :ci_pipelines, column: :pipeline_id, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToUserIdOnAlertManagementAlertAssignees < ActiveRecord::Migra
def up
with_lock_retries do
add_foreign_key :alert_management_alert_assignees, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :alert_management_alert_assignees, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToAlertIdOnAlertMangagementAlertAssignees < ActiveRecord::Mig
def up
with_lock_retries do
add_foreign_key :alert_management_alert_assignees, :alert_management_alerts, column: :alert_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :alert_management_alert_assignees, :alert_management_alerts, column: :alert_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToOpsFeatureFlagsIssues < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :operations_feature_flags_issues, :issues, column: :issue_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :operations_feature_flags_issues, :issues, column: :issue_id, on_delete: :cascade
end
end
......
......@@ -11,7 +11,7 @@ class AddRequirementsBuildReference < ActiveRecord::Migration[6.0]
add_index :requirements_management_test_reports, :build_id, name: INDEX_NAME # rubocop:disable Migration/AddIndex
with_lock_retries do
add_foreign_key :requirements_management_test_reports, :ci_builds, column: :build_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :requirements_management_test_reports, :ci_builds, column: :build_id, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToBuildIdOnBuildReportResults < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :ci_build_report_results, :ci_builds, column: :build_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_build_report_results, :ci_builds, column: :build_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToProjectIdOnBuildReportResults < ActiveRecord::Migration[6.0
def up
with_lock_retries do
add_foreign_key :ci_build_report_results, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_build_report_results, :projects, column: :project_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddUsersForeignKeyToBoardUserPreferences < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :board_user_preferences, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :board_user_preferences, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddBoardsForeignKeyToBoardUserPreferences < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :board_user_preferences, :boards, column: :board_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :board_user_preferences, :boards, column: :board_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToAlertManagementAlertUserMentions < ActiveRecord::Migration[
def up
with_lock_retries do
add_foreign_key :alert_management_alert_user_mentions, :notes, column: :note_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :alert_management_alert_user_mentions, :notes, column: :note_id, on_delete: :cascade
end
end
......
......@@ -20,7 +20,7 @@ class AddSourceMergeRequestIdToResourceStateEvents < ActiveRecord::Migration[6.0
unless foreign_key_exists?(:resource_state_events, :merge_requests, column: :source_merge_request_id)
with_lock_retries do
add_foreign_key :resource_state_events, :merge_requests, column: :source_merge_request_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_state_events, :merge_requests, column: :source_merge_request_id, on_delete: :nullify
end
end
end
......
......@@ -7,7 +7,7 @@ class AddUsersFkToResourceIterationEventsTable < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :resource_iteration_events, :users, column: :user_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_iteration_events, :users, column: :user_id, on_delete: :nullify
end
end
......
......@@ -7,7 +7,7 @@ class AddIssuesFkToResourceIterationEventsTable < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :resource_iteration_events, :issues, column: :issue_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_iteration_events, :issues, column: :issue_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddMergeRequestsFkToResourceIterationEventsTable < ActiveRecord::Migration
def up
with_lock_retries do
add_foreign_key :resource_iteration_events, :merge_requests, column: :merge_request_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_iteration_events, :merge_requests, column: :merge_request_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddIterationsFkToResourceIterationEventsTable < ActiveRecord::Migration[6.
def up
with_lock_retries do
add_foreign_key :resource_iteration_events, :sprints, column: :iteration_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :resource_iteration_events, :sprints, column: :iteration_id, on_delete: :cascade
end
end
......
......@@ -8,7 +8,7 @@ class AddForeignKeyToExperimentOnExperimentUsers < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
# There is no need to use add_concurrent_foreign_key since it's an empty table
add_foreign_key :experiment_users, :experiments, column: :experiment_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :experiment_users, :experiments, column: :experiment_id, on_delete: :cascade
end
end
......
......@@ -8,7 +8,7 @@ class AddForeignKeyToUserOnExperimentUsers < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
# There is no need to use add_concurrent_foreign_key since it's an empty table
add_foreign_key :experiment_users, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :experiment_users, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToPipelineIdOnPipelineArtifact < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :ci_pipeline_artifacts, :ci_pipelines, column: :pipeline_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_pipeline_artifacts, :ci_pipelines, column: :pipeline_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyToProjectIdOnPipelineArtifact < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :ci_pipeline_artifacts, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :ci_pipeline_artifacts, :projects, column: :project_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class BoardsEpicUserPreferencesFkBoard < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :boards_epic_user_preferences, :boards, column: :board_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey
add_foreign_key :boards_epic_user_preferences, :boards, column: :board_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class BoardsEpicUserPreferencesFkUser < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :boards_epic_user_preferences, :users, column: :user_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey
add_foreign_key :boards_epic_user_preferences, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class BoardsEpicUserPreferencesFkEpic < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :boards_epic_user_preferences, :epics, column: :epic_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey
add_foreign_key :boards_epic_user_preferences, :epics, column: :epic_id, on_delete: :cascade
end
end
......
......@@ -10,7 +10,7 @@ class AddMergeRequestForeignKeyToMergeRequestReviewers < ActiveRecord::Migration
def up
with_lock_retries do
add_foreign_key :merge_request_reviewers, :merge_requests, column: :merge_request_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :merge_request_reviewers, :merge_requests, column: :merge_request_id, on_delete: :cascade
end
end
......
......@@ -10,7 +10,7 @@ class AddUserForeignKeyToMergeRequestReviewers < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :merge_request_reviewers, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :merge_request_reviewers, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyOnScanIdToSecurityScans < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :security_findings, :security_scans, column: :scan_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :security_findings, :security_scans, column: :scan_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddForeignKeyOnScannerIdToVulnerabilityScanners < ActiveRecord::Migration[
def up
with_lock_retries do
add_foreign_key :security_findings, :vulnerability_scanners, column: :scanner_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :security_findings, :vulnerability_scanners, column: :scanner_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddPagesDeploymentProjectForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :pages_deployments, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :pages_deployments, :projects, column: :project_id, on_delete: :cascade
end
end
......
......@@ -7,7 +7,7 @@ class AddPagesDeploymentCiBuildForeignKey < ActiveRecord::Migration[6.0]
def up
with_lock_retries do
add_foreign_key :pages_deployments, :ci_builds, column: :ci_build_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :pages_deployments, :ci_builds, column: :ci_build_id, on_delete: :nullify
end
end
......
......@@ -13,9 +13,7 @@ class RemovePipelineIdFromTestReports < ActiveRecord::Migration[6.0]
add_column :requirements_management_test_reports, :pipeline_id, :integer
with_lock_retries do
# rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :requirements_management_test_reports, :ci_pipelines, column: :pipeline_id, on_delete: :nullify
# rubocop:enable Migration/AddConcurrentForeignKey
end
end
end
......@@ -300,7 +300,7 @@ include Gitlab::Database::MigrationHelpers
def up
with_lock_retries do
add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade
end
end
......@@ -318,7 +318,7 @@ include Gitlab::Database::MigrationHelpers
def up
with_lock_retries do
add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade
end
end
......
......@@ -11,7 +11,11 @@ module RuboCop
MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead'.freeze
def_node_matcher :false_node?, <<~PATTERN
(false)
(false)
PATTERN
def_node_matcher :with_lock_retries?, <<~PATTERN
(:send nil? :with_lock_retries)
PATTERN
def on_send(node)
......@@ -19,9 +23,11 @@ module RuboCop
name = node.children[1]
if name == :add_foreign_key && !not_valid_fk?(node)
add_offense(node, location: :selector)
end
return unless name == :add_foreign_key
return if in_with_lock_retries?(node)
return if not_valid_fk?(node)
add_offense(node, location: :selector)
end
def method_name(node)
......@@ -33,6 +39,12 @@ module RuboCop
pair.children[0].children[0] == :validate && false_node?(pair.children[1])
end
end
def in_with_lock_retries?(node)
node.each_ancestor(:block).any? do |parent|
with_lock_retries?(parent.to_a.first)
end
end
end
end
end
......
......@@ -36,5 +36,15 @@ RSpec.describe RuboCop::Cop::Migration::AddConcurrentForeignKey, type: :rubocop
expect(cop.offenses).to be_empty
end
it 'does not register an offense when `add_foreign_key` is within `with_lock_retries`' do
inspect_source <<~RUBY
with_lock_retries do
add_foreign_key :key, :projects, column: :project_id, on_delete: :cascade
end
RUBY
expect(cop.offenses).to be_empty
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