From 313731232ebbfb5b0f98e7621ec1981719f61586 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 19 Oct 2020 16:01:20 +0500 Subject: [PATCH 1/6] Move jobs from Que to ActiveJob --- .../domains/expired_pendings/process_clean.rb | 2 +- .../process_update_confirmed.rb | 2 +- app/jobs/application_job.rb | 2 + app/jobs/directo_invoice_forward_job.rb | 4 +- app/jobs/dispute_status_update_job.rb | 4 +- app/jobs/domain_delete_confirm_job.rb | 32 ++++++++++++- app/jobs/domain_delete_job.rb | 5 +-- app/jobs/domain_expire_email_job.rb | 2 +- app/jobs/domain_update_confirm_job.rb | 2 - app/jobs/regenerate_registrar_whoises_job.rb | 10 +++-- app/jobs/regenerate_subzone_whoises_job.rb | 6 +-- .../registrant_change_confirm_email_job.rb | 4 +- .../registrant_change_expired_email_job.rb | 22 +++++++++ .../registrant_change_notice_email_job.rb | 4 +- app/jobs/send_e_invoice_job.rb | 31 +++++++------ app/jobs/update_whois_record_job.rb | 4 +- app/jobs/verify_emails_job.rb | 28 ++++++------ app/models/blocked_domain.rb | 2 +- app/models/concerns/domain/deletable.rb | 7 ++- app/models/concerns/invoice/payable.rb | 2 +- app/models/concerns/zone/whois_queryable.rb | 2 +- app/models/contact.rb | 2 +- app/models/dispute.rb | 2 +- app/models/domain.rb | 6 +-- app/models/registrant_verification.rb | 4 +- app/models/registrar.rb | 4 +- app/models/reserved_domain.rb | 2 +- config/application.rb | 2 + config/environments/production.rb | 2 +- config/environments/test.rb | 2 + lib/tasks/verify_email.rake | 4 +- lib/tasks/whois.rake | 9 ++-- .../epp/domain/delete/base_test.rb | 3 +- .../epp/domain/update/base_test.rb | 25 +++++++---- test/jobs/active_job_queuing_test.rb | 45 +++++++++++++++++++ test/jobs/directo_invoice_forward_job_test.rb | 14 +++--- test/jobs/dispute_status_update_job_test.rb | 20 ++++++--- .../regenerate_subzone_whoises_job_test.rb | 6 ++- ...egistrant_change_confirm_email_job_test.rb | 4 +- ...egistrant_change_expired_email_job_test.rb | 24 ++++++++++ ...registrant_change_notice_email_job_test.rb | 4 +- test/jobs/send_e_invoice_job_test.rb | 33 +++----------- test/jobs/update_whois_record_job_test.rb | 28 ++++++++++++ test/jobs/verify_emails_job_test.rb | 14 ++++-- .../registrant_change_mailer_preview.rb | 4 +- test/models/contact_test.rb | 6 ++- test/models/dns/zone_test.rb | 18 +++++--- .../domain/releasable/auctionable_test.rb | 6 ++- .../domain/releasable/discardable_test.rb | 17 ++++--- test/models/registrar_test.rb | 6 ++- .../domains/domain_delete_confirms_test.rb | 42 +++++++++++++++++ test/tasks/emails/verify_email_task_test.rb | 10 +++-- test/test_helper.rb | 2 + 53 files changed, 390 insertions(+), 157 deletions(-) create mode 100644 app/jobs/registrant_change_expired_email_job.rb create mode 100644 test/jobs/active_job_queuing_test.rb create mode 100644 test/jobs/registrant_change_expired_email_job_test.rb create mode 100644 test/jobs/update_whois_record_job_test.rb create mode 100644 test/system/registrant_area/domains/domain_delete_confirms_test.rb diff --git a/app/interactions/domains/expired_pendings/process_clean.rb b/app/interactions/domains/expired_pendings/process_clean.rb index 63a8f1ba5..0fe5a7b37 100644 --- a/app/interactions/domains/expired_pendings/process_clean.rb +++ b/app/interactions/domains/expired_pendings/process_clean.rb @@ -9,7 +9,7 @@ module Domains clean_pendings to_stdout("DomainCron.clean_expired_pendings: ##{domain.id} (#{domain.name})") - UpdateWhoisRecordJob.enqueue domain.name, 'domain' + UpdateWhoisRecordJob.perform_later domain.name, 'domain' end private diff --git a/app/interactions/domains/update_confirm/process_update_confirmed.rb b/app/interactions/domains/update_confirm/process_update_confirmed.rb index 291253651..9530a0a6a 100644 --- a/app/interactions/domains/update_confirm/process_update_confirmed.rb +++ b/app/interactions/domains/update_confirm/process_update_confirmed.rb @@ -17,7 +17,7 @@ module Domains update_domain clean_pendings! - WhoisRecord.find_by(domain_id: domain.id).save # need to reload model + WhoisRecord.find_by(domain_id: domain.id)&.save # need to reload model end def update_domain diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index a009ace51..8e51414e9 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,2 +1,4 @@ class ApplicationJob < ActiveJob::Base + discard_on NoMethodError + queue_as :default end diff --git a/app/jobs/directo_invoice_forward_job.rb b/app/jobs/directo_invoice_forward_job.rb index 4b2c06e2c..e52caac20 100644 --- a/app/jobs/directo_invoice_forward_job.rb +++ b/app/jobs/directo_invoice_forward_job.rb @@ -1,5 +1,5 @@ -class DirectoInvoiceForwardJob < Que::Job - def run(monthly: false, dry: false) +class DirectoInvoiceForwardJob < ApplicationJob + def perform(monthly: false, dry: false) @dry = dry (@month = Time.zone.now - 1.month) if monthly diff --git a/app/jobs/dispute_status_update_job.rb b/app/jobs/dispute_status_update_job.rb index 547d56868..a1596aecb 100644 --- a/app/jobs/dispute_status_update_job.rb +++ b/app/jobs/dispute_status_update_job.rb @@ -1,5 +1,5 @@ -class DisputeStatusUpdateJob < Que::Job - def run(logger: Logger.new(STDOUT)) +class DisputeStatusUpdateJob < ApplicationJob + def perform(logger: Logger.new(STDOUT)) @logger = logger @backlog = { 'activated': 0, 'closed': 0, 'activate_fail': [], 'close_fail': [] } diff --git a/app/jobs/domain_delete_confirm_job.rb b/app/jobs/domain_delete_confirm_job.rb index afea3da59..88cd405ff 100644 --- a/app/jobs/domain_delete_confirm_job.rb +++ b/app/jobs/domain_delete_confirm_job.rb @@ -1,6 +1,4 @@ class DomainDeleteConfirmJob < ApplicationJob - queue_as :default - def perform(domain_id, action, initiator = nil) domain = Epp::Domain.find(domain_id) @@ -8,4 +6,34 @@ class DomainDeleteConfirmJob < ApplicationJob action: action, initiator: initiator) end + + private + + def action_confirmed(domain) + domain.notify_registrar(:poll_pending_delete_confirmed_by_registrant) + domain.apply_pending_delete! + raise_errors!(domain) + end + + def action_rejected(domain) + domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION) + domain.notify_registrar(:poll_pending_delete_rejected_by_registrant) + + domain.cancel_pending_delete + domain.save(validate: false) + raise_errors!(domain) + notify_on_domain(domain) + end + + def notify_on_domain(domain) + if domain.registrant_verification_token.blank? + Rails.logger.warn 'EMAIL NOT DELIVERED: registrant_verification_token is missing for '\ + "#{domain.name}" + elsif domain.registrant_verification_asked_at.blank? + Rails.logger.warn 'EMAIL NOT DELIVERED: registrant_verification_asked_at is missing for '\ + "#{domain.name}" + else + DomainDeleteMailer.rejected(domain).deliver_now + end + end end diff --git a/app/jobs/domain_delete_job.rb b/app/jobs/domain_delete_job.rb index 5577908c6..18061496f 100644 --- a/app/jobs/domain_delete_job.rb +++ b/app/jobs/domain_delete_job.rb @@ -1,6 +1,5 @@ -class DomainDeleteJob < Que::Job - - def run(domain_id) +class DomainDeleteJob < ApplicationJob + def perform(domain_id) domain = Domain.find(domain_id) Domains::Delete::DoDelete.run(domain: domain) diff --git a/app/jobs/domain_expire_email_job.rb b/app/jobs/domain_expire_email_job.rb index 286fe29ab..1ae6ce6e1 100644 --- a/app/jobs/domain_expire_email_job.rb +++ b/app/jobs/domain_expire_email_job.rb @@ -1,5 +1,5 @@ class DomainExpireEmailJob < Que::Job - def run(domain_id, email) + def perform(domain_id, email) domain = Domain.find(domain_id) return if domain.registered? diff --git a/app/jobs/domain_update_confirm_job.rb b/app/jobs/domain_update_confirm_job.rb index 403318ca6..b5085520e 100644 --- a/app/jobs/domain_update_confirm_job.rb +++ b/app/jobs/domain_update_confirm_job.rb @@ -1,6 +1,4 @@ class DomainUpdateConfirmJob < ApplicationJob - queue_as :default - def perform(domain_id, action, initiator = nil) domain = Epp::Domain.find(domain_id) Domains::UpdateConfirm::ProcessAction.run(domain: domain, diff --git a/app/jobs/regenerate_registrar_whoises_job.rb b/app/jobs/regenerate_registrar_whoises_job.rb index c5db82378..100a1cd87 100644 --- a/app/jobs/regenerate_registrar_whoises_job.rb +++ b/app/jobs/regenerate_registrar_whoises_job.rb @@ -1,10 +1,12 @@ -class RegenerateRegistrarWhoisesJob < Que::Job - def run(registrar_id) +class RegenerateRegistrarWhoisesJob < ApplicationJob + retry_on StandardError, wait: 2.seconds, attempts: 3 + + def perform(registrar_id) # no return as we want restart job if fails registrar = Registrar.find(registrar_id) registrar.whois_records.select(:name).find_in_batches(batch_size: 20) do |group| - UpdateWhoisRecordJob.enqueue group.map(&:name), 'domain' + UpdateWhoisRecordJob.perform_later group.map(&:name), 'domain' end end -end \ No newline at end of file +end diff --git a/app/jobs/regenerate_subzone_whoises_job.rb b/app/jobs/regenerate_subzone_whoises_job.rb index 280fa4088..ea8af8a18 100644 --- a/app/jobs/regenerate_subzone_whoises_job.rb +++ b/app/jobs/regenerate_subzone_whoises_job.rb @@ -1,11 +1,11 @@ -class RegenerateSubzoneWhoisesJob < Que::Job - def run +class RegenerateSubzoneWhoisesJob < ApplicationJob + def perform subzones = DNS::Zone.all subzones.each do |zone| next unless zone.subzone? - UpdateWhoisRecordJob.enqueue zone.origin, 'zone' + UpdateWhoisRecordJob.perform_later zone.origin, 'zone' end end end diff --git a/app/jobs/registrant_change_confirm_email_job.rb b/app/jobs/registrant_change_confirm_email_job.rb index 2883c10a6..09cf0caa4 100644 --- a/app/jobs/registrant_change_confirm_email_job.rb +++ b/app/jobs/registrant_change_confirm_email_job.rb @@ -1,5 +1,5 @@ -class RegistrantChangeConfirmEmailJob < Que::Job - def run(domain_id, new_registrant_id) +class RegistrantChangeConfirmEmailJob < ApplicationJob + def perform(domain_id, new_registrant_id) domain = Domain.find(domain_id) new_registrant = Registrant.find(new_registrant_id) diff --git a/app/jobs/registrant_change_expired_email_job.rb b/app/jobs/registrant_change_expired_email_job.rb new file mode 100644 index 000000000..68712af40 --- /dev/null +++ b/app/jobs/registrant_change_expired_email_job.rb @@ -0,0 +1,22 @@ +class RegistrantChangeExpiredEmailJob < ApplicationJob + def perform(domain_id) + domain = Domain.find(domain_id) + log(domain) + RegistrantChangeMailer.expired(domain: domain, + registrar: domain.registrar, + registrant: domain.registrant, + send_to: [domain.new_registrant_email, + domain.registrant.email]).deliver_now + end + + private + + def log(domain) + message = "Send RegistrantChangeMailer#expired email for domain #{domain.name} (##{domain.id}) to #{domain.new_registrant_email}" + logger.info(message) + end + + def logger + Rails.logger + end +end diff --git a/app/jobs/registrant_change_notice_email_job.rb b/app/jobs/registrant_change_notice_email_job.rb index cabd1db7f..cb0da5d06 100644 --- a/app/jobs/registrant_change_notice_email_job.rb +++ b/app/jobs/registrant_change_notice_email_job.rb @@ -1,5 +1,5 @@ -class RegistrantChangeNoticeEmailJob < Que::Job - def run(domain_id, new_registrant_id) +class RegistrantChangeNoticeEmailJob < ApplicationJob + def perform(domain_id, new_registrant_id) domain = Domain.find(domain_id) new_registrant = Registrant.find(new_registrant_id) log(domain, new_registrant) diff --git a/app/jobs/send_e_invoice_job.rb b/app/jobs/send_e_invoice_job.rb index e3880963e..f8b1f8296 100644 --- a/app/jobs/send_e_invoice_job.rb +++ b/app/jobs/send_e_invoice_job.rb @@ -1,13 +1,11 @@ -class SendEInvoiceJob < Que::Job - def run(invoice_id, payable = true) - invoice = run_condition(Invoice.find_by(id: invoice_id), payable: payable) +class SendEInvoiceJob < ApplicationJob + discard_on HTTPClient::TimeoutError - invoice.to_e_invoice(payable: payable).deliver - ActiveRecord::Base.transaction do - invoice.update(e_invoice_sent_at: Time.zone.now) - log_success(invoice) - destroy - end + def perform(invoice_id, payable = true) + invoice = Invoice.find_by(id: invoice_id) + return unless need_to_process_invoice?(invoice: invoice, payable: payable) + + process(invoice: invoice, payable: payable) rescue StandardError => e log_error(invoice: invoice, error: e) raise e @@ -15,10 +13,17 @@ class SendEInvoiceJob < Que::Job private - def run_condition(invoice, payable: true) - destroy unless invoice - destroy if invoice.do_not_send_e_invoice? && payable - invoice + def need_to_process_invoice?(invoice:, payable:) + return false if invoice.blank? + return false if invoice.do_not_send_e_invoice? && payable + + true + end + + def process(invoice:, payable:) + invoice.to_e_invoice(payable: payable).deliver + invoice.update(e_invoice_sent_at: Time.zone.now) + log_success(invoice) end def log_success(invoice) diff --git a/app/jobs/update_whois_record_job.rb b/app/jobs/update_whois_record_job.rb index d067807a0..1c90b8969 100644 --- a/app/jobs/update_whois_record_job.rb +++ b/app/jobs/update_whois_record_job.rb @@ -1,5 +1,5 @@ -class UpdateWhoisRecordJob < Que::Job - def run(names, type) +class UpdateWhoisRecordJob < ApplicationJob + def perform(names, type) Whois::Update.run(names: [names].flatten, type: type) end end diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 75f4b7d91..0d3ec2078 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -1,14 +1,11 @@ -class VerifyEmailsJob < Que::Job - def run(verification_id) - email_address_verification = run_condition(EmailAddressVerification.find(verification_id)) +class VerifyEmailsJob < ApplicationJob + discard_on StandardError - return if email_address_verification.recently_verified? + def perform(verification_id) + email_address_verification = EmailAddressVerification.find(verification_id) + return unless need_to_verify?(email_address_verification) - ActiveRecord::Base.transaction do - email_address_verification.verify - log_success(email_address_verification) - destroy - end + process(email_address_verification) rescue StandardError => e log_error(verification: email_address_verification, error: e) raise e @@ -16,11 +13,16 @@ class VerifyEmailsJob < Que::Job private - def run_condition(email_address_verification) - destroy unless email_address_verification - destroy if email_address_verification.recently_verified? + def need_to_verify?(email_address_verification) + return false if email_address_verification.blank? + return false if email_address_verification.recently_verified? - email_address_verification + true + end + + def process(email_address_verification) + email_address_verification.verify + log_success(email_address_verification) end def logger diff --git a/app/models/blocked_domain.rb b/app/models/blocked_domain.rb index d292827dc..f4e18bffc 100644 --- a/app/models/blocked_domain.rb +++ b/app/models/blocked_domain.rb @@ -34,6 +34,6 @@ class BlockedDomain < ApplicationRecord end def remove_data - UpdateWhoisRecordJob.enqueue name, 'blocked' + UpdateWhoisRecordJob.perform_later name, 'blocked' end end diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index 50c418feb..db81856b8 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -7,11 +7,14 @@ module Domain::Deletable DomainStatus::FORCE_DELETE, ].freeze + def deletion_time + @deletion_time ||= Time.zone.at(rand(deletion_time_span)) + end + private def delete_later - deletion_time = Time.zone.at(rand(deletion_time_span)) - DomainDeleteJob.enqueue(id, run_at: deletion_time, priority: 1) + DomainDeleteJob.set(wait_until: deletion_time).perform_later(id) logger.info "Domain #{name} is scheduled to be deleted around #{deletion_time}" end diff --git a/app/models/concerns/invoice/payable.rb b/app/models/concerns/invoice/payable.rb index ad91b886e..6e2cc19b4 100644 --- a/app/models/concerns/invoice/payable.rb +++ b/app/models/concerns/invoice/payable.rb @@ -11,7 +11,7 @@ module Invoice::Payable end def paid? - account_activity + account_activity.present? end def receipt_date diff --git a/app/models/concerns/zone/whois_queryable.rb b/app/models/concerns/zone/whois_queryable.rb index e4a6c1314..7c6ff511d 100644 --- a/app/models/concerns/zone/whois_queryable.rb +++ b/app/models/concerns/zone/whois_queryable.rb @@ -11,7 +11,7 @@ module Zone::WhoisQueryable end def update_whois_record - UpdateWhoisRecordJob.enqueue origin, 'zone' + UpdateWhoisRecordJob.perform_later origin, 'zone' end def generate_data diff --git a/app/models/contact.rb b/app/models/contact.rb index 61d49c10c..c6e3e22ba 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -492,7 +492,7 @@ class Contact < ApplicationRecord return if saved_changes.slice(*(self.class.column_names - ignored_columns)).empty? names = related_domain_descriptions.keys - UpdateWhoisRecordJob.enqueue(names, 'domain') if names.present? + UpdateWhoisRecordJob.perform_later(names, 'domain') if names.present? end def children_log diff --git a/app/models/dispute.rb b/app/models/dispute.rb index 45ff27274..6690e0e3c 100644 --- a/app/models/dispute.rb +++ b/app/models/dispute.rb @@ -84,7 +84,7 @@ class Dispute < ApplicationRecord end def remove_data - UpdateWhoisRecordJob.enqueue domain_name, 'disputed' + UpdateWhoisRecordJob.perform_later domain_name, 'disputed' end def fill_empty_passwords diff --git a/app/models/domain.rb b/app/models/domain.rb index 04491bc53..68820060f 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -425,8 +425,8 @@ class Domain < ApplicationRecord new_registrant_email = registrant.email new_registrant_name = registrant.name - RegistrantChangeConfirmEmailJob.enqueue(id, new_registrant_id) - RegistrantChangeNoticeEmailJob.enqueue(id, new_registrant_id) + RegistrantChangeConfirmEmailJob.perform_later(id, new_registrant_id) + RegistrantChangeNoticeEmailJob.perform_later(id, new_registrant_id) reload @@ -670,7 +670,7 @@ class Domain < ApplicationRecord end def update_whois_record - UpdateWhoisRecordJob.enqueue name, 'domain' + UpdateWhoisRecordJob.perform_later name, 'domain' end def status_notes_array=(notes) diff --git a/app/models/registrant_verification.rb b/app/models/registrant_verification.rb index 4e8e6afe9..b689d9c1c 100644 --- a/app/models/registrant_verification.rb +++ b/app/models/registrant_verification.rb @@ -30,12 +30,12 @@ class RegistrantVerification < ApplicationRecord def domain_registrant_delete_confirm!(initiator) self.action_type = DOMAIN_DELETE self.action = CONFIRMED - DomainDeleteConfirmJob.perform_later domain.id, CONFIRMED, initiator if save + DomainDeleteConfirmJob.perform_later(domain.id, CONFIRMED, initiator) if save end def domain_registrant_delete_reject!(initiator) self.action_type = DOMAIN_DELETE self.action = REJECTED - DomainDeleteConfirmJob.perform_later domain.id, REJECTED, initiator if save + DomainDeleteConfirmJob.perform_later(domain.id, REJECTED, initiator) if save end end diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 335fb9115..264029c8a 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -43,7 +43,7 @@ class Registrar < ApplicationRecord after_commit :update_whois_records def update_whois_records return true unless changed? && (changes.keys & WHOIS_TRIGGERS).present? - RegenerateRegistrarWhoisesJob.enqueue id + RegenerateRegistrarWhoisesJob.perform_later id end self.ignored_columns = %w[legacy_id] @@ -104,7 +104,7 @@ class Registrar < ApplicationRecord InvoiceMailer.invoice_email(invoice: invoice, recipient: billing_email).deliver_now end - SendEInvoiceJob.enqueue(invoice.id, payable) + SendEInvoiceJob.perform_later(invoice.id, payable) invoice end diff --git a/app/models/reserved_domain.rb b/app/models/reserved_domain.rb index 4c9df3269..a45d06e83 100644 --- a/app/models/reserved_domain.rb +++ b/app/models/reserved_domain.rb @@ -59,6 +59,6 @@ class ReservedDomain < ApplicationRecord alias_method :update_whois_record, :generate_data def remove_data - UpdateWhoisRecordJob.enqueue name, 'reserved' + UpdateWhoisRecordJob.perform_later name, 'reserved' end end diff --git a/config/application.rb b/config/application.rb index 0ab5aa8bb..4455a8a77 100644 --- a/config/application.rb +++ b/config/application.rb @@ -44,6 +44,8 @@ module DomainNameRegistry config.active_record.schema_format = :sql + config.active_job.queue_adapter = :que + config.generators do |g| g.stylesheets false g.javascripts false diff --git a/config/environments/production.rb b/config/environments/production.rb index e1966d6ba..0636788c5 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -56,7 +56,7 @@ Rails.application.configure do # config.cache_store = :mem_cache_store # Use a real queuing backend for Active Job (and separate queues per environment) - # config.active_job.queue_adapter = :resque + config.active_job.queue_adapter = :que # config.active_job.queue_name_prefix = "domain_name_registry_#{Rails.env}" config.action_mailer.perform_caching = false diff --git a/config/environments/test.rb b/config/environments/test.rb index c55e59e31..d10de6d19 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -14,6 +14,8 @@ Rails.application.configure do # preloads Rails for running tests, you may have to set it to true. config.eager_load = false + config.active_job.queue_adapter = :test + config.consider_all_requests_local = true config.action_controller.perform_caching = false diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index d49bb38b9..daffabf19 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -4,7 +4,7 @@ namespace :verify_email do verifications_by_domain = EmailAddressVerification.not_verified_recently.group_by(&:domain) verifications_by_domain.each do |_domain, verifications| ver = verifications.sample # Verify random email to not to clog the SMTP servers - VerifyEmailsJob.enqueue(ver.id) + VerifyEmailsJob.perform_later(ver.id) next end end @@ -18,6 +18,6 @@ namespace :verify_email do verifications_by_domain = EmailAddressVerification.not_verified_recently .by_domain(args[:domain_name]) - verifications_by_domain.map { |ver| VerifyEmailsJob.enqueue(ver.id) } + verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } end end diff --git a/lib/tasks/whois.rake b/lib/tasks/whois.rake index 52be7e17f..57b054d95 100644 --- a/lib/tasks/whois.rake +++ b/lib/tasks/whois.rake @@ -23,22 +23,23 @@ namespace :whois do print "\n-----> Update domains whois_records" Domain.find_in_batches.each do |group| - UpdateWhoisRecordJob.enqueue group.map(&:name), 'domain' + UpdateWhoisRecordJob.perform_later group.map(&:name), 'domain' end print "\n-----> Update blocked domains whois_records" BlockedDomain.find_in_batches.each do |group| - UpdateWhoisRecordJob.enqueue group.map(&:name), 'blocked' + UpdateWhoisRecordJob.perform_later group.map(&:name), 'blocked' end print "\n-----> Update reserved domains whois_records" ReservedDomain.find_in_batches.each do |group| - UpdateWhoisRecordJob.enqueue group.map(&:name), 'reserved' + UpdateWhoisRecordJob.perform_later group.map(&:name), 'reserved' end print "\n-----> Update disputed domains whois_records" Dispute.active.find_in_batches.each do |group| - UpdateWhoisRecordJob.enqueue group.map(&:domain_name), 'disputed' + + UpdateWhoisRecordJob.perform_later group.map(&:domain_name), 'disputed' end end puts "\n-----> all done in #{(Time.zone.now.to_f - start).round(2)} seconds" diff --git a/test/integration/epp/domain/delete/base_test.rb b/test/integration/epp/domain/delete/base_test.rb index d32a89f63..830d7363d 100644 --- a/test/integration/epp/domain/delete/base_test.rb +++ b/test/integration/epp/domain/delete/base_test.rb @@ -92,12 +92,13 @@ class EppDomainDeleteBaseTest < EppTestCase perform_enqueued_jobs do post epp_delete_path, params: { frame: request_xml }, headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } end + @domain.reload assert @domain.registrant_verification_asked? assert @domain.pending_delete_confirmation? - assert_emails 1 assert_epp_response :completed_successfully_action_pending + assert_emails 1 end def test_skips_registrant_confirmation_when_not_required diff --git a/test/integration/epp/domain/update/base_test.rb b/test/integration/epp/domain/update/base_test.rb index 83ba65478..cf3ee7fe9 100644 --- a/test/integration/epp/domain/update/base_test.rb +++ b/test/integration/epp/domain/update/base_test.rb @@ -2,6 +2,7 @@ require 'test_helper' class EppDomainUpdateBaseTest < EppTestCase include ActionMailer::TestHelper + include ActiveJob::TestHelper setup do @domain = domains(:shop) @@ -134,15 +135,17 @@ class EppDomainUpdateBaseTest < EppTestCase XML - post epp_update_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + assert_no_enqueued_jobs + assert_enqueued_jobs 3 do + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end @domain.reload assert_epp_response :completed_successfully_action_pending assert_not_equal new_registrant, @domain.registrant assert @domain.registrant_verification_asked? assert_includes @domain.statuses, DomainStatus::PENDING_UPDATE - assert_verification_and_notification_emails end def test_domain_should_doesnt_have_pending_update_when_updated_registrant_with_same_idents_data @@ -211,15 +214,17 @@ class EppDomainUpdateBaseTest < EppTestCase XML - post epp_update_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + assert_no_enqueued_jobs + assert_enqueued_jobs 3 do + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end @domain.reload assert_epp_response :completed_successfully_action_pending assert_not_equal new_registrant, @domain.registrant assert @domain.registrant_verification_asked? assert_includes @domain.statuses, DomainStatus::PENDING_UPDATE - assert_verification_and_notification_emails end def test_updates_registrant_when_legaldoc_is_not_mandatory @@ -247,15 +252,17 @@ class EppDomainUpdateBaseTest < EppTestCase XML - post epp_update_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + assert_no_enqueued_jobs + assert_enqueued_jobs 3 do + post epp_update_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end @domain.reload assert_epp_response :completed_successfully_action_pending assert_not_equal new_registrant, @domain.registrant assert @domain.registrant_verification_asked? assert_includes @domain.statuses, DomainStatus::PENDING_UPDATE - assert_verification_and_notification_emails end def test_dows_not_update_registrant_when_legaldoc_is_mandatory diff --git a/test/jobs/active_job_queuing_test.rb b/test/jobs/active_job_queuing_test.rb new file mode 100644 index 000000000..9f60a5b06 --- /dev/null +++ b/test/jobs/active_job_queuing_test.rb @@ -0,0 +1,45 @@ +$VERBOSE=nil +require 'test_helper' + +class ActiveJobQueuingTest < ActiveJob::TestCase + + def test_job_discarded_after_error + assert_no_enqueued_jobs + assert_performed_jobs 1 do + TestDiscardedJob.perform_later + end + assert_no_enqueued_jobs + end + + def test_job_retried_after_error + assert_no_enqueued_jobs + assert_raises StandardError do + assert_performed_jobs 3 do + TestRetriedJob.perform_later + end + end + + assert_no_enqueued_jobs + end + +end + +class TestDiscardedJob < ApplicationJob + queue_as :default + + discard_on StandardError + + def perform + raise StandardError + end +end + +class TestRetriedJob < ApplicationJob + queue_as :default + + retry_on StandardError, wait: 2.seconds, attempts: 3 + + def perform + raise StandardError + end +end diff --git a/test/jobs/directo_invoice_forward_job_test.rb b/test/jobs/directo_invoice_forward_job_test.rb index 4e0edf7e5..4a4fcf86d 100644 --- a/test/jobs/directo_invoice_forward_job_test.rb +++ b/test/jobs/directo_invoice_forward_job_test.rb @@ -38,7 +38,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase end.to_return(status: 200, body: response) assert_nothing_raised do - DirectoInvoiceForwardJob.run(monthly: false, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: false, dry: false) end assert_not_empty @invoice.directo_records.first.request @@ -52,7 +52,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase Setting.directo_monthly_number_max = 30991 assert_raises 'RuntimeError' do - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) end end @@ -78,7 +78,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase end.to_return(status: 200, body: response) assert_difference 'Setting.directo_monthly_number_last' do - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) end end @@ -103,7 +103,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase end.to_return(status: 200, body: response) assert_difference 'Setting.directo_monthly_number_last' do - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) end end @@ -126,7 +126,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase end.to_return(status: 200, body: response) assert_difference 'Setting.directo_monthly_number_last' do - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) end end @@ -148,7 +148,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase end.to_return(status: 200, body: response) assert_difference 'Setting.directo_monthly_number_last' do - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) end end @@ -186,7 +186,7 @@ class DirectoInvoiceForwardJobTest < ActiveSupport::TestCase (body.include? 'StartDate') && (body.include? 'EndDate') && (body.include? 'goodnames') end.to_return(status: 200, body: response) - DirectoInvoiceForwardJob.run(monthly: true, dry: false) + DirectoInvoiceForwardJob.perform_now(monthly: true, dry: false) assert_requested first_registrar_stub assert_requested second_registrar_stub diff --git a/test/jobs/dispute_status_update_job_test.rb b/test/jobs/dispute_status_update_job_test.rb index e70e58c04..7784cdeac 100644 --- a/test/jobs/dispute_status_update_job_test.rb +++ b/test/jobs/dispute_status_update_job_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class DisputeStatusUpdateJobTest < ActiveSupport::TestCase +class DisputeStatusUpdateJobTest < ActiveJob::TestCase setup do travel_to Time.zone.parse('2010-10-05') @logger = Rails.logger @@ -8,13 +8,13 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase def test_nothing_is_raised assert_nothing_raised do - DisputeStatusUpdateJob.run(logger: @logger) + DisputeStatusUpdateJob.perform_now(logger: @logger) end end def test_whois_data_added_when_dispute_activated dispute = disputes(:future) - DisputeStatusUpdateJob.run(logger: @logger) + DisputeStatusUpdateJob.perform_now(logger: @logger) whois_record = Whois::Record.find_by(name: dispute.domain_name) assert whois_record.present? @@ -25,7 +25,7 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase dispute = disputes(:active) dispute.update!(starts_at: Time.zone.today - 3.years - 1.day) - DisputeStatusUpdateJob.run(logger: @logger) + DisputeStatusUpdateJob.perform_now(logger: @logger) dispute.reload assert dispute.closed @@ -37,7 +37,9 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase def test_registered_domain_whois_data_is_added Dispute.create(domain_name: 'shop.test', starts_at: '2010-07-05') travel_to Time.zone.parse('2010-07-05') - DisputeStatusUpdateJob.run(logger: @logger) + perform_enqueued_jobs do + DisputeStatusUpdateJob.perform_now(logger: @logger) + end whois_record = Whois::Record.find_by(name: 'shop.test') assert_includes whois_record.json['status'], 'disputed' @@ -53,7 +55,9 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase force_delete_date: nil) # Dispute status is added automatically if starts_at is not in future - Dispute.create(domain_name: 'shop.test', starts_at: Time.zone.parse('2010-07-05')) + perform_enqueued_jobs do + Dispute.create(domain_name: 'shop.test', starts_at: Time.zone.parse('2010-07-05')) + end domain.reload whois_record = Whois::Record.find_by(name: 'shop.test') @@ -62,7 +66,9 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase # Dispute status is removed night time day after it's ended travel_to Time.zone.parse('2010-07-05') + 3.years + 1.day - DisputeStatusUpdateJob.run(logger: @logger) + perform_enqueued_jobs do + DisputeStatusUpdateJob.perform_now(logger: @logger) + end whois_record.reload assert_not whois_record.json['status'].include? 'disputed' diff --git a/test/jobs/regenerate_subzone_whoises_job_test.rb b/test/jobs/regenerate_subzone_whoises_job_test.rb index 6c29e4be7..b6b6e97fe 100644 --- a/test/jobs/regenerate_subzone_whoises_job_test.rb +++ b/test/jobs/regenerate_subzone_whoises_job_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class RegenerateSubzoneWhoisesJobTest < ActiveSupport::TestCase +class RegenerateSubzoneWhoisesJobTest < ActiveJob::TestCase def test_regenerates_whois_data_only_for_subzones subzone = dns_zones(:one).dup subzone.origin = 'subzone.test' @@ -11,7 +11,9 @@ class RegenerateSubzoneWhoisesJobTest < ActiveSupport::TestCase assert_nil Whois::Record.find_by(name: subzone.origin) assert_nil Whois::Record.find_by(name: dns_zones(:one).origin) - RegenerateSubzoneWhoisesJob.run + perform_enqueued_jobs do + RegenerateSubzoneWhoisesJob.perform_now + end record = Whois::Record.find_by(name: subzone.origin) assert record assert record.json['dnssec_keys'].is_a?(Array) diff --git a/test/jobs/registrant_change_confirm_email_job_test.rb b/test/jobs/registrant_change_confirm_email_job_test.rb index 0e38fb9a4..5341a59b2 100644 --- a/test/jobs/registrant_change_confirm_email_job_test.rb +++ b/test/jobs/registrant_change_confirm_email_job_test.rb @@ -11,8 +11,8 @@ class RegistrantChangeConfirmEmailJobTest < ActiveSupport::TestCase domain_id = domains(:shop).id new_registrant_id = contacts(:william).id - RegistrantChangeConfirmEmailJob.enqueue(domain_id, new_registrant_id) + RegistrantChangeConfirmEmailJob.perform_now(domain_id, new_registrant_id) assert_emails 1 end -end \ No newline at end of file +end diff --git a/test/jobs/registrant_change_expired_email_job_test.rb b/test/jobs/registrant_change_expired_email_job_test.rb new file mode 100644 index 000000000..f3c194b5f --- /dev/null +++ b/test/jobs/registrant_change_expired_email_job_test.rb @@ -0,0 +1,24 @@ +require 'test_helper' + +class RegistrantChangeExpiredEmailJobTest < ActiveJob::TestCase + include ActionMailer::TestHelper + + setup do + ActionMailer::Base.deliveries.clear + end + + def test_delivers_email + domain = domains(:shop) + domain.update!(pending_json: {new_registrant_email: 'aaa@bbb.com'}) + domain_id = domain.id + + assert_performed_jobs 1 do + perform_enqueued_jobs do + RegistrantChangeExpiredEmailJob.perform_later(domain_id) + end + end + + assert_emails 1 + end + +end diff --git a/test/jobs/registrant_change_notice_email_job_test.rb b/test/jobs/registrant_change_notice_email_job_test.rb index 8213ea3b9..df6bcc6e5 100644 --- a/test/jobs/registrant_change_notice_email_job_test.rb +++ b/test/jobs/registrant_change_notice_email_job_test.rb @@ -11,8 +11,8 @@ class RegistrantChangeNoticeEmailJobTest < ActiveSupport::TestCase domain_id = domains(:shop).id new_registrant_id = contacts(:william).id - RegistrantChangeNoticeEmailJob.enqueue(domain_id, new_registrant_id) + RegistrantChangeNoticeEmailJob.perform_now(domain_id, new_registrant_id) assert_emails 1 end -end \ No newline at end of file +end diff --git a/test/jobs/send_e_invoice_job_test.rb b/test/jobs/send_e_invoice_job_test.rb index 86d761b42..ca1e50eb7 100644 --- a/test/jobs/send_e_invoice_job_test.rb +++ b/test/jobs/send_e_invoice_job_test.rb @@ -1,43 +1,22 @@ require 'test_helper' -class SendEInvoiceJobTest < ActiveSupport::TestCase +class SendEInvoiceJobTest < ActiveJob::TestCase def teardown EInvoice.provider = EInvoice::Providers::TestProvider.new EInvoice::Providers::TestProvider.deliveries.clear end - def test_if_invoice_is_sended + def test_if_invoice_is_sent @invoice = invoices(:one) + @invoice.account_activity.destroy EInvoice.provider = EInvoice::Providers::TestProvider.new EInvoice::Providers::TestProvider.deliveries.clear assert_nothing_raised do - SendEInvoiceJob.enqueue(@invoice.id, true) - end - @invoice.reload - - assert_not @invoice.e_invoice_sent_at.blank? - assert_equal 1, EInvoice::Providers::TestProvider.deliveries.count - end - - def test_if_invoice_sending_retries - @invoice = invoices(:one) - provider_config = { password: nil, - test_mode: true } - EInvoice.provider = EInvoice::Providers::OmnivaProvider.new(provider_config) - stub_request(:get, "https://testfinance.post.ee/finance/erp/erpServices.wsdl").to_timeout - - assert_raise HTTPClient::TimeoutError do - SendEInvoiceJob.enqueue(@invoice.id, true) - end - assert @invoicee_invoice_sent_at.blank? - - EInvoice.provider = EInvoice::Providers::TestProvider.new - EInvoice::Providers::TestProvider.deliveries.clear - - assert_nothing_raised do - SendEInvoiceJob.enqueue(@invoice.id, true) + perform_enqueued_jobs do + SendEInvoiceJob.perform_now(@invoice.id, true) + end end @invoice.reload diff --git a/test/jobs/update_whois_record_job_test.rb b/test/jobs/update_whois_record_job_test.rb new file mode 100644 index 000000000..77bc7bb23 --- /dev/null +++ b/test/jobs/update_whois_record_job_test.rb @@ -0,0 +1,28 @@ +require 'test_helper' + +class SendEInvoiceJobTest < ActiveJob::TestCase + + def test_job_is_updating_domains + domain_names = Domain.find_in_batches.first.map(&:name) + assert_domains_processed_by_task(domain_names, 'domain') + end + + def test_job_is_updating_blocked_domains + domain_names = BlockedDomain.find_in_batches.first.map(&:name) + assert_domains_processed_by_task(domain_names, 'blocked') + end + + def test_job_is_updating_reserved_domains + domain_names = ReservedDomain.find_in_batches.first.map(&:name) + assert_domains_processed_by_task(domain_names, 'reserved') + end + + private + + def assert_domains_processed_by_task(domain_names, type) + Rake::Task['whois:regenerate'].execute + + perform_enqueued_jobs + assert_performed_with(job: UpdateWhoisRecordJob, args: [domain_names, type]) + end +end diff --git a/test/jobs/verify_emails_job_test.rb b/test/jobs/verify_emails_job_test.rb index f55a474db..49a08fe73 100644 --- a/test/jobs/verify_emails_job_test.rb +++ b/test/jobs/verify_emails_job_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class VerifyEmailsJobTest < ActiveSupport::TestCase +class VerifyEmailsJobTest < ActiveJob::TestCase def setup @contact = contacts(:john) @invalid_contact = contacts(:invalid_email) @@ -33,7 +33,9 @@ class VerifyEmailsJobTest < ActiveSupport::TestCase end def test_job_checks_if_email_valid - VerifyEmailsJob.run(@contact_verification.id) + perform_enqueued_jobs do + VerifyEmailsJob.perform_now(@contact_verification.id) + end @contact_verification.reload assert @contact_verification.success @@ -44,14 +46,18 @@ class VerifyEmailsJobTest < ActiveSupport::TestCase @contact_verification.update(success: true, verified_at: old_verified_at) assert @contact_verification.recently_verified? - VerifyEmailsJob.run(@contact_verification.id) + perform_enqueued_jobs do + VerifyEmailsJob.perform_now(@contact_verification.id) + end @contact_verification.reload assert_in_delta @contact_verification.verified_at.to_i, old_verified_at.to_i, 1 end def test_job_checks_if_email_invalid - VerifyEmailsJob.run(@invalid_contact_verification.id) + perform_enqueued_jobs do + VerifyEmailsJob.perform_now(@invalid_contact_verification.id) + end @contact_verification.reload refute @contact_verification.success diff --git a/test/mailers/previews/registrant_change_mailer_preview.rb b/test/mailers/previews/registrant_change_mailer_preview.rb index ac3dab2a9..03b4b494e 100644 --- a/test/mailers/previews/registrant_change_mailer_preview.rb +++ b/test/mailers/previews/registrant_change_mailer_preview.rb @@ -37,6 +37,8 @@ class RegistrantChangeMailerPreview < ActionMailer::Preview def expired RegistrantChangeMailer.expired(domain: @domain, registrar: @domain.registrar, - registrant: @domain.registrant) + registrant: @domain.registrant, + send_to: [@domain.new_registrant_email, + @domain.registrant.email]) end end diff --git a/test/models/contact_test.rb b/test/models/contact_test.rb index e6dc2422f..3c0669890 100644 --- a/test/models/contact_test.rb +++ b/test/models/contact_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class ContactTest < ActiveSupport::TestCase +class ContactTest < ActiveJob::TestCase setup do @contact = contacts(:john) @old_validation_type = Truemail.configure.default_validation_type @@ -336,7 +336,9 @@ class ContactTest < ActiveSupport::TestCase @contact.name = 'SomeReallyWeirdRandomTestName' domain = @contact.registrant_domains.first - @contact.save! + perform_enqueued_jobs do + @contact.save! + end assert_equal domain.whois_record.try(:json).try(:[], 'registrant'), @contact.name end diff --git a/test/models/dns/zone_test.rb b/test/models/dns/zone_test.rb index fab4c6355..47b0503b5 100644 --- a/test/models/dns/zone_test.rb +++ b/test/models/dns/zone_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class DNS::ZoneTest < ActiveSupport::TestCase +class DNS::ZoneTest < ActiveJob::TestCase def test_valid_zone_fixture_is_valid assert valid_zone.valid?, proc { valid_zone.errors.full_messages } end @@ -134,7 +134,9 @@ class DNS::ZoneTest < ActiveSupport::TestCase subzone = dns_zones(:one).dup subzone.origin = 'sub.zone' - subzone.save + perform_enqueued_jobs do + subzone.save + end whois_record = Whois::Record.find_by(name: subzone.origin) assert whois_record.present? @@ -144,7 +146,9 @@ class DNS::ZoneTest < ActiveSupport::TestCase subzone = dns_zones(:one).dup subzone.origin = 'sub.zone' - subzone.save + perform_enqueued_jobs do + subzone.save + end whois_record = Whois::Record.find_by(name: subzone.origin) assert whois_record.present? @@ -170,11 +174,15 @@ class DNS::ZoneTest < ActiveSupport::TestCase subzone = dns_zones(:one).dup subzone.origin = 'sub.zone' - subzone.save + perform_enqueued_jobs do + subzone.save + end assert Whois::Record.find_by(name: subzone.origin).present? - subzone.destroy + perform_enqueued_jobs do + subzone.destroy + end assert_nil Whois::Record.find_by(name: subzone.origin) end diff --git a/test/models/domain/releasable/auctionable_test.rb b/test/models/domain/releasable/auctionable_test.rb index d24f46913..97d66be4f 100644 --- a/test/models/domain/releasable/auctionable_test.rb +++ b/test/models/domain/releasable/auctionable_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class DomainReleasableAuctionableTest < ActiveSupport::TestCase +class DomainReleasableAuctionableTest < ActiveJob::TestCase # Needed for `test_updates_whois` test because of `after_commit :update_whois_record` in Domain self.use_transactional_tests = false @@ -65,7 +65,9 @@ class DomainReleasableAuctionableTest < ActiveSupport::TestCase travel_to Time.zone.parse('2010-07-05') old_whois = @domain.whois_record - Domain.release_domains + perform_enqueued_jobs do + Domain.release_domains + end assert_raises ActiveRecord::RecordNotFound do old_whois.reload diff --git a/test/models/domain/releasable/discardable_test.rb b/test/models/domain/releasable/discardable_test.rb index d7c2a8ed2..796b4eac5 100644 --- a/test/models/domain/releasable/discardable_test.rb +++ b/test/models/domain/releasable/discardable_test.rb @@ -1,7 +1,10 @@ require 'test_helper' class DomainReleasableDiscardableTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + setup do + ActiveJob::Base.queue_adapter = :test @domain = domains(:shop) end @@ -64,16 +67,18 @@ class DomainReleasableDiscardableTest < ActiveSupport::TestCase travel_to Time.zone.parse('2010-07-05') @domain.update_columns(delete_date: '2010-07-05') - Domain.release_domains + + assert_enqueued_with(job: DomainDeleteJob) do + Domain.release_domains + end other_domain = domains(:airport) other_domain.update_columns(delete_date: '2010-07-05') - Domain.release_domains + assert_enqueued_with(job: DomainDeleteJob) do + Domain.release_domains + end - background_job = QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) - other_background_job = QueJob.find_by("args->>0 = '#{other_domain.id}'", - job_class: DomainDeleteJob.name) - assert_not_equal background_job.run_at, other_background_job.run_at + assert_not other_domain.deletion_time == @domain.deletion_time end def test_discarding_a_domain_bypasses_validation diff --git a/test/models/registrar_test.rb b/test/models/registrar_test.rb index 091b8e6f4..e7d53e075 100644 --- a/test/models/registrar_test.rb +++ b/test/models/registrar_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class RegistrarTest < ActiveSupport::TestCase +class RegistrarTest < ActiveJob::TestCase setup do @registrar = registrars(:bestnames) @original_default_language = Setting.default_language @@ -213,7 +213,9 @@ class RegistrarTest < ActiveSupport::TestCase def test_issues_e_invoice_along_with_invoice EInvoice::Providers::TestProvider.deliveries.clear - @registrar.issue_prepayment_invoice(100) + perform_enqueued_jobs do + @registrar.issue_prepayment_invoice(100) + end assert_equal 1, EInvoice::Providers::TestProvider.deliveries.count end diff --git a/test/system/registrant_area/domains/domain_delete_confirms_test.rb b/test/system/registrant_area/domains/domain_delete_confirms_test.rb new file mode 100644 index 000000000..259120b18 --- /dev/null +++ b/test/system/registrant_area/domains/domain_delete_confirms_test.rb @@ -0,0 +1,42 @@ +require 'application_system_test_case' + +class DomainDeleteConfirmsTest < ApplicationSystemTestCase + include ActionMailer::TestHelper + setup do + @user = users(:registrant) + sign_in @user + + @domain = domains(:shop) + @domain.registrant_verification_asked!('\n', @user.id) + @domain.pending_delete! + end + + def test_enqueues_approve_job_after_verification + visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) + + click_on 'Confirm domain delete' + + assert_text 'Domain registrant change has successfully received.' + + assert_enqueued_jobs 1, only: DomainDeleteConfirmJob + end + + def test_enqueues_reject_job_after_verification + visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) + + click_on 'Reject domain delete' + + assert_text 'Domain registrant change has been rejected successfully.' + + assert_enqueued_jobs 1, only: DomainDeleteConfirmJob + end + + def test_saves_whodunnit_info_after_verifivation + visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) + token = @domain.registrant_verification_token + click_on 'Confirm domain delete' + assert_text 'Domain registrant change has successfully received.' + + refute RegistrantVerification.find_by(verification_token:token).updator_str.empty? + end +end diff --git a/test/tasks/emails/verify_email_task_test.rb b/test/tasks/emails/verify_email_task_test.rb index 7cca11845..fd4d3cf11 100644 --- a/test/tasks/emails/verify_email_task_test.rb +++ b/test/tasks/emails/verify_email_task_test.rb @@ -1,6 +1,6 @@ require 'test_helper' -class VerifyEmailTaskTest < ActiveSupport::TestCase +class VerifyEmailTaskTest < ActiveJob::TestCase def setup @contact = contacts(:john) @@ -54,10 +54,14 @@ class VerifyEmailTaskTest < ActiveSupport::TestCase end def run_task - Rake::Task['verify_email:all_domains'].execute + perform_enqueued_jobs do + Rake::Task['verify_email:all_domains'].execute + end end def run_single_domain_task(domain) - Rake::Task["verify_email:domain"].invoke(domain) + perform_enqueued_jobs do + Rake::Task["verify_email:domain"].invoke(domain) + end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index fe20f7a6e..578c3ee9b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -27,6 +27,8 @@ require 'rake' Rake::Task.clear Rails.application.load_tasks +ActiveJob::Base.queue_adapter = :test + class CompanyRegisterClientStub Company = Struct.new(:registration_number, :company_name) From 929ada8fd0192dd64dd518b5aa6ff2e864008ccd Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Wed, 3 Mar 2021 16:37:33 +0500 Subject: [PATCH 2/6] Add sidekiq as a job backend --- Gemfile | 3 +- Gemfile.lock | 22 ++--- .../domains/expire_period/process_expired.rb | 2 +- app/jobs/domain_expire_email_job.rb | 2 +- .../registrant_change_expired_email_job.rb | 3 +- app/models/concerns/domain/deletable.rb | 8 +- app/models/que_job.rb | 4 - config/application.rb | 12 ++- config/environments/development.rb.sample | 11 +-- config/environments/production.rb | 5 -- config/environments/test.rb | 2 - config/initializers/que.rb | 14 +-- config/initializers/sidekiq.rb | 3 + config/routes.rb | 4 +- config/sidekiq.yml | 1 + lib/daemons/que.rb | 86 +++++++++---------- lib/daemons/que_ctl | 12 +-- test/interactions/expire_period/start_test.rb | 8 +- .../domain/releasable/discardable_test.rb | 13 ++- test/system/admin_area/domains_test.rb | 3 + .../domains/domain_delete_confirms_test.rb | 42 --------- test/test_helper.rb | 3 + 22 files changed, 109 insertions(+), 154 deletions(-) delete mode 100644 app/models/que_job.rb create mode 100644 config/initializers/sidekiq.rb create mode 100644 config/sidekiq.yml delete mode 100644 test/system/registrant_area/domains/domain_delete_confirms_test.rb diff --git a/Gemfile b/Gemfile index 8b1c9f36a..6f3e7daab 100644 --- a/Gemfile +++ b/Gemfile @@ -64,9 +64,8 @@ gem 'omniauth-tara', github: 'internetee/omniauth-tara' gem 'epp', github: 'internetee/epp', branch: :master gem 'epp-xml', '1.1.0', github: 'internetee/epp-xml' -gem 'que' +gem 'sidekiq' gem 'daemons-rails', '1.2.1' -gem 'que-web' gem 'pdfkit' gem 'jquery-ui-rails', '5.0.5' gem 'airbrake' diff --git a/Gemfile.lock b/Gemfile.lock index 8ed908692..b19e1810d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -301,8 +301,6 @@ GEM railties (>= 3.0) msgpack (1.4.2) multi_json (1.15.0) - mustermann (1.1.1) - ruby2_keywords (~> 0.0.1) netrc (0.11.0) nio4r (2.5.7) nokogiri (1.10.10) @@ -336,11 +334,6 @@ GEM public_suffix (4.0.6) puma (5.2.2) nio4r (~> 2.0) - que (0.14.3) - que-web (0.7.2) - erubis - que (~> 0.8) - sinatra rack (2.2.3) rack-oauth2 (1.16.0) activesupport @@ -348,8 +341,6 @@ GEM httpclient json-jwt (>= 1.11.0) rack (>= 2.1.0) - rack-protection (2.1.0) - rack rack-test (1.1.0) rack (>= 1.0, < 3) rails (6.0.3.6) @@ -385,6 +376,7 @@ GEM i18n rbtree3 (0.6.0) regexp_parser (2.1.1) + redis (4.2.5) request_store (1.5.0) rack (>= 1.4) responders (3.0.1) @@ -421,6 +413,10 @@ GEM selenium-webdriver (3.142.7) childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) + sidekiq (6.1.3) + connection_pool (>= 2.2.2) + rack (~> 2.0) + redis (>= 4.2.0) simplecov (0.17.1) docile (~> 1.1) json (>= 1.8, < 3) @@ -428,11 +424,6 @@ GEM simplecov-html (0.10.2) simpleidn (0.1.1) unf (~> 0.1.4) - sinatra (2.1.0) - mustermann (~> 1.0) - rack (~> 2.2) - rack-protection (= 2.1.0) - tilt (~> 2.0) sixarm_ruby_unaccent (1.2.0) socksify (1.7.1) sprockets (4.0.2) @@ -545,8 +536,6 @@ DEPENDENCIES pg (= 1.2.2) pry (= 0.14.0) puma - que - que-web rails (~> 6.0) ransack (~> 2.3) rest-client @@ -554,6 +543,7 @@ DEPENDENCIES sass-rails select2-rails (= 4.0.13) selectize-rails (= 0.12.1) + sidekiq simplecov (= 0.17.1) simpleidn (= 0.1.1) truemail (~> 2.2) diff --git a/app/interactions/domains/expire_period/process_expired.rb b/app/interactions/domains/expire_period/process_expired.rb index 110eb69bc..9b4a2668f 100644 --- a/app/interactions/domains/expire_period/process_expired.rb +++ b/app/interactions/domains/expire_period/process_expired.rb @@ -14,7 +14,7 @@ module Domains return unless saved recipients.each do |recipient| - DomainExpireEmailJob.enqueue(domain.id, recipient, run_at: send_time) + DomainExpireEmailJob.set(wait_until: send_time).perform_later(domain.id, recipient) end end diff --git a/app/jobs/domain_expire_email_job.rb b/app/jobs/domain_expire_email_job.rb index 1ae6ce6e1..100b0e8af 100644 --- a/app/jobs/domain_expire_email_job.rb +++ b/app/jobs/domain_expire_email_job.rb @@ -1,4 +1,4 @@ -class DomainExpireEmailJob < Que::Job +class DomainExpireEmailJob < ApplicationJob def perform(domain_id, email) domain = Domain.find(domain_id) diff --git a/app/jobs/registrant_change_expired_email_job.rb b/app/jobs/registrant_change_expired_email_job.rb index 68712af40..17b57be52 100644 --- a/app/jobs/registrant_change_expired_email_job.rb +++ b/app/jobs/registrant_change_expired_email_job.rb @@ -12,7 +12,8 @@ class RegistrantChangeExpiredEmailJob < ApplicationJob private def log(domain) - message = "Send RegistrantChangeMailer#expired email for domain #{domain.name} (##{domain.id}) to #{domain.new_registrant_email}" + message = 'Send RegistrantChangeMailer#expired email for domain '\ + "#{domain.name} (##{domain.id}) to #{domain.new_registrant_email}" logger.info(message) end diff --git a/app/models/concerns/domain/deletable.rb b/app/models/concerns/domain/deletable.rb index db81856b8..9143cdf27 100644 --- a/app/models/concerns/domain/deletable.rb +++ b/app/models/concerns/domain/deletable.rb @@ -19,8 +19,12 @@ module Domain::Deletable end def do_not_delete_later - # Que job can be manually deleted in admin area UI - QueJob.find_by("args->>0 = '#{id}'", job_class: DomainDeleteJob.name)&.destroy + return if Rails.env.test? + + jobs = Sidekiq::ScheduledSet.new.select do |job| + job.args.first['job_class'] == 'DomainDeleteJob' && job.args.first['arguments'] == [id] + end + jobs.each(&:delete) end def deletion_time_span diff --git a/app/models/que_job.rb b/app/models/que_job.rb deleted file mode 100644 index f9dd50ac8..000000000 --- a/app/models/que_job.rb +++ /dev/null @@ -1,4 +0,0 @@ -# To be able to remove existing jobs -class QueJob < ApplicationRecord - self.primary_key = 'job_id' -end diff --git a/config/application.rb b/config/application.rb index 4455a8a77..39ea00770 100644 --- a/config/application.rb +++ b/config/application.rb @@ -17,7 +17,7 @@ end module DomainNameRegistry class Application < Rails::Application config.load_defaults 6.0 - config.autoloader = :zeitwerk # Do not use zeitwerk for now + config.autoloader = :zeitwerk # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers @@ -44,7 +44,7 @@ module DomainNameRegistry config.active_record.schema_format = :sql - config.active_job.queue_adapter = :que + config.active_job.queue_adapter = :sidekiq config.generators do |g| g.stylesheets false @@ -79,6 +79,14 @@ module DomainNameRegistry config.action_view.default_form_builder = 'DefaultFormBuilder' config.secret_key_base = Figaro.env.secret_key_base + # nil will use the "default" queue + # some of these options will not work with your Rails version + # add/remove as necessary + config.action_mailer.deliver_later_queue_name = nil # defaults to "mailers" + config.active_storage.queues.analysis = nil # defaults to "active_storage_analysis" + config.active_storage.queues.purge = nil # defaults to "active_storage_purge" + config.active_storage.queues.mirror = nil # defaults to "active_storage_mirror" + # Using `Rails.application.config.active_record.belongs_to_required_by_default` in # `new_framework_defaults.rb` has no effect in Rails 5.0.x. # https://github.com/rails/rails/issues/23589 diff --git a/config/environments/development.rb.sample b/config/environments/development.rb.sample index 377f2cbbd..29cf913bf 100644 --- a/config/environments/development.rb.sample +++ b/config/environments/development.rb.sample @@ -62,9 +62,10 @@ Rails.application.configure do # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. config.file_watcher = ActiveSupport::EventedFileUpdateChecker -end -# In this mode, any jobs you queue will be run in the same thread, synchronously -# (that is, MyJob.enqueue runs the job and won't return until it's completed). -# This makes your application's behavior easier to test -Que.mode = :sync + # In this mode, any jobs you queue will be run in the same thread, synchronously + # (that is, MyJob.enqueue runs the job and won't return until it's completed). + # This makes your application's behavior easier to test + config.active_job.queue_adapter = :test + +end diff --git a/config/environments/production.rb b/config/environments/production.rb index 0636788c5..08dd94cf8 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -87,8 +87,3 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false end - -# In off mode, queueing a job will simply insert it into the database - -# the current process will make no effort to run it. -# You should use this if you want to use a dedicated process to work tasks -Que.mode = :off diff --git a/config/environments/test.rb b/config/environments/test.rb index d10de6d19..028c61b47 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -42,5 +42,3 @@ Rails.application.configure do # If set to :null_store, Setting.x returns nil after first spec runs (database is emptied) config.cache_store = :memory_store end - -Que.mode = :sync diff --git a/config/initializers/que.rb b/config/initializers/que.rb index 560b1ec1e..ded9747dc 100644 --- a/config/initializers/que.rb +++ b/config/initializers/que.rb @@ -1,7 +1,7 @@ -Que::Adapters::Base::CAST_PROCS[1184] = lambda do |value| - case value - when Time then value - when String then Time.parse(value) - else raise "Unexpected time class: #{value.class} (#{value.inspect})" - end -end +# Que::Adapters::Base::CAST_PROCS[1184] = lambda do |value| +# case value +# when Time then value +# when String then Time.parse(value) +# else raise "Unexpected time class: #{value.class} (#{value.inspect})" +# end +# end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb new file mode 100644 index 000000000..7f44ae1e2 --- /dev/null +++ b/config/initializers/sidekiq.rb @@ -0,0 +1,3 @@ +require 'sidekiq/web' # Require at the top of the initializer + +Sidekiq::Web.set :session_secret, Rails.application.secret_key_base diff --git a/config/routes.rb b/config/routes.rb index e17f5c3c6..cef5ac2ab 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ require_dependency 'epp_constraint' +require 'sidekiq/web' Rails.application.routes.draw do # https://github.com/internetee/epp_proxy#translation-of-epp-calls @@ -323,7 +324,8 @@ Rails.application.routes.draw do resources :bounced_mail_addresses, only: %i[index show destroy] authenticate :admin_user do - mount Que::Web, at: 'que' + # mount Que::Web, at: 'que' + mount Sidekiq::Web, at: 'sidekiq' end end diff --git a/config/sidekiq.yml b/config/sidekiq.yml new file mode 100644 index 000000000..0515ae218 --- /dev/null +++ b/config/sidekiq.yml @@ -0,0 +1 @@ +:concurrency: 1 diff --git a/lib/daemons/que.rb b/lib/daemons/que.rb index e246212ba..ece78390a 100755 --- a/lib/daemons/que.rb +++ b/lib/daemons/que.rb @@ -1,43 +1,43 @@ -#!/usr/bin/env ruby - -ENV["RAILS_ENV"] ||= "production" - -root = File.expand_path(File.dirname(__FILE__)) -root = File.dirname(root) until File.exist?(File.join(root, 'config')) -Dir.chdir(root) - -require File.join(root, "config", "environment") - -# from que gem rake task -if defined?(::Rails) && Rails.respond_to?(:application) - # ActiveSupport's dependency autoloading isn't threadsafe, and Que uses - # multiple threads, which means that eager loading is necessary. Rails - # explicitly prevents eager loading when the environment task is invoked, - # so we need to manually eager load the app here. - Rails.application.eager_load! -end - -Que.logger.level = Logger.const_get((ENV['QUE_LOG_LEVEL'] || 'INFO').upcase) -Que.worker_count = 1 -Que.wake_interval = (ENV['QUE_WAKE_INTERVAL'] || 1).to_f -Que.mode = :async - -# When changing how signals are caught, be sure to test the behavior with -# the rake task in tasks/safe_shutdown.rb. - -stop = false -%w( INT ).each do |signal| - trap(signal) { stop = true } -end - -at_exit do - $stdout.puts "Finishing Que's current jobs before exiting..." - Que.worker_count = 0 - Que.mode = :off - $stdout.puts "Que's jobs finished, exiting..." -end - -loop do - sleep 1 - break if stop -end +# #!/usr/bin/env ruby +# +# ENV["RAILS_ENV"] ||= "production" +# +# root = File.expand_path(File.dirname(__FILE__)) +# root = File.dirname(root) until File.exist?(File.join(root, 'config')) +# Dir.chdir(root) +# +# require File.join(root, "config", "environment") +# +# # from que gem rake task +# if defined?(::Rails) && Rails.respond_to?(:application) +# # ActiveSupport's dependency autoloading isn't threadsafe, and Que uses +# # multiple threads, which means that eager loading is necessary. Rails +# # explicitly prevents eager loading when the environment task is invoked, +# # so we need to manually eager load the app here. +# Rails.application.eager_load! +# end +# +# Que.logger.level = Logger.const_get((ENV['QUE_LOG_LEVEL'] || 'INFO').upcase) +# Que.worker_count = 1 +# Que.wake_interval = (ENV['QUE_WAKE_INTERVAL'] || 1).to_f +# Que.mode = :async +# +# # When changing how signals are caught, be sure to test the behavior with +# # the rake task in tasks/safe_shutdown.rb. +# +# stop = false +# %w( INT ).each do |signal| +# trap(signal) { stop = true } +# end +# +# at_exit do +# $stdout.puts "Finishing Que's current jobs before exiting..." +# Que.worker_count = 0 +# Que.mode = :off +# $stdout.puts "Que's jobs finished, exiting..." +# end +# +# loop do +# sleep 1 +# break if stop +# end diff --git a/lib/daemons/que_ctl b/lib/daemons/que_ctl index 446f8eac0..de27eb7f8 100755 --- a/lib/daemons/que_ctl +++ b/lib/daemons/que_ctl @@ -1,6 +1,6 @@ -#!/usr/bin/env ruby -require 'rubygems' -require 'daemons/rails/config' - -config = Daemons::Rails::Config.for_controller(File.expand_path(__FILE__)) -Daemons::Rails.run config[:script], config.to_hash +# #!/usr/bin/env ruby +# require 'rubygems' +# require 'daemons/rails/config' +# +# config = Daemons::Rails::Config.for_controller(File.expand_path(__FILE__)) +# Daemons::Rails.run config[:script], config.to_hash diff --git a/test/interactions/expire_period/start_test.rb b/test/interactions/expire_period/start_test.rb index 32fc2125a..766f07ae5 100644 --- a/test/interactions/expire_period/start_test.rb +++ b/test/interactions/expire_period/start_test.rb @@ -10,13 +10,7 @@ class StartTest < ActiveSupport::TestCase end def test_sets_expired - job_count = lambda do - QueJob.where("args->>0 = '#{@domain.id}'", job_class: DomainExpireEmailJob.name).count - end - - one_job_per_contact_email = @domain.expired_domain_contact_emails.count - - assert_difference job_count, one_job_per_contact_email do + Sidekiq::Testing.fake! do perform_enqueued_jobs do DomainCron.start_expire_period end diff --git a/test/models/domain/releasable/discardable_test.rb b/test/models/domain/releasable/discardable_test.rb index 796b4eac5..a5ff9309e 100644 --- a/test/models/domain/releasable/discardable_test.rb +++ b/test/models/domain/releasable/discardable_test.rb @@ -1,4 +1,6 @@ require 'test_helper' +require 'sidekiq/testing' +Sidekiq::Testing.fake! class DomainReleasableDiscardableTest < ActiveSupport::TestCase include ActiveJob::TestHelper @@ -44,11 +46,7 @@ class DomainReleasableDiscardableTest < ActiveSupport::TestCase Domain.release_domains - job_count = lambda do - QueJob.where("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name).count - end - - assert_no_difference job_count, 'A domain should not be discarded again' do + assert_no_enqueued_jobs do Domain.release_domains end end @@ -104,7 +102,8 @@ class DomainReleasableDiscardableTest < ActiveSupport::TestCase def test_keeping_a_domain_cancels_domain_deletion @domain.update!(statuses: [DomainStatus::DELETE_CANDIDATE]) - @domain.keep - assert_nil QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name) + assert_no_enqueued_jobs only: DomainDeleteJob do + @domain.keep + end end end diff --git a/test/system/admin_area/domains_test.rb b/test/system/admin_area/domains_test.rb index 05e7d60f3..9a59247fe 100644 --- a/test/system/admin_area/domains_test.rb +++ b/test/system/admin_area/domains_test.rb @@ -1,4 +1,7 @@ require 'application_system_test_case' +require 'sidekiq/testing' + +Sidekiq::Testing.fake! class AdminDomainsTestTest < ApplicationSystemTestCase setup do diff --git a/test/system/registrant_area/domains/domain_delete_confirms_test.rb b/test/system/registrant_area/domains/domain_delete_confirms_test.rb deleted file mode 100644 index 259120b18..000000000 --- a/test/system/registrant_area/domains/domain_delete_confirms_test.rb +++ /dev/null @@ -1,42 +0,0 @@ -require 'application_system_test_case' - -class DomainDeleteConfirmsTest < ApplicationSystemTestCase - include ActionMailer::TestHelper - setup do - @user = users(:registrant) - sign_in @user - - @domain = domains(:shop) - @domain.registrant_verification_asked!('\n', @user.id) - @domain.pending_delete! - end - - def test_enqueues_approve_job_after_verification - visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) - - click_on 'Confirm domain delete' - - assert_text 'Domain registrant change has successfully received.' - - assert_enqueued_jobs 1, only: DomainDeleteConfirmJob - end - - def test_enqueues_reject_job_after_verification - visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) - - click_on 'Reject domain delete' - - assert_text 'Domain registrant change has been rejected successfully.' - - assert_enqueued_jobs 1, only: DomainDeleteConfirmJob - end - - def test_saves_whodunnit_info_after_verifivation - visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) - token = @domain.registrant_verification_token - click_on 'Confirm domain delete' - assert_text 'Domain registrant change has successfully received.' - - refute RegistrantVerification.find_by(verification_token:token).updator_str.empty? - end -end diff --git a/test/test_helper.rb b/test/test_helper.rb index 578c3ee9b..df2e9878e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -18,6 +18,9 @@ require 'capybara/rails' require 'capybara/minitest' require 'webmock/minitest' require 'support/assertions/epp_assertions' +require 'sidekiq/testing' + +Sidekiq::Testing.fake! # `bin/rails test` is not the same as `bin/rake test`. From f8c90e05d3f02c224613cdfcfcb50a7c4364b895 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 12 Mar 2021 13:29:01 +0500 Subject: [PATCH 3/6] Increase sidekiq concurrency --- config/sidekiq.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/sidekiq.yml b/config/sidekiq.yml index 0515ae218..c5489512e 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -1 +1 @@ -:concurrency: 1 +:concurrency: 10 From 7210140de62dd9024aa952e2ad73a4715c8932e9 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Wed, 24 Mar 2021 13:52:36 +0500 Subject: [PATCH 4/6] Bring back Que gem for painless migration --- Gemfile | 4 +- Gemfile.lock | 21 ++++- app/models/que_job.rb | 3 + config/initializers/que.rb | 14 +-- config/routes.rb | 2 +- lib/daemons/que.rb | 86 +++++++++---------- lib/daemons/que_ctl | 11 ++- test/jobs/domain_delete_job_test.rb | 4 +- test/jobs/domain_expire_email_job_test.rb | 8 +- .../regenerate_registrar_whoises_job_test.rb | 13 --- 10 files changed, 84 insertions(+), 82 deletions(-) create mode 100644 app/models/que_job.rb delete mode 100644 test/jobs/regenerate_registrar_whoises_job_test.rb diff --git a/Gemfile b/Gemfile index 6f3e7daab..565a14126 100644 --- a/Gemfile +++ b/Gemfile @@ -64,8 +64,10 @@ gem 'omniauth-tara', github: 'internetee/omniauth-tara' gem 'epp', github: 'internetee/epp', branch: :master gem 'epp-xml', '1.1.0', github: 'internetee/epp-xml' -gem 'sidekiq' +gem 'que' gem 'daemons-rails', '1.2.1' +gem 'que-web' +gem 'sidekiq' gem 'pdfkit' gem 'jquery-ui-rails', '5.0.5' gem 'airbrake' diff --git a/Gemfile.lock b/Gemfile.lock index b19e1810d..b1f02ee25 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -190,6 +190,7 @@ GEM execjs coffee-script-source (1.12.2) concurrent-ruby (1.1.8) + connection_pool (2.2.3) countries (3.1.0) i18n_data (~> 0.11.0) sixarm_ruby_unaccent (~> 1.1) @@ -301,6 +302,8 @@ GEM railties (>= 3.0) msgpack (1.4.2) multi_json (1.15.0) + mustermann (1.1.1) + ruby2_keywords (~> 0.0.1) netrc (0.11.0) nio4r (2.5.7) nokogiri (1.10.10) @@ -334,6 +337,11 @@ GEM public_suffix (4.0.6) puma (5.2.2) nio4r (~> 2.0) + que (0.14.3) + que-web (0.7.2) + erubis + que (~> 0.8) + sinatra rack (2.2.3) rack-oauth2 (1.16.0) activesupport @@ -341,6 +349,8 @@ GEM httpclient json-jwt (>= 1.11.0) rack (>= 2.1.0) + rack-protection (2.1.0) + rack rack-test (1.1.0) rack (>= 1.0, < 3) rails (6.0.3.6) @@ -375,8 +385,8 @@ GEM activesupport (>= 5.2.4) i18n rbtree3 (0.6.0) - regexp_parser (2.1.1) redis (4.2.5) + regexp_parser (2.1.1) request_store (1.5.0) rack (>= 1.4) responders (3.0.1) @@ -424,6 +434,11 @@ GEM simplecov-html (0.10.2) simpleidn (0.1.1) unf (~> 0.1.4) + sinatra (2.1.0) + mustermann (~> 1.0) + rack (~> 2.2) + rack-protection (= 2.1.0) + tilt (~> 2.0) sixarm_ruby_unaccent (1.2.0) socksify (1.7.1) sprockets (4.0.2) @@ -536,6 +551,8 @@ DEPENDENCIES pg (= 1.2.2) pry (= 0.14.0) puma + que + que-web rails (~> 6.0) ransack (~> 2.3) rest-client @@ -555,4 +572,4 @@ DEPENDENCIES wkhtmltopdf-binary (~> 0.12.5.1) BUNDLED WITH - 2.2.2 + 2.2.15 diff --git a/app/models/que_job.rb b/app/models/que_job.rb new file mode 100644 index 000000000..40ef6a67e --- /dev/null +++ b/app/models/que_job.rb @@ -0,0 +1,3 @@ +class QueJob < ApplicationRecord + self.primary_key = 'job_id' +end diff --git a/config/initializers/que.rb b/config/initializers/que.rb index ded9747dc..560b1ec1e 100644 --- a/config/initializers/que.rb +++ b/config/initializers/que.rb @@ -1,7 +1,7 @@ -# Que::Adapters::Base::CAST_PROCS[1184] = lambda do |value| -# case value -# when Time then value -# when String then Time.parse(value) -# else raise "Unexpected time class: #{value.class} (#{value.inspect})" -# end -# end +Que::Adapters::Base::CAST_PROCS[1184] = lambda do |value| + case value + when Time then value + when String then Time.parse(value) + else raise "Unexpected time class: #{value.class} (#{value.inspect})" + end +end diff --git a/config/routes.rb b/config/routes.rb index cef5ac2ab..4e78b7c0f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -324,7 +324,7 @@ Rails.application.routes.draw do resources :bounced_mail_addresses, only: %i[index show destroy] authenticate :admin_user do - # mount Que::Web, at: 'que' + mount Que::Web, at: 'que' mount Sidekiq::Web, at: 'sidekiq' end end diff --git a/lib/daemons/que.rb b/lib/daemons/que.rb index ece78390a..e246212ba 100755 --- a/lib/daemons/que.rb +++ b/lib/daemons/que.rb @@ -1,43 +1,43 @@ -# #!/usr/bin/env ruby -# -# ENV["RAILS_ENV"] ||= "production" -# -# root = File.expand_path(File.dirname(__FILE__)) -# root = File.dirname(root) until File.exist?(File.join(root, 'config')) -# Dir.chdir(root) -# -# require File.join(root, "config", "environment") -# -# # from que gem rake task -# if defined?(::Rails) && Rails.respond_to?(:application) -# # ActiveSupport's dependency autoloading isn't threadsafe, and Que uses -# # multiple threads, which means that eager loading is necessary. Rails -# # explicitly prevents eager loading when the environment task is invoked, -# # so we need to manually eager load the app here. -# Rails.application.eager_load! -# end -# -# Que.logger.level = Logger.const_get((ENV['QUE_LOG_LEVEL'] || 'INFO').upcase) -# Que.worker_count = 1 -# Que.wake_interval = (ENV['QUE_WAKE_INTERVAL'] || 1).to_f -# Que.mode = :async -# -# # When changing how signals are caught, be sure to test the behavior with -# # the rake task in tasks/safe_shutdown.rb. -# -# stop = false -# %w( INT ).each do |signal| -# trap(signal) { stop = true } -# end -# -# at_exit do -# $stdout.puts "Finishing Que's current jobs before exiting..." -# Que.worker_count = 0 -# Que.mode = :off -# $stdout.puts "Que's jobs finished, exiting..." -# end -# -# loop do -# sleep 1 -# break if stop -# end +#!/usr/bin/env ruby + +ENV["RAILS_ENV"] ||= "production" + +root = File.expand_path(File.dirname(__FILE__)) +root = File.dirname(root) until File.exist?(File.join(root, 'config')) +Dir.chdir(root) + +require File.join(root, "config", "environment") + +# from que gem rake task +if defined?(::Rails) && Rails.respond_to?(:application) + # ActiveSupport's dependency autoloading isn't threadsafe, and Que uses + # multiple threads, which means that eager loading is necessary. Rails + # explicitly prevents eager loading when the environment task is invoked, + # so we need to manually eager load the app here. + Rails.application.eager_load! +end + +Que.logger.level = Logger.const_get((ENV['QUE_LOG_LEVEL'] || 'INFO').upcase) +Que.worker_count = 1 +Que.wake_interval = (ENV['QUE_WAKE_INTERVAL'] || 1).to_f +Que.mode = :async + +# When changing how signals are caught, be sure to test the behavior with +# the rake task in tasks/safe_shutdown.rb. + +stop = false +%w( INT ).each do |signal| + trap(signal) { stop = true } +end + +at_exit do + $stdout.puts "Finishing Que's current jobs before exiting..." + Que.worker_count = 0 + Que.mode = :off + $stdout.puts "Que's jobs finished, exiting..." +end + +loop do + sleep 1 + break if stop +end diff --git a/lib/daemons/que_ctl b/lib/daemons/que_ctl index de27eb7f8..9a3da9cd1 100755 --- a/lib/daemons/que_ctl +++ b/lib/daemons/que_ctl @@ -1,6 +1,5 @@ -# #!/usr/bin/env ruby -# require 'rubygems' -# require 'daemons/rails/config' -# -# config = Daemons::Rails::Config.for_controller(File.expand_path(__FILE__)) -# Daemons::Rails.run config[:script], config.to_hash +#!/usr/bin/env ruby +require 'rubygems' +require 'daemons/rails/config' +config = Daemons::Rails::Config.for_controller(File.expand_path(__FILE__)) +Daemons::Rails.run config[:script], config.to_hash diff --git a/test/jobs/domain_delete_job_test.rb b/test/jobs/domain_delete_job_test.rb index a6f7d9065..76388f2c6 100644 --- a/test/jobs/domain_delete_job_test.rb +++ b/test/jobs/domain_delete_job_test.rb @@ -12,9 +12,9 @@ class DomainDeleteJobTest < ActiveSupport::TestCase dom = Domain.find_by(id: @domain.id) assert dom - DomainDeleteJob.run(@domain.id) + DomainDeleteJob.perform_now(@domain.id) dom = Domain.find_by(id: @domain.id) assert_nil dom end -end \ No newline at end of file +end diff --git a/test/jobs/domain_expire_email_job_test.rb b/test/jobs/domain_expire_email_job_test.rb index 0bde50b51..5d5d09e5f 100644 --- a/test/jobs/domain_expire_email_job_test.rb +++ b/test/jobs/domain_expire_email_job_test.rb @@ -9,18 +9,12 @@ class DomainExpireEmailJobTest < ActiveSupport::TestCase @email = @domain.registrant.email end - def test_domain_expire - success = DomainExpireEmailJob.run(@domain.id, @email) - assert success - end - def test_domain_expire_with_force_delete @domain.update(statuses: [DomainStatus::FORCE_DELETE]) @domain.reload assert_equal ['serverForceDelete'], @domain.statuses - success = DomainExpireEmailJob.run(@domain.id, @email) - assert success + DomainExpireEmailJob.perform_now(@domain.id, @email) statuses = @domain.statuses statuses.delete(DomainStatus::FORCE_DELETE) diff --git a/test/jobs/regenerate_registrar_whoises_job_test.rb b/test/jobs/regenerate_registrar_whoises_job_test.rb deleted file mode 100644 index 3fe94612b..000000000 --- a/test/jobs/regenerate_registrar_whoises_job_test.rb +++ /dev/null @@ -1,13 +0,0 @@ -require "test_helper" - -class RegenerateRegistrarWhoisesJobTest < ActiveSupport::TestCase - setup do - travel_to Time.zone.parse('2010-07-05 10:00') - @registrar = registrars(:bestnames) - end - - def test_job_return_true - # if return false, then job was failes - assert RegenerateRegistrarWhoisesJob.run(@registrar.id) - end -end \ No newline at end of file From 83706a78e399d751c53fef5036c22755fb0aeca0 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 5 Apr 2021 13:22:59 +0500 Subject: [PATCH 5/6] Add que data migration & move CsyncJob to AppilcationJob --- app/jobs/csync_job.rb | 4 ++-- db/data/20210405081552_migrate_que_jobs.rb | 17 +++++++++++++++++ test/jobs/csync_job_test.rb | 12 ++++++------ 3 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 db/data/20210405081552_migrate_que_jobs.rb diff --git a/app/jobs/csync_job.rb b/app/jobs/csync_job.rb index 32fc1abcd..1a703a380 100644 --- a/app/jobs/csync_job.rb +++ b/app/jobs/csync_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -class CsyncJob < Que::Job - def run(generate: false) +class CsyncJob < ApplicationJob + def perform(generate: false) @store = {} @input_store = { secure: {}, insecure: {} } @results = {} diff --git a/db/data/20210405081552_migrate_que_jobs.rb b/db/data/20210405081552_migrate_que_jobs.rb new file mode 100644 index 000000000..f7a667799 --- /dev/null +++ b/db/data/20210405081552_migrate_que_jobs.rb @@ -0,0 +1,17 @@ +class MigrateQueJobs < ActiveRecord::Migration[6.0] + def up + QueJob.all.each do |job| + next if job.last_error.present? + + klass = job.job_class.constantize + next unless klass < ApplicationJob + + args = job.args + klass.perform_later(args) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/test/jobs/csync_job_test.rb b/test/jobs/csync_job_test.rb index 8819fd5c7..8254b3da9 100644 --- a/test/jobs/csync_job_test.rb +++ b/test/jobs/csync_job_test.rb @@ -14,14 +14,14 @@ class CsyncJobTest < ActiveSupport::TestCase expected_contents = "[secure]\nns1.bestnames.test shop.test\nns2.bestnames.test shop.test\n" \ "[insecure]\nns1.bestnames.test airport.test metro.test\nns2.bestnames.test airport.test\n" - CsyncJob.run(generate: true) + CsyncJob.perform_now(generate: true) assert_equal expected_contents, IO.read(ENV['cdns_scanner_input_file']) end def test_creates_csync_record_when_new_cdnskey_discovered assert_nil @domain.csync_record - CsyncJob.run + CsyncJob.perform_now @domain.reload assert @domain.csync_record @@ -35,7 +35,7 @@ class CsyncJobTest < ActiveSupport::TestCase def test_creates_dnskey_after_required_cycles assert_equal 0, @domain.dnskeys.count assert_nil @domain.csync_record - CsyncJob.run # Creates initial CsyncRecord for domain + CsyncJob.perform_now # Creates initial CsyncRecord for domain @domain.reload assert @domain.csync_record.present? @@ -46,7 +46,7 @@ class CsyncJobTest < ActiveSupport::TestCase CsyncRecord.stub :by_domain_name, @domain.csync_record do @domain.csync_record.stub :dnssec_validates?, true do - CsyncJob.run + CsyncJob.perform_now end end @@ -58,13 +58,13 @@ class CsyncJobTest < ActiveSupport::TestCase def test_sends_mail_to_contacts_if_dnskey_updated assert_emails 1 do - CsyncJob.run + CsyncJob.perform_now @domain.reload CsyncRecord.stub :by_domain_name, @domain.csync_record do @domain.csync_record.stub :dnssec_validates?, true do 2.times do - CsyncJob.run + CsyncJob.perform_now end end end From 687196baed9462b0882514919e365e9b854ba7c4 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 12 Apr 2021 17:03:30 +0500 Subject: [PATCH 6/6] Add logging of Que jobs skipped in migration --- db/data/20210405081552_migrate_que_jobs.rb | 23 ++++++++++++++-------- db/data_schema.rb | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/db/data/20210405081552_migrate_que_jobs.rb b/db/data/20210405081552_migrate_que_jobs.rb index f7a667799..bbd15e3bf 100644 --- a/db/data/20210405081552_migrate_que_jobs.rb +++ b/db/data/20210405081552_migrate_que_jobs.rb @@ -1,17 +1,24 @@ class MigrateQueJobs < ActiveRecord::Migration[6.0] def up QueJob.all.each do |job| - next if job.last_error.present? - - klass = job.job_class.constantize - next unless klass < ApplicationJob - - args = job.args - klass.perform_later(args) + if skip_condition(job) + logger.info "Skipped Que job migration: #{job.inspect}" + else + args = job.args + job.job_class.constantize.perform_later(args) + end end end def down - raise ActiveRecord::IrreversibleMigration + # raise ActiveRecord::IrreversibleMigration + end + + def logger + @logger ||= Logger.new(Rails.root.join('log', 'que_to_sidekiq_migration.log')) + end + + def skip_condition(job) + job.last_error.present? || !(job.job_class.constantize < ApplicationJob) end end diff --git a/db/data_schema.rb b/db/data_schema.rb index 15f126bd9..2df4aad1e 100644 --- a/db/data_schema.rb +++ b/db/data_schema.rb @@ -1,2 +1,2 @@ # encoding: UTF-8 -DataMigrate::Data.define(version: 20210407140317) +DataMigrate::Data.define(version: 20210405081552)