Use cache to avoid duplicate URL checks. Refactor

Author: fbacall

lib/link_checker.rb

@@ -1,8 +1,22 @@
 class LinkChecker
-  def self.check(record, log: true)
+  attr_reader :log
+
+  def initialize(log: true)
+    @log = log
+    @cache = {}
+  end
+
+  def check(collection)
+    collection.find_each do |record|
+      check_record(record)
+    end
+  end
+
+  def check_record(record)
     [record, *record.external_resources].each do |item|
       next if item.url.blank?
-      code = bad_response(item.url, log: log)
+      @cache[item.url] ||= bad_response(item.url) # Cache the result, there could be multiple resources linked to same URL
+      code = @cache[item.url]
       if code
         puts "  #{code} - #{item.class.name} #{item.id}: #{item.url}" if log
         if item.link_monitor
@@ -18,9 +32,11 @@ def self.check(record, log: true)
     end
   end
 
+  private
+
   # The fake return codes on an exception are so the LinkMonitor object has something
   # to store as "code" which might be tracked back to a particular problem.
-  def self.bad_response(url, log: true)
+  def bad_response(url)
     begin
       host = URI.parse(url).host rescue nil
       if @prev_host == host
@@ -48,7 +64,7 @@ def self.bad_response(url, log: true)
   end
 
   # Gets a response code using a GET request, for the case where HEAD is not supported.
-  def self.get_code(url, redirect_limit: 5, open_timeout: 5, read_timeout: 5, use_range: true)
+  def get_code(url, redirect_limit: 5, open_timeout: 5, read_timeout: 5, use_range: true)
     raise StandardError, 'too many redirects' if redirect_limit <= 0
 
     uri = URI(url)

lib/tasks/check_urls.rake

@@ -24,14 +24,10 @@ end
 
 def check_materials
   puts 'Checking material URLs'
-  Material.find_each do |mat|
-    LinkChecker.check(mat)
-  end
+  LinkChecker.new.check(Material)
 end
 
 def check_events
   puts 'Checking event URLs'
-  Event.find_each do |event|
-    LinkChecker.check(event)
-  end
+  LinkChecker.new.check(Event)
 end

test/unit/link_checker_test.rb

@@ -2,13 +2,29 @@
 
 class LinkCheckerTest < ActiveSupport::TestCase
 
+  setup do
+    @link_checker = LinkChecker.new(log: false)
+  end
+
+  test 'check collection' do
+    event = events(:two)
+    WebMock.stub_request(:head, event.url).to_return(status: 200)
+    assert_nil event.link_monitor
+
+    assert_no_difference('LinkMonitor.count') do
+      @link_checker.check(Event.where(id: event.id))
+    end
+
+    assert_nil event.link_monitor
+  end
+
   test 'check passing event url' do
     event = events(:two)
     WebMock.stub_request(:head, event.url).to_return(status: 200)
     assert_nil event.link_monitor
 
     assert_no_difference('LinkMonitor.count') do
-      LinkChecker.check(event, log: false)
+      @link_checker.check_record(event)
     end
 
     assert_nil event.link_monitor
@@ -20,7 +36,7 @@ class LinkCheckerTest < ActiveSupport::TestCase
     assert_nil event.link_monitor
 
     assert_difference('LinkMonitor.count', 1) do
-      LinkChecker.check(event, log: false)
+      @link_checker.check_record(event)
     end
 
     assert event.link_monitor
@@ -35,7 +51,7 @@ class LinkCheckerTest < ActiveSupport::TestCase
     assert_nil event.link_monitor
 
     assert_no_difference('LinkMonitor.count') do
-      LinkChecker.check(event, log: false)
+      @link_checker.check_record(event)
     end
 
     assert_nil event.link_monitor
@@ -49,7 +65,7 @@ class LinkCheckerTest < ActiveSupport::TestCase
     assert_nil event.link_monitor
 
     assert_difference('LinkMonitor.count', 1) do
-      LinkChecker.check(event, log: false)
+      @link_checker.check_record(event)
     end
 
     assert event.link_monitor
@@ -65,7 +81,7 @@ class LinkCheckerTest < ActiveSupport::TestCase
     assert_nil event.link_monitor
 
     assert_difference('LinkMonitor.count', 1) do
-      LinkChecker.check(event, log: false)
+      @link_checker.check_record(event)
     end
 
     assert event.link_monitor
@@ -81,7 +97,7 @@ class LinkCheckerTest < ActiveSupport::TestCase
     material = materials(:material_with_external_resource)
 
     assert_difference('LinkMonitor.count', 2) do
-      LinkChecker.check(material, log: false)
+      @link_checker.check_record(material)
     end
   end