From 8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Jun 2020 11:18:50 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-1-stable-ee --- rubocop/cop/migration/add_limit_to_text_columns.rb | 3 ++ rubocop/cop/migration/drop_table.rb | 52 ++++++++++++++++++++++ rubocop/cop/migration/prevent_strings.rb | 3 ++ rubocop/cop/migration/update_large_table.rb | 49 -------------------- 4 files changed, 58 insertions(+), 49 deletions(-) create mode 100644 rubocop/cop/migration/drop_table.rb delete mode 100644 rubocop/cop/migration/update_large_table.rb (limited to 'rubocop/cop/migration') 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 -- cgit v1.2.3