change: bust the cache if the time since last lookup is greater than …

…the cached ttl

lib/valid_email2/address.rb

@@ -8,6 +8,7 @@ module ValidEmail2
   class Address
     attr_accessor :address
 
+    # Cache structure: { domain: { records: [], cached_at: Time, ttl: Integer } }
     @@mx_servers_cache = {}
     @@mx_or_a_servers_cache = {}
 
@@ -140,34 +141,58 @@ 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?
       mx_servers.length == 1 && mx_servers.first.preference == 0 && mx_servers.first.exchange.length == 0
     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

spec/address_spec.rb

@@ -38,8 +38,9 @@
   describe "caching" do
     let(:email_address) { "[email protected]" }
     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,28 @@
         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
+
+  it 'test' do
+    a = ValidEmail2::Address.new('[email protected]')
+    # a.valid_strict_mx?
+    a.valid_mx?
+  end
 end