Add latest changes from gitlab-org/gitlab@12-9-stable-ee

7 files changed, 220 insertions, 7 deletions

diff --git a/rubocop/cop/ban_catch_throw.rb b/rubocop/cop/ban_catch_throw.rb
new file mode 100644
index 00000000000..42301d5512f
--- /dev/null
+++ b/rubocop/cop/ban_catch_throw.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Bans the use of 'catch/throw', as exceptions are better for errors and
+    # they are equivalent to 'goto' for flow control, with all the problems
+    # that implies.
+    #
+    # @example
+    #   # bad
+    #   catch(:error) do
+    #     throw(:error)
+    #   end
+    #
+    #   # good
+    #   begin
+    #     raise StandardError
+    #   rescue StandardError => err
+    #     # ...
+    #   end
+    #
+    class BanCatchThrow < RuboCop::Cop::Cop
+      MSG = "Do not use catch or throw unless a gem's API demands it."
+
+      def on_send(node)
+        receiver, method_name, _ = *node
+
+        return unless receiver.nil? && %i[catch throw].include?(method_name)
+
+        add_offense(node, location: :expression)
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/gitlab/keys-first-and-values-first.rb b/rubocop/cop/gitlab/keys-first-and-values-first.rb
index 9b68957cba2..544f9800304 100644
--- a/rubocop/cop/gitlab/keys-first-and-values-first.rb
+++ b/rubocop/cop/gitlab/keys-first-and-values-first.rb
@@ -26,7 +26,7 @@ module RuboCop
                            elsif node.descendants.first.method_name == :keys
                              '.each_key'
                            else
-                             throw("Expect '.values.first' or '.keys.first', but get #{node.descendants.first.method_name}.first")
+                             throw("Expect '.values.first' or '.keys.first', but get #{node.descendants.first.method_name}.first") # rubocop:disable Cop/BanCatchThrow
                            end
 
             upto_including_keys_or_values = node.descendants.first.source_range
diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb
index d9f8fe62a86..68e53b17f19 100644
--- a/rubocop/cop/migration/add_column_with_default.rb
+++ b/rubocop/cop/migration/add_column_with_default.rb
@@ -10,7 +10,37 @@ module RuboCop
       class AddColumnWithDefault < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        WHITELISTED_TABLES = [:application_settings].freeze
+        # 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
@@ -23,21 +53,23 @@ module RuboCop
           return unless in_migration?(node)
 
           add_column_with_default?(node) do |table, options|
-            break if table_whitelisted?(table) || nulls_allowed?(options)
-
-            add_offense(node, location: :selector)
+            add_offense(node, location: :selector) if offensive?(table, options)
           end
         end
 
         private
 
+        def offensive?(table, options)
+          table_blacklisted?(table) && !nulls_allowed?(options)
+        end
+
         def nulls_allowed?(options)
           options.find { |opt| opt.key.value == :allow_null && opt.value.true_type? }
         end
 
-        def table_whitelisted?(symbol)
+        def table_blacklisted?(symbol)
           symbol && symbol.type == :sym &&
-            WHITELISTED_TABLES.include?(symbol.children[0])
+            BLACKLISTED_TABLES.include?(symbol.children[0])
         end
       end
     end
diff --git a/rubocop/cop/migration/schedule_async.rb b/rubocop/cop/migration/schedule_async.rb
new file mode 100644
index 00000000000..f296628c3d6
--- /dev/null
+++ b/rubocop/cop/migration/schedule_async.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      class ScheduleAsync < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        ENFORCED_SINCE = 2020_02_12_00_00_00
+
+        MSG = <<~MSG
+          Don't call the background migration worker directly, use the `#migrate_async`,
+          `#migrate_in`, `#bulk_migrate_async` or `#bulk_migrate_in` migration helpers
+          instead.
+        MSG
+
+        def_node_matcher :calls_background_migration_worker?, <<~PATTERN
+          (send (const nil? :BackgroundMigrationWorker) {:perform_async :perform_in :bulk_perform_async :bulk_perform_in} ... )
+        PATTERN
+
+        def on_send(node)
+          return unless in_migration?(node)
+          return if version(node) < ENFORCED_SINCE
+
+          add_offense(node, location: :expression) if calls_background_migration_worker?(node)
+        end
+
+        def autocorrect(node)
+          # This gets rid of the receiver `BackgroundMigrationWorker` and
+          # replaces `perform` with `schedule`
+          schedule_method = method_name(node).to_s.sub('perform', 'migrate')
+          arguments = arguments(node).map(&:source).join(', ')
+
+          replacement = "#{schedule_method}(#{arguments})"
+          lambda do |corrector|
+            corrector.replace(node.source_range, replacement)
+          end
+        end
+
+        private
+
+        def method_name(node)
+          node.children.second
+        end
+
+        def arguments(node)
+          node.children[2..-1]
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/with_lock_retries_with_change.rb b/rubocop/cop/migration/with_lock_retries_with_change.rb
new file mode 100644
index 00000000000..36fc1f92833
--- /dev/null
+++ b/rubocop/cop/migration/with_lock_retries_with_change.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that prevents usage of `with_lock_retries` within the `change` method.
+      class WithLockRetriesWithChange < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = '`with_lock_retries` cannot be used within `change` so you must manually define ' \
+          'the `up` and `down` methods in your migration class and use `with_lock_retries` in both methods'.freeze
+
+        def on_send(node)
+          return unless in_migration?(node)
+          return unless node.children[1] == :with_lock_retries
+
+          node.each_ancestor(:def) do |def_node|
+            add_offense(def_node, location: :name) if method_name(def_node) == :change
+          end
+        end
+
+        def method_name(node)
+          node.children.first
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/scalability/idempotent_worker.rb b/rubocop/cop/scalability/idempotent_worker.rb
new file mode 100644
index 00000000000..a38b457b7c7
--- /dev/null
+++ b/rubocop/cop/scalability/idempotent_worker.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require_relative '../../code_reuse_helpers.rb'
+
+module RuboCop
+  module Cop
+    module Scalability
+      # This cop checks for the `idempotent!` call in the worker scope.
+      #
+      # @example
+      #
+      # # bad
+      # class TroubleMakerWorker
+      #   def perform
+      #   end
+      # end
+      #
+      # # good
+      # class NiceWorker
+      #   idempotent!
+      #
+      #   def perform
+      #   end
+      # end
+      #
+      class IdempotentWorker < RuboCop::Cop::Cop
+        include CodeReuseHelpers
+
+        HELP_LINK = 'https://github.com/mperham/sidekiq/wiki/Best-Practices#2-make-your-job-idempotent-and-transactional'
+
+        MSG = <<~MSG
+          Avoid adding not idempotent workers.
+
+          A worker is considered idempotent if:
+
+          1. It can safely run multiple times with the same arguments
+          2. The application side-effects are expected to happen once (or side-effects of a second run are not impactful)
+          3. It can safely be skipped if another job with the same arguments is already in the queue
+
+          If all the above is true, make sure to mark it as so by calling the `idempotent!`
+          method in the worker scope.
+
+          See #{HELP_LINK}
+        MSG
+
+        def_node_search :idempotent?, <<~PATTERN
+          (send nil? :idempotent!)
+        PATTERN
+
+        def on_class(node)
+          return unless in_worker?(node)
+          return if idempotent?(node)
+
+          add_offense(node, location: :expression)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index 577f768da67..767cacaecaf 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -10,6 +10,10 @@ module RuboCop
       dirname(node).end_with?('db/post_migrate', 'db/geo/post_migrate')
     end
 
+    def version(node)
+      File.basename(node.location.expression.source_buffer.name).split('_').first.to_i
+    end
+
     private
 
     def dirname(node)