Additional raster opt fallback cases (DLCFEED-277), plus ruby/gem version updates

.github/workflows/ci.yml

@@ -11,7 +11,7 @@ jobs:
     runs-on: ubuntu-24.04
     strategy:
       matrix:
-        ruby-version: ['3.2.2']
+        ruby-version: ['3.3.8']
         node: ['20']
         redis-version: [7]
     env:

.ruby-version

@@ -1 +1 @@
-ruby-3.2.2
+3.3.8

Gemfile

@@ -19,6 +19,8 @@ gem 'jbuilder', '~> 2.7'
 gem 'jwt', '~> 2.7.1'
 # Use mysql as a database option for Active Record
 gem 'mysql2', '~> 0.5.5'
+# For building and parsing XML
+gem 'nokogiri', '~> 1.18', '>= 1.18.10', force_ruby_platform: true
 # Use Puma as the app server
 gem 'puma', '~> 5.2'
 # Bundle edge Rails instead: gem 'rails', github: 'rails/rails'

Gemfile.lock

@@ -257,7 +257,7 @@ GEM
     puma (5.6.8)
       nio4r (~> 2.0)
     racc (1.8.1)
-    rack (2.2.20)
+    rack (2.2.21)
     rack-protection (3.2.0)
       base64 (>= 0.1.0)
       rack (~> 2.2, >= 2.2.4)
@@ -418,6 +418,7 @@ GEM
       sprockets (>= 3.0.0)
     sqlite3 (1.7.0)
       mini_portile2 (~> 2.8.0)
+    sqlite3 (1.7.0-x86_64-linux)
     sshkit (1.24.0)
       base64
       logger
@@ -446,14 +447,15 @@ GEM
       activemodel (>= 6.0.0)
       bindex (>= 0.4.0)
       railties (>= 6.0.0)
-    webrick (1.8.1)
+    webrick (1.9.1)
     websocket-driver (0.7.6)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
     zeitwerk (2.6.13)
 
 PLATFORMS
   ruby
+  x86_64-linux
 
 DEPENDENCIES
   addressable (~> 2.8.0)
@@ -475,6 +477,7 @@ DEPENDENCIES
   jwt (~> 2.7.1)
   listen (~> 3.3)
   mysql2 (~> 0.5.5)
+  nokogiri (~> 1.18, >= 1.18.10)
   omniauth
   omniauth-cul (~> 0.2.0)
   puma (~> 5.2)
@@ -495,4 +498,4 @@ DEPENDENCIES
   web-console (>= 3.3.0)
 
 BUNDLED WITH
-   2.3.13
+   2.6.9

app/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic.rb

@@ -4,7 +4,7 @@ module ImagesController
       module RasterOptFallbackLogic
         extend ActiveSupport::Concern
 
-        # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting
+        # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
         def raster_opts_for_ready_resource_with_fallback(resource, base_type, original_raster_opts, normalized_raster_opts)
           raster_opts_to_try = normalized_raster_opts.dup
           cache_hit = resource.raster_exists?(base_type, raster_opts_to_try)
@@ -41,10 +41,36 @@ def raster_opts_for_ready_resource_with_fallback(resource, base_type, original_r
 
                   raster_opts_to_try = normalized_raster_opts.merge(size: "!#{long_side},#{long_side}")
                   cache_hit = resource.raster_exists?(base_type, raster_opts_to_try)
-                  Rails.logger.error(
-                    "[#{resource.identifier}] "\
-                    "Third try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}"
-                  )
+                  unless cache_hit
+                    Rails.logger.error(
+                      "[#{resource.identifier}] "\
+                      "Third try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}"
+                    )
+
+                    # Unfortunately, we cannot guarantee that converting a rounded width or height value to a
+                    # "!#{long_side},#{long_side}" value will always result in the long_size value in the cache,
+                    # since the generation of "!#{long_side},#{long_side}" cached items was based on the original
+                    # ratio and rounding errors can occur.  So we'll also fall back to checking for
+                    # "!#{long_side - 1},#{long_side - 1}" and "!#{long_side + 1},#{long_side + 1}" versions.
+
+                    raster_opts_to_try = normalized_raster_opts.merge(size: "!#{long_side - 1},#{long_side - 1}")
+                    cache_hit = resource.raster_exists?(base_type, raster_opts_to_try)
+                    unless cache_hit
+                      Rails.logger.error(
+                        "[#{resource.identifier}] "\
+                        "Fourth try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}"
+                      )
+
+                      raster_opts_to_try = normalized_raster_opts.merge(size: "!#{long_side + 1},#{long_side + 1}")
+                      cache_hit = resource.raster_exists?(base_type, raster_opts_to_try)
+                      unless cache_hit
+                        Rails.logger.error(
+                          "[#{resource.identifier}] "\
+                          "Fifth try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}"
+                        )
+                      end
+                    end
+                  end
                 end
               end
             end
@@ -53,7 +79,7 @@ def raster_opts_for_ready_resource_with_fallback(resource, base_type, original_r
 
           [raster_opts_to_try, cache_hit]
         end
-        # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting
+        # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
       end
     end
   end

spec/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic_spec.rb

@@ -38,6 +38,12 @@
   let(:normalized_raster_opts_with_exclamation_point_long_side_size_notation) do
     normalized_raster_opts.merge({ size: "!#{long_side_length},#{long_side_length}" })
   end
+  let(:normalized_raster_opts_with_exclamation_point_long_side_minus_one_size_notation) do
+    normalized_raster_opts.merge({ size: "!#{long_side_length - 1},#{long_side_length - 1}" })
+  end
+  let(:normalized_raster_opts_with_exclamation_point_long_side_plus_one_size_notation) do
+    normalized_raster_opts.merge({ size: "!#{long_side_length + 1},#{long_side_length + 1}" })
+  end
 
   before do
     allow(resource).to receive(:identifier).and_return(identifier)
@@ -57,7 +63,7 @@
     end
 
     context 'when an associated file does not exist at the normalized_raster_opts path, '\
-            'but an associated file does exist at the normalized_raster_opts path with the size'\
+            'but an associated file does exist at the normalized_raster_opts path with the size '\
             'opt replaced with the original_raster_opts size opt' do
       before do
         allow(resource).to receive(:raster_exists?).with(base_type, normalized_raster_opts).and_return(false)
@@ -74,8 +80,8 @@
     end
 
     context 'when an associated file does not exist at the normalized_raster_opts path, '\
-            'and it does not exist at the normalized_raster_opts path with the size'\
-            'opt replaced with the original_raster_opts size opt, but it does exist at'\
+            'and it does not exist at the normalized_raster_opts path with the size '\
+            'opt replaced with the original_raster_opts size opt, but it does exist at '\
             'the normalized_raster_opts path with the size replaced with "!long_side,long_side"' do
       before do
         allow(resource).to receive(:raster_exists?).with(base_type, normalized_raster_opts).and_return(false)
@@ -95,6 +101,66 @@
     end
   end
 
+  context 'when an associated file does not exist at the normalized_raster_opts path, '\
+    'and it does not exist at the normalized_raster_opts path with the size '\
+    'opt replaced with the original_raster_opts size opt, '\
+    'and it does exist at the normalized_raster_opts path with the size replaced with "!long_side,long_side", '\
+    'but it does exist at the normalized_raster_opts path with the size '\
+    'replaced with "!long_side - 1,long_side - 1",' do
+    before do
+      allow(resource).to receive(:raster_exists?).with(base_type, normalized_raster_opts).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_original_size_opt
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_size_notation
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_minus_one_size_notation
+      ).and_return(true)
+    end
+
+    it 'returns the expected opts, and a cache hit of true' do
+      expect(
+        instance.raster_opts_for_ready_resource_with_fallback(
+          resource, base_type, original_raster_opts, normalized_raster_opts
+        )
+      ).to eq([normalized_raster_opts_with_exclamation_point_long_side_minus_one_size_notation, true])
+    end
+  end
+
+  context 'when an associated file does not exist at the normalized_raster_opts path, '\
+    'and it does not exist at the normalized_raster_opts path with the size '\
+    'opt replaced with the original_raster_opts size opt, '\
+    'and it does exist at the normalized_raster_opts path with the size replaced with "!long_side,long_side", '\
+    'and it does exist at the normalized_raster_opts path with the size replaced with "!long_side - 1,long_side - 1", '\
+    'but it does exist at the normalized_raster_opts path with the size '\
+    'replaced with "!long_side + 1,long_side + 1",' do
+    before do
+      allow(resource).to receive(:raster_exists?).with(base_type, normalized_raster_opts).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_original_size_opt
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_size_notation
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_minus_one_size_notation
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_plus_one_size_notation
+      ).and_return(true)
+    end
+
+    it 'returns the expected opts, and a cache hit of true' do
+      expect(
+        instance.raster_opts_for_ready_resource_with_fallback(
+          resource, base_type, original_raster_opts, normalized_raster_opts
+        )
+      ).to eq([normalized_raster_opts_with_exclamation_point_long_side_plus_one_size_notation, true])
+    end
+  end
+
   context 'when an associated file does not exist at the normalized_raster_opts path, '\
             'and it does not exist at any other fallback variants' do
     before do
@@ -105,12 +171,18 @@
       allow(resource).to receive(:raster_exists?).with(
         base_type, normalized_raster_opts_with_exclamation_point_long_side_size_notation
       ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_minus_one_size_notation
+      ).and_return(false)
+      allow(resource).to receive(:raster_exists?).with(
+        base_type, normalized_raster_opts_with_exclamation_point_long_side_plus_one_size_notation
+      ).and_return(false)
     end
 
     it 'returns the expected opts, and a cache hit of false' do
       expect(instance.raster_opts_for_ready_resource_with_fallback(
                resource, base_type, original_raster_opts, normalized_raster_opts
-             )).to eq([normalized_raster_opts_with_exclamation_point_long_side_size_notation, false])
+             )).to eq([normalized_raster_opts_with_exclamation_point_long_side_plus_one_size_notation, false])
     end
   end
 end