DEV-1119: refactor healthchecks (#19)

* DEV-1119: refactor healthchecks

splits the generalized new healthcheck introduced to fix this issue
into its own class; IIIF item checks should happen separately, even
if they rely on similar logic

* Apply suggestions from code review

Co-authored-by: Anna Wilcox <AWilcox@Wilcox-Tech.com>

---------

Co-authored-by: Anna Wilcox <AWilcox@Wilcox-Tech.com>

Author: anarchivist

app/lib/health_checks.rb

@@ -2,6 +2,7 @@
 
 module HealthChecks
   CHECK_FILES = %w[
+    iiif_item_check
     iiif_server_check
     lending_root_path
     test_item_exists

app/lib/health_checks/iiif_item_check.rb

@@ -0,0 +1,36 @@
+require 'health_checks/iiif_server_check'
+
+module HealthChecks
+  class IIIFItemCheck < IIIFServerCheck
+
+    private
+
+    def iiif_test_uri
+      @iiif_test_uri ||= begin
+        item = Item.active.first || Item.inactive.first
+        if item && iiif_base_uri
+          BerkeleyLibrary::Util::URIs.append(
+            iiif_base_uri,
+            item.iiif_directory.first_image_url_path,
+            'info.json'
+          )
+        end
+      end
+    end
+
+    def perform_request
+      # iipsrv won't return an CORS header for the "health" endpoint, so
+      # we still call a test item to do a separate check
+      response = super
+      return response if response[:failure]
+
+      acao_header = response[:rsp].headers['Access-Control-Allow-Origin']
+      if acao_header.blank?
+        response.merge!({
+                          message: "GET #{iiif_test_uri} missing Access-Control-Allow-Origin header", failure: true
+                        })
+      end
+      response
+    end
+  end
+end

app/lib/health_checks/iiif_server_check.rb

@@ -3,7 +3,7 @@ class IIIFServerCheck < OkComputer::Check
     include BerkeleyLibrary::Logging
 
     def check
-      result = validate_iiif_server
+      result = perform_request
       mark_message result[:message]
       mark_failure if result[:failure]
     rescue StandardError => e
@@ -21,32 +21,22 @@ def iiif_connection
       end
     end
 
-    def iiif_test_uri
-      base_uri = Lending::Config.iiif_base_uri
-      return unless base_uri
-
-      item = Item.active.first || Item.inactive.first
-      return unless item
-
-      BerkeleyLibrary::Util::URIs.append(
-        base_uri,
-        item.iiif_directory.first_image_url_path,
-        'info.json'
-      )
+    def iiif_base_uri
+      @iiif_base_uri ||= Lending::Config.iiif_base_uri
     end
 
-    # Returns a hash with :message and :failure keys
-    def validate_iiif_server
-      test_uri = iiif_test_uri
-      return { message: 'Unable to construct test image URI', failure: true } unless test_uri
+    def iiif_test_uri
+      @iiif_test_uri ||= (URI.join(iiif_base_uri, '/health') if iiif_base_uri)
+    end
 
-      response = iiif_connection.head(test_uri)
-      return { message: "HEAD #{test_uri} returned status #{response.status}", failure: true } unless response.success?
+    # Returns a hash with :message, :failure, and :rsp keys
+    def perform_request
+      return { message: 'Unable to construct healthcheck URI', failure: true, rsp: nil } unless iiif_test_uri
 
-      acao_header = response.headers['Access-Control-Allow-Origin']
-      return { message: "HEAD #{test_uri} missing Access-Control-Allow-Origin header", failure: true } if acao_header.blank?
+      response = iiif_connection.get(iiif_test_uri)
+      return { message: "GET #{iiif_test_uri} returned status #{response.status}", failure: true, rsp: response } unless response.success?
 
-      { message: 'IIIF server reachable', failure: false }
+      { message: 'HTTP check successful', failure: false, rsp: response }
     end
   end
 end

config/initializers/okcomputer.rb

@@ -9,8 +9,9 @@
 # Check that DB migrations have run
 OkComputer::Registry.register 'database-migrations', OkComputer::ActiveRecordMigrationsCheck.new
 
-# Custom IIIF server check
+# Custom IIIF server checks
 OkComputer::Registry.register 'iiif-server', HealthChecks::IIIFServerCheck.new
+OkComputer::Registry.register 'iiif-item', HealthChecks::IIIFItemCheck.new
 
 # TODO: Custom Test Item Exists
 OkComputer::Registry.register 'test-item-exists', HealthChecks::TestItemExists.new

spec/lib/health_checks/iiif_item_check_spec.rb

@@ -0,0 +1,206 @@
+require 'rails_helper'
+require 'support/iiif_check_helper'
+
+RSpec.describe HealthChecks::IIIFItemCheck do
+  subject(:check) { described_class.new }
+
+  def run_check
+    check.run
+    check
+  end
+
+  describe '#check' do
+
+    context 'with a valid IIIF base URI' do
+      include_context 'IIIF item checks'
+
+      it 'fails and sets message when the GET health check response is not successful' do
+        response = instance_double('Faraday::Response',
+                                   success?: false,
+                                   status: 503,
+                                   headers: {})
+
+        allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+        run_check
+
+        expect(check.message).to match(/returned status 503/)
+
+        if check.respond_to?(:failure?)
+          expect(check.failure?).to be(true)
+        else
+          expect(check.instance_variable_get(:@failure_occurred)).to be(true)
+        end
+      end
+
+      it 'fails and sets message when Access-Control-Allow-Origin header is missing/blank' do
+        response = instance_double('Faraday::Response',
+                                   success?: true,
+                                   status: 200,
+                                   headers: { 'Access-Control-Allow-Origin' => '' })
+
+        allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+        run_check
+
+        expect(check.message).to eq(
+          "GET #{test_uri} missing Access-Control-Allow-Origin header"
+        )
+
+        if check.respond_to?(:failure?)
+          expect(check.failure?).to be(true)
+        else
+          expect(check.instance_variable_get(:@failure_occurred)).to be(true)
+        end
+      end
+
+      it 'does not fail when reachable and ACAO header present' do
+        response = instance_double('Faraday::Response',
+                                   success?: true,
+                                   status: 200,
+                                   headers: { 'Access-Control-Allow-Origin' => '*' })
+
+        allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+        run_check
+
+        expect(check.message).to eq('HTTP check successful')
+
+        if check.respond_to?(:failure?)
+          expect(check.failure?).to be(false)
+        else
+          expect(check.instance_variable_get(:@failure_occurred)).not_to be(true)
+        end
+      end
+
+      it 'fails and sets message when an exception is raised' do
+        allow(connection).to receive(:get).with(test_uri).and_raise(StandardError, 'boom')
+
+        run_check
+
+        expect(check.message).to match('StandardError')
+
+        if check.respond_to?(:failure?)
+          expect(check.failure?).to be(true)
+        else
+          expect(check.instance_variable_get(:@failure_occurred)).to be(true)
+        end
+      end
+    end
+  end
+
+  describe 'private helpers' do
+    describe '#iiif_connection' do
+      it 'configures Faraday timeouts' do
+        conn = check.send(:iiif_connection)
+
+        expect(conn.options.open_timeout).to eq(2)
+        expect(conn.options.timeout).to eq(3)
+      end
+    end
+
+    describe '#iiif_base_uri' do
+      it 'returns nil when iiif_base_uri is nil' do
+        allow(Lending::Config).to receive(:iiif_base_uri).and_return(nil)
+
+        expect(check.send(:iiif_base_uri)).to be_nil
+      end
+
+      it 'returns the Lending::Config value when set' do
+        allow(Lending::Config).to receive(:iiif_base_uri).and_return('http://example.test/iiif')
+        expect(check.send(:iiif_base_uri)).to eq('http://example.test/iiif')
+      end
+    end
+
+    describe '#iiif_test_uri' do
+      it 'returns nil when iiif_base_uri is nil' do
+        allow(Lending::Config).to receive(:iiif_base_uri).and_return(nil)
+
+        expect(check.send(:iiif_test_uri)).to be_nil
+      end
+
+      context 'with IIIF item checks' do
+        include_context 'IIIF item checks'
+
+        it 'returns nil when no Item exists' do
+          allow(Lending::Config).to receive(:iiif_base_uri).and_return(base_uri)
+          stub_items(active_first: nil, inactive_first: nil)
+
+          expect(check.send(:iiif_test_uri)).to be_nil
+        end
+
+        it 'builds a test uri from an active item (or inactive fallback)' do
+          base_uri = URI('http://example.test/iiif/')
+          allow(Lending::Config).to receive(:iiif_base_uri).and_return(base_uri)
+
+          iiif_dir = instance_double('IiifDirectory', first_image_url_path: 'some/path')
+          item = instance_double('Item', iiif_directory: iiif_dir)
+          stub_items(active_first: item, inactive_first: nil)
+
+          expect(BerkeleyLibrary::Util::URIs).to receive(:append)
+            .with(base_uri, 'some/path', 'info.json')
+            .and_return('http://example.test/iiif/some/path/info.json')
+
+          expect(check.send(:iiif_test_uri)).to eq('http://example.test/iiif/some/path/info.json')
+        end
+      end
+    end
+
+    describe '#perform_request' do
+      it 'returns a failure when it cannot construct test uri' do
+        allow(Lending::Config).to receive(:iiif_base_uri).and_return(nil)
+
+        result = check.send(:perform_request)
+
+        expect(result).to eq(message: 'Unable to construct healthcheck URI', failure: true, rsp: nil)
+      end
+
+      context 'with IIIF item checks' do
+        include_context 'IIIF item checks'
+        it 'returns a failure when the GET response is not successful' do
+          response = instance_double('Faraday::Response',
+                                     success?: false,
+                                     status: 503,
+                                     headers: {})
+
+          allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+          result = check.send(:perform_request)
+
+          expect(result[:failure]).to be(true)
+          expect(result[:message]).to match(/returned status 503/)
+        end
+
+        it 'returns a failure when Access-Control-Allow-Origin header is missing/blank' do
+          response = instance_double('Faraday::Response',
+                                     success?: true,
+                                     status: 200,
+                                     headers: { 'Access-Control-Allow-Origin' => '' })
+
+          allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+          result = check.send(:perform_request)
+
+          expect(result).to eq(
+            message: "GET #{test_uri} missing Access-Control-Allow-Origin header",
+            failure: true,
+            rsp: response
+          )
+        end
+
+        it 'returns ok when reachable and ACAO header present' do
+          response = instance_double('Faraday::Response',
+                                     success?: true,
+                                     status: 200,
+                                     headers: { 'Access-Control-Allow-Origin' => '*' })
+
+          allow(connection).to receive(:get).with(test_uri).and_return(response)
+
+          result = check.send(:perform_request)
+
+          expect(result).to eq(message: 'HTTP check successful', failure: false, rsp: response)
+        end
+      end
+    end
+  end
+end