# bad - passing by id
# Deserialization error is reported, the job *is* scheduled for retry.
class SomeJob < ApplicationJob
  def perform(model_id)
    model = Model.find(model_id)
    do_something_with(model)
  end
end

# bad - model mismatch
class SomeJob < ApplicationJob
  def perform(model_id)
    Model.find(model_id)
    # ...
  end
end

# Will try to fetch a Model using another model class, e.g. User's id.
SomeJob.perform_later(user.id)

# acceptable - passing by id
# Deserialization error is reported, the job is *not* scheduled for retry.
class SomeJob < ApplicationJob
  def perform(model_id)
    model = Model.find(model_id)
    do_something_with(model)
  rescue ActiveRecord::RecordNotFound
    Rollbar.warning('Not found')
  end
end

# good - passing with GlobalID
# Deserialization error is reported, the job is *not* scheduled for retry.
class SomeJob < ApplicationJob
  def perform(model)
    do_something_with(model)
  end
end

class SomeJob < ApplicationJob
  include TransitionHelper

  def perform(model)
    # TODO: remove this when all jobs with numeric id arguments are processed
    model = fetch(model, Model)
    do_something_with(model)
  end
end

module TransitionHelper
  def fetch(id_or_object, model_class)
    case id_or_object
    when Numeric
      model_class.find(id_or_object)
    when model_class
      id_or_object
    else
      fail "Object type mismatch #{model_class}, #{id_or_object}"
    end
  end
end

# bad - no queue specified
class SomeJob < ApplicationJob
  def perform
    # ...
  end
end

# bad - the wrong queue specified
class SomeJob < ApplicationJob
  queue_as :hgh_prioriti # nonexistent queue specified

  def perform
    # ...
  end
end

# good
class SomeJob < ApplicationJob
  queue_as :high_priority

  def perform
    # ...
  end
end

class UserNotificationJob < ApplicationJob
  def perform(user)
    send_email_to(user) unless already_notified?(user)
  end
end

# bad - consumes all available connections
class SomeJob < ApplicationJob
  def perform
    User.find_each |user|
      Thread.new do
        ExternalService.update(user)
      end
    end
  end
end

# good
class SomeJob < ApplicationJob
  def perform(user)
    ExternalService.update(user)
  end
end

User.find_each |user|
  SomeJob.perform_later(user)
end

# bad - makes three attempts without submitting to Rollbar,
# fails and relies on Sidekiq's retry that would also make several
# retry attempts, submitting each of the failures to Rollbar.
class SomeJob < ApplicationJob
  retry_on ThirdParty::Api::Errors::SomeError, wait: 1.minute, attempts: 3

  def perform(user)
    # ...
  end
end

# bad - it's not clear upfront if the job will be retried or not
class SomeJob < ApplicationJob
  include ReliableJob

  def perform(user)
    # ...
  end
end

# good - Sidekiq deals with retries
class SomeJob < ApplicationJob
  sidekiq_options retry: 3

  def perform(user)
    # ...
  end
end

# bad - job may perform earlier than the transaction is committed
User.transaction do
  users_params.each do |user_params|
    user = User.create!(user_params)
    NotifyUserJob.perform_later(user)
  end
end

# good
users = User.transaction do
          users_params.map do |user_params|
            User.create!(user_params)
          end
        end
users.each { |user| NotifyUserJob.perform_later(user) }

EAGER_LOAD=true ALLOW_CONCURRENCY=true bundle exec sidekiq

# bad
class SendUserAgreementJob < ApplicationJob
  # Convenient method to check if preconditions are satisfied to avoid
  # scheduling unnecessary jobs.
  def self.perform_later_if_applies(user)
    job = new(user)
    return unless job.satisfy_preconditions?

    job.enqueue
  end

  def perform(user)
    @user = user
    return unless satisfy_preconditions?

    agreement = agreement_for(user: user)
    AgreementMailer.deliver_now(agreement)
  end

  def satisfy_preconditions?
    legal_agreement_signed? &&
      !user.removed? &&
      !user.referral? &&
      !(user.active? || user.pending?) &&
      !user.has_flag?(:on_hold)
  end

  private

  attr_reader :user

  # business logic
end

# good - business logic is not coupled to the job
class SendUserAgreementJob < ApplicationJob
  def perform(user)
    agreement = agreement_for(user: user)
    AgreementMailer.deliver_now(agreement)
  end
end

SendUserAgreementJob.perform_later(user) if satisfy_preconditions?

# good - error kernel pattern
# bad - additional jobs are spawned
class SomeJob < ApplicationJob
  def perform
    SomeMailer.some_notification.deliver_later
    OtherJob.perform_later
  end
end

# good - no additional jobs
# bad - if `OtherJob` fails, `SomeMailer` will be re-executed on retry as well
class SomeJob < ApplicationJob
  def perform
    SomeMailer.some_notification.deliver_now
    OtherJob.perform_now
  end
end

# good - keep the old class
# TODO: Delete this alias in a few weeks when old jobs are safely gone
OldJob = NewJob

# bad
class SomeJob < ApplicationJob
  def perform(user)
    attempts_number = 3
    ThirdParty::Api::User.renew(user.external_id)
  rescue ThirdParty::Api::Errors::TooManyRequestsError => error
    sleep(error.retry_after)
    attempts_number -= 1
    retry unless attempts_number.zero?
    raise
  end
end

# good - retry job in a while, a limited number of times
class SomeJob < ApplicationJob
  sidekiq_options retry: 3
  sidekiq_retry_in do |count, exception|
    case exception
    when ThirdParty::Api::Errors::TooManyRequestsError
      count + 1 # i.e. 1s, 2s, 3s
    end
  end

  def perform(user)
    ThirdParty::Api::User.renew(user.external_id)
  end
end

# good - fine-grained control of API usage in jobs
class SomeJob < ApplicationJob
  def perform(user)
    LIMITER.within_limit do
      ThirdParty::Api::User.renew(user.external_id)
    end
  end
end

# config/initializers/sidekiq.rb
Sidekiq::Limiter.configure do |config|
  config.errors << ThirdParty::Api::Errors::TooManyRequestsError
end

# bad
class BackfillMissingDataJob < ApplicationJob
  def self.run_batch
    Model.where(attribute: nil).find_each do |model|
      perform_later(model)
    end
  end

  def perform(model)
    # do the job
  end
end

# good
class BackfillMissingDataJob < ApplicationJob
  def self.run_batch
    batch = Sidekiq::Batch.new
    batch.description = 'Backfill missing data'
    batch.on(:success, BackfillComplete, to: SysAdmin.email)
    batch.jobs do
      Model.where(attribute: nil).find_each do |model|
        perform_later(model)
      end
    end
  end

  def perform(model)
    # do the job
  end
end

# bad
class BackfillMissingDataJob < ApplicationJob
  SIZE = 20
  def perform(offset = 0)
    models = Model.where(attribute: nil)
      .order(:id).offset(offset).limit(SIZE)
    return if models.empty?

    models.each do |model|
      model.update!(attribute: for(model))
    end
    self.class.perform_later(offset + SIZE)
  end
end

# good
class BackfillMissingDataJob < ApplicationJob
  def self.run_batch
    Sidekiq::Batch.new.jobs do
      Model.where(attribute: nil)
        .find_in_batches(20) do |models|
        BackfillMissingDataJob.perform_later(models)
      end
    end
  end

  def perform(models)
    models.each do |model|
      model.update!(attribute: for(model))
    end
  end
end

# bad
class UpdateExternalDataJob < ApplicationJob
  def perform(user)
    new_attribute = ThirdParty::Api.get_attribute(user.external_id)
    user.update!(attribute: new_attribute)
  end
end

User.where.not(external_id: nil)
  .find_in_batches.with_index do |group_number, users|
  users.each do |user|
    UpdateExternalDataJob
      .set(wait: group_number.minutes)
      .perform_later(users)
    end
end

# good
class UpdateExternalDataJob < ApplicationJob
  LIMITER = Sidekiq::Limiter.window('third-party-attribute-update', 20, :minute, wait_timeout: 0)

  def perform(user)
    LIMITER.within_limit do
      new_attribute = ThirdParty::Api.get_attribute(user.external_id)
      user.update!(attribute: new_attribute)
    end
  end
end

# Application code
User.where.not(external_id: nil).find_each do |user|
  UpdateExternalDataJob.perform_later(user)
end

# config/initializers/sidekiq.rb
Sidekiq::Limiter.configure do |config|
  config.errors << ThirdParty::Api::Errors::TooManyRequestsError
end

DEFAULT_BACKOFF = ->(limiter, job) do
  (300 * job['overrated']) + rand(300) + 1
end

Sidekiq::Limiter.configure do |config|
  config.backoff = ->(limiter, job) do
    case limiter.name
    when 'daily-third-party-api-limit'
      12.hours
    else
      (300 * job['overrated']) + rand(300) + 1 # fallback to default
    end
  end
end

 Sidekiq::Limiter.bucket('custom-limiter', 1, :day) == Sidekiq::Limiter.bucket('custom-limiter', 1, :day) # => false

# bad - limiter is re-created on each job call
class SomeJob < ApplicationJob
  def perform(...)
    limiter = Sidekiq::Limiter.concurrent('erp', 50, wait_timeout: 0, lock_timeout: 30)
    limiter.within_limit do
      # call ERP
    end
  end
end

# good
class SomeJob < ApplicationJob
  ERP_LIMIT = Sidekiq::Limiter.concurrent('erp', 50, wait_timeout: 0, lock_timeout: 30)

  def perform(...)
    ERP_LIMIT.within_limit do
      # call ERP
    end
  end
end

# acceptable - an exception is when the limiter is specific to something, and that is used as a distinction key in limiter name.
class SomeJob < ApplicationJob
  def perform(user)
    # Rate limiting is per user account
    user_throttle = Sidekiq::Limiter.bucket("stripe-#{user.id}", 30, :second, wait_timeout: 0)
    user_throttle.within_limit do
      # call stripe with user's account creds
    end
  end
end

# Middleware
class SaturationLimiter
  SaturationOverLimit = Class.new(StandardError)

  def self.wrapper(job, block)
    LIMITER.within_limit { block.call }
  rescue Sidekiq::Limiter::OverLimit => e
    limiter_name = e.limiter.name
    # Re-raise if an over the limit exception is coming from a limiter
    # defined on the job level.
    raise unless limiter_name == LIMITER.name

    # Use a custom exception that Sidekiq::Limiter is using to re-schedule
    # the job to a later time, but in a way that doesn't overlap with the
    # limiters defined on the job level.
    raise SaturationOverLimit, limiter_name
  end
end

# config/initializers/active_job.rb
ActiveJob::Base.around_perform(&SidekiqLimiter.method(:wrapper))

# config/initializers/rollbar.rb
Rollbar.configure do |config|
  config.exception_level_filters.merge!('Sidekiq::Limiter::OverLimit' => 'ignore')
end

# config/newrelic.yml
production:
  error_collector:
    enabled: true
    ignore_errors: "Sidekiq::Limiter::OverLimit"

# bad - `perform` method is called directly on an implicitly defined subject
RSpec.describe SomeJob do
  # implicitly defined `subject` is `SomeJob.new`
  it 'updates user status' do
    expect { subject.perform(user) }.to change { user.status }.to(:updated) }
  end
end

# bad - `perform` method is called directly on a job instance
RSpec.describe SomeJob do
  it 'updates user status' do
    expect { SomeJob.new.perform(user) }.to change { user.status }.to(:updated) }
  end
end

# good
RSpec.describe SomeJob do
  it 'updates user status' do
    expect { SomeJob.perform_now(user) }.to change { user.status }.to(:updated) }
  end
end

# bad - unnecessary roundtrip to Redis
RSpec.describe SomeJob do
  it 'updates user status' do
    expect do
      SomeJob.perform_later(user)
      perform_scheduled_jobs
    end.to change { user.status }.to(:updated) }
  end
end

# good
RSpec.describe SomeJob do
  it 'updates user status' do
    expect { SomeJob.perform_now(user) }.to change { user.status }.to(:updated) }
  end
end