Add latest changes from gitlab-org/gitlab@13-1-stable-ee

4 files changed, 58 insertions, 49 deletions

diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb
index 15c28bb9266..b578e73f19e 100644
--- a/rubocop/cop/migration/add_limit_to_text_columns.rb
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -39,6 +39,9 @@ module RuboCop
         private
 
         def text_operation?(node)
+          # Don't complain about text arrays
+          return false if array_column?(node)
+
           modifier = node.children[0]
           migration_method = node.children[1]
 
diff --git a/rubocop/cop/migration/drop_table.rb b/rubocop/cop/migration/drop_table.rb
new file mode 100644
index 00000000000..2a0f57c0c13
--- /dev/null
+++ b/rubocop/cop/migration/drop_table.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if `drop_table` is called in deployment migrations.
+      # Calling it in deployment migrations can cause downtimes as there still may be code using the target tables.
+      class DropTable < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = <<-MESSAGE.delete("\n").squeeze
+          `drop_table` in deployment migrations requires downtime.
+          Drop tables in post-deployment migrations instead.
+        MESSAGE
+
+        def on_def(node)
+          return unless in_deployment_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            next unless offensible?(send_node)
+
+            add_offense(send_node, location: :selector)
+          end
+        end
+
+        private
+
+        def offensible?(node)
+          drop_table?(node) || drop_table_in_execute?(node)
+        end
+
+        def drop_table?(node)
+          node.children[1] == :drop_table
+        end
+
+        def drop_table_in_execute?(node)
+          execute?(node) && drop_table_in_execute_sql?(node)
+        end
+
+        def execute?(node)
+          node.children[1] == :execute
+        end
+
+        def drop_table_in_execute_sql?(node)
+          node.children[2].to_s.match?(/drop\s+table/i)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb
index 25c00194698..bfeabd2c78d 100644
--- a/rubocop/cop/migration/prevent_strings.rb
+++ b/rubocop/cop/migration/prevent_strings.rb
@@ -33,6 +33,9 @@ module RuboCop
         private
 
         def string_operation?(node)
+          # Don't complain about string arrays
+          return false if array_column?(node)
+
           modifier = node.children[0]
           migration_method = node.children[1]
 
diff --git a/rubocop/cop/migration/update_large_table.rb b/rubocop/cop/migration/update_large_table.rb
deleted file mode 100644
index e44eadbdb92..00000000000
--- a/rubocop/cop/migration/update_large_table.rb
+++ /dev/null
@@ -1,49 +0,0 @@
-require_relative '../../migration_helpers'
-
-module RuboCop
-  module Cop
-    module Migration
-      # This cop checks for `add_column_with_default` on a table that's been
-      # explicitly blacklisted because of its size.
-      #
-      # Even though this helper performs the update in batches to avoid
-      # downtime, using it with tables with millions of rows still causes a
-      # significant delay in the deploy process and is best avoided.
-      #
-      # See https://gitlab.com/gitlab-com/infrastructure/issues/1602 for more
-      # information.
-      class UpdateLargeTable < RuboCop::Cop::Cop
-        include MigrationHelpers
-
-        MSG = 'Using `%s` on the `%s` table will take a long time to ' \
-              'complete, and should be avoided unless absolutely ' \
-              'necessary'.freeze
-
-        BATCH_UPDATE_METHODS = %w[
-          :add_column_with_default
-          :change_column_type_concurrently
-          :rename_column_concurrently
-          :update_column_in_batches
-        ].join(' ').freeze
-
-        def_node_matcher :batch_update?, <<~PATTERN
-          (send nil? ${#{BATCH_UPDATE_METHODS}} $(sym ...) ...)
-        PATTERN
-
-        def on_send(node)
-          return unless in_migration?(node)
-
-          matches = batch_update?(node)
-          return unless matches
-
-          update_method = matches.first
-          table = matches.last.to_a.first
-
-          return unless BLACKLISTED_TABLES.include?(table)
-
-          add_offense(node, location: :expression, message: format(MSG, update_method, table))
-        end
-      end
-    end
-  end
-end