summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@selenight.nl>2017-03-07 10:08:53 -0600
committerDouwe Maan <douwe@selenight.nl>2017-03-07 10:09:01 -0600
commit5d9762b3b5ace3da397b83f501d103a5152f0dd3 (patch)
treebed282cc9a07e165842c6b28e18746c8558c0c21
parent6a52cda31da4becc3e342530a2bdf0868d8921cc (diff)
downloadgitlab-ce-5d9762b3b5ace3da397b83f501d103a5152f0dd3.tar.gz
Add cop to ensure reversibility of add_concurrent_index
-rw-r--r--db/migrate/20160615142710_add_index_on_requested_at_to_members.rb6
-rw-r--r--db/migrate/20160620115026_add_index_on_runners_locked.rb6
-rw-r--r--db/migrate/20160715134306_add_index_for_pipeline_user_id.rb6
-rw-r--r--db/migrate/20160805041956_add_deleted_at_to_namespaces.rb9
-rw-r--r--db/migrate/20160808085602_add_index_for_build_token.rb6
-rw-r--r--db/migrate/20160819221631_add_index_to_note_discussion_id.rb6
-rw-r--r--db/migrate/20160819232256_add_incoming_email_token_to_users.rb9
-rw-r--r--db/migrate/20160919145149_add_group_id_to_labels.rb8
-rw-r--r--db/migrate/20160920160832_add_index_to_labels_title.rb6
-rw-r--r--db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb10
-rw-r--r--db/migrate/20161106185620_add_project_import_data_project_index.rb6
-rw-r--r--db/migrate/20161124111395_add_index_to_parent_id.rb6
-rw-r--r--db/migrate/20161202152035_add_index_to_routes.rb7
-rw-r--r--db/migrate/20161209153400_add_unique_index_for_environment_slug.rb6
-rw-r--r--db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb6
-rw-r--r--db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb7
-rw-r--r--db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb6
-rw-r--r--db/migrate/20170210103609_add_index_to_user_agent_detail.rb8
-rw-r--r--db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb8
-rw-r--r--rubocop/cop/migration/add_concurrent_index.rb34
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/rubocop/cop/migration/add_concurrent_index_spec.rb41
22 files changed, 186 insertions, 22 deletions
diff --git a/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb b/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb
index 63f7392e54f..e23b33f78d8 100644
--- a/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb
+++ b/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb
@@ -3,7 +3,11 @@ class AddIndexOnRequestedAtToMembers < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :members, :requested_at
end
+
+ def down
+ remove_index :members, :requested_at if index_exists? :members, :requested_at
+ end
end
diff --git a/db/migrate/20160620115026_add_index_on_runners_locked.rb b/db/migrate/20160620115026_add_index_on_runners_locked.rb
index dfa5110dea4..62b00cc3e2c 100644
--- a/db/migrate/20160620115026_add_index_on_runners_locked.rb
+++ b/db/migrate/20160620115026_add_index_on_runners_locked.rb
@@ -6,7 +6,11 @@ class AddIndexOnRunnersLocked < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :ci_runners, :locked
end
+
+ def down
+ remove_index :ci_runners, :locked if index_exists? :ci_runners, :locked
+ end
end
diff --git a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb
index 7c991c6d998..12fd25b420d 100644
--- a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb
+++ b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb
@@ -3,7 +3,11 @@ class AddIndexForPipelineUserId < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :ci_commits, :user_id
end
+
+ def down
+ remove_index :ci_commits, :user_id if index_exists? :ci_commits, :user_id
+ end
end
diff --git a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb
index a853de3abfb..3f074723b4a 100644
--- a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb
+++ b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb
@@ -5,8 +5,15 @@ class AddDeletedAtToNamespaces < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_column :namespaces, :deleted_at, :datetime
+
add_concurrent_index :namespaces, :deleted_at
end
+
+ def down
+ remove_index :namespaces, :deleted_at if index_exists? :namespaces, :deleted_at
+
+ remove_column :namespaces, :deleted_at
+ end
end
diff --git a/db/migrate/20160808085602_add_index_for_build_token.rb b/db/migrate/20160808085602_add_index_for_build_token.rb
index 10ef42afce1..6c5d7268e72 100644
--- a/db/migrate/20160808085602_add_index_for_build_token.rb
+++ b/db/migrate/20160808085602_add_index_for_build_token.rb
@@ -6,7 +6,11 @@ class AddIndexForBuildToken < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :ci_builds, :token, unique: true
end
+
+ def down
+ remove_index :ci_builds, :token, unique: true if index_exists? :ci_builds, :token, unique: true
+ end
end
diff --git a/db/migrate/20160819221631_add_index_to_note_discussion_id.rb b/db/migrate/20160819221631_add_index_to_note_discussion_id.rb
index b6e8bb18e7b..8f693e97a58 100644
--- a/db/migrate/20160819221631_add_index_to_note_discussion_id.rb
+++ b/db/migrate/20160819221631_add_index_to_note_discussion_id.rb
@@ -8,7 +8,11 @@ class AddIndexToNoteDiscussionId < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :notes, :discussion_id
end
+
+ def down
+ remove_index :notes, :discussion_id if index_exists? :notes, :discussion_id
+ end
end
diff --git a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
index f2cf956adc9..bcad3416d04 100644
--- a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
+++ b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
@@ -9,8 +9,15 @@ class AddIncomingEmailTokenToUsers < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_column :users, :incoming_email_token, :string
+
add_concurrent_index :users, :incoming_email_token
end
+
+ def down
+ remove_index :users, :incoming_email_token if index_exists? :users, :incoming_email_token
+
+ remove_column :users, :incoming_email_token
+ end
end
diff --git a/db/migrate/20160919145149_add_group_id_to_labels.rb b/db/migrate/20160919145149_add_group_id_to_labels.rb
index d10f3a6d104..828b6afddb1 100644
--- a/db/migrate/20160919145149_add_group_id_to_labels.rb
+++ b/db/migrate/20160919145149_add_group_id_to_labels.rb
@@ -5,9 +5,15 @@ class AddGroupIdToLabels < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_column :labels, :group_id, :integer
add_foreign_key :labels, :namespaces, column: :group_id, on_delete: :cascade # rubocop: disable Migration/AddConcurrentForeignKey
add_concurrent_index :labels, :group_id
end
+
+ def down
+ remove_index :labels, :group_id if index_exists? :labels, :group_id
+ remove_foreign_key :labels, :namespaces, column: :group_id
+ remove_column :labels, :group_id
+ end
end
diff --git a/db/migrate/20160920160832_add_index_to_labels_title.rb b/db/migrate/20160920160832_add_index_to_labels_title.rb
index b5de552b98c..19f7b1076a7 100644
--- a/db/migrate/20160920160832_add_index_to_labels_title.rb
+++ b/db/migrate/20160920160832_add_index_to_labels_title.rb
@@ -5,7 +5,11 @@ class AddIndexToLabelsTitle < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :labels, :title
end
+
+ def down
+ remove_index :labels, :title if index_exists? :labels, :title
+ 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 2abfe47b776..ad3eb4a26f9 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
@@ -25,9 +25,15 @@ class AddPipelineIdToMergeRequestMetrics < ActiveRecord::Migration
# comments:
# disable_ddl_transaction!
- def change
+ def up
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 # rubocop: disable Migration/AddConcurrentForeignKey
+ add_concurrent_index :merge_request_metrics, :pipeline_id
+ end
+
+ def down
+ remove_index :merge_request_metrics, :pipeline_id if index_exists? :merge_request_metrics, :pipeline_id
+ remove_foreign_key :merge_request_metrics, :ci_commits, column: :pipeline_id
+ remove_column :merge_request_metrics, :pipeline_id
end
end
diff --git a/db/migrate/20161106185620_add_project_import_data_project_index.rb b/db/migrate/20161106185620_add_project_import_data_project_index.rb
index 750a6a8c51e..94b8ddd46f5 100644
--- a/db/migrate/20161106185620_add_project_import_data_project_index.rb
+++ b/db/migrate/20161106185620_add_project_import_data_project_index.rb
@@ -6,7 +6,11 @@ class AddProjectImportDataProjectIndex < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :project_import_data, :project_id
end
+
+ def down
+ remove_index :project_import_data, :project_id if index_exists? :project_import_data, :project_id
+ end
end
diff --git a/db/migrate/20161124111395_add_index_to_parent_id.rb b/db/migrate/20161124111395_add_index_to_parent_id.rb
index eab74c01dfd..73f9d92bb22 100644
--- a/db/migrate/20161124111395_add_index_to_parent_id.rb
+++ b/db/migrate/20161124111395_add_index_to_parent_id.rb
@@ -8,7 +8,11 @@ class AddIndexToParentId < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index(:namespaces, [:parent_id, :id], unique: true)
end
+
+ def down
+ remove_index :namespaces, [:parent_id, :id] if index_exists? :namespaces, [:parent_id, :id]
+ end
end
diff --git a/db/migrate/20161202152035_add_index_to_routes.rb b/db/migrate/20161202152035_add_index_to_routes.rb
index 4a51337bda6..6d6c8906204 100644
--- a/db/migrate/20161202152035_add_index_to_routes.rb
+++ b/db/migrate/20161202152035_add_index_to_routes.rb
@@ -9,8 +9,13 @@ class AddIndexToRoutes < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index(:routes, :path, unique: true)
add_concurrent_index(:routes, [:source_type, :source_id], unique: true)
end
+
+ def down
+ remove_index(:routes, :path) if index_exists? :routes, :path
+ remove_index(:routes, [:source_type, :source_id]) if index_exists? :routes, [:source_type, :source_id]
+ end
end
diff --git a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb
index e9fcef1cd45..d7ef1aa83d9 100644
--- a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb
+++ b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb
@@ -9,7 +9,11 @@ class AddUniqueIndexForEnvironmentSlug < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :environments, [:project_id, :slug], unique: true
end
+
+ def down
+ remove_index :environments, [:project_id, :slug], unique: true if index_exists? :environments, [:project_id, :slug]
+ end
end
diff --git a/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb b/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb
index 8f944930807..31ef458c44f 100644
--- a/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb
+++ b/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb
@@ -5,7 +5,11 @@ class AddIndexToLabelsForTypeAndProject < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :labels, [:type, :project_id]
end
+
+ def down
+ remove_index :labels, [:type, :project_id] if index_exists? :labels, [:type, :project_id]
+ end
end
diff --git a/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb
index f922ed209aa..70fb0ef12f9 100644
--- a/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb
+++ b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb
@@ -5,8 +5,13 @@ class AddIndexToLabelsForTitleAndProject < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :labels, :title
add_concurrent_index :labels, :project_id
end
+
+ def down
+ remove_index :labels, :title if index_exists? :labels, :title
+ remove_index :labels, :project_id if index_exists? :labels, :project_id
+ end
end
diff --git a/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb b/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb
index 61e49c14fc0..07d4f8af27f 100644
--- a/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb
+++ b/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb
@@ -5,7 +5,11 @@ class AddIndexToCiTriggerRequestsForCommitId < ActiveRecord::Migration
disable_ddl_transaction!
- def change
+ def up
add_concurrent_index :ci_trigger_requests, :commit_id
end
+
+ def down
+ remove_index :ci_trigger_requests, :commit_id if index_exists? :ci_trigger_requests, :commit_id
+ end
end
diff --git a/db/migrate/20170210103609_add_index_to_user_agent_detail.rb b/db/migrate/20170210103609_add_index_to_user_agent_detail.rb
index c01753cfbd2..2d8329b7862 100644
--- a/db/migrate/20170210103609_add_index_to_user_agent_detail.rb
+++ b/db/migrate/20170210103609_add_index_to_user_agent_detail.rb
@@ -8,7 +8,11 @@ class AddIndexToUserAgentDetail < ActiveRecord::Migration
disable_ddl_transaction!
- def change
- add_concurrent_index(:user_agent_details, [:subject_id, :subject_type])
+ def up
+ add_concurrent_index :user_agent_details, [:subject_id, :subject_type]
+ end
+
+ def down
+ remove_index :user_agent_details, [:subject_id, :subject_type] if index_exists? :user_agent_details, [:subject_id, :subject_type]
end
end
diff --git a/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb b/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb
index 7b1e687977b..65adc90c2c1 100644
--- a/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb
+++ b/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb
@@ -4,7 +4,11 @@ class AddIndexForLatestSuccessfulPipeline < ActiveRecord::Migration
disable_ddl_transaction!
- def change
- add_concurrent_index(:ci_commits, [:gl_project_id, :ref, :status])
+ def up
+ add_concurrent_index :ci_commits, [:gl_project_id, :ref, :status]
+ end
+
+ def down
+ remove_index :ci_commits, [:gl_project_id, :ref, :status] if index_exists? :ci_commits, [:gl_project_id, :ref, :status]
end
end
diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb
new file mode 100644
index 00000000000..332fb7dcbd7
--- /dev/null
+++ b/rubocop/cop/migration/add_concurrent_index.rb
@@ -0,0 +1,34 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+ module Cop
+ module Migration
+ # Cop that checks if `add_concurrent_index` is used with `up`/`down` methods
+ # and not `change`.
+ class AddConcurrentIndex < RuboCop::Cop::Cop
+ include MigrationHelpers
+
+ MSG = '`add_concurrent_index` is not reversible so you must manually define ' \
+ 'the `up` and `down` methods in your migration class, using `remove_index` in `down`'.freeze
+
+ def on_send(node)
+ return unless in_migration?(node)
+
+ name = node.children[1]
+
+ return unless name == :add_concurrent_index
+
+ node.each_ancestor(:def) do |def_node|
+ next unless method_name(def_node) == :change
+
+ add_offense(def_node, :name)
+ end
+ end
+
+ def method_name(node)
+ node.children.first
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index ea8e0f64b0d..a50a522cf9d 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -3,4 +3,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_concurrent_index'
require_relative 'cop/migration/add_index'
diff --git a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb
new file mode 100644
index 00000000000..19a5718b0b1
--- /dev/null
+++ b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb
@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/migration/add_concurrent_index'
+
+describe RuboCop::Cop::Migration::AddConcurrentIndex do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ context 'in migration' do
+ before do
+ allow(cop).to receive(:in_migration?).and_return(true)
+ end
+
+ it 'registers an offense when add_concurrent_index is used inside a change method' do
+ inspect_source(cop, 'def change; add_concurrent_index :table, :column; end')
+
+ aggregate_failures do
+ expect(cop.offenses.size).to eq(1)
+ expect(cop.offenses.map(&:line)).to eq([1])
+ end
+ end
+
+ it 'registers no offense when add_concurrent_index is used inside an up method' do
+ inspect_source(cop, 'def up; add_concurrent_index :table, :column; end')
+
+ expect(cop.offenses.size).to eq(0)
+ end
+ end
+
+ context 'outside of migration' do
+ it 'registers no offense' do
+ inspect_source(cop, 'def change; add_concurrent_index :table, :column; end')
+
+ expect(cop.offenses.size).to eq(0)
+ end
+ end
+end