From 34c14e5befe949c6a28bef7098b64076b56eca96 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 8 Feb 2024 15:00:41 +0200 Subject: [PATCH 1/8] changed count of contact validations --- .../api/v1/registrant/contacts_controller.rb | 2 +- app/interactions/actions/email_check.rb | 16 ++++-- app/models/concerns/email_verifable.rb | 19 +++---- test/models/contact_test.rb | 53 +++++++++++++++++++ 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/app/controllers/api/v1/registrant/contacts_controller.rb b/app/controllers/api/v1/registrant/contacts_controller.rb index 9bc66d6eb..1c22277df 100644 --- a/app/controllers/api/v1/registrant/contacts_controller.rb +++ b/app/controllers/api/v1/registrant/contacts_controller.rb @@ -145,7 +145,7 @@ module Api contact.transaction do contact.save! - contact.validate_email_by_regex_and_mx + contact.validate_email_by_regex_and_mx(single_email: true) contact.remove_force_delete_for_valid_contact action = current_registrant_user.actions.create!(contact: contact, operation: :update) diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index b15e4bad2..433f4a476 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -1,18 +1,19 @@ module Actions class EmailCheck - attr_reader :email, :validation_eventable, :check_level + attr_reader :email, :validation_eventable, :check_level, :single_email SAVE_RESULTS_BATCH_SIZE = 500 - def initialize(email:, validation_eventable:, check_level: nil) + def initialize(email:, validation_eventable:, single_email: false, check_level: nil) @email = email @validation_eventable = validation_eventable @check_level = check_level || :mx + @single_email = single_email end def call result = check_email(email) - save_result(result) + single_email ? save_single_result(validation_eventable: validation_eventable, result: result) : save_result(result) handle_logging(result) result.success end @@ -48,6 +49,15 @@ module Actions contact.validation_events.destroy_all if success end + def save_single_result(validation_eventable:, result:) + handle_mx_validation(result) if !result.success && @check_level == 'mx' + result.configuration = nil + + handle_saving_result(validation_eventable, result) + rescue ActiveRecord::RecordNotSaved + logger.info "Cannot save validation result for #{log_object_id}" and return true + end + def save_result(result) contacts = Contact.where(email: email) diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index e2d55f299..ba436f5cb 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -5,16 +5,12 @@ module EmailVerifable scope :recently_not_validated, -> { where.not(id: ValidationEvent.validated_ids_by(name)) } end - def validate_email_by_regex_and_mx - # return if Rails.env.test? - - verify_email(check_level: 'regex') - verify_email(check_level: 'mx') + def validate_email_by_regex_and_mx(single_email: false) + verify_email(check_level: 'regex', single_email: single_email) + verify_email(check_level: 'mx', single_email: single_email) end def remove_force_delete_for_valid_contact - # return if Rails.env.test? - domains.each do |domain| contact_emails_valid?(domain) ? domain.cancel_force_delete : nil end @@ -68,14 +64,15 @@ module EmailVerifable process_error(:billing_email) unless result end - def verify_email(check_level: 'regex') - verify(email: email, check_level: check_level) + def verify_email(check_level: 'regex', single_email: false) + verify(email: email, check_level: check_level, single_email: single_email) end - def verify(email:, check_level: 'regex') + def verify(email:, check_level: 'regex', single_email: false) action = Actions::EmailCheck.new(email: email, validation_eventable: self, - check_level: check_level) + check_level: check_level, + single_email: single_email) action.call end diff --git a/test/models/contact_test.rb b/test/models/contact_test.rb index 31f171e1f..fb34ff9e3 100644 --- a/test/models/contact_test.rb +++ b/test/models/contact_test.rb @@ -375,6 +375,59 @@ class ContactTest < ActiveJob::TestCase # assert_not @contact.domains.first.force_delete_scheduled? # end + def test_should_be_validated_only_one_contact_with_same_email + 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]) + + contact_1 = contacts(:john) + contact_2 = contacts(:william) + + assert_equal contact_1.validation_events.count, 0 + assert_equal contact_2.validation_events.count, 0 + + contact_1.email = 'test@example.com' && contact_1.save! && contact_1.reload + contact_2.email = 'test@example.com' && contact_2.save! && contact_2.reload + + contact_1.validate_email_by_regex_and_mx(single_email: true) + + contact_1.reload && contact_2.reload + + assert_equal contact_1.validation_events.count, 1 + assert_equal contact_2.validation_events.count, 0 + end + + def test_should_be_validated_serial_contacts_with_same_email + 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]) + + contact_1 = contacts(:john) + contact_2 = contacts(:william) + + assert_equal contact_1.validation_events.count, 0 + assert_equal contact_2.validation_events.count, 0 + + contact_1.update_attribute(:email, 'tester@example.com') + contact_2.update_attribute(:email, 'tester@example.com') + contact_1.reload && contact_2.reload + + contact_1.validate_email_by_regex_and_mx(single_email: false) + + contact_1.reload && contact_2.reload + + assert_equal contact_1.validation_events.count, 1 + assert_equal contact_2.validation_events.count, 1 + end + private def make_contact_free_of_domains_where_it_acts_as_a_registrant(contact) From 7e6dee7f10823c912b589811c38eea18b5331eea Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 8 Feb 2024 15:12:15 +0200 Subject: [PATCH 2/8] added filter for domains with fd --- app/models/concerns/email_verifable.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index ba436f5cb..d9237df48 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -11,7 +11,9 @@ module EmailVerifable end def remove_force_delete_for_valid_contact - domains.each do |domain| + domain_with_fd = domains.select(&:force_delete_scheduled?) + + domain_with_fd.each do |domain| contact_emails_valid?(domain) ? domain.cancel_force_delete : nil end end From 66f3fca97a01b13eb22b912b1405fae5e1c07529 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Thu, 8 Feb 2024 15:55:38 +0200 Subject: [PATCH 3/8] added single contact process --- app/interactions/actions/contact_create.rb | 2 +- app/interactions/actions/contact_update.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index b24f891e5..989ded90d 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -22,7 +22,7 @@ module Actions %i[regex mx].each do |m| result = Actions::SimpleMailValidator.run(email: contact.email, level: m) if result - @contact.validate_email_by_regex_and_mx + @contact.validate_email_by_regex_and_mx(single_email: true) @contact.remove_force_delete_for_valid_contact next diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index bf544097a..a7c2c5889 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -26,7 +26,7 @@ module Actions %i[regex mx].each do |m| result = Actions::SimpleMailValidator.run(email: @new_attributes[:email], level: m) if result - @contact.validate_email_by_regex_and_mx + @contact.validate_email_by_regex_and_mx(single_email: true) @contact.remove_force_delete_for_valid_contact next From 1ce5ebd62925df617ec49504fe745a25743f8488 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Mon, 12 Feb 2024 09:41:00 +0200 Subject: [PATCH 4/8] remove validation from contact creation --- app/interactions/actions/contact_create.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index 989ded90d..33995bef3 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -1,11 +1,12 @@ module Actions class ContactCreate - attr_reader :contact, :legal_document, :ident + attr_reader :contact, :legal_document, :ident, :result def initialize(contact, legal_document, ident) @contact = contact @legal_document = legal_document @ident = ident + @result = nil end def call @@ -20,13 +21,8 @@ module Actions return if Rails.env.test? %i[regex mx].each do |m| - result = Actions::SimpleMailValidator.run(email: contact.email, level: m) - if result - @contact.validate_email_by_regex_and_mx(single_email: true) - @contact.remove_force_delete_for_valid_contact - - next - end + @result = Actions::SimpleMailValidator.run(email: contact.email, level: m) + next if @result err_text = "email '#{contact.email}' didn't pass validation" contact.add_epp_error('2005', nil, nil, "#{I18n.t(:parameter_value_syntax_error)} #{err_text}") From 20cbdbfa755e75e386067f51ac7e9866c090ef5d Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Mon, 12 Feb 2024 14:12:04 +0200 Subject: [PATCH 5/8] added validation command --- app/jobs/verify_emails_job.rb | 4 ++-- lib/tasks/verify_domain.rake | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 lib/tasks/verify_domain.rake diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index faefa4cee..4095697b6 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -1,7 +1,7 @@ class VerifyEmailsJob < ApplicationJob discard_on StandardError - def perform(email:, check_level: 'mx') + def perform(email:, check_level: 'mx', single_check: false) contact = Contact.find_by(email: email) return logger.info "Contact #{email} not found!" if contact.nil? @@ -11,7 +11,7 @@ class VerifyEmailsJob < ApplicationJob validate_check_level(check_level) logger.info "Trying to verify contact email #{email} with check_level #{check_level}" - contact.verify_email(check_level: check_level) + contact.verify_email(check_level: check_level, single_email: single_check) rescue StandardError => e handle_error(e) end diff --git a/lib/tasks/verify_domain.rake b/lib/tasks/verify_domain.rake new file mode 100644 index 000000000..c800719c8 --- /dev/null +++ b/lib/tasks/verify_domain.rake @@ -0,0 +1,29 @@ +require 'optparse' +require 'rake_option_parser_boilerplate' +require 'syslog/logger' +require 'active_record' + +SPAM_PROTECT_TIMEOUT = 30.seconds + +task verify_domain: :environment do + options = { + domain_name: nil, + check_level: 'mx', + spam_protect: false, + } + banner = 'Usage: rake verify_domain -- [options]' + options = RakeOptionParserBoilerplate.process_args(options: options, + banner: banner, + hash: opts_hash) + + + domain = Domain.find_by(name: options[:domain_name]) + check_level = options[:check_level] + + domain.domain_contacts.each do |dc| + dc.contact.verify_email(check_level: check_level, single_email: true) + + Rails.logger.info "Validated contact with code #{dc.contact.code} and email #{dc.contact.email} of #{domain.name} domain" + Rails.logger.info "Result - #{dc.contact.validation_events.last.success}" + end +end From 681092bc1bf22b475ade75c2fa80839a4ae0bd90 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Mon, 12 Feb 2024 14:32:58 +0200 Subject: [PATCH 6/8] validation when contact create --- app/interactions/actions/contact_create.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index 33995bef3..80761c1af 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -15,6 +15,7 @@ module Actions validate_ident maybe_change_email commit + validate_contact end def maybe_change_email @@ -88,5 +89,11 @@ module Actions contact.email_history = contact.email contact.save end + + def validate_contact + [:regex, :mx].each do |m| + contact.verify_email(check_level: m, single_email: true) + end + end end end From fa3a629305f5376b6e2ade5033072105d5f506c1 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Mon, 12 Feb 2024 14:35:17 +0200 Subject: [PATCH 7/8] added validation signel after update --- app/interactions/actions/contact_update.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index a7c2c5889..11477b4c8 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -18,6 +18,7 @@ module Actions maybe_change_email if new_attributes[:email].present? maybe_filtering_old_failed_records commit + validate_contact end def maybe_change_email @@ -25,12 +26,7 @@ module Actions %i[regex mx].each do |m| result = Actions::SimpleMailValidator.run(email: @new_attributes[:email], level: m) - if result - @contact.validate_email_by_regex_and_mx(single_email: true) - @contact.remove_force_delete_for_valid_contact - - next - end + next if result err_text = "email '#{new_attributes[:email]}' didn't pass validation" contact.add_epp_error('2005', nil, nil, "#{I18n.t(:parameter_value_syntax_error)} #{err_text}") @@ -128,5 +124,14 @@ module Actions updated end + + def validate_contact + [:regex, :mx].each do |m| + @contact.verify_email(check_level: m, single_email: true) + end + + @contact.remove_force_delete_for_valid_contact + end + end end end From 25effa8d86bf358d8180cfd9cddfacf73dea1a09 Mon Sep 17 00:00:00 2001 From: Oleg Hasjanov Date: Mon, 12 Feb 2024 14:54:31 +0200 Subject: [PATCH 8/8] added condition for errors --- app/interactions/actions/contact_create.rb | 2 ++ app/interactions/actions/contact_update.rb | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/interactions/actions/contact_create.rb b/app/interactions/actions/contact_create.rb index 80761c1af..c1b091a1f 100644 --- a/app/interactions/actions/contact_create.rb +++ b/app/interactions/actions/contact_create.rb @@ -91,6 +91,8 @@ module Actions end def validate_contact + return if @error || !contact.valid? + [:regex, :mx].each do |m| contact.verify_email(check_level: m, single_email: true) end diff --git a/app/interactions/actions/contact_update.rb b/app/interactions/actions/contact_update.rb index 11477b4c8..a37f81b60 100644 --- a/app/interactions/actions/contact_update.rb +++ b/app/interactions/actions/contact_update.rb @@ -126,12 +126,13 @@ module Actions end def validate_contact + return if @error || !contact.valid? + [:regex, :mx].each do |m| @contact.verify_email(check_level: m, single_email: true) end @contact.remove_force_delete_for_valid_contact end - end end end