Concentrate whitelisted and blacklisted tables

Whitelisted and blacklisted tables were defined in different rubocop
files, this commit changes that so they can be defined on the
MigrationHelpers.

Specs were updated as well

db/migrate/20181016141739_add_status_to_deployments.rb

@@ -14,14 +14,18 @@ class AddStatusToDeployments < ActiveRecord::Migration[4.2]
   # Ideally, `status` column should not have default value because it should be leveraged by state machine (i.e. application level).
   # However, we have to use the default value for avoiding `NOT NULL` violation during the transition period.
   # The default value should be removed in the future release.
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
   def up
-    add_column_with_default(:deployments, # rubocop:disable Migration/AddColumnWithDefault
+    add_column_with_default(:deployments,
       :status,
       :integer,
       limit: 2,
       default: DEPLOYMENT_STATUS_SUCCESS,
       allow_null: false)
   end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     remove_column(:deployments, :status)

db/migrate/20190416185130_add_merge_train_enabled_to_ci_cd_settings.rb

@@ -7,10 +7,12 @@ class AddMergeTrainEnabledToCiCdSettings < ActiveRecord::Migration[5.1]
 
   disable_ddl_transaction!
 
+  # rubocop:disable Migration/AddColumnWithDefault
   # rubocop:disable Migration/UpdateLargeTable
   def up
     add_column_with_default :project_ci_cd_settings, :merge_trains_enabled, :boolean, default: false, allow_null: false
   end
+  # rubocop:enable Migration/AddColumnWithDefault
   # rubocop:enable Migration/UpdateLargeTable
 
   def down

db/migrate/20190416213615_add_variable_type_to_ci_pipeline_variables.rb

@@ -7,9 +7,13 @@ class AddVariableTypeToCiPipelineVariables < ActiveRecord::Migration[5.0]
   DOWNTIME = false
   ENV_VAR_VARIABLE_TYPE = 1
 
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
   def up
-    add_column_with_default(:ci_pipeline_variables, :variable_type, :smallint, default: ENV_VAR_VARIABLE_TYPE) # rubocop:disable Migration/AddColumnWithDefault
+    add_column_with_default(:ci_pipeline_variables, :variable_type, :smallint, default: ENV_VAR_VARIABLE_TYPE)
   end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     remove_column(:ci_pipeline_variables, :variable_type)

db/migrate/20190426180107_add_deployment_events_to_services.rb

@@ -7,9 +7,13 @@ class AddDeploymentEventsToServices < ActiveRecord::Migration[5.0]
 
   disable_ddl_transaction!
 
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
   def up
     add_column_with_default(:services, :deployment_events, :boolean, default: false, allow_null: false)
   end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     remove_column(:services, :deployment_events)

db/migrate/20191023093207_add_comment_actions_to_services.rb

@@ -7,9 +7,13 @@ class AddCommentActionsToServices < ActiveRecord::Migration[5.2]
 
   disable_ddl_transaction!
 
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
   def up
     add_column_with_default(:services, :comment_on_event_enabled, :boolean, default: true)
   end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     remove_column(:services, :comment_on_event_enabled)

db/migrate/20200310132654_add_instance_to_services.rb

@@ -7,9 +7,13 @@ class AddInstanceToServices < ActiveRecord::Migration[6.0]
 
   disable_ddl_transaction!
 
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
   def up
     add_column_with_default(:services, :instance, :boolean, default: false)
   end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     remove_column(:services, :instance)

db/post_migrate/20200212052620_readd_template_column_to_services.rb

@@ -7,6 +7,7 @@ class ReaddTemplateColumnToServices < ActiveRecord::Migration[6.0]
 
   disable_ddl_transaction!
 
+  # rubocop:disable Migration/UpdateLargeTable
   def up
     return if column_exists? :services, :template
 
@@ -16,6 +17,7 @@ class ReaddTemplateColumnToServices < ActiveRecord::Migration[6.0]
     # of `template`, we would look for entries with `project_id IS NULL`.
     add_column_with_default :services, :template, :boolean, default: false, allow_null: true
   end
+  # rubocop:enable Migration/UpdateLargeTable
 
   def down
     # NOP since the column is expected to exist

rubocop/cop/migration/add_column.rb

@@ -8,11 +8,6 @@ module RuboCop
       class AddColumn < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        WHITELISTED_TABLES = %i[
-          application_settings
-          plan_limits
-        ].freeze
-
         MSG = '`add_column` with a default value requires downtime, ' \
           'use `add_column_with_default` instead'.freeze
 

rubocop/cop/migration/add_column_with_default.rb

@@ -10,38 +10,6 @@ module RuboCop
       class AddColumnWithDefault < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        # Tables >= 10 GB on GitLab.com as of 02/2020
-        BLACKLISTED_TABLES = %i[
-          audit_events
-          ci_build_trace_sections
-          ci_builds
-          ci_builds_metadata
-          ci_job_artifacts
-          ci_pipeline_variables
-          ci_pipelines
-          ci_stages
-          deployments
-          events
-          issues
-          merge_request_diff_commits
-          merge_request_diff_files
-          merge_request_diffs
-          merge_request_metrics
-          merge_requests
-          note_diff_files
-          notes
-          project_authorizations
-          projects
-          push_event_payloads
-          resource_label_events
-          sent_notifications
-          system_note_metadata
-          taggings
-          todos
-          users
-          web_hook_logs
-        ].freeze
-
         MSG = '`add_column_with_default` without `allow_null: true` may cause prolonged lock situations and downtime, ' \
           'see https://gitlab.com/gitlab-org/gitlab/issues/38060'.freeze
 

rubocop/cop/migration/safer_boolean_column.rb

@@ -23,10 +23,6 @@ module RuboCop
         NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.'.freeze
         DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.'.freeze
 
-        SMALL_TABLES = %i[
-          application_settings
-        ].freeze
-
         def_node_matcher :add_column?, <<~PATTERN
           (send nil? :add_column $...)
         PATTERN
@@ -41,7 +37,7 @@ module RuboCop
           table, _, type = matched.to_a.take(3).map(&:children).map(&:first)
           opts = matched[3]
 
-          return unless SMALL_TABLES.include?(table) && type == :boolean
+          return unless WHITELISTED_TABLES.include?(table) && type == :boolean
 
           no_default = no_default?(opts)
           nulls_allowed = nulls_allowed?(opts)

rubocop/cop/migration/update_large_table.rb

@@ -19,26 +19,6 @@ module RuboCop
               'complete, and should be avoided unless absolutely ' \
               'necessary'.freeze
 
-        LARGE_TABLES = %i[
-          ci_build_trace_sections
-          ci_builds
-          ci_job_artifacts
-          ci_pipelines
-          ci_stages
-          events
-          issues
-          merge_request_diff_commits
-          merge_request_diff_files
-          merge_request_diffs
-          merge_requests
-          namespaces
-          notes
-          projects
-          project_ci_cd_settings
-          routes
-          users
-        ].freeze
-
         BATCH_UPDATE_METHODS = %w[
           :add_column_with_default
           :change_column_type_concurrently
@@ -59,7 +39,7 @@ module RuboCop
           update_method = matches.first
           table = matches.last.to_a.first
 
-          return unless LARGE_TABLES.include?(table)
+          return unless BLACKLISTED_TABLES.include?(table)
 
           add_offense(node, location: :expression, message: format(MSG, update_method, table))
         end

rubocop/migration_helpers.rb

 module RuboCop
   # Module containing helper methods for writing migration cops.
   module MigrationHelpers
+    WHITELISTED_TABLES = %i[
+      application_settings
+      plan_limits
+    ].freeze
+
+    # Blacklisted table due to:
+    #   - size in GB (>= 10 GB on GitLab.com as of 02/2020)
+    #   - number of records
+    BLACKLISTED_TABLES = %i[
+       audit_events
+       ci_build_trace_sections
+       ci_builds
+       ci_builds_metadata
+       ci_job_artifacts
+       ci_pipeline_variables
+       ci_pipelines
+       ci_stages
+       deployments
+       events
+       issues
+       merge_request_diff_commits
+       merge_request_diff_files
+       merge_request_diffs
+       merge_request_metrics
+       merge_requests
+       namespaces
+       note_diff_files
+       notes
+       project_authorizations
+       projects
+       project_ci_cd_settings
+       push_event_payloads
+       resource_label_events
+       routes
+       sent_notifications
+       services
+       system_note_metadata
+       taggings
+       todos
+       users
+       web_hook_logs
+    ].freeze
+
     # Returns true if the given node originated from the db/migrate directory.
     def in_migration?(node)
       dirname(node).end_with?('db/migrate', 'db/geo/migrate') || in_post_deployment_migration?(node)

spec/rubocop/cop/migration/safer_boolean_column_spec.rb

@@ -17,7 +17,7 @@ describe RuboCop::Cop::Migration::SaferBooleanColumn do
       allow(cop).to receive(:in_migration?).and_return(true)
     end
 
-    described_class::SMALL_TABLES.each do |table|
+    described_class::WHITELISTED_TABLES.each do |table|
       context "for the #{table} table" do
         sources_and_offense = [
           ["add_column :#{table}, :column, :boolean, default: true", 'should disallow nulls'],
@@ -62,14 +62,14 @@ describe RuboCop::Cop::Migration::SaferBooleanColumn do
       end
     end
 
-    it 'registers no offense for tables not listed in SMALL_TABLES' do
+    it 'registers no offense for tables not listed in WHITELISTED_TABLES' do
       inspect_source("add_column :large_table, :column, :boolean")
 
       expect(cop.offenses).to be_empty
     end
 
     it 'registers no offense for non-boolean columns' do
-      table = described_class::SMALL_TABLES.sample
+      table = described_class::WHITELISTED_TABLES.sample
       inspect_source("add_column :#{table}, :column, :string")
 
       expect(cop.offenses).to be_empty
@@ -78,7 +78,7 @@ describe RuboCop::Cop::Migration::SaferBooleanColumn do
 
   context 'outside of migration' do
     it 'registers no offense' do
-      table = described_class::SMALL_TABLES.sample
+      table = described_class::WHITELISTED_TABLES.sample
       inspect_source("add_column :#{table}, :column, :boolean")
 
       expect(cop.offenses).to be_empty

spec/rubocop/cop/migration/update_large_table_spec.rb

@@ -18,7 +18,7 @@ describe RuboCop::Cop::Migration::UpdateLargeTable do
     end
 
     shared_examples 'large tables' do |update_method|
-      described_class::LARGE_TABLES.each do |table|
+      described_class::BLACKLISTED_TABLES.each do |table|
         it "registers an offense for the #{table} table" do
           inspect_source("#{update_method} :#{table}, :column, default: true")
 
@@ -53,7 +53,7 @@ describe RuboCop::Cop::Migration::UpdateLargeTable do
     end
 
     it 'registers no offense for non-blacklisted methods' do
-      table = described_class::LARGE_TABLES.sample
+      table = described_class::BLACKLISTED_TABLES.sample
 
       inspect_source("some_other_method :#{table}, :column, default: true")
 
@@ -62,7 +62,7 @@ describe RuboCop::Cop::Migration::UpdateLargeTable do
   end
 
   context 'outside of migration' do
-    let(:table) { described_class::LARGE_TABLES.sample }
+    let(:table) { described_class::BLACKLISTED_TABLES.sample }
 
     it 'registers no offense for add_column_with_default' do
       inspect_source("add_column_with_default :#{table}, :column, default: true")