From 104a35583e342158682cbe50ab9588333808717a Mon Sep 17 00:00:00 2001 From: Ian Bayne Date: Sat, 26 Oct 2024 11:58:00 +0900 Subject: [PATCH] change: bust the cache if the time since last lookup is greater than the cached ttl --- lib/valid_email2/address.rb | 41 +++++++++++++++++++++++++------- spec/address_spec.rb | 47 ++++++++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index 6a20671..63963b4 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -8,6 +8,7 @@ module ValidEmail2 class Address attr_accessor :address + # Cache structure: { domain (String): { records: [], cached_at: Time, ttl: Integer } } @@mx_servers_cache = {} @@mx_or_a_servers_cache = {} @@ -140,16 +141,28 @@ def address_contain_emoticons?(email) end def mx_servers - return @@mx_servers_cache[address.domain.downcase] if @@mx_servers_cache.key?(address.domain.downcase) + domain = address.domain.downcase + + 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) + end + end - result = Resolv::DNS.open(@resolv_config) do |dns| + records = Resolv::DNS.open(@resolv_config) do |dns| dns.timeouts = @dns_timeout dns.getresources(address.domain, Resolv::DNS::Resource::IN::MX) end - @@mx_servers_cache[address.domain.downcase] = result + if records.any? + ttl = records.map(&:ttl).min + @@mx_servers_cache[domain] = { records: records, cached_at: Time.now, ttl: ttl } + end - result + records end def null_mx? @@ -157,17 +170,29 @@ def null_mx? end def mx_or_a_servers - return @@mx_or_a_servers_cache[address.domain.downcase] if @@mx_or_a_servers_cache.key?(address.domain.downcase) + 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) + end + end - result = Resolv::DNS.open(@resolv_config) do |dns| + 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 - @@mx_or_a_servers_cache[address.domain.downcase] = result + if records.any? + ttl = records.map(&:ttl).min + @@mx_or_a_servers_cache[domain] = { records: records, cached_at: Time.now, ttl: ttl } + end - result + records end end end diff --git a/spec/address_spec.rb b/spec/address_spec.rb index 0887e0b..a43686c 100644 --- a/spec/address_spec.rb +++ b/spec/address_spec.rb @@ -38,8 +38,9 @@ describe "caching" do let(:email_address) { "example@ymail.com" } let(:email_instance) { described_class.new(email_address) } + let(:ttl) { 1_000 } let(:mock_resolv_dns) { instance_double(Resolv::DNS) } - let(:mock_mx_records) { [double('MX', exchange: 'mx.ymail.com', preference: 10)] } + 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) @@ -80,10 +81,26 @@ second_result = email_instance.valid_strict_mx? 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 end describe "#valid_mx?" do - let(:mock_a_records) { [double('A', address: '192.168.1.1')] } + 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, {}) @@ -93,11 +110,13 @@ .and_return(mock_a_records) end - it "calls the MX or A servers lookup when the email is not cached" do - result = email_instance.valid_mx? + context "when the email is not cached" do + it "calls the MX or A servers lookup" do + result = email_instance.valid_mx? - expect(Resolv::DNS).to have_received(:open).once - expect(result).to be true + expect(Resolv::DNS).to have_received(:open).once + expect(result).to be true + end end it "does not call the MX or A servers lookup when the email is cached" do @@ -118,6 +137,22 @@ second_result = email_instance.valid_mx? 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 end end end