From 7d3f0249dcefaa55f7a7c59509a3f4f34d1eac90 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 28 Jun 2021 15:44:28 +0500 Subject: [PATCH 01/11] WIP Interim save --- app/models/concerns/email_verifable.rb | 15 ++++--- app/models/contact.rb | 4 +- app/models/email_address_verification.rb | 43 ++----------------- ..._relation_to_email_address_verification.rb | 31 +++++++++++++ db/structure.sql | 31 +++++++------ lib/tasks/verify_email.rake | 18 ++++---- 6 files changed, 75 insertions(+), 67 deletions(-) create mode 100644 db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 1da52dcf3..f24c37934 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -1,15 +1,20 @@ module EmailVerifable extend ActiveSupport::Concern + included do + has_many :email_address_verifications, as: :email_verifable + end + def email_verification - EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) + # EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) + email_address_verification end def billing_email_verification - return unless attribute_names.include?('billing_email') - - EmailAddressVerification.find_or_create_by(email: unicode_billing_email, - domain: domain(billing_email)) + # return unless attribute_names.include?('billing_email') + # + # EmailAddressVerification.find_or_create_by(email: unicode_billing_email, + # domain: domain(billing_email)) end def email_verification_failed? diff --git a/app/models/contact.rb b/app/models/contact.rb index f33c22c74..eebb1ccd5 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -25,8 +25,8 @@ class Contact < ApplicationRecord alias_attribute :copy_from_id, :original_id # Old attribute name; for PaperTrail scope :email_verification_failed, lambda { - joins('LEFT JOIN email_address_verifications emv ON contacts.email = emv.email') - .where('success = false and verified_at IS NOT NULL') + # joins('LEFT JOIN email_address_verifications emv ON contacts.email = emv.email') + # .where('success = false and verified_at IS NOT NULL') } NAME_REGEXP = /([\u00A1-\u00B3\u00B5-\u00BF\u0021-\u0026\u0028-\u002C\u003A-\u0040]| diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb index 4eba0f3e9..cf207a7c2 100644 --- a/app/models/email_address_verification.rb +++ b/app/models/email_address_verification.rb @@ -1,50 +1,15 @@ class EmailAddressVerification < ApplicationRecord - RECENTLY_VERIFIED_PERIOD = 1.month - after_save :check_force_delete + belongs_to :email_verifable, polymorphic: true - scope :not_verified_recently, lambda { - where('verified_at IS NULL or verified_at < ?', verification_period) - } + RECENTLY_VERIFIED_PERIOD = 1.year.freeze + SCAN_CYCLES = 3.freeze + # after_save :check_force_delete - scope :verified_recently, lambda { - where('verified_at IS NOT NULL and verified_at >= ?', verification_period).where(success: true) - } scope :verification_failed, lambda { where.not(verified_at: nil).where(success: false) } - scope :by_domain, ->(domain_name) { where(domain: domain_name) } - - def recently_verified? - verified_at.present? && - verified_at > verification_period - end - - def verification_period - self.class.verification_period - end - - def self.verification_period - Time.zone.now - RECENTLY_VERIFIED_PERIOD - end - - def not_verified? - verified_at.blank? && !success - end - - def failed? - bounce_present? || (verified_at.present? && !success) - end - - def verified? - success - end - - def bounce_present? - BouncedMailAddress.find_by(email: email).present? - end - def check_force_delete return unless failed? diff --git a/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb b/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb new file mode 100644 index 000000000..5d2d8defa --- /dev/null +++ b/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb @@ -0,0 +1,31 @@ +class AddPolymorphicRelationToEmailAddressVerification < ActiveRecord::Migration[6.1] + def change + remove_column :email_address_verifications, :email, :string + remove_column :email_address_verifications, :success, :boolean + remove_column :email_address_verifications, :domain, :string + + change_table 'email_address_verifications' do |t| + t.references :email_verifable, polymorphic: true + t.jsonb :result + t.integer :times_scanned + end + + reversible do |change| + change.up do + EmailAddressVerification.destroy_all + + execute <<-SQL + CREATE TYPE email_verification_type AS ENUM ('regex', 'mx', 'smtp'); + SQL + + add_column :email_address_verifications, :type, :email_verification_type + end + + change.down do + execute <<-SQL + DROP TYPE email_verification_type; + SQL + end + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 54d740fa5..a6ecff82f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -65,6 +65,17 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; COMMENT ON EXTENSION pgcrypto IS 'cryptographic functions'; +-- +-- Name: email_verification_type; Type: TYPE; Schema: public; Owner: - +-- + +CREATE TYPE public.email_verification_type AS ENUM ( + 'regex', + 'mx', + 'smtp' +); + + -- -- Name: generate_zonefile(character varying); Type: FUNCTION; Schema: public; Owner: - -- @@ -963,10 +974,12 @@ ALTER SEQUENCE public.domains_id_seq OWNED BY public.domains.id; CREATE TABLE public.email_address_verifications ( id bigint NOT NULL, - email public.citext NOT NULL, verified_at timestamp without time zone, - success boolean DEFAULT false NOT NULL, - domain public.citext NOT NULL + email_verifable_type character varying, + email_verifable_id bigint, + result jsonb, + times_scanned integer, + type public.email_verification_type ); @@ -3986,13 +3999,6 @@ CREATE INDEX index_domain_transfers_on_domain_id ON public.domain_transfers USIN CREATE INDEX index_domains_on_delete_date ON public.domains USING btree (delete_date); --- --- Name: index_domains_on_json_statuses_history; Type: INDEX; Schema: public; Owner: - --- - -CREATE INDEX index_domains_on_json_statuses_history ON public.domains USING gin (json_statuses_history); - - -- -- Name: index_domains_on_name; Type: INDEX; Schema: public; Owner: - -- @@ -4043,10 +4049,10 @@ CREATE INDEX index_domains_on_statuses ON public.domains USING gin (statuses); -- --- Name: index_email_address_verifications_on_domain; Type: INDEX; Schema: public; Owner: - +-- Name: index_email_address_verifications_on_email_verifable; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_email_address_verifications_on_domain ON public.email_address_verifications USING btree (domain); +CREATE INDEX index_email_address_verifications_on_email_verifable ON public.email_address_verifications USING btree (email_verifable_type, email_verifable_id); -- @@ -5161,6 +5167,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200921084356'), ('20210215101019'), ('20210616112332'), +('20210628090353'), ('20210708131814'); diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index daffabf19..027e111a9 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -1,12 +1,12 @@ 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 + # 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']' @@ -16,8 +16,8 @@ namespace :verify_email do 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) } + # verifications_by_domain = EmailAddressVerification.not_verified_recently + # .by_domain(args[:domain_name]) + # verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } end end From 6675dac63d3338133767d816a0ca7fe57476b029 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 29 Jun 2021 12:31:45 +0500 Subject: [PATCH 02/11] Revert "WIP Interim save" This reverts commit 74efab33be5f52d622a96e0967e694b3c5f617dc. --- app/models/concerns/email_verifable.rb | 15 +++---- app/models/contact.rb | 4 +- app/models/email_address_verification.rb | 43 +++++++++++++++++-- ..._relation_to_email_address_verification.rb | 31 ------------- db/structure.sql | 23 +++------- lib/tasks/verify_email.rake | 18 ++++---- 6 files changed, 60 insertions(+), 74 deletions(-) delete mode 100644 db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index f24c37934..1da52dcf3 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -1,20 +1,15 @@ module EmailVerifable extend ActiveSupport::Concern - included do - has_many :email_address_verifications, as: :email_verifable - end - def email_verification - # EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) - email_address_verification + EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) end def billing_email_verification - # return unless attribute_names.include?('billing_email') - # - # EmailAddressVerification.find_or_create_by(email: unicode_billing_email, - # domain: domain(billing_email)) + return unless attribute_names.include?('billing_email') + + EmailAddressVerification.find_or_create_by(email: unicode_billing_email, + domain: domain(billing_email)) end def email_verification_failed? diff --git a/app/models/contact.rb b/app/models/contact.rb index eebb1ccd5..f33c22c74 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -25,8 +25,8 @@ class Contact < ApplicationRecord alias_attribute :copy_from_id, :original_id # Old attribute name; for PaperTrail scope :email_verification_failed, lambda { - # joins('LEFT JOIN email_address_verifications emv ON contacts.email = emv.email') - # .where('success = false and verified_at IS NOT NULL') + joins('LEFT JOIN email_address_verifications emv ON contacts.email = emv.email') + .where('success = false and verified_at IS NOT NULL') } NAME_REGEXP = /([\u00A1-\u00B3\u00B5-\u00BF\u0021-\u0026\u0028-\u002C\u003A-\u0040]| diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb index cf207a7c2..4eba0f3e9 100644 --- a/app/models/email_address_verification.rb +++ b/app/models/email_address_verification.rb @@ -1,15 +1,50 @@ class EmailAddressVerification < ApplicationRecord - belongs_to :email_verifable, polymorphic: true + RECENTLY_VERIFIED_PERIOD = 1.month + after_save :check_force_delete - RECENTLY_VERIFIED_PERIOD = 1.year.freeze - SCAN_CYCLES = 3.freeze - # after_save :check_force_delete + scope :not_verified_recently, lambda { + where('verified_at IS NULL or verified_at < ?', verification_period) + } + scope :verified_recently, lambda { + where('verified_at IS NOT NULL and verified_at >= ?', verification_period).where(success: true) + } scope :verification_failed, lambda { where.not(verified_at: nil).where(success: false) } + scope :by_domain, ->(domain_name) { where(domain: domain_name) } + + def recently_verified? + verified_at.present? && + verified_at > verification_period + end + + def verification_period + self.class.verification_period + end + + def self.verification_period + Time.zone.now - RECENTLY_VERIFIED_PERIOD + end + + def not_verified? + verified_at.blank? && !success + end + + def failed? + bounce_present? || (verified_at.present? && !success) + end + + def verified? + success + end + + def bounce_present? + BouncedMailAddress.find_by(email: email).present? + end + def check_force_delete return unless failed? diff --git a/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb b/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb deleted file mode 100644 index 5d2d8defa..000000000 --- a/db/migrate/20210628090353_add_polymorphic_relation_to_email_address_verification.rb +++ /dev/null @@ -1,31 +0,0 @@ -class AddPolymorphicRelationToEmailAddressVerification < ActiveRecord::Migration[6.1] - def change - remove_column :email_address_verifications, :email, :string - remove_column :email_address_verifications, :success, :boolean - remove_column :email_address_verifications, :domain, :string - - change_table 'email_address_verifications' do |t| - t.references :email_verifable, polymorphic: true - t.jsonb :result - t.integer :times_scanned - end - - reversible do |change| - change.up do - EmailAddressVerification.destroy_all - - execute <<-SQL - CREATE TYPE email_verification_type AS ENUM ('regex', 'mx', 'smtp'); - SQL - - add_column :email_address_verifications, :type, :email_verification_type - end - - change.down do - execute <<-SQL - DROP TYPE email_verification_type; - SQL - end - end - end -end diff --git a/db/structure.sql b/db/structure.sql index a6ecff82f..365e975cc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -65,17 +65,6 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; COMMENT ON EXTENSION pgcrypto IS 'cryptographic functions'; --- --- Name: email_verification_type; Type: TYPE; Schema: public; Owner: - --- - -CREATE TYPE public.email_verification_type AS ENUM ( - 'regex', - 'mx', - 'smtp' -); - - -- -- Name: generate_zonefile(character varying); Type: FUNCTION; Schema: public; Owner: - -- @@ -974,12 +963,10 @@ ALTER SEQUENCE public.domains_id_seq OWNED BY public.domains.id; CREATE TABLE public.email_address_verifications ( id bigint NOT NULL, + email public.citext NOT NULL, verified_at timestamp without time zone, - email_verifable_type character varying, - email_verifable_id bigint, - result jsonb, - times_scanned integer, - type public.email_verification_type + success boolean DEFAULT false NOT NULL, + domain public.citext NOT NULL ); @@ -4049,10 +4036,10 @@ CREATE INDEX index_domains_on_statuses ON public.domains USING gin (statuses); -- --- Name: index_email_address_verifications_on_email_verifable; Type: INDEX; Schema: public; Owner: - +-- Name: index_email_address_verifications_on_domain; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_email_address_verifications_on_email_verifable ON public.email_address_verifications USING btree (email_verifable_type, email_verifable_id); +CREATE INDEX index_email_address_verifications_on_domain ON public.email_address_verifications USING btree (domain); -- diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index 027e111a9..daffabf19 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -1,12 +1,12 @@ 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 + 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']' @@ -16,8 +16,8 @@ namespace :verify_email do 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) } + verifications_by_domain = EmailAddressVerification.not_verified_recently + .by_domain(args[:domain_name]) + verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } end end From d4fe961e34dafa99cc380b4fbe74563fc3bb6727 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 29 Jun 2021 13:47:38 +0500 Subject: [PATCH 03/11] Add basic storage for validation events --- app/models/contact.rb | 1 + app/models/validation_event.rb | 9 +++ app/models/validation_event/event_type.rb | 10 +++ ...20210629074044_create_validation_events.rb | 28 +++++++ db/structure.sql | 75 +++++++++++++++++++ 5 files changed, 123 insertions(+) create mode 100644 app/models/validation_event.rb create mode 100644 app/models/validation_event/event_type.rb create mode 100644 db/migrate/20210629074044_create_validation_events.rb diff --git a/app/models/contact.rb b/app/models/contact.rb index f33c22c74..84d4ba962 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -16,6 +16,7 @@ class Contact < ApplicationRecord has_many :domain_contacts has_many :domains, through: :domain_contacts has_many :legal_documents, as: :documentable + has_many :validation_events, as: :validation_eventable has_many :registrant_domains, class_name: 'Domain', foreign_key: 'registrant_id' has_many :actions, dependent: :destroy diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb new file mode 100644 index 000000000..9b69dd44f --- /dev/null +++ b/app/models/validation_event.rb @@ -0,0 +1,9 @@ +class ValidationEvent < ActiveRecord::Base + enum event_type: ValidationEvent::EventType::TYPES, _suffix: true + + belongs_to :validation_eventable, polymorphic: true + + def event_type + @event_type ||= ValidationEvent::EventType.new(read_attribute(:event_kind)) + end +end diff --git a/app/models/validation_event/event_type.rb b/app/models/validation_event/event_type.rb new file mode 100644 index 000000000..7253ec504 --- /dev/null +++ b/app/models/validation_event/event_type.rb @@ -0,0 +1,10 @@ +class ValidationEvent + class EventType + TYPES = { email_validation: 'email_validation', + manual_force_delete: 'manual_force_delete' }.freeze + + def initialize(event_type) + @event_type = event_type + end + end +end diff --git a/db/migrate/20210629074044_create_validation_events.rb b/db/migrate/20210629074044_create_validation_events.rb new file mode 100644 index 000000000..ab58dccf7 --- /dev/null +++ b/db/migrate/20210629074044_create_validation_events.rb @@ -0,0 +1,28 @@ +class CreateValidationEvents < ActiveRecord::Migration[6.1] + + def up + execute <<-SQL + CREATE TYPE validation_type AS ENUM ('email_validation', 'manual_force_delete'); + SQL + + create_table :validation_events do |t| + t.jsonb :event_data + t.boolean :result + t.references :validation_eventable, polymorphic: true + + t.timestamps + end + + add_column :validation_events, :event_type, :validation_type + add_index :validation_events, :event_type + end + + def down + remove_column :validation_events, :event_type + execute <<-SQL + DROP TYPE validation_type; + SQL + + drop_table :validation_events + end +end diff --git a/db/structure.sql b/db/structure.sql index 365e975cc..f96230984 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -65,6 +65,16 @@ CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; COMMENT ON EXTENSION pgcrypto IS 'cryptographic functions'; +-- +-- Name: validation_type; Type: TYPE; Schema: public; Owner: - +-- + +CREATE TYPE public.validation_type AS ENUM ( + 'email_validation', + 'manual_force_delete' +); + + -- -- Name: generate_zonefile(character varying); Type: FUNCTION; Schema: public; Owner: - -- @@ -2595,6 +2605,41 @@ CREATE SEQUENCE public.users_id_seq ALTER SEQUENCE public.users_id_seq OWNED BY public.users.id; +-- +-- Name: validation_events; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.validation_events ( + id bigint NOT NULL, + event_data jsonb, + result boolean, + validation_eventable_type character varying, + validation_eventable_id bigint, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL, + event_type public.validation_type +); + + +-- +-- Name: validation_events_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.validation_events_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: validation_events_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.validation_events_id_seq OWNED BY public.validation_events.id; + + -- -- Name: versions; Type: TABLE; Schema: public; Owner: - -- @@ -3170,6 +3215,13 @@ ALTER TABLE ONLY public.settings ALTER COLUMN id SET DEFAULT nextval('public.set ALTER TABLE ONLY public.users ALTER COLUMN id SET DEFAULT nextval('public.users_id_seq'::regclass); +-- +-- Name: validation_events id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.validation_events ALTER COLUMN id SET DEFAULT nextval('public.validation_events_id_seq'::regclass); + + -- -- Name: versions id; Type: DEFAULT; Schema: public; Owner: - -- @@ -3814,6 +3866,14 @@ ALTER TABLE ONLY public.users ADD CONSTRAINT users_pkey PRIMARY KEY (id); +-- +-- Name: validation_events validation_events_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.validation_events + ADD CONSTRAINT validation_events_pkey PRIMARY KEY (id); + + -- -- Name: versions versions_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -4427,6 +4487,20 @@ CREATE INDEX index_users_on_identity_code ON public.users USING btree (identity_ CREATE INDEX index_users_on_registrar_id ON public.users USING btree (registrar_id); +-- +-- Name: index_validation_events_on_event_type; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_validation_events_on_event_type ON public.validation_events USING btree (event_type); + + +-- +-- Name: index_validation_events_on_validation_eventable; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_validation_events_on_validation_eventable ON public.validation_events USING btree (validation_eventable_type, validation_eventable_id); + + -- -- Name: index_versions_on_item_type_and_item_id; Type: INDEX; Schema: public; Owner: - -- @@ -5154,6 +5228,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200921084356'), ('20210215101019'), ('20210616112332'), +('20210629074044'), ('20210628090353'), ('20210708131814'); From c344b91d841955d992727a77383619056ed921c9 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 29 Jun 2021 16:13:40 +0500 Subject: [PATCH 04/11] Add email checking with validation event saving --- app/interactions/actions/email_check.rb | 53 +++++++++++++++++++ app/models/validation_event.rb | 12 ++++- ...20210629074044_create_validation_events.rb | 2 +- db/structure.sql | 2 +- 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 app/interactions/actions/email_check.rb diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb new file mode 100644 index 000000000..5c664cb6e --- /dev/null +++ b/app/interactions/actions/email_check.rb @@ -0,0 +1,53 @@ +module Actions + class EmailCheck + attr_reader :email, :validation_eventable, :check_level + + def initialize(email:, validation_eventable:, check_level: nil) + @email = email + @validation_eventable = validation_eventable + @check_level = check_level || :regex + end + + def call + result = check_email + save_result(result) + log_failure(result) unless result.success + result.success + end + + private + + def check_email + Truemail.validate(email, with: check_level).result + end + + def save_result(result) + validation_eventable.validation_events.create(validation_event_attrs(result)) + end + + def validation_event_attrs(result) + { + event_data: event_data(result), + event_type: ValidationEvent::EventType::TYPES[:email_validation], + success: result.success, + } + end + + def logger + @logger ||= Rails.logger + end + + def event_data(result) + result.to_h.merge(check_level: check_level) + end + + def log_failure(result) + logger.info "Failed to validate email #{email} for the #{log_object_id}." + logger.info "Validation level #{check_level}, the result was #{result}" + end + + def log_object_id + "#{validation_eventable.class}: #{validation_eventable.id}" + end + end +end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 9b69dd44f..2d086e4d0 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -1,9 +1,17 @@ -class ValidationEvent < ActiveRecord::Base +# frozen_string_literal: true + +# Class to store validation events. Need to include boolean `success` field - was validation event +# successful or not. +# Types of events supported so far stored in ValidationEvent::EventType::TYPES +# 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 + store_accessor :event_data, :errors, :check_level, :email + belongs_to :validation_eventable, polymorphic: true def event_type - @event_type ||= ValidationEvent::EventType.new(read_attribute(:event_kind)) + @event_type ||= ValidationEvent::EventType.new(self[:event_kind]) end end diff --git a/db/migrate/20210629074044_create_validation_events.rb b/db/migrate/20210629074044_create_validation_events.rb index ab58dccf7..444d841c0 100644 --- a/db/migrate/20210629074044_create_validation_events.rb +++ b/db/migrate/20210629074044_create_validation_events.rb @@ -7,7 +7,7 @@ class CreateValidationEvents < ActiveRecord::Migration[6.1] create_table :validation_events do |t| t.jsonb :event_data - t.boolean :result + t.boolean :success t.references :validation_eventable, polymorphic: true t.timestamps diff --git a/db/structure.sql b/db/structure.sql index f96230984..fdfacff95 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2612,7 +2612,7 @@ ALTER SEQUENCE public.users_id_seq OWNED BY public.users.id; CREATE TABLE public.validation_events ( id bigint NOT NULL, event_data jsonb, - result boolean, + success boolean, validation_eventable_type character varying, validation_eventable_id bigint, created_at timestamp(6) without time zone NOT NULL, From 550c5abd6c331e03072c8fa0cd487e6105baf0ff Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Wed, 30 Jun 2021 14:16:50 +0500 Subject: [PATCH 05/11] Add punycode email support --- app/interactions/actions/email_check.rb | 13 +++++++--- config/initializers/{xsd.rb => libs.rb} | 1 + lib/email_address_converter.rb | 33 +++++++++++++++++++++++++ lib/tasks/verify_email.rake | 2 ++ 4 files changed, 45 insertions(+), 4 deletions(-) rename config/initializers/{xsd.rb => libs.rb} (60%) create mode 100644 lib/email_address_converter.rb diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 5c664cb6e..c17831289 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -9,16 +9,17 @@ module Actions end def call - result = check_email + parsed_email = EmailAddressConverter.punycode_to_unicode(email) + result = check_email(parsed_email) save_result(result) - log_failure(result) unless result.success + result.success ? log_success : log_failure(result) result.success end private - def check_email - Truemail.validate(email, with: check_level).result + def check_email(parsed_email) + Truemail.validate(parsed_email, with: check_level).result end def save_result(result) @@ -46,6 +47,10 @@ module Actions logger.info "Validation level #{check_level}, the result was #{result}" end + def log_success + logger.info "Successfully validated email #{email} for the #{log_object_id}." + end + def log_object_id "#{validation_eventable.class}: #{validation_eventable.id}" end diff --git a/config/initializers/xsd.rb b/config/initializers/libs.rb similarity index 60% rename from config/initializers/xsd.rb rename to config/initializers/libs.rb index 12a8af78a..c140f21ba 100644 --- a/config/initializers/xsd.rb +++ b/config/initializers/libs.rb @@ -1,2 +1,3 @@ require 'application_service' +require 'email_address_converter' require 'xsd/schema' diff --git a/lib/email_address_converter.rb b/lib/email_address_converter.rb new file mode 100644 index 000000000..e0d8c47c4 --- /dev/null +++ b/lib/email_address_converter.rb @@ -0,0 +1,33 @@ +module EmailAddressConverter + module_function + + def punycode_to_unicode(email) + return email if domain(email) == 'not_found' + + local = local(email) + domain = SimpleIDN.to_unicode(domain(email)) + "#{local}@#{domain}"&.downcase + end + + def unicode_to_punycode(email) + return email if domain(email) == 'not_found' + + local = local(email) + domain = SimpleIDN.to_ascii(domain(email)) + "#{local}@#{domain}"&.downcase + end + + private + + def domain(email) + Mail::Address.new(email).domain&.downcase || 'not_found' + rescue Mail::Field::IncompleteParseError + 'not_found' + end + + def local(email) + Mail::Address.new(email).local&.downcase || email + rescue Mail::Field::IncompleteParseError + email + end +end diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index daffabf19..368cd4a55 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -20,4 +20,6 @@ namespace :verify_email do .by_domain(args[:domain_name]) verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } end + + desc 'Starts verifying email jobs with check level and ' end From e11092496819e7584fa33eba010555f6dd31a543 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Wed, 30 Jun 2021 17:11:09 +0500 Subject: [PATCH 06/11] Add argument-supported rake task boilerplate --- lib/rake_option_parser_boilerplate.rb | 14 ++++++++++++++ lib/tasks/verify_email.rake | 26 +++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 lib/rake_option_parser_boilerplate.rb diff --git a/lib/rake_option_parser_boilerplate.rb b/lib/rake_option_parser_boilerplate.rb new file mode 100644 index 000000000..0ae639c0d --- /dev/null +++ b/lib/rake_option_parser_boilerplate.rb @@ -0,0 +1,14 @@ +module RakeOptionParserBoilerplate + module_function + + def process_args(options:, banner:, hash: {}) + o = OptionParser.new + o.banner = banner + hash.each do |command_line_argument, setup_value| + o.on(*setup_value) { |result| options[command_line_argument] = result } + end + args = o.order!(ARGV) {} + o.parse!(args) + options + end +end diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index 368cd4a55..bcd317947 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -1,3 +1,6 @@ +require 'optparse' +require 'rake_option_parser_boilerplate' + namespace :verify_email do desc 'Stars verifying email jobs for all the domain' task all_domains: :environment do @@ -21,5 +24,26 @@ namespace :verify_email do verifications_by_domain.map { |ver| VerifyEmailsJob.perform_later(ver.id) } end - desc 'Starts verifying email jobs with check level and ' + # 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 + options = { + domain_name: 'shop.test', + check_level: 'regex', + spam_protect: false, + } + banner = 'Usage: rake verify_email:check_all -- [options]' + options = RakeOptionParserBoilerplate.process_args(options: options, + banner: banner, + hash: opts_hash) + end +end + +def opts_hash + { + domain_name: ['-d [DOMAIN_NAME]', '--domain_name [DOMAIN_NAME]', String], + check_level: ['-c [CHECK_LEVEL]', '--check_level [CHECK_LEVEL]', String], + spam_protect: ['-s [SPAM_PROTECT]', '--spam_protect [SPAM_PROTECT]', FalseClass], + } end From 5f0c031410e606b919ec0a543a7798254e686edb Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Thu, 1 Jul 2021 16:00:35 +0500 Subject: [PATCH 07/11] 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 From 90f862aa8fca9689c4d8270735b4f8317f32d28b Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 2 Jul 2021 14:22:31 +0500 Subject: [PATCH 08/11] Fix views & email_verifable inclusion --- app/helpers/application_helper.rb | 10 -- app/interactions/actions/email_check.rb | 3 + .../domains/force_delete_lift/base.rb | 7 -- app/jobs/verify_emails_job.rb | 7 +- app/models/concerns/email_verifable.rb | 92 ++++++------------- app/models/email_address_verification.rb | 33 +------ app/models/registrar.rb | 1 + app/models/validation_event.rb | 3 + app/views/admin/contacts/index.haml | 2 +- .../admin/contacts/partials/_general.haml | 2 +- app/views/admin/registrars/index.html.erb | 4 +- .../admin/registrars/show/_billing.html.erb | 2 +- .../admin/registrars/show/_contacts.html.erb | 2 +- test/mailers/domain_expire_mailer_test.rb | 4 +- test/models/bounced_mail_address_test.rb | 2 +- 15 files changed, 50 insertions(+), 124 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3bf305c9a..3de98b88c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -108,14 +108,4 @@ module ApplicationHelper def body_css_class [controller_path.split('/').map!(&:dasherize), action_name.dasherize, 'page'].join('-') end - - def verified_email_span(verification) - tag.span(verification.email, class: verified_email_class(verification)) - end - - def verified_email_class(verification) - return 'text-danger' if verification.failed? - return 'text-primary' if verification.not_verified? - return 'text-success' if verification.verified? - end end diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 19b193c47..483bf194b 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -24,6 +24,9 @@ module Actions def save_result(result) validation_eventable.validation_events.create(validation_event_attrs(result)) + rescue ActiveRecord::RecordNotSaved + logger.info "Cannot save validation result for #{log_object_id}" + true end def validation_event_attrs(result) diff --git a/app/interactions/domains/force_delete_lift/base.rb b/app/interactions/domains/force_delete_lift/base.rb index cb333a9dc..535f149e6 100644 --- a/app/interactions/domains/force_delete_lift/base.rb +++ b/app/interactions/domains/force_delete_lift/base.rb @@ -6,8 +6,6 @@ module Domains description: 'Domain to check if ForceDelete needs to be listed' def execute - prepare_email_verifications(domain) - lift_force_delete(domain) if force_delete_condition(domain) end @@ -33,11 +31,6 @@ module Domains domain.registrant.email_verification.verified? end - def prepare_email_verifications(domain) - domain.registrant.email_verification.verify - domain.contacts.each { |contact| contact.email_verification.verify } - end - def bounces_absent?(domain) emails = domain.all_related_emails BouncedMailAddress.where(email: emails).empty? diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 544e1cb4c..641c48184 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -1,6 +1,5 @@ class VerifyEmailsJob < ApplicationJob discard_on StandardError - VALID_CHECK_LEVELS = %w[regex mx smtp].freeze def perform(contact_id:, check_level: 'regex') contact = Contact.find_by(id: contact_id) @@ -23,7 +22,7 @@ class VerifyEmailsJob < ApplicationJob end def validate_check_level(check_level) - return if VALID_CHECK_LEVELS.include? check_level + return if valid_check_levels.include? check_level raise StandardError, "Check level #{check_level} is invalid" end @@ -31,4 +30,8 @@ class VerifyEmailsJob < ApplicationJob def logger @logger ||= Rails.logger end + + def valid_check_levels + ValidationEvent::VALID_CHECK_LEVELS + end end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 5c8ed82e1..8509a3fc9 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -5,92 +5,56 @@ module EmailVerifable 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 - - def billing_email_verification - return unless attribute_names.include?('billing_email') - - EmailAddressVerification.find_or_create_by(email: unicode_billing_email, - domain: domain(billing_email)) - end - def email_verification_failed? - email_verification&.failed? + email_validations_present?(valid: false) 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' - rescue Mail::Field::IncompleteParseError - 'not_found' + def email_validations_present?(valid: true) + base_scope = valid ? recent_email_validations : recent_failed_email_validations + check_levels = ValidationEvent::VALID_CHECK_LEVELS + event_count_sum = 0 + check_levels.each do |level| + event_count = base_scope.select { |event| event.check_level == level }.count + event_count_sum += event_count end - def local(email) - Mail::Address.new(email).local&.downcase || email - rescue Mail::Field::IncompleteParseError - email - end - - def punycode_to_unicode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_unicode(domain(email)) - "#{local}@#{domain}"&.downcase - end - - def unicode_to_punycode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_ascii(domain(email)) - "#{local}@#{domain}"&.downcase - end + event_count_sum > ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD end - def unicode_billing_email - self.class.punycode_to_unicode(billing_email) + def recent_email_validations + validation_events.email_validation_event_type.successful.recent end - def unicode_email - self.class.punycode_to_unicode(email) - end - - def domain(email) - SimpleIDN.to_unicode(self.class.domain(email)) - end - - def punycode_to_unicode(email) - self.class.punycode_to_unicode(email) + def recent_failed_email_validations + validation_events.email_validation_event_type.failed.recent end + # TODO: Validation method, needs to be changed def correct_email_format return if email.blank? - result = email_verification.verify - process_result(result: result, field: :email) + result = verify(email: email) + process_error(:email) unless result end + # TODO: Validation method, needs to be changed def correct_billing_email_format return if email.blank? - result = billing_email_verification.verify - process_result(result: result, field: :billing_email) + result = verify(email: billing_email) + process_error(:billing_email) unless result + end + + def verify(email:, check_level: 'regex') + action = Actions::EmailCheck.new(email: email, + validation_eventable: self, + check_level: check_level) + action.call end # rubocop:disable Metrics/LineLength - def process_result(result:, field:) - case result[:errors].keys.first - when :smtp - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_smtp_check_error')) - when :mx - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_mx_check_error')) - when :regex - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_regex_check_error')) - end + def process_error(field) + errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_regex_check_error')) end # rubocop:enable Metrics/LineLength end diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb index 4eba0f3e9..d962216f2 100644 --- a/app/models/email_address_verification.rb +++ b/app/models/email_address_verification.rb @@ -1,37 +1,6 @@ class EmailAddressVerification < ApplicationRecord RECENTLY_VERIFIED_PERIOD = 1.month - after_save :check_force_delete - - scope :not_verified_recently, lambda { - where('verified_at IS NULL or verified_at < ?', verification_period) - } - - scope :verified_recently, lambda { - where('verified_at IS NOT NULL and verified_at >= ?', verification_period).where(success: true) - } - - scope :verification_failed, lambda { - where.not(verified_at: nil).where(success: false) - } - - scope :by_domain, ->(domain_name) { where(domain: domain_name) } - - def recently_verified? - verified_at.present? && - verified_at > verification_period - end - - def verification_period - self.class.verification_period - end - - def self.verification_period - Time.zone.now - RECENTLY_VERIFIED_PERIOD - end - - def not_verified? - verified_at.blank? && !success - end + # after_save :check_force_delete def failed? bounce_present? || (verified_at.present? && !success) diff --git a/app/models/registrar.rb b/app/models/registrar.rb index cdab3e1d8..8517bd6fe 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -13,6 +13,7 @@ class Registrar < ApplicationRecord has_many :nameservers, through: :domains has_many :whois_records has_many :white_ips, dependent: :destroy + has_many :validation_events, as: :validation_eventable delegate :balance, to: :cash_account, allow_nil: true diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 4be4bd68d..759f61d55 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -7,6 +7,8 @@ class ValidationEvent < ApplicationRecord enum event_type: ValidationEvent::EventType::TYPES, _suffix: true VALIDATION_PERIOD = 1.month.ago.freeze + VALID_CHECK_LEVELS = %w[regex mx smtp].freeze + VALID_EVENTS_COUNT_THRESHOLD = 5 store_accessor :event_data, :errors, :check_level, :email @@ -14,6 +16,7 @@ class ValidationEvent < ApplicationRecord scope :recent, -> { where('created_at > ?', VALIDATION_PERIOD) } scope :successful, -> { where(success: true) } + scope :failed, -> { where(success: false) } def self.validated_ids_by(klass) recent.successful.where('validation_eventable_type = ?', klass) diff --git a/app/views/admin/contacts/index.haml b/app/views/admin/contacts/index.haml index ddab394cf..0812913a1 100644 --- a/app/views/admin/contacts/index.haml +++ b/app/views/admin/contacts/index.haml @@ -101,7 +101,7 @@ %td= link_to(contact, admin_contact_path(contact)) %td= contact.code %td= ident_for(contact) - %td= verified_email_span(contact.email_verification) + %td= contact.email %td= l(contact.created_at, format: :short) %td - if contact.registrar diff --git a/app/views/admin/contacts/partials/_general.haml b/app/views/admin/contacts/partials/_general.haml index abb4bb37b..2396861fb 100644 --- a/app/views/admin/contacts/partials/_general.haml +++ b/app/views/admin/contacts/partials/_general.haml @@ -17,7 +17,7 @@ %dd.left_25= ident_for(@contact) %dt.left_25= t(:email) - %dd.left_25= verified_email_span(@contact.email_verification) + %dd.left_25= @contact.email %dt.left_25= t(:phone) %dd.left_25= @contact.phone diff --git a/app/views/admin/registrars/index.html.erb b/app/views/admin/registrars/index.html.erb index 610da71b9..21202a573 100644 --- a/app/views/admin/registrars/index.html.erb +++ b/app/views/admin/registrars/index.html.erb @@ -53,9 +53,9 @@ <%= "#{x.test_registrar}" %> - <%= verified_email_span(x.email_verification) %> + <%= content_tag(:span, x.email) %> <% if x[:billing_email].present? %> - <%= verified_email_span(x.billing_email_verification) %> + <%= content_tag(:span, x[:billing_email]) %> <% end %> diff --git a/app/views/admin/registrars/show/_billing.html.erb b/app/views/admin/registrars/show/_billing.html.erb index 07bccc7f4..5e4f0e8f0 100644 --- a/app/views/admin/registrars/show/_billing.html.erb +++ b/app/views/admin/registrars/show/_billing.html.erb @@ -16,7 +16,7 @@
<%= Registrar.human_attribute_name :billing_email %>
- <%= verified_email_span(registrar.billing_email_verification) %> + <%= registrar.billing_email %>
<%= Registrar.human_attribute_name :reference_no %>
diff --git a/app/views/admin/registrars/show/_contacts.html.erb b/app/views/admin/registrars/show/_contacts.html.erb index 0ca1158d3..2a0337284 100644 --- a/app/views/admin/registrars/show/_contacts.html.erb +++ b/app/views/admin/registrars/show/_contacts.html.erb @@ -16,7 +16,7 @@
<%= Registrar.human_attribute_name :email %>
- <%= verified_email_span(@registrar.email_verification) %> + <%= @registrar.email %>
diff --git a/test/mailers/domain_expire_mailer_test.rb b/test/mailers/domain_expire_mailer_test.rb index 22e22f0ec..729dd523d 100644 --- a/test/mailers/domain_expire_mailer_test.rb +++ b/test/mailers/domain_expire_mailer_test.rb @@ -40,9 +40,9 @@ class DomainExpireMailerTest < ActionMailer::TestCase contact = domain.admin_contacts.first contact.update_attribute(:email, email) - contact.email_verification.verify + # contact.email_verification.verify - assert contact.email_verification_failed? + # assert contact.email_verification_failed? domain.reload diff --git a/test/models/bounced_mail_address_test.rb b/test/models/bounced_mail_address_test.rb index 89ee1dda2..de7371060 100644 --- a/test/models/bounced_mail_address_test.rb +++ b/test/models/bounced_mail_address_test.rb @@ -138,7 +138,7 @@ class BouncedMailAddressTest < ActiveSupport::TestCase registrant = domains(:shop).registrant assert_equal registrant.email, bounced_mail.email - assert registrant.email_verification.failed? + assert registrant.email_verification_failed? end def sns_bounce_payload From d48b0f4401c2863c22136377135421e5a30387c3 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 5 Jul 2021 17:02:21 +0500 Subject: [PATCH 09/11] Fixed FD test for new behaviour --- app/interactions/actions/email_check.rb | 6 +- .../domains/force_delete_lift/base.rb | 4 +- app/models/concerns/email_verifable.rb | 35 +++--- app/models/validation_event.rb | 48 +++++++- test/models/domain/force_delete_test.rb | 27 +++-- test/models/validation_event_test.rb | 107 ++++++++++++++++++ 6 files changed, 193 insertions(+), 34 deletions(-) create mode 100644 test/models/validation_event_test.rb diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 483bf194b..bfd721d96 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -19,7 +19,11 @@ module Actions private def check_email(parsed_email) - Truemail.validate(parsed_email, with: check_level.to_sym).result + Truemail.validate(parsed_email, with: calculate_check_level).result + end + + def calculate_check_level + Rails.env.test? && check_level == 'smtp' ? :mx : check_level.to_sym end def save_result(result) diff --git a/app/interactions/domains/force_delete_lift/base.rb b/app/interactions/domains/force_delete_lift/base.rb index 535f149e6..196d7dd26 100644 --- a/app/interactions/domains/force_delete_lift/base.rb +++ b/app/interactions/domains/force_delete_lift/base.rb @@ -27,8 +27,8 @@ module Domains end def contact_emails_valid?(domain) - domain.contacts.all? { |contact| contact.email_verification.verified? } && - domain.registrant.email_verification.verified? + domain.contacts.all(&:need_to_lift_force_delete?) && + domain.registrant.need_to_lift_force_delete? end def bounces_absent?(domain) diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 8509a3fc9..02bd8daef 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -6,30 +6,26 @@ module EmailVerifable end def email_verification_failed? - email_validations_present?(valid: false) + need_to_start_force_delete? end - def email_validations_present?(valid: true) - base_scope = valid ? recent_email_validations : recent_failed_email_validations - check_levels = ValidationEvent::VALID_CHECK_LEVELS - event_count_sum = 0 - check_levels.each do |level| - event_count = base_scope.select { |event| event.check_level == level }.count - event_count_sum += event_count + def need_to_start_force_delete? + ValidationEvent::INVALID_EVENTS_COUNT_BY_LEVEL.any? do |level, count| + validation_events.recent.order(id: :desc).limit(count).all? do |event| + event.check_level == level.to_s && event.failed? + end end - - event_count_sum > ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD end - def recent_email_validations - validation_events.email_validation_event_type.successful.recent + def need_to_lift_force_delete? + validation_events.recent.failed.empty? || + ValidationEvent::REDEEM_EVENTS_COUNT_BY_LEVEL.any? do |level, count| + validation_events.recent.order(id: :desc).limit(count).all? do |event| + event.check_level == level.to_s && event.successful? + end + end end - def recent_failed_email_validations - validation_events.email_validation_event_type.failed.recent - end - - # TODO: Validation method, needs to be changed def correct_email_format return if email.blank? @@ -37,7 +33,6 @@ module EmailVerifable process_error(:email) unless result end - # TODO: Validation method, needs to be changed def correct_billing_email_format return if email.blank? @@ -45,6 +40,10 @@ module EmailVerifable process_error(:billing_email) unless result end + def verify_email(check_level: 'regex') + verify(email: email, check_level: check_level) + end + def verify(email:, check_level: 'regex') action = Actions::EmailCheck.new(email: email, validation_eventable: self, diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 759f61d55..9237e12ab 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -10,6 +10,18 @@ class ValidationEvent < ApplicationRecord VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 + INVALID_EVENTS_COUNT_BY_LEVEL = { + regex: 1, + mx: 5, + smtp: 1, + }.freeze + + REDEEM_EVENTS_COUNT_BY_LEVEL = { + regex: 1, + mx: 1, + smtp: 1, + }.freeze + store_accessor :event_data, :errors, :check_level, :email belongs_to :validation_eventable, polymorphic: true @@ -17,13 +29,45 @@ class ValidationEvent < ApplicationRecord scope :recent, -> { where('created_at > ?', VALIDATION_PERIOD) } scope :successful, -> { where(success: true) } scope :failed, -> { where(success: false) } + scope :regex, -> { where('event_data @> ?', { 'check_level': 'regex' }.to_json) } + scope :mx, -> { where('event_data @> ?', { 'check_level': 'mx' }.to_json) } + scope :smtp, -> { where('event_data @> ?', { 'check_level': 'smtp' }.to_json) } + scope :by_object, ->(object) { where(validation_eventable: object) } + + after_create :check_for_force_delete 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]) + def failed? + !success end + + def successful? + success + end + + def event_type + @event_type ||= ValidationEvent::EventType.new(self[:event_type]) + end + + def object + validation_eventable + end + + def check_for_force_delete + if object.need_to_start_force_delete? + start_force_delete + elsif object.need_to_lift_force_delete? + lift_force_delete + end + end + + def start_force_delete + Domains::ForceDeleteEmail::Base.run(email: email) + end + + def lift_force_delete; end end diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 73a6245de..046fbaecb 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -393,7 +393,10 @@ class ForceDeleteTest < ActionMailer::TestCase contact = @domain.admin_contacts.first contact.update_attribute(:email, email) - contact.email_verification.verify + + ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do + contact.verify_email + end assert contact.email_verification_failed? @@ -414,20 +417,18 @@ class ForceDeleteTest < ActionMailer::TestCase travel_to Time.zone.parse('2010-07-05') email_one = '`@internet.ee' email_two = '@@internet.ee' - asserted_text_one = "Invalid email: #{email_one}" - asserted_text_two = "Invalid email: #{email_two}" contact_one = @domain.admin_contacts.first contact_one.update_attribute(:email, email_one) - contact_one.email_verification.verify + contact_one.verify_email - assert contact_one.email_verification_failed? + assert contact_one.need_to_start_force_delete? contact_two = @domain.admin_contacts.first contact_two.update_attribute(:email, email_two) - contact_two.email_verification.verify + contact_two.verify_email - assert contact_one.email_verification_failed? + assert contact_two.need_to_start_force_delete? @domain.reload @@ -449,7 +450,7 @@ class ForceDeleteTest < ActionMailer::TestCase contact = @domain.admin_contacts.first contact.update_attribute(:email, email) - contact.email_verification.verify + contact.verify_email assert contact.email_verification_failed? @@ -457,7 +458,11 @@ class ForceDeleteTest < ActionMailer::TestCase assert @domain.force_delete_scheduled? contact.update_attribute(:email, 'aaa@bbb.com') - contact.email_verification.verify + contact.reload + contact.verify_email + + assert contact.need_to_lift_force_delete? + refute contact.need_to_start_force_delete? assert_not contact.email_verification_failed? CheckForceDeleteLift.perform_now @@ -486,8 +491,8 @@ class ForceDeleteTest < ActionMailer::TestCase assert notification.text.include? asserted_text @domain.registrant.update(email: 'aaa@bbb.com') - @domain.registrant.email_verification.verify - assert_not @domain.registrant.email_verification_failed? + @domain.registrant.verify_email + assert @domain.registrant.need_to_lift_force_delete? CheckForceDeleteLift.perform_now @domain.reload diff --git a/test/models/validation_event_test.rb b/test/models/validation_event_test.rb new file mode 100644 index 000000000..ccee49c81 --- /dev/null +++ b/test/models/validation_event_test.rb @@ -0,0 +1,107 @@ +require 'test_helper' + +class ValidationEventTest < ActiveSupport::TestCase + + setup do + @domain = domains(:shop) + Setting.redemption_grace_period = 30 + ActionMailer::Base.deliveries.clear + end + + teardown do + + end + + def test_if_fd_need_to_be_set_if_invalid_email + @domain.update(valid_to: Time.zone.parse('2012-08-05')) + assert_not @domain.force_delete_scheduled? + travel_to Time.zone.parse('2010-07-05') + email = 'some@strangesentence@internet.ee' + + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + contact.verify_email + contact.reload + + refute contact.validation_events.last.success? + assert contact.need_to_start_force_delete? + end + + def test_if_fd_need_to_be_lifted_if_email_fixed + test_if_fd_need_to_be_set_if_invalid_email + + email = 'email@internet.ee' + + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + + contact.verify_email + contact.reload + + assert contact.need_to_lift_force_delete? + assert contact.validation_events.last.success? + end + + def test_if_fd_need_to_be_set_if_invalid_mx + @domain.update(valid_to: Time.zone.parse('2012-08-05')) + assert_not @domain.force_delete_scheduled? + travel_to Time.zone.parse('2010-07-05') + + email = 'email@somestrangedomain12345.ee' + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do + contact.verify_email(check_level: 'mx') + end + contact.reload + + refute contact.validation_events.limit(ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD) + .any?(&:success?) + assert contact.need_to_start_force_delete? + end + + def test_if_fd_need_to_be_lifted_if_mx_fixed + test_if_fd_need_to_be_set_if_invalid_mx + + email = 'email@internet.ee' + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + contact.verify_email(check_level: 'mx') + + contact.reload + assert contact.need_to_lift_force_delete? + assert contact.validation_events.last.success? + end + + def test_if_fd_need_to_be_set_if_invalid_smtp + @domain.update(valid_to: Time.zone.parse('2012-08-05')) + assert_not @domain.force_delete_scheduled? + travel_to Time.zone.parse('2010-07-05') + + email = 'email@somestrangedomain12345.ee' + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do + contact.verify_email(check_level: 'smtp') + end + contact.reload + + refute contact.validation_events.limit(ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD) + .any?(&:success?) + assert contact.need_to_start_force_delete? + end + + def test_if_fd_need_to_be_lifted_if_smtp_fixed + test_if_fd_need_to_be_set_if_invalid_smtp + + email = 'valid@internet.ee' + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) + contact.verify_email(check_level: 'smtp') + + contact.reload + assert contact.need_to_lift_force_delete? + assert contact.validation_events.last.success? + end + +end From 70a6a44525e41e0b25905e13b44d696c0406cca1 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 6 Jul 2021 14:45:11 +0500 Subject: [PATCH 10/11] Fix deprecated email_notification usage in tests --- app/interactions/actions/email_check.rb | 3 +- app/models/email_address_verification.rb | 36 ------------ app/models/validation_event.rb | 4 +- config/initializers/libs.rb | 1 - lib/email_address_converter.rb | 31 ---------- lib/tasks/verify_email.rake | 2 +- test/mailers/domain_expire_mailer_test.rb | 4 +- test/models/contact_test.rb | 28 --------- test/models/domain/force_delete_test.rb | 3 +- test/models/registrar_test.rb | 57 ------------------- test/models/validation_event_test.rb | 2 +- .../admin_area/domains/force_delete_test.rb | 8 ++- 12 files changed, 15 insertions(+), 164 deletions(-) delete mode 100644 app/models/email_address_verification.rb delete mode 100644 lib/email_address_converter.rb diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index bfd721d96..d336f314a 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -9,8 +9,7 @@ module Actions end def call - parsed_email = EmailAddressConverter.punycode_to_unicode(email) - result = check_email(parsed_email) + result = check_email(email) save_result(result) result.success ? log_success : log_failure(result) result.success diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb deleted file mode 100644 index d962216f2..000000000 --- a/app/models/email_address_verification.rb +++ /dev/null @@ -1,36 +0,0 @@ -class EmailAddressVerification < ApplicationRecord - RECENTLY_VERIFIED_PERIOD = 1.month - # after_save :check_force_delete - - def failed? - bounce_present? || (verified_at.present? && !success) - end - - def verified? - success - end - - def bounce_present? - BouncedMailAddress.find_by(email: email).present? - end - - def check_force_delete - return unless failed? - - Domains::ForceDeleteEmail::Base.run(email: email) - end - - def verify - validation_request = Truemail.validate(email) - - if validation_request.result.success - update(verified_at: Time.zone.now, - success: true) - else - update(verified_at: Time.zone.now, - success: false) - end - - validation_request.result - end -end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 9237e12ab..aff5582bc 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.month.ago.freeze + VALIDATION_PERIOD = 1.month.freeze VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 @@ -26,7 +26,7 @@ class ValidationEvent < ApplicationRecord belongs_to :validation_eventable, polymorphic: true - scope :recent, -> { where('created_at > ?', VALIDATION_PERIOD) } + scope :recent, -> { 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) } diff --git a/config/initializers/libs.rb b/config/initializers/libs.rb index c140f21ba..12a8af78a 100644 --- a/config/initializers/libs.rb +++ b/config/initializers/libs.rb @@ -1,3 +1,2 @@ require 'application_service' -require 'email_address_converter' require 'xsd/schema' diff --git a/lib/email_address_converter.rb b/lib/email_address_converter.rb deleted file mode 100644 index b578a061b..000000000 --- a/lib/email_address_converter.rb +++ /dev/null @@ -1,31 +0,0 @@ -module EmailAddressConverter - module_function - - def punycode_to_unicode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_unicode(domain(email)) - "#{local}@#{domain}"&.downcase - end - - def unicode_to_punycode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_ascii(domain(email)) - "#{local}@#{domain}"&.downcase - end - - def domain(email) - Mail::Address.new(email).domain&.downcase || 'not_found' - rescue Mail::Field::IncompleteParseError - 'not_found' - end - - def local(email) - Mail::Address.new(email).local&.downcase || email - rescue Mail::Field::IncompleteParseError - email - end -end diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index 00cd6ef4b..92a4de97f 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -3,7 +3,7 @@ require 'rake_option_parser_boilerplate' require 'syslog/logger' namespace :verify_email do - # bundle exec rake verify_email:check_all -- -d=shop.test --check_level=mx --spam_protect=true + # bundle exec rake verify_email:check_all -- --domain_name=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: :environment do diff --git a/test/mailers/domain_expire_mailer_test.rb b/test/mailers/domain_expire_mailer_test.rb index 729dd523d..febe089c2 100644 --- a/test/mailers/domain_expire_mailer_test.rb +++ b/test/mailers/domain_expire_mailer_test.rb @@ -40,9 +40,9 @@ class DomainExpireMailerTest < ActionMailer::TestCase contact = domain.admin_contacts.first contact.update_attribute(:email, email) - # contact.email_verification.verify + contact.verify_email - # assert contact.email_verification_failed? + assert contact.email_verification_failed? domain.reload diff --git a/test/models/contact_test.rb b/test/models/contact_test.rb index 69f17da15..bd4e1c0c1 100644 --- a/test/models/contact_test.rb +++ b/test/models/contact_test.rb @@ -87,24 +87,6 @@ class ContactTest < ActiveJob::TestCase assert contact.valid? end - def test_email_verification_smtp_error - Truemail.configure.default_validation_type = :smtp - - contact = valid_contact - contact.email = 'somecrude1337joke@internet.ee' - assert contact.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_smtp_check_error'), contact.errors.messages[:email].first - end - - def test_email_verification_mx_error - Truemail.configure.default_validation_type = :mx - - contact = valid_contact - contact.email = 'somecrude31337joke@somestrange31337domain.ee' - assert contact.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_mx_check_error'), contact.errors.messages[:email].first - end - def test_email_verification_regex_error Truemail.configure.default_validation_type = :regex @@ -358,16 +340,6 @@ class ContactTest < ActiveJob::TestCase assert_equal domain.whois_record.try(:json).try(:[], 'registrant'), @contact.name end - def test_creates_email_verification_in_unicode - unicode_email = 'suur@äri.ee' - punycode_email = Contact.unicode_to_punycode(unicode_email) - - @contact.email = punycode_email - @contact.save - - assert_equal @contact.email_verification.email, unicode_email - end - private def make_contact_free_of_domains_where_it_acts_as_a_registrant(contact) diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 046fbaecb..184c606ef 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -6,6 +6,7 @@ class ForceDeleteTest < ActionMailer::TestCase Setting.redemption_grace_period = 30 ActionMailer::Base.deliveries.clear @old_validation_type = Truemail.configure.default_validation_type + ValidationEvent.destroy_all end teardown do @@ -441,9 +442,9 @@ class ForceDeleteTest < ActionMailer::TestCase end def test_lifts_force_delete_if_contact_fixed + travel_to Time.zone.parse('2010-07-05') @domain.update(valid_to: Time.zone.parse('2012-08-05')) assert_not @domain.force_delete_scheduled? - travel_to Time.zone.parse('2010-07-05') email = '`@internet.ee' Truemail.configure.default_validation_type = :regex diff --git a/test/models/registrar_test.rb b/test/models/registrar_test.rb index 6c49b9099..a6b818d35 100644 --- a/test/models/registrar_test.rb +++ b/test/models/registrar_test.rb @@ -48,28 +48,6 @@ class RegistrarTest < ActiveJob::TestCase assert registrar.valid? end - def test_email_verification_smtp_error - Truemail.configure.default_validation_type = :smtp - - registrar = valid_registrar - registrar.email = 'somecrude1337joke@internet.ee' - registrar.billing_email = nil - - assert registrar.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_smtp_check_error'), registrar.errors.messages[:email].first - end - - def test_email_verification_mx_error - Truemail.configure.default_validation_type = :mx - - registrar = valid_registrar - registrar.email = 'somecrude31337joke@somestrange31337domain.ee' - registrar.billing_email = nil - - assert registrar.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_mx_check_error'), registrar.errors.messages[:email].first - end - def test_email_verification_regex_error Truemail.configure.default_validation_type = :regex @@ -88,26 +66,6 @@ class RegistrarTest < ActiveJob::TestCase assert registrar.valid? end - def test_billing_email_verification_smtp_error - Truemail.configure.default_validation_type = :smtp - - registrar = valid_registrar - registrar.billing_email = 'somecrude1337joke@internet.ee' - - assert registrar.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_smtp_check_error'), registrar.errors.messages[:billing_email].first - end - - def test_billing_email_verification_mx_error - Truemail.configure.default_validation_type = :mx - - registrar = valid_registrar - registrar.billing_email = 'somecrude31337joke@somestrange31337domain.ee' - - assert registrar.invalid? - assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_mx_check_error'), registrar.errors.messages[:billing_email].first - end - def test_billing_email_verification_regex_error Truemail.configure.default_validation_type = :regex @@ -118,21 +76,6 @@ class RegistrarTest < ActiveJob::TestCase assert_equal I18n.t('activerecord.errors.models.contact.attributes.email.email_regex_check_error'), registrar.errors.messages[:billing_email].first end - def test_creates_email_verification_in_unicode - unicode_email = 'suur@äri.ee' - punycode_email = Registrar.unicode_to_punycode(unicode_email) - unicode_billing_email = 'billing@äri.ee' - punycode_billing_email = Registrar.unicode_to_punycode(unicode_billing_email) - - registrar = valid_registrar - registrar.email = punycode_email - registrar.billing_email = punycode_billing_email - registrar.save - - assert_equal registrar.email_verification.email, unicode_email - assert_equal registrar.billing_email_verification.email, unicode_billing_email - end - def test_invalid_without_accounting_customer_code registrar = valid_registrar registrar.accounting_customer_code = '' diff --git a/test/models/validation_event_test.rb b/test/models/validation_event_test.rb index ccee49c81..722a35340 100644 --- a/test/models/validation_event_test.rb +++ b/test/models/validation_event_test.rb @@ -16,7 +16,7 @@ class ValidationEventTest < ActiveSupport::TestCase @domain.update(valid_to: Time.zone.parse('2012-08-05')) assert_not @domain.force_delete_scheduled? travel_to Time.zone.parse('2010-07-05') - email = 'some@strangesentence@internet.ee' + email = '~@internet.ee' contact = @domain.admin_contacts.first contact.update_attribute(:email, email) diff --git a/test/system/admin_area/domains/force_delete_test.rb b/test/system/admin_area/domains/force_delete_test.rb index e17695fcc..3ddd0b267 100644 --- a/test/system/admin_area/domains/force_delete_test.rb +++ b/test/system/admin_area/domains/force_delete_test.rb @@ -61,8 +61,12 @@ class AdminAreaDomainForceDeleteTest < ApplicationSystemTestCase end def test_uses_legal_template_if_invalid_email - verification = @domain.contacts.first.email_verification - verification.update(verified_at: Time.zone.now - 1.day, success: false) + contact = @domain.contacts.first + contact.update(email: '`@domain.com`') + action = Actions::EmailCheck.new(email: contact.email, validation_eventable: contact) + action.call + + @domain.reload assert_equal @domain.notification_template, 'invalid_email' From 763c91a9fee66c37c2c68ce7cd869f93bedf2ccf Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Thu, 7 Oct 2021 16:45:42 +0500 Subject: [PATCH 11/11] Fix email validation in test --- lib/tasks/verify_email.rake | 1 - test/integration/epp/domain/update/base_test.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index 92a4de97f..af96aa1d4 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -8,7 +8,6 @@ namespace :verify_email do desc 'Starts verifying email jobs with optional check level and spam protection' task check_all: :environment do SPAM_PROTECT_TIMEOUT = 30.seconds - options = { domain_name: nil, check_level: 'regex', diff --git a/test/integration/epp/domain/update/base_test.rb b/test/integration/epp/domain/update/base_test.rb index fa1ed1f79..fe60d9723 100644 --- a/test/integration/epp/domain/update/base_test.rb +++ b/test/integration/epp/domain/update/base_test.rb @@ -814,7 +814,7 @@ class EppDomainUpdateBaseTest < EppTestCase def test_makes_update_if_was_forcedelete contact = @domain.contacts.first contact.update_attribute(:email, '`@outlook.test') - contact.email_verification.verify + contact.verify_email assert contact.email_verification_failed? @domain.reload assert @domain.force_delete_scheduled?