Merge branch 'centralized-whitelisted-and-blacklisted-tables' into 'master'

Concentrates whitelisted and blacklisted tables

See merge request gitlab-org/gitlab!25624

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")