diff options
6 files changed, 64 insertions, 3 deletions
diff --git a/db/migrate/20160919145149_add_group_id_to_labels.rb b/db/migrate/20160919145149_add_group_id_to_labels.rb index 05e21af0584..d10f3a6d104 100644 --- a/db/migrate/20160919145149_add_group_id_to_labels.rb +++ b/db/migrate/20160919145149_add_group_id_to_labels.rb @@ -7,7 +7,7 @@ class AddGroupIdToLabels < ActiveRecord::Migration def change add_column :labels, :group_id, :integer - add_foreign_key :labels, :namespaces, column: :group_id, on_delete: :cascade + add_foreign_key :labels, :namespaces, column: :group_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey add_concurrent_index :labels, :group_id end end diff --git a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb index f49df6802a7..2abfe47b776 100644 --- a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb +++ b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb @@ -28,6 +28,6 @@ class AddPipelineIdToMergeRequestMetrics < ActiveRecord::Migration def change add_column :merge_request_metrics, :pipeline_id, :integer add_concurrent_index :merge_request_metrics, :pipeline_id - add_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id, on_delete: :cascade + add_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey end end diff --git a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb index 97534679b59..d5c343dc527 100644 --- a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb +++ b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb @@ -5,7 +5,7 @@ class AddProjectIdToSubscriptions < ActiveRecord::Migration def up add_column :subscriptions, :project_id, :integer - add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade + add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey end def down diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb new file mode 100644 index 00000000000..e40a7087a47 --- /dev/null +++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb @@ -0,0 +1,27 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `add_concurrent_foreign_key` is used instead of + # `add_foreign_key`. + class AddConcurrentForeignKey < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead' + + def on_send(node) + return unless in_migration?(node) + + name = node.children[1] + + add_offense(node, :selector) if name == :add_foreign_key + end + + def method_name(node) + node.children.first + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index aa35fb1701c..d4266d0deae 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,4 +1,5 @@ require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default' +require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_index' diff --git a/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb new file mode 100644 index 00000000000..7cb24dc5646 --- /dev/null +++ b/spec/rubocop/cop/migration/add_concurrent_foreign_key_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/migration/add_concurrent_foreign_key' + +describe RuboCop::Cop::Migration::AddConcurrentForeignKey do + include CopHelper + + let(:cop) { described_class.new } + + context 'outside of a migration' do + it 'does not register any offenses' do + inspect_source(cop, 'def up; add_foreign_key(:projects, :users, column: :user_id); end') + + expect(cop.offenses).to be_empty + end + end + + context 'in a migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when using add_foreign_key' do + inspect_source(cop, 'def up; add_foreign_key(:projects, :users, column: :user_id); end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + end +end |