diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-19 18:09:10 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-19 18:09:10 +0000 |
commit | 33795139ea8e72756bee3675b4e16387425e6ab1 (patch) | |
tree | 3ca568fca61482e57810ee30ad5ce4b964a82c4e /rubocop | |
parent | c7e385e282bcb8505589bce526e692b7bb819ffa (diff) | |
download | gitlab-ce-33795139ea8e72756bee3675b4e16387425e6ab1.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/migration/schedule_async.rb | 54 | ||||
-rw-r--r-- | rubocop/cop/scalability/idempotent_worker.rb | 59 | ||||
-rw-r--r-- | rubocop/migration_helpers.rb | 4 |
3 files changed, 117 insertions, 0 deletions
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/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) |