From 5f0c031410e606b919ec0a543a7798254e686edb Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Thu, 1 Jul 2021 16:00:35 +0500 Subject: [PATCH] Add a rake task running job & test for job --- app/interactions/actions/email_check.rb | 2 +- app/jobs/verify_emails_job.rb | 47 +++++--------- app/models/concerns/email_verifable.rb | 5 ++ app/models/validation_event.rb | 9 +++ lib/email_address_converter.rb | 2 - lib/tasks/verify_email.rake | 70 ++++++++++++++------- test/jobs/verify_emails_job_test.rb | 33 +++------- test/tasks/emails/verify_email_task_test.rb | 29 ++------- 8 files changed, 92 insertions(+), 105 deletions(-) diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index c17831289..19b193c47 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -19,7 +19,7 @@ module Actions private def check_email(parsed_email) - Truemail.validate(parsed_email, with: check_level).result + Truemail.validate(parsed_email, with: check_level.to_sym).result end def save_result(result) diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 1de244221..544e1cb4c 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -1,47 +1,34 @@ class VerifyEmailsJob < ApplicationJob discard_on StandardError + VALID_CHECK_LEVELS = %w[regex mx smtp].freeze - def perform(verification_id) - email_address_verification = EmailAddressVerification.find(verification_id) - return unless need_to_verify?(email_address_verification) + def perform(contact_id:, check_level: 'regex') + contact = Contact.find_by(id: contact_id) + contact_not_found(contact_id) unless contact + validate_check_level(check_level) - process(email_address_verification) + action = Actions::EmailCheck.new(email: contact.email, + validation_eventable: contact, + check_level: check_level) + action.call rescue StandardError => e - log_error(verification: email_address_verification, error: e) + logger.error e.message raise e end private - def need_to_verify?(email_address_verification) - return false if email_address_verification.blank? - return false if email_address_verification.recently_verified? - - true + def contact_not_found(contact_id) + raise StandardError, "Contact with contact_id #{contact_id} not found" end - def process(email_address_verification) - email_address_verification.verify - log_success(email_address_verification) + def validate_check_level(check_level) + return if VALID_CHECK_LEVELS.include? check_level + + raise StandardError, "Check level #{check_level} is invalid" end def logger - @logger ||= Logger.new(Rails.root.join('log/email_verification.log')) - end - - def log_success(verification) - email = verification.try(:email) || verification - message = "Email address #{email} verification done" - logger.info message - end - - def log_error(verification:, error:) - email = verification.try(:email) || verification - message = <<~TEXT.squish - There was an error verifying email #{email}. - The error message was the following: #{error} - This job will retry. - TEXT - logger.error message + @logger ||= Rails.logger end end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 1da52dcf3..5c8ed82e1 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -1,6 +1,10 @@ module EmailVerifable extend ActiveSupport::Concern + included do + scope :recently_not_validated, -> { where.not(id: ValidationEvent.validated_ids_by(name)) } + end + def email_verification EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) end @@ -16,6 +20,7 @@ module EmailVerifable email_verification&.failed? end + # TODO: The following methods are deprecated and need to be moved to ValidationEvent class class_methods do def domain(email) Mail::Address.new(email).domain&.downcase || 'not_found' diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 2d086e4d0..4be4bd68d 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,11 +6,20 @@ # For email_validation event kind also check_level (regex/mx/smtp) is stored in the event_data class ValidationEvent < ApplicationRecord enum event_type: ValidationEvent::EventType::TYPES, _suffix: true + VALIDATION_PERIOD = 1.month.ago.freeze store_accessor :event_data, :errors, :check_level, :email belongs_to :validation_eventable, polymorphic: true + scope :recent, -> { where('created_at > ?', VALIDATION_PERIOD) } + scope :successful, -> { where(success: true) } + + def self.validated_ids_by(klass) + recent.successful.where('validation_eventable_type = ?', klass) + .pluck(:validation_eventable_id) + end + def event_type @event_type ||= ValidationEvent::EventType.new(self[:event_kind]) end diff --git a/lib/email_address_converter.rb b/lib/email_address_converter.rb index e0d8c47c4..b578a061b 100644 --- a/lib/email_address_converter.rb +++ b/lib/email_address_converter.rb @@ -17,8 +17,6 @@ module EmailAddressConverter "#{local}@#{domain}"&.downcase end - private - def domain(email) Mail::Address.new(email).domain&.downcase || 'not_found' rescue Mail::Field::IncompleteParseError diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index bcd317947..00cd6ef4b 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -1,35 +1,16 @@ require 'optparse' require 'rake_option_parser_boilerplate' +require 'syslog/logger' namespace :verify_email do - desc 'Stars verifying email jobs for all the domain' - task all_domains: :environment 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.perform_later(ver.id) - next - end - end - - # Need to be run like 'bundle exec rake verify_email:domain['gmail.com']' - # In zsh syntax will be 'bundle exec rake verify_email:domain\['gmail.com'\]' - # Default 'bundle exec rake verify_email:domain' wil use 'internet.ee' domain - desc 'Stars verifying email jobs for domain stated in argument' - task :domain, [:domain_name] => [:environment] do |_task, args| - args.with_defaults(domain_name: 'internet.ee') - - verifications_by_domain = EmailAddressVerification.not_verified_recently - .by_domain(args[:domain_name]) - verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } - end - # bundle exec rake verify_email:check_all -- -d=shop.test --check_level=mx --spam_protect=true # bundle exec rake verify_email:check_all -- -dshop.test -cmx -strue desc 'Starts verifying email jobs with optional check level and spam protection' - task :check_all do + task check_all: :environment do + SPAM_PROTECT_TIMEOUT = 30.seconds + options = { - domain_name: 'shop.test', + domain_name: nil, check_level: 'regex', spam_protect: false, } @@ -37,9 +18,50 @@ namespace :verify_email do options = RakeOptionParserBoilerplate.process_args(options: options, banner: banner, hash: opts_hash) + + contacts = prepare_contacts(options) + logger.info 'No contacts to check email selected' and next if contacts.blank? + + contacts.find_each do |contact| + VerifyEmailsJob.set(wait_until: spam_protect_timeout(options)).perform_later( + contact_id: contact.id, + check_level: check_level(options) + ) + end end end +def check_level(options) + options[:check_level] +end + +def spam_protect(options) + options[:spam_protect] +end + +def spam_protect_timeout(options) + spam_protect(options) ? 0.seconds : SPAM_PROTECT_TIMEOUT +end + +def logger + @logger ||= ActiveSupport::TaggedLogging.new(Syslog::Logger.new('registry')) +end + +def prepare_contacts(options) + if options[:domain_name].present? + contacts_by_domain(options[:domain_name]) + else + Contact.recently_not_validated + end +end + +def contacts_by_domain(domain_name) + domain = ::Domain.find_by(name: domain_name) + return unless domain + + domain.contacts.recently_not_validated +end + def opts_hash { domain_name: ['-d [DOMAIN_NAME]', '--domain_name [DOMAIN_NAME]', String], diff --git a/test/jobs/verify_emails_job_test.rb b/test/jobs/verify_emails_job_test.rb index 49a08fe73..d4eed0ec0 100644 --- a/test/jobs/verify_emails_job_test.rb +++ b/test/jobs/verify_emails_job_test.rb @@ -4,9 +4,6 @@ class VerifyEmailsJobTest < ActiveJob::TestCase def setup @contact = contacts(:john) @invalid_contact = contacts(:invalid_email) - @contact_verification = @contact.email_verification - @invalid_contact_verification = @invalid_contact.email_verification - @default_whitelist = Truemail.configure.whitelisted_domains @default_blacklist = Truemail.configure.blacklisted_domains Truemail.configure.whitelisted_domains = whitelisted_domains @@ -33,33 +30,21 @@ class VerifyEmailsJobTest < ActiveJob::TestCase end def test_job_checks_if_email_valid - perform_enqueued_jobs do - VerifyEmailsJob.perform_now(@contact_verification.id) + assert_difference 'ValidationEvent.successful.count', 1 do + perform_enqueued_jobs do + VerifyEmailsJob.perform_now(contact_id: @contact.id, check_level: 'regex') + end end - @contact_verification.reload - - assert @contact_verification.success - end - - def test_job_checks_does_not_run_if_recent - old_verified_at = Time.zone.now - 10.days - @contact_verification.update(success: true, verified_at: old_verified_at) - assert @contact_verification.recently_verified? - - 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 + assert ValidationEvent.validated_ids_by(Contact).include? @contact.id end def test_job_checks_if_email_invalid perform_enqueued_jobs do - VerifyEmailsJob.perform_now(@invalid_contact_verification.id) + VerifyEmailsJob.perform_now(contact_id: @invalid_contact.id, check_level: 'regex') end - @contact_verification.reload + @invalid_contact.reload - refute @contact_verification.success + refute @invalid_contact.validation_events.last.success + refute ValidationEvent.validated_ids_by(Contact).include? @invalid_contact.id end end diff --git a/test/tasks/emails/verify_email_task_test.rb b/test/tasks/emails/verify_email_task_test.rb index fd4d3cf11..f12d15b3d 100644 --- a/test/tasks/emails/verify_email_task_test.rb +++ b/test/tasks/emails/verify_email_task_test.rb @@ -5,8 +5,6 @@ class VerifyEmailTaskTest < ActiveJob::TestCase def setup @contact = contacts(:john) @invalid_contact = contacts(:invalid_email) - @contact_verification = @contact.email_verification - @invalid_contact_verification = @invalid_contact.email_verification @default_whitelist = Truemail.configure.whitelisted_domains @default_blacklist = Truemail.configure.blacklisted_domains @@ -36,32 +34,15 @@ class VerifyEmailTaskTest < ActiveJob::TestCase def test_tasks_verifies_emails capture_io { run_task } - @contact_verification.reload - @invalid_contact_verification.reload - - assert @contact_verification.verified? - assert @invalid_contact_verification.failed? - end - - def test_domain_task_verifies_for_one_domain - capture_io { run_single_domain_task(@contact_verification.domain) } - - @contact_verification.reload - @invalid_contact_verification.reload - - assert @contact_verification.verified? - assert @invalid_contact_verification.not_verified? + assert ValidationEvent.validated_ids_by(Contact).include? @contact.id + assert @contact.validation_events.last.success + refute @invalid_contact.validation_events.last.success + refute ValidationEvent.validated_ids_by(Contact).include? @invalid_contact.id end def run_task perform_enqueued_jobs do - Rake::Task['verify_email:all_domains'].execute - end - end - - def run_single_domain_task(domain) - perform_enqueued_jobs do - Rake::Task["verify_email:domain"].invoke(domain) + Rake::Task['verify_email:check_all'].execute end end end