From 294b60ebdc912bddb15dae85b5d8e5d7a92372cd Mon Sep 17 00:00:00 2001 From: dinsmol Date: Sat, 19 Feb 2022 22:43:05 +0300 Subject: [PATCH 1/7] Add removing old records for validation events --- app/interactions/actions/email_check.rb | 5 +++++ app/models/validation_event.rb | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 4b026ec2e4..248952a473 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -12,12 +12,17 @@ def call result = check_email(email) save_result(result) filtering_old_failed_records(result) + remove_old_records! result.success ? log_success : log_failure(result) result.success end private + def remove_old_records! + validation_eventable.validation_events.old_records.destroy_all + end + def check_email(parsed_email) Truemail.validate(parsed_email, with: calculate_check_level).result end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 621fec0301..1c0ec155de 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,7 +6,7 @@ # 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.year.freeze + VALIDATION_PERIOD = 4.month.freeze VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 MX_CHECK = 3 @@ -27,7 +27,7 @@ class ValidationEvent < ApplicationRecord belongs_to :validation_eventable, polymorphic: true - scope :recent, -> { where('created_at < ?', Time.zone.now - VALIDATION_PERIOD) } + scope :old_records, -> { where('created_at < ?', Time.zone.now - VALIDATION_PERIOD) } scope :successful, -> { where(success: true) } scope :failed, -> { where(success: false) } scope :regex, -> { where('event_data @> ?', { 'check_level': 'regex' }.to_json) } @@ -38,7 +38,7 @@ class ValidationEvent < ApplicationRecord after_create :check_for_force_delete def self.validated_ids_by(klass) - recent.successful.where('validation_eventable_type = ?', klass) + old_records.successful.where('validation_eventable_type = ?', klass) .pluck(:validation_eventable_id) end From c13a3e39fd57825be14425a9c631a53e7bc576a4 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Tue, 22 Feb 2022 23:51:39 +0300 Subject: [PATCH 2/7] Add tests --- test/interactions/email_check_test.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/interactions/email_check_test.rb b/test/interactions/email_check_test.rb index 4e77a5beee..0aed12ea0b 100644 --- a/test/interactions/email_check_test.rb +++ b/test/interactions/email_check_test.rb @@ -95,4 +95,27 @@ def test_should_remove_valid_validation_record_if_there_count_more_than_one assert_equal @contact.validation_events.count, 1 assert @contact.validation_events.last.success end + + def test_should_remove_old_validation_records + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: "box.tests", + errors: {:mx=>"target host(s) not found"}, + ) + + Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([true]) + + action = Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'regex') + + + action.call + assert_equal @contact.validation_events.count, 1 + + travel_to(Time.zone.now + ::ValidationEvent::VALIDATION_PERIOD + 1.minute) + action.call + assert_equal @contact.validation_events.count, 1 + end end From ab3ababc1a139cc43c4cc6c684e91684efcee604 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Wed, 23 Feb 2022 00:17:18 +0300 Subject: [PATCH 3/7] Fix codeclimate errors --- app/models/validation_event.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 1c0ec155de..dd8edd057e 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,7 +6,7 @@ # 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 = 4.month.freeze + VALIDATION_PERIOD = 4.month VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 MX_CHECK = 3 @@ -38,8 +38,10 @@ class ValidationEvent < ApplicationRecord after_create :check_for_force_delete def self.validated_ids_by(klass) - old_records.successful.where('validation_eventable_type = ?', klass) - .pluck(:validation_eventable_id) + old_records + .successful + .where('validation_eventable_type = ?', klass) + .pluck(:validation_eventable_id) end def failed? From 3586eb999bdd4e23f1cfc9c25bc721db06868412 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Wed, 23 Feb 2022 09:41:03 +0300 Subject: [PATCH 4/7] Return constant for validation period --- app/models/validation_event.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index dd8edd057e..28fc2173b7 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,7 +6,7 @@ # 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 = 4.month + VALIDATION_PERIOD = 4.month.freeze VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 MX_CHECK = 3 From 2dc5685393623639a15af6ea520427a516978655 Mon Sep 17 00:00:00 2001 From: dinsmol Date: Wed, 23 Feb 2022 09:48:51 +0300 Subject: [PATCH 5/7] Fix validation period --- app/models/validation_event.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 28fc2173b7..69fd660e55 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,7 +6,7 @@ # 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 = 4.month.freeze + VALIDATION_PERIOD = 1.year.freeze VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 MX_CHECK = 3 From 4dcd6d765c8daf8c6e7d2c9c17323344a065bb12 Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Wed, 18 May 2022 11:08:37 +0300 Subject: [PATCH 6/7] Task remove all old validation event records --- app/interactions/actions/email_check.rb | 13 ++--------- app/jobs/verify_emails_job.rb | 8 ------- lib/tasks/verify_email.rake | 12 ++++------ test/interactions/email_check_test.rb | 23 ------------------- test/tasks/emails/verify_email_task_test.rb | 25 ++++++++++++++++++--- 5 files changed, 28 insertions(+), 53 deletions(-) diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index c16c035ed7..ca36649625 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -12,17 +12,12 @@ def call result = check_email(email) save_result(result) filtering_old_failed_records(result) - remove_old_records! result.success ? log_success : log_failure(result) result.success end private - def remove_old_records! - validation_eventable.validation_events.old_records.destroy_all - end - def check_email(parsed_email) Truemail.validate(parsed_email, with: calculate_check_level).result end @@ -53,14 +48,10 @@ def save_result(result) if !result.success && @check_level == 'mx' result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'A') - output_a_and_aaaa_validation_results(email: @email, - result: result_validation, - type: 'A') + output_a_and_aaaa_validation_results(email: @email, result: result_validation, type: 'A') result_validation = Actions::AAndAaaaEmailValidation.call(email: @email, value: 'AAAA') if result_validation.empty? - output_a_and_aaaa_validation_results(email: @email, - result: result_validation, - type: 'AAAA') + output_a_and_aaaa_validation_results(email: @email, result: result_validation, type: 'AAAA') result.success = result_validation.present? end diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 7274fcad48..441a465699 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -20,10 +20,6 @@ def perform(email:, check_level: 'mx') private - def contact_not_found(contact_id) - raise StandardError, "Contact with contact_id #{contact_id} not found" - end - def validate_check_level(check_level) return if valid_check_levels.include? check_level @@ -38,10 +34,6 @@ def valid_check_levels ValidationEvent::VALID_CHECK_LEVELS end - def get_validation_results(contact) - ValidationEvent.where(created_at: Time.zone.now.beginning_of_day..Time.zone.now.end_of_day) - end - def filter_check_level(contact) return true unless contact.validation_events.exists? diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index b90fde0d66..ebc9c234b6 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -20,11 +20,11 @@ namespace :verify_email do hash: opts_hash) email_contacts = prepare_contacts(options) email_contacts.each do |email| - VerifyEmailsJob.set(wait_until: spam_protect_timeout(options)).perform_later( - email: email, - check_level: check_level(options) - ) + VerifyEmailsJob.set(wait_until: spam_protect_timeout(options)) + .perform_later(email: email, check_level: check_level(options)) end + + ValidationEvent.old_records.destroy_all end end @@ -40,10 +40,6 @@ 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]) diff --git a/test/interactions/email_check_test.rb b/test/interactions/email_check_test.rb index 0aed12ea0b..4e77a5beee 100644 --- a/test/interactions/email_check_test.rb +++ b/test/interactions/email_check_test.rb @@ -95,27 +95,4 @@ def test_should_remove_valid_validation_record_if_there_count_more_than_one assert_equal @contact.validation_events.count, 1 assert @contact.validation_events.last.success end - - def test_should_remove_old_validation_records - trumail_results = OpenStruct.new(success: false, - email: @contact.email, - domain: "box.tests", - errors: {:mx=>"target host(s) not found"}, - ) - - Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([true]) - - action = Actions::EmailCheck.new(email: @contact.email, - validation_eventable: @contact, - check_level: 'regex') - - - action.call - assert_equal @contact.validation_events.count, 1 - - travel_to(Time.zone.now + ::ValidationEvent::VALIDATION_PERIOD + 1.minute) - action.call - assert_equal @contact.validation_events.count, 1 - end end diff --git a/test/tasks/emails/verify_email_task_test.rb b/test/tasks/emails/verify_email_task_test.rb index a366708d96..256bc0fdd3 100644 --- a/test/tasks/emails/verify_email_task_test.rb +++ b/test/tasks/emails/verify_email_task_test.rb @@ -116,13 +116,32 @@ def test_fd_should_not_removed_if_change_email_to_another_invalid_one ) Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) - 1.times do - run_task - end + run_task assert contact.domains.last.force_delete_scheduled? end + def test_should_remove_old_validation_records + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: 'box.tests', + errors: { mx: 'target host(s) not found' }) + + Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([true]) + + Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'regex').call + + travel_to(Time.zone.now + ::ValidationEvent::VALIDATION_PERIOD + 1.minute) + assert_equal ValidationEvent.old_records.count, 1 + + run_task + + assert_predicate ValidationEvent.old_records.count, :zero? + end + def run_task perform_enqueued_jobs do Rake::Task['verify_email:check_all'].execute From de53f0422f307f6d344abbb7d3cd425f37f66c7c Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Thu, 19 May 2022 09:15:53 +0300 Subject: [PATCH 7/7] Destroy validation events before job --- lib/tasks/verify_email.rake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index ebc9c234b6..818684be2b 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -18,13 +18,12 @@ namespace :verify_email do options = RakeOptionParserBoilerplate.process_args(options: options, banner: banner, hash: opts_hash) + ValidationEvent.old_records.destroy_all email_contacts = prepare_contacts(options) email_contacts.each do |email| VerifyEmailsJob.set(wait_until: spam_protect_timeout(options)) .perform_later(email: email, check_level: check_level(options)) end - - ValidationEvent.old_records.destroy_all end end