Add latest changes from gitlab-org/gitlab@14-3-stable-eev14.3.0-rc42

10 files changed, 405 insertions, 82 deletions

diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb
index a0de43abe85..03ee4805f4e 100644
--- a/rubocop/cop/gitlab/mark_used_feature_flags.rb
+++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb
@@ -47,10 +47,21 @@ module RuboCop
           :usage_data_static_site_editor_merge_requests # https://gitlab.com/gitlab-org/gitlab/-/issues/284083
         ].freeze
 
+        class << self
+          # We track feature flags in `on_new_investigation` only once per
+          # rubocop whole run instead once per file.
+          attr_accessor :feature_flags_already_tracked
+        end
+
         # Called before all on_... have been called
         # When refining this method, always call `super`
         def on_new_investigation
           super
+
+          return if self.class.feature_flags_already_tracked
+
+          self.class.feature_flags_already_tracked = true
+
           track_dynamic_feature_flags!
           track_usage_data_counters_known_events!
         end
@@ -69,7 +80,7 @@ module RuboCop
           flag_value = flag_value(node)
           return unless flag_value
 
-          if flag_arg_is_str_or_sym?(node)
+          if flag_arg_is_str_or_sym?(flag_arg)
             if caller_is_feature_gitaly?(node)
               save_used_feature_flag("gitaly_#{flag_value}")
             else
@@ -84,9 +95,9 @@ module RuboCop
                 save_used_feature_flag(matching_feature_flag)
               end
             end
-          elsif flag_arg_is_send_type?(node)
+          elsif flag_arg_is_send_type?(flag_arg)
             puts_if_ci(node, "Feature flag is dynamic: '#{flag_value}.")
-          elsif flag_arg_is_dstr_or_dsym?(node)
+          elsif flag_arg_is_dstr_or_dsym?(flag_arg)
             str_prefix = flag_arg.children[0]
             rest_children = flag_arg.children[1..]
 
@@ -159,18 +170,16 @@ module RuboCop
           end.to_s.tr("\n/", ' _')
         end
 
-        def flag_arg_is_str_or_sym?(node)
-          flag_arg = flag_arg(node)
+        def flag_arg_is_str_or_sym?(flag_arg)
           flag_arg.str_type? || flag_arg.sym_type?
         end
 
-        def flag_arg_is_send_type?(node)
-          flag_arg(node).send_type?
+        def flag_arg_is_send_type?(flag_arg)
+          flag_arg.send_type?
         end
 
-        def flag_arg_is_dstr_or_dsym?(node)
-          flag = flag_arg(node)
-          (flag.dstr_type? || flag.dsym_type?) && flag.children[0].str_type?
+        def flag_arg_is_dstr_or_dsym?(flag_arg)
+          (flag_arg.dstr_type? || flag_arg.dsym_type?) && flag_arg.children[0].str_type?
         end
 
         def caller_is_feature?(node)
diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb
index f45551e60a4..b5780e87c19 100644
--- a/rubocop/cop/migration/add_limit_to_text_columns.rb
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -13,8 +13,13 @@ module RuboCop
       class AddLimitToTextColumns < RuboCop::Cop::Cop
         include MigrationHelpers
 
+        TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE = 2021_09_10_00_00_00
+
         MSG = 'Text columns should always have a limit set (255 is suggested). ' \
-          'You can add a limit to a `text` column by using `add_text_limit`'
+          'You can add a limit to a `text` column by using `add_text_limit` or by using `.text... limit: 255` inside `create_table`'
+
+        TEXT_LIMIT_ATTRIBUTE_NOT_ALLOWED = 'Text columns should always have a limit set (255 is suggested). Using limit: is not supported in this version. ' \
+        'You can add a limit to a `text` column by using `add_text_limit` or `.text_limit` inside `create_table`'
 
         def_node_matcher :reverting?, <<~PATTERN
           (def :down ...)
@@ -37,15 +42,29 @@ module RuboCop
           node.each_descendant(:send) do |send_node|
             next unless text_operation?(send_node)
 
-            # We require a limit for the same table and attribute name
-            if text_limit_missing?(node, *table_and_attribute_name(send_node))
-              add_offense(send_node, location: :selector)
+            if text_operation_with_limit?(send_node)
+              add_offense(send_node, location: :selector, message: TEXT_LIMIT_ATTRIBUTE_NOT_ALLOWED) if version(node) < TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE
+            else
+              # We require a limit for the same table and attribute name
+              if text_limit_missing?(node, *table_and_attribute_name(send_node))
+                add_offense(send_node, location: :selector)
+              end
             end
           end
         end
 
         private
 
+        def text_operation_with_limit?(node)
+          migration_method = node.children[1]
+
+          return unless migration_method == :text
+
+          if attributes = node.children[3]
+            attributes.pairs.find { |pair| pair.key.value == :limit }.present?
+          end
+        end
+
         def text_operation?(node)
           # Don't complain about text arrays
           return false if array_column?(node)
diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb
index c90f911d24e..c383466f73b 100644
--- a/rubocop/cop/migration/prevent_index_creation.rb
+++ b/rubocop/cop/migration/prevent_index_creation.rb
@@ -12,16 +12,29 @@ module RuboCop
 
         MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886"
 
+        def on_new_investigation
+          super
+          @forbidden_tables_used = false
+        end
+
         def_node_matcher :add_index?, <<~PATTERN
-          (send nil? :add_index (sym #forbidden_tables?) ...)
+          (send nil? :add_index ({sym|str} #forbidden_tables?) ...)
         PATTERN
 
         def_node_matcher :add_concurrent_index?, <<~PATTERN
-          (send nil? :add_concurrent_index (sym #forbidden_tables?) ...)
+          (send nil? :add_concurrent_index ({sym|str} #forbidden_tables?) ...)
         PATTERN
 
-        def forbidden_tables?(node)
-          FORBIDDEN_TABLES.include?(node)
+        def_node_matcher :forbidden_constant_defined?, <<~PATTERN
+          (casgn nil? _ ({sym|str} #forbidden_tables?))
+        PATTERN
+
+        def_node_matcher :add_concurrent_index_with_constant?, <<~PATTERN
+          (send nil? :add_concurrent_index (const nil? _) ...)
+        PATTERN
+
+        def on_casgn(node)
+          @forbidden_tables_used = !!forbidden_constant_defined?(node)
         end
 
         def on_def(node)
@@ -32,8 +45,18 @@ module RuboCop
           end
         end
 
+        private
+
+        def forbidden_tables?(node)
+          FORBIDDEN_TABLES.include?(node.to_sym)
+        end
+
         def offense?(node)
-          add_index?(node) || add_concurrent_index?(node)
+          add_index?(node) || add_concurrent_index?(node) || any_constant_used_with_forbidden_tables?(node)
+        end
+
+        def any_constant_used_with_forbidden_tables?(node)
+          add_concurrent_index_with_constant?(node) && @forbidden_tables_used
         end
       end
     end
diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb
new file mode 100644
index 00000000000..f2e4550c691
--- /dev/null
+++ b/rubocop/cop/migration/versioned_migration_class.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      class VersionedMigrationClass < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        ENFORCED_SINCE = 2021_09_02_00_00_00
+
+        MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
+        MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
+
+        ACTIVERECORD_MIGRATION_CLASS = 'ActiveRecord::Migration'
+
+        def_node_search :includes_helpers?, <<~PATTERN
+        (send nil? :include
+          (const
+            (const
+              (const nil? :Gitlab) :Database) :MigrationHelpers))
+        PATTERN
+
+        def on_class(node)
+          return unless relevant_migration?(node)
+          return unless activerecord_migration_class?(node)
+
+          add_offense(node, location: :expression, message: MSG_INHERIT)
+        end
+
+        def on_send(node)
+          return unless relevant_migration?(node)
+
+          add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_helpers?(node)
+        end
+
+        private
+
+        def relevant_migration?(node)
+          in_migration?(node) && version(node) >= ENFORCED_SINCE
+        end
+
+        def activerecord_migration_class?(node)
+          superclass(node) == ACTIVERECORD_MIGRATION_CLASS
+        end
+
+        def superclass(class_node)
+          _, *others = class_node.descendants
+
+          others.find { |node| node.const_type? && node&.const_name != 'Types' }&.const_name
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/performance/active_record_subtransaction_methods.rb b/rubocop/cop/performance/active_record_subtransaction_methods.rb
new file mode 100644
index 00000000000..3b89d3ab858
--- /dev/null
+++ b/rubocop/cop/performance/active_record_subtransaction_methods.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      # Cop that disallows certain methods that rely on subtransactions in their implementation.
+      # Companion to Performance/ActiveRecordSubtransactions, which bans direct usage of subtransactions.
+      class ActiveRecordSubtransactionMethods < RuboCop::Cop::Cop
+        MSG = 'Methods that rely on subtransactions should not be used. ' \
+          'For more information see: https://gitlab.com/gitlab-org/gitlab/-/issues/338346'
+
+        DISALLOWED_METHODS = %i[
+          safe_ensure_unique
+          safe_find_or_create_by
+          safe_find_or_create_by!
+          with_fast_read_statement_timeout
+          create_or_find_by
+          create_or_find_by!
+        ].to_set.freeze
+
+        def on_send(node)
+          return unless DISALLOWED_METHODS.include?(node.method_name)
+
+          add_offense(node, location: :selector)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/performance/active_record_subtransactions.rb b/rubocop/cop/performance/active_record_subtransactions.rb
new file mode 100644
index 00000000000..a550b558e52
--- /dev/null
+++ b/rubocop/cop/performance/active_record_subtransactions.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Performance
+      class ActiveRecordSubtransactions < RuboCop::Cop::Cop
+        MSG = 'Subtransactions should not be used. ' \
+          'For more information see: https://gitlab.com/gitlab-org/gitlab/-/issues/338346'
+
+        def_node_matcher :match_transaction_with_options, <<~PATTERN
+          (send _ :transaction (hash $...))
+        PATTERN
+
+        def_node_matcher :subtransaction_option?, <<~PATTERN
+          (pair (:sym :requires_new) (true))
+        PATTERN
+
+        def on_send(node)
+          match_transaction_with_options(node) do |option_nodes|
+            option_nodes.each do |option_node|
+              next unless subtransaction_option?(option_node)
+
+              add_offense(option_node)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb
new file mode 100644
index 00000000000..7a2e7ee00b4
--- /dev/null
+++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb
@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require_relative '../../code_reuse_helpers'
+
+module RuboCop
+  module Cop
+    module SidekiqLoadBalancing
+      # This cop checks for a call to `data_consistency` to exist in Sidekiq workers.
+      #
+      # @example
+      #
+      # # bad
+      # class BadWorker
+      #   def perform
+      #   end
+      # end
+      #
+      # # good
+      # class GoodWorker
+      #   data_consistency :delayed
+      #
+      #   def perform
+      #   end
+      # end
+      #
+      class WorkerDataConsistency < RuboCop::Cop::Cop
+        include CodeReuseHelpers
+
+        HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#job-data-consistency-strategies'
+
+        MSG = <<~MSG
+          Should define data_consistency expectation.
+
+          It is encouraged for workers to use database replicas as much as possible by declaring
+          data_consistency to use the :delayed or :sticky modes. Mode :always will result in the
+          worker always hitting the primary database node for both reads and writes, which limits
+          scalability.
+
+          Some guidelines:
+
+          1. If your worker mostly writes or reads its own writes, use mode :always. TRY TO AVOID THIS.
+          2. If your worker performs mostly reads and can tolerate small delays, use mode :delayed.
+          3. If your worker performs mostly reads but cannot tolerate any delays, use mode :sticky.
+
+          See #{HELP_LINK} for a more detailed explanation of these settings.
+        MSG
+
+        def_node_search :application_worker?, <<~PATTERN
+        `(send nil? :include (const nil? :ApplicationWorker))
+        PATTERN
+
+        def_node_search :data_consistency_defined?, <<~PATTERN
+          `(send nil? :data_consistency ...)
+        PATTERN
+
+        def on_class(node)
+          return unless in_worker?(node) && application_worker?(node)
+          return if data_consistency_defined?(node)
+
+          add_offense(node, location: :expression)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb
new file mode 100644
index 00000000000..e8b4b513a23
--- /dev/null
+++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb
@@ -0,0 +1,154 @@
+# frozen_string_literal: true
+
+require_relative '../../code_reuse_helpers'
+
+module RuboCop
+  module Cop
+    module SidekiqLoadBalancing
+      # This cop checks for including_scheduled: true option in idempotent Sidekiq workers that utilize load balancing capabilities.
+      #
+      # @example
+      #
+      # # bad
+      # class BadWorker
+      #   include ApplicationWorker
+      #
+      #   data_consistency :delayed
+      #   idempotent!
+      #
+      #   def perform
+      #   end
+      # end
+      #
+      # # bad
+      # class BadWorker
+      #   include ApplicationWorker
+      #
+      #   data_consistency :delayed
+      #
+      #   deduplicate :until_executing
+      #   idempotent!
+      #
+      #   def perform
+      #   end
+      # end
+      #
+      # # good
+      # class GoodWorker
+      #   include ApplicationWorker
+      #
+      #   data_consistency :delayed
+      #
+      #   deduplicate :until_executing, including_scheduled: true
+      #   idempotent!
+      #
+      #   def perform
+      #   end
+      # end
+      #
+      class WorkerDataConsistencyWithDeduplication < RuboCop::Cop::Base
+        include CodeReuseHelpers
+        extend AutoCorrector
+
+        HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#scheduling-jobs-in-the-future'
+        REPLACEMENT = ', including_scheduled: true'
+        DEFAULT_STRATEGY = ':until_executing'
+
+        MSG = <<~MSG
+          Workers that declare either `:sticky` or `:delayed` data consistency become eligible for database load-balancing.
+          In both cases, jobs are enqueued with a short delay.
+
+          If you do want to deduplicate jobs that utilize load-balancing, you need to specify including_scheduled: true
+          argument when defining deduplication strategy.
+
+          See #{HELP_LINK} for a more detailed explanation of these settings.
+        MSG
+
+        def_node_search :application_worker?, <<~PATTERN
+          `(send nil? :include (const nil? :ApplicationWorker))
+        PATTERN
+
+        def_node_search :idempotent_worker?, <<~PATTERN
+          `(send nil? :idempotent!)
+        PATTERN
+
+        def_node_search :data_consistency_defined?, <<~PATTERN
+          `(send nil? :data_consistency (sym {:sticky :delayed }))
+        PATTERN
+
+        def_node_matcher :including_scheduled?, <<~PATTERN
+          `(hash <(pair (sym :including_scheduled) (%1)) ...>)
+        PATTERN
+
+        def_node_matcher :deduplicate_strategy?, <<~PATTERN
+          `(send nil? :deduplicate (sym $_) $(...)?)
+        PATTERN
+
+        def on_class(node)
+          return unless in_worker?(node)
+          return unless application_worker?(node)
+          return unless idempotent_worker?(node)
+          return unless data_consistency_defined?(node)
+
+          @strategy, options = deduplicate_strategy?(node)
+          including_scheduled = false
+          if options
+            @deduplicate_options = options[0]
+            including_scheduled = including_scheduled?(@deduplicate_options, :true) # rubocop:disable Lint/BooleanSymbol
+          end
+
+          @offense = !(including_scheduled || @strategy == :none)
+        end
+
+        def on_send(node)
+          return unless offense
+
+          if node.children[1] == :deduplicate
+            add_offense(node.loc.expression) do |corrector|
+              autocorrect_deduplicate_strategy(node, corrector)
+            end
+          elsif node.children[1] == :idempotent! && !strategy
+            add_offense(node.loc.expression) do |corrector|
+              autocorrect_missing_deduplicate_strategy(node, corrector)
+            end
+          end
+        end
+
+        private
+
+        attr_reader :offense, :deduplicate_options, :strategy
+
+        def autocorrect_deduplicate_with_options(corrector)
+          if including_scheduled?(deduplicate_options, :false) # rubocop:disable Lint/BooleanSymbol
+            replacement = deduplicate_options.source.sub("including_scheduled: false", "including_scheduled: true")
+            corrector.replace(deduplicate_options.loc.expression, replacement)
+          else
+            corrector.insert_after(deduplicate_options.loc.expression, REPLACEMENT)
+          end
+        end
+
+        def autocorrect_deduplicate_without_options(node, corrector)
+          corrector.insert_after(node.loc.expression, REPLACEMENT)
+        end
+
+        def autocorrect_missing_deduplicate_strategy(node, corrector)
+          indent_found = node.source_range.source_line =~ /^( +)/
+          # Get indentation size
+          whitespaces = Regexp.last_match(1).size if indent_found
+          replacement = "deduplicate #{DEFAULT_STRATEGY}#{REPLACEMENT}\n"
+          # Add indentation in the end since we are inserting a whole line before idempotent!
+          replacement += ' ' * whitespaces.to_i
+          corrector.insert_before(node.source_range, replacement)
+        end
+
+        def autocorrect_deduplicate_strategy(node, corrector)
+          if deduplicate_options
+            autocorrect_deduplicate_with_options(corrector)
+          else
+            autocorrect_deduplicate_without_options(node, corrector)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/worker_data_consistency.rb b/rubocop/cop/worker_data_consistency.rb
deleted file mode 100644
index e9047750f3e..00000000000
--- a/rubocop/cop/worker_data_consistency.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../code_reuse_helpers'
-
-module RuboCop
-  module Cop
-    # This cop checks for a call to `data_consistency` to exist in Sidekiq workers.
-    #
-    # @example
-    #
-    # # bad
-    # class BadWorker
-    #   def perform
-    #   end
-    # end
-    #
-    # # good
-    # class GoodWorker
-    #   data_consistency :delayed
-    #
-    #   def perform
-    #   end
-    # end
-    #
-    class WorkerDataConsistency < RuboCop::Cop::Cop
-      include CodeReuseHelpers
-
-      HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#job-data-consistency-strategies'
-
-      MSG = <<~MSG
-        Should define data_consistency expectation.
-
-        It is encouraged for workers to use database replicas as much as possible by declaring
-        data_consistency to use the :delayed or :sticky modes. Mode :always will result in the
-        worker always hitting the primary database node for both reads and writes, which limits
-        scalability.
-
-        Some guidelines:
-        
-        1. If your worker mostly writes or reads its own writes, use mode :always. TRY TO AVOID THIS.
-        2. If your worker performs mostly reads and can tolerate small delays, use mode :delayed.
-        3. If your worker performs mostly reads but cannot tolerate any delays, use mode :sticky.
-
-        See #{HELP_LINK} for a more detailed explanation of these settings.
-      MSG
-
-      def_node_search :application_worker?, <<~PATTERN
-      `(send nil? :include (const nil? :ApplicationWorker))
-      PATTERN
-
-      def_node_search :data_consistency_defined?, <<~PATTERN
-        `(send nil? :data_consistency ...)
-      PATTERN
-
-      def on_class(node)
-        return unless in_worker?(node) && application_worker?(node)
-        return if data_consistency_defined?(node)
-
-        add_offense(node, location: :expression)
-      end
-    end
-  end
-end
diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml
index 979b0fa2936..a98a059df1d 100644
--- a/rubocop/rubocop-migrations.yml
+++ b/rubocop/rubocop-migrations.yml
@@ -14,6 +14,7 @@ Migration/UpdateLargeTable:
     - :events
     - :gitlab_subscriptions
     - :issues
+    - :members
     - :merge_request_diff_commits
     - :merge_request_diff_files
     - :merge_request_diffs