diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index 47ba769..cbf408c 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -3,6 +3,7 @@ require "valid_email2" require "resolv" require "mail" +require "valid_email2/dns_records_cache" module ValidEmail2 class Address @@ -11,7 +12,6 @@ class Address PROHIBITED_DOMAIN_CHARACTERS_REGEX = /[+!_\/\s'`]/ DEFAULT_RECIPIENT_DELIMITER = '+' DOT_DELIMITER = '.' - MAX_CACHE_SIZE = 1_000 # Cache structure: { domain (String): { records: [], cached_at: Time, ttl: Integer } } @@mx_servers_cache = {} @@ -142,32 +142,14 @@ def address_contain_emoticons?(email) end def mx_servers - if @@mx_servers_cache.size > MAX_CACHE_SIZE - prune_cache(@@mx_servers_cache) - end - - domain = address.domain.downcase + @mx_servers_cache ||= ValidEmail2::DnsRecordsCache.new - if @@mx_servers_cache[domain] - cache_entry = @@mx_servers_cache[domain] - if (Time.now - cache_entry[:cached_at]) < cache_entry[:ttl] - return cache_entry[:records] - else - @@mx_servers_cache.delete(domain) + @mx_servers_cache.fetch(address.domain.downcase) do + Resolv::DNS.open(@resolv_config) do |dns| + dns.timeouts = @dns_timeout + dns.getresources(address.domain, Resolv::DNS::Resource::IN::MX) end end - - records = Resolv::DNS.open(@resolv_config) do |dns| - dns.timeouts = @dns_timeout - dns.getresources(address.domain, Resolv::DNS::Resource::IN::MX) - end - - if records.any? - ttl = records.map(&:ttl).min - @@mx_servers_cache[domain] = { records: records, cached_at: Time.now, ttl: ttl } - end - - records end def null_mx? @@ -175,39 +157,15 @@ def null_mx? end def mx_or_a_servers - if @@mx_or_a_servers_cache.size > MAX_CACHE_SIZE - prune_cache(@@mx_or_a_servers_cache) - end + @mx_or_a_servers_cache ||= ValidEmail2::DnsRecordsCache.new - domain = address.domain.downcase - - if @@mx_or_a_servers_cache[domain] - cache_entry = @@mx_or_a_servers_cache[domain] - if (Time.now - cache_entry[:cached_at]) < cache_entry[:ttl] - return cache_entry[:records] - else - @@mx_or_a_servers_cache.delete(domain) + @mx_or_a_servers_cache.fetch(address.domain.downcase) do + Resolv::DNS.open(@resolv_config) do |dns| + dns.timeouts = @dns_timeout + (mx_servers.any? && mx_servers) || + dns.getresources(address.domain, Resolv::DNS::Resource::IN::A) end end - - records = Resolv::DNS.open(@resolv_config) do |dns| - dns.timeouts = @dns_timeout - (mx_servers.any? && mx_servers) || - dns.getresources(address.domain, Resolv::DNS::Resource::IN::A) - end - - if records.any? - ttl = records.map(&:ttl).min - @@mx_or_a_servers_cache[domain] = { records: records, cached_at: Time.now, ttl: ttl } - end - - records - end - - def prune_cache(cache) - entries_sorted_by_cached_at_asc = (cache.sort_by { |_domain, data| data[:cached_at] }).flatten - entries_to_remove = entries_sorted_by_cached_at_asc.first(cache.size - MAX_CACHE_SIZE) - entries_to_remove.each { |domain| cache.delete(domain) } end end end diff --git a/lib/valid_email2/dns_records_cache.rb b/lib/valid_email2/dns_records_cache.rb new file mode 100644 index 0000000..78ecbad --- /dev/null +++ b/lib/valid_email2/dns_records_cache.rb @@ -0,0 +1,37 @@ +module ValidEmail2 + class DnsRecordsCache + MAX_CACHE_SIZE = 1_000 + + def initialize + # Cache structure: { domain (String): { records: [], cached_at: Time, ttl: Integer } } + @cache = {} + end + + def fetch(domain, &block) + prune(@cache) if @cache.size > MAX_CACHE_SIZE + + cache_entry = @cache[domain] + + if cache_entry && (Time.now - cache_entry[:cached_at]) < cache_entry[:ttl] + return cache_entry[:records] + else + @cache.delete(domain) + end + + records = block.call + + if records.any? + ttl = records.map(&:ttl).min + @cache[domain] = { records: records, cached_at: Time.now, ttl: ttl } + end + + records + end + + def prune(cache) + entries_sorted_by_cached_at_asc = (cache.sort_by { |_domain, data| data[:cached_at] }).flatten + entries_to_remove = entries_sorted_by_cached_at_asc.first(cache.size - MAX_CACHE_SIZE) + entries_to_remove.each { |domain| cache.delete(domain) } + end + end +end \ No newline at end of file diff --git a/spec/address_spec.rb b/spec/address_spec.rb index dbf0aac..bd62572 100644 --- a/spec/address_spec.rb +++ b/spec/address_spec.rb @@ -38,9 +38,10 @@ describe "caching" do let(:email_address) { "example@ymail.com" } let(:email_instance) { described_class.new(email_address) } + let(:dns_cache_instance) { ValidEmail2::DnsRecordsCache.new } let(:ttl) { 1_000 } let(:mock_resolv_dns) { instance_double(Resolv::DNS) } - let(:mock_mx_records) { [double('MX', exchange: 'mx.ymail.com', preference: 10, ttl: ttl)] } + let(:mock_mx_records) { [double("MX", exchange: "mx.ymail.com", preference: 10, ttl: ttl)] } before do allow(email_instance).to receive(:null_mx?).and_return(false) @@ -49,8 +50,10 @@ end describe "#valid_strict_mx?" do + let(:cached_at) { Time.now } + let(:mock_cache_data) { { email_instance.address.domain => { records: mock_mx_records, cached_at: cached_at, ttl: ttl } } } + before do - described_class.class_variable_set(:@@mx_servers_cache, {}) allow(mock_resolv_dns).to receive(:getresources) .with(email_instance.address.domain, Resolv::DNS::Resource::IN::MX) .and_return(mock_mx_records) @@ -82,100 +85,118 @@ expect(second_result).to be true end - it "does not call the MX servers lookup when the cached time since last lookup is less than the cached ttl entry" do - described_class.class_variable_set(:@@mx_servers_cache, { email_instance.address.domain => { records: mock_mx_records, cached_at: Time.now, ttl: ttl }}) - - email_instance.valid_strict_mx? - - expect(Resolv::DNS).not_to have_received(:open) - end - - it "calls the MX servers lookup when the cached time since last lookup is greater than the cached ttl entry" do - described_class.class_variable_set(:@@mx_servers_cache, { email_instance.address.domain => { records: mock_mx_records, cached_at: Time.now - ttl, ttl: ttl }}) # Cached 1 day ago - - email_instance.valid_strict_mx? - - expect(Resolv::DNS).to have_received(:open).once - end - - it "does not prune the cache when the cache size is less than the max cache size" do - expect(email_instance).not_to receive(:prune_cache) - - email_instance.valid_strict_mx? - end - - it "prunes the cache when the cache size is greater than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - - expect(email_instance).to receive(:prune_cache).with(described_class.class_variable_get(:@@mx_servers_cache)).once - - email_instance.valid_strict_mx? - email_instance.valid_strict_mx? - end - - it "does not call the MX or A servers lookup when there is a cached entry for the domain and the cache size is less than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 1) - described_class.class_variable_set(:@@mx_servers_cache, { email_instance.address.domain => { records: mock_mx_records, cached_at: Time.now, ttl: ttl }}) - - email_instance.valid_strict_mx? - - expect(Resolv::DNS).not_to have_received(:open) - end + describe "ttl" do + before do + dns_cache_instance.instance_variable_set(:@cache, mock_cache_data) + allow(ValidEmail2::DnsRecordsCache).to receive(:new).and_return(dns_cache_instance) + allow(dns_cache_instance).to receive(:fetch).with(email_instance.address.domain).and_call_original + end - it "calls the MX or A servers lookup when there is a cached entry for the domain but the cache size is greater than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - described_class.class_variable_set(:@@mx_servers_cache, { email_instance.address.domain => { records: mock_mx_records, cached_at: Time.now, ttl: ttl }}) + context "when the time since last lookup is less than the cached ttl entry" do + let(:cached_at) { Time.now } - email_instance.valid_strict_mx? + it "does not call the MX servers lookup" do + email_instance.valid_strict_mx? - expect(Resolv::DNS).to have_received(:open).once - end - - it "does not prune older entries when the cache size is less than the max size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 1) - described_class.class_variable_set(:@@mx_servers_cache, { - 'another_domain.com' => { - records: mock_mx_records, cached_at: Time.now - 100, ttl: ttl - } - }) + expect(Resolv::DNS).not_to have_received(:open) + end + end - email_instance.valid_strict_mx? + context "when the time since last lookup is greater than the cached ttl entry" do + let(:cached_at) { Time.now - ttl } + + it "calls the MX servers lookup" do + email_instance.valid_strict_mx? - expect(described_class.class_variable_get(:@@mx_servers_cache).keys).to match_array([email_instance.address.domain, 'another_domain.com']) + expect(Resolv::DNS).to have_received(:open).once + end + end end - it "prunes older entries when the cache size is greater than the max size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - described_class.class_variable_set(:@@mx_servers_cache, { - 'another_domain.com' => { - records: mock_mx_records, cached_at: Time.now - 100, ttl: ttl - } - }) + describe "cache size" do + before do + dns_cache_instance.instance_variable_set(:@cache, mock_cache_data) + allow(ValidEmail2::DnsRecordsCache).to receive(:new).and_return(dns_cache_instance) + allow(dns_cache_instance).to receive(:fetch).with(email_instance.address.domain).and_call_original + end - email_instance.valid_strict_mx? + context "when the cache size is less than or equal to the max cache size" do + before do + stub_const("ValidEmail2::DnsRecordsCache::MAX_CACHE_SIZE", 1) + end + + it "does not prune the cache" do + expect(dns_cache_instance).not_to receive(:prune) + + email_instance.valid_strict_mx? + end + + it "does not call the MX servers lookup" do + email_instance.valid_strict_mx? + + expect(Resolv::DNS).not_to have_received(:open) + end + + context "and there are older cached entries" do + let(:mock_cache_data) { { "another_domain.com" => { records: mock_mx_records, cached_at: cached_at - 100, ttl: ttl } } } + + it "does not prune those entries" do + email_instance.valid_strict_mx? + + expect(dns_cache_instance.instance_variable_get(:@cache).keys.size).to eq 2 + expect(dns_cache_instance.instance_variable_get(:@cache).keys).to match_array([email_instance.address.domain, "another_domain.com"]) + end + end + end - expect(described_class.class_variable_get(:@@mx_servers_cache).keys).to match_array([email_instance.address.domain]) + context "when the cache size is greater than the max cache size" do + before do + stub_const("ValidEmail2::DnsRecordsCache::MAX_CACHE_SIZE", 0) + end + + it "prunes the cache" do + expect(dns_cache_instance).to receive(:prune).once + + email_instance.valid_strict_mx? + end + + it "calls the the MX servers lookup" do + email_instance.valid_strict_mx? + + expect(Resolv::DNS).to have_received(:open).once + end + + context "and there are older cached entries" do + let(:mock_cache_data) { { "another_domain.com" => { records: mock_mx_records, cached_at: cached_at - 100, ttl: ttl } } } + + it "prunes those entries" do + email_instance.valid_strict_mx? + + expect(dns_cache_instance.instance_variable_get(:@cache).keys.size).to eq 1 + expect(dns_cache_instance.instance_variable_get(:@cache).keys).to match_array([email_instance.address.domain]) + end + end + end end end describe "#valid_mx?" do - let(:mock_a_records) { [double('A', address: '192.168.1.1', ttl: ttl)] } + let(:cached_at) { Time.now } + let(:mock_cache_data) { { email_instance.address.domain => { records: mock_a_records, cached_at: cached_at, ttl: ttl } } } + let(:mock_a_records) { [double("A", address: "192.168.1.1", ttl: ttl)] } before do - described_class.class_variable_set(:@@mx_or_a_servers_cache, {}) allow(email_instance).to receive(:mx_servers).and_return(mock_mx_records) allow(mock_resolv_dns).to receive(:getresources) .with(email_instance.address.domain, Resolv::DNS::Resource::IN::A) .and_return(mock_a_records) end - context "when the email is not cached" do - it "calls the MX or A servers lookup" do - result = email_instance.valid_mx? + it "calls the MX or A servers lookup when the email is not cached" do + result = email_instance.valid_mx? - expect(Resolv::DNS).to have_received(:open).once - expect(result).to be true - end + expect(Resolv::DNS).to have_received(:open).once + expect(result).to be true end it "does not call the MX or A servers lookup when the email is cached" do @@ -197,80 +218,99 @@ expect(second_result).to be true end - it "does not call the MX or A servers lookup when the time since last lookup is less than the cached ttl entry" do - described_class.class_variable_set(:@@mx_or_a_servers_cache, { email_instance.address.domain => { records: mock_a_records, cached_at: Time.now, ttl: ttl }}) - - email_instance.valid_mx? - - expect(Resolv::DNS).not_to have_received(:open) - end - - it "calls the MX or A servers lookup when the time since last lookup is greater than the cached ttl entry" do - described_class.class_variable_set(:@@mx_or_a_servers_cache, { email_instance.address.domain => { records: mock_a_records, cached_at: Time.now - ttl, ttl: ttl }}) - - email_instance.valid_mx? - - expect(Resolv::DNS).to have_received(:open).once - end - - it "does not prune the cache when the cache size is less than the max cache size" do - expect(email_instance).not_to receive(:prune_cache) - - email_instance.valid_mx? - end - - it "prunes the cache when the cache size is greater than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - - expect(email_instance).to receive(:prune_cache).with(described_class.class_variable_get(:@@mx_or_a_servers_cache)).once - - email_instance.valid_mx? - email_instance.valid_mx? - end - - it "does not call the MX or A servers lookup when there is a cached entry for the domain and the cache size is less than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 1) - described_class.class_variable_set(:@@mx_or_a_servers_cache, { email_instance.address.domain => { records: mock_a_records, cached_at: Time.now, ttl: ttl }}) - - email_instance.valid_mx? - - expect(Resolv::DNS).not_to have_received(:open) - end - - it "calls the MX or A servers lookup when there is a cached entry for the domain but the cache size is greater than the max cache size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - described_class.class_variable_set(:@@mx_or_a_servers_cache, { email_instance.address.domain => { records: mock_a_records, cached_at: Time.now, ttl: ttl }}) - - email_instance.valid_mx? + describe "ttl" do + before do + dns_cache_instance.instance_variable_set(:@cache, mock_cache_data) + allow(ValidEmail2::DnsRecordsCache).to receive(:new).and_return(dns_cache_instance) + allow(dns_cache_instance).to receive(:fetch).with(email_instance.address.domain).and_call_original + end - expect(Resolv::DNS).to have_received(:open).once - end + context "when the time since last lookup is less than the cached ttl entry" do + let(:cached_at) { Time.now } - it "does not prune older entries when the cache size is less than the max size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 1) - described_class.class_variable_set(:@@mx_or_a_servers_cache, { - 'another_domain.com' => { - records: mock_a_records, cached_at: Time.now - 100, ttl: ttl - } - }) + it "does not call the MX or A servers lookup" do + email_instance.valid_mx? - email_instance.valid_mx? - - expect(described_class.class_variable_get(:@@mx_or_a_servers_cache).keys).to match_array([email_instance.address.domain, 'another_domain.com']) + expect(Resolv::DNS).not_to have_received(:open) + end + end + + context "when the time since last lookup is greater than the cached ttl entry" do + let(:cached_at) { Time.now - ttl } + + it "calls the MX or A servers lookup " do + email_instance.valid_mx? + + expect(Resolv::DNS).to have_received(:open).once + end + end end - it "prunes older entries when the cache size is greater than the max size" do - stub_const("#{described_class}::MAX_CACHE_SIZE", 0) - described_class.class_variable_set(:@@mx_or_a_servers_cache, { - 'another_domain.com' => { - records: mock_a_records, cached_at: Time.now - 100, ttl: ttl - } - }) + describe "cache size" do + before do + dns_cache_instance.instance_variable_set(:@cache, mock_cache_data) + allow(ValidEmail2::DnsRecordsCache).to receive(:new).and_return(dns_cache_instance) + allow(dns_cache_instance).to receive(:fetch).with(email_instance.address.domain).and_call_original + end - email_instance.valid_mx? + context "when the cache size is less than or equal to the max cache size" do + before do + stub_const("ValidEmail2::DnsRecordsCache::MAX_CACHE_SIZE", 1) + end + + it "does not prune the cache" do + expect(email_instance).not_to receive(:prune_cache) + + email_instance.valid_mx? + end + + it "does not call the MX or A servers lookup" do + email_instance.valid_mx? + + expect(Resolv::DNS).not_to have_received(:open) + end + + context "and there are older cached entries" do + let(:mock_cache_data) { { "another_domain.com" => { records: mock_a_records, cached_at: cached_at - 100, ttl: ttl } } } + + it "does not prune those entries" do + email_instance.valid_mx? + + expect(dns_cache_instance.instance_variable_get(:@cache).keys.size).to eq 2 + expect(dns_cache_instance.instance_variable_get(:@cache).keys).to match_array([email_instance.address.domain, "another_domain.com"]) + end + end + end - expect(described_class.class_variable_get(:@@mx_or_a_servers_cache).keys).to match_array([email_instance.address.domain]) - end + context "when the cache size is greater than the max cache size" do + before do + stub_const("ValidEmail2::DnsRecordsCache::MAX_CACHE_SIZE", 0) + end + + it "prunes the cache" do + expect(dns_cache_instance).to receive(:prune).once + + email_instance.valid_mx? + end + + it "calls the MX or A servers lookup" do + email_instance.valid_mx? + + expect(Resolv::DNS).to have_received(:open).once + end + + context "and there are older cached entries" do + let(:mock_cache_data) { { "another_domain.com" => { records: mock_a_records, cached_at: cached_at - 100, ttl: ttl } } } + + it "prunes those entries" do + email_instance.valid_mx? + + expect(dns_cache_instance.instance_variable_get(:@cache).keys.size).to eq 1 + expect(dns_cache_instance.instance_variable_get(:@cache).keys).to match_array([email_instance.address.domain]) + end + end + end + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3a2a1ec..d5e02cc 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,7 @@ require 'rspec-benchmark' RSpec.configure do |config| config.include RSpec::Benchmark::Matchers + config.default_formatter = 'doc' end RSpec::Benchmark.configure do |config| config.disable_gc = true diff --git a/spec/valid_email2_spec.rb b/spec/valid_email2_spec.rb index 3e71feb..c1c1a3a 100644 --- a/spec/valid_email2_spec.rb +++ b/spec/valid_email2_spec.rb @@ -79,11 +79,6 @@ class TestUserMultiple < TestModel let(:disposable_domain) { ValidEmail2.disposable_emails.first } - before do - ValidEmail2::Address.class_variable_set(:@@mx_servers_cache, {}) - ValidEmail2::Address.class_variable_set(:@@mx_or_a_servers_cache, {}) - end - describe "basic validation" do subject(:user) { TestUser.new(email: "") }