diff options
Diffstat (limited to 'rubocop')
| -rw-r--r-- | rubocop/cop/migration/add_column_with_default_to_large_table.rb | 51 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_concurrent_index.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/migration/remove_concurrent_index.rb | 29 | ||||
| -rw-r--r-- | rubocop/cop/migration/remove_index.rb | 26 | ||||
| -rw-r--r-- | rubocop/cop/migration/reversible_add_column_with_default.rb (renamed from rubocop/cop/migration/add_column_with_default.rb) | 21 | ||||
| -rw-r--r-- | rubocop/rubocop.rb | 5 |
6 files changed, 122 insertions, 12 deletions
diff --git a/rubocop/cop/migration/add_column_with_default_to_large_table.rb b/rubocop/cop/migration/add_column_with_default_to_large_table.rb new file mode 100644 index 00000000000..2372e6b60ea --- /dev/null +++ b/rubocop/cop/migration/add_column_with_default_to_large_table.rb @@ -0,0 +1,51 @@ +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 AddColumnWithDefaultToLargeTable < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = 'Using `add_column_with_default` on the `%s` table will take a ' \ + 'long time to complete, and should be avoided unless absolutely ' \ + 'necessary'.freeze + + LARGE_TABLES = %i[ + events + issues + merge_requests + namespaces + notes + projects + routes + users + ].freeze + + def_node_matcher :add_column_with_default?, <<~PATTERN + (send nil :add_column_with_default $(sym ...) ...) + PATTERN + + def on_send(node) + return unless in_migration?(node) + + matched = add_column_with_default?(node) + return unless matched + + table = matched.to_a.first + return unless LARGE_TABLES.include?(table) + + add_offense(node, :expression, format(MSG, table)) + end + end + end + end +end diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb index 332fb7dcbd7..69852f4d580 100644 --- a/rubocop/cop/migration/add_concurrent_index.rb +++ b/rubocop/cop/migration/add_concurrent_index.rb @@ -9,7 +9,7 @@ module RuboCop include MigrationHelpers MSG = '`add_concurrent_index` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `remove_index` in `down`'.freeze + 'the `up` and `down` methods in your migration class, using `remove_concurrent_index` in `down`'.freeze def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/remove_concurrent_index.rb b/rubocop/cop/migration/remove_concurrent_index.rb new file mode 100644 index 00000000000..268c49865cb --- /dev/null +++ b/rubocop/cop/migration/remove_concurrent_index.rb @@ -0,0 +1,29 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `remove_concurrent_index` is used with `up`/`down` methods + # and not `change`. + class RemoveConcurrentIndex < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`remove_concurrent_index` is not reversible so you must manually define ' \ + 'the `up` and `down` methods in your migration class, using `add_concurrent_index` in `down`'.freeze + + def on_send(node) + return unless in_migration?(node) + return unless node.children[1] == :remove_concurrent_index + + node.each_ancestor(:def) do |def_node| + add_offense(def_node, :name) if method_name(def_node) == :change + end + end + + def method_name(node) + node.children[0] + end + end + end + end +end diff --git a/rubocop/cop/migration/remove_index.rb b/rubocop/cop/migration/remove_index.rb new file mode 100644 index 00000000000..613b35dd00d --- /dev/null +++ b/rubocop/cop/migration/remove_index.rb @@ -0,0 +1,26 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if indexes are removed in a concurrent manner. + class RemoveIndex < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`remove_index` requires downtime, use `remove_concurrent_index` instead'.freeze + + def on_def(node) + return unless in_migration?(node) + + node.each_descendant(:send) do |send_node| + add_offense(send_node, :selector) if method_name(send_node) == :remove_index + end + end + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/reversible_add_column_with_default.rb index 54a920d4b49..f413f06f39b 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/reversible_add_column_with_default.rb @@ -5,29 +5,30 @@ module RuboCop module Migration # Cop that checks if `add_column_with_default` is used with `up`/`down` methods # and not `change`. - class AddColumnWithDefault < RuboCop::Cop::Cop + class ReversibleAddColumnWithDefault < RuboCop::Cop::Cop include MigrationHelpers + def_node_matcher :add_column_with_default?, <<~PATTERN + (send nil :add_column_with_default $...) + PATTERN + + def_node_matcher :defines_change?, <<~PATTERN + (def :change ...) + PATTERN + MSG = '`add_column_with_default` is not reversible so you must manually define ' \ 'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze def on_send(node) return unless in_migration?(node) - - name = node.children[1] - - return unless name == :add_column_with_default + return unless add_column_with_default?(node) node.each_ancestor(:def) do |def_node| - next unless method_name(def_node) == :change + next unless defines_change?(def_node) add_offense(def_node, :name) end end - - def method_name(node) - node.children.first - end end end end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index a50a522cf9d..4ff204f939e 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,7 +1,10 @@ require_relative 'cop/custom_error_class' require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' -require_relative 'cop/migration/add_column_with_default' +require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' +require_relative 'cop/migration/remove_concurrent_index' +require_relative 'cop/migration/remove_index' +require_relative 'cop/migration/reversible_add_column_with_default' |