From 835b2ff3a3c4ec8b04dde45130456297deec862a Mon Sep 17 00:00:00 2001 From: Eric O Date: Wed, 13 Aug 2025 18:15:58 -0400 Subject: [PATCH 01/15] Add tests for Triclops::Iiif::RasterOptNormalizer module --- .../iiif/raster_opt_normalizer_spec.rb | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 spec/triclops/iiif/raster_opt_normalizer_spec.rb diff --git a/spec/triclops/iiif/raster_opt_normalizer_spec.rb b/spec/triclops/iiif/raster_opt_normalizer_spec.rb new file mode 100644 index 0000000..111c2d0 --- /dev/null +++ b/spec/triclops/iiif/raster_opt_normalizer_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe Triclops::Iiif::RasterOptNormalizer do + let(:resource) { FactoryBot.create(:resource) } + let(:region) { 'full' } + let(:size) { 'full' } + let(:rotation) { 0 } + let(:quality) { 'color' } + let(:raster_opts) do + { + region: region, + size: size, + rotation: rotation, + quality: quality + } + end + + describe '.normalize_raster_opts' do + it 'returns the same opts for opts that do not need conversion' do + expect(described_class.normalize_raster_opts(resource, raster_opts)).to eq(raster_opts) + end + + context 'when region is "square"' do + let(:region) { 'square' } + let(:expected_output) { raster_opts.merge({ region: '320,616,1280,1280' }) } + + it 'converts the region to the x,y,w,h version of the region' do + expect(described_class.normalize_raster_opts(resource, raster_opts)).to eq(expected_output) + end + end + + context 'when quality is "default"' do + let(:quality) { 'default' } + let(:expected_output) { raster_opts.merge({ quality: 'color' }) } + + it 'converts the quality to "color"' do + expect(described_class.normalize_raster_opts(resource, raster_opts)).to eq(expected_output) + end + end + end +end From b4e6fb72cedf9086a49b41cd7c0389d32fad8427 Mon Sep 17 00:00:00 2001 From: Eric O Date: Thu, 14 Aug 2025 18:40:18 -0400 Subject: [PATCH 02/15] Add specs for upcoming Triclops::Iiif::RasterOptNormalizer.normalize_raster_size method --- .../iiif/raster_opt_normalizer_spec.rb | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/spec/triclops/iiif/raster_opt_normalizer_spec.rb b/spec/triclops/iiif/raster_opt_normalizer_spec.rb index 111c2d0..2afbc42 100644 --- a/spec/triclops/iiif/raster_opt_normalizer_spec.rb +++ b/spec/triclops/iiif/raster_opt_normalizer_spec.rb @@ -20,6 +20,11 @@ expect(described_class.normalize_raster_opts(resource, raster_opts)).to eq(raster_opts) end + it 'processes the size parameter with an underlying call to normalize_raster_size' do + expect(described_class).to receive(:normalize_raster_size).with(resource, raster_opts[:size]) + described_class.normalize_raster_opts(resource, raster_opts) + end + context 'when region is "square"' do let(:region) { 'square' } let(:expected_output) { raster_opts.merge({ region: '320,616,1280,1280' }) } @@ -38,4 +43,35 @@ end end end + + describe '.normalize_raster_size' do + it 'does not modify an input size of "full"' do + expect(described_class.normalize_raster_size(resource, 'full')).to eq('full') + end + + it 'does not modify an input size of "max"' do + expect(described_class.normalize_raster_size(resource, 'max')).to eq('max') + end + + it 'does not modify an input size with the format "123,456"' do + expect(described_class.normalize_raster_size(resource, '123,456')).to eq('123,456') + end + + context 'when various convertable size values are provided' do + let(:resource) { FactoryBot.create(:resource, standard_width: 6485, standard_height: 8690) } + + { + '573,768' => '573,768', + '573,' => '573,768', + ',768' => '573,768', + '!768,768' => '573,768', + '!100,1234' => '100,134', + '!1234,100' => '75,100' + }.each do |input_value, output_value| + it "converts an input value of #{input_value} to #{output_value}" do + expect(described_class.normalize_raster_size(resource, input_value)).to eq(output_value) + end + end + end + end end From 18a7ebf5b6b8cef060b6707b8e28be961b231ab8 Mon Sep 17 00:00:00 2001 From: Eric O Date: Fri, 22 Aug 2025 18:20:25 -0400 Subject: [PATCH 03/15] Use normalized raster opts throughout the application; Normalize various image sizes to underlying 'width,height' paths; If image isn't found at normalized path, also check originally requested path --- app/controllers/iiif/images_controller.rb | 16 +++++- .../resource/derivative_generation.rb | 57 ++++++++++--------- lib/triclops/iiif/raster_opt_normalizer.rb | 55 ++++++++++++++++++ 3 files changed, 98 insertions(+), 30 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index 6bdb8ac..bfc953e 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -96,7 +96,7 @@ def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts, end end - # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts) cache_hit = @resource.raster_exists?(base_type, normalized_raster_opts) unless cache_hit @@ -115,10 +115,20 @@ def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opt send_raster_file(raster_file, normalized_raster_opts, @resource.updated_at, delivery_method: :send_data) end else # TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR - render plain: 'not found', status: :not_found + + # !!! THIS IS TEMPORARY. REMOVE THIS AFTER RASTER CACHE RESTRUCTURING IS COMPLETE. !!! + # If a raster is not found at the given path and the normalized_raster_opts != original_raster_opts, + # try checking the original_raster_opts path. + if normalized_raster_opts != original_raster_opts # rubocop:disable Style/IfInsideElse + # NOTE: Below, we are passing original_raster_opts to the normalized_raster_opts parameter + handle_ready_resource(base_type, original_raster_opts, original_raster_opts) + else + render plain: 'not found', status: :not_found + end + end end - # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/MethodLength, Metrics/AbcSize def error_response(errors) { result: false, errors: errors } diff --git a/app/models/concerns/triclops/resource/derivative_generation.rb b/app/models/concerns/triclops/resource/derivative_generation.rb index ddeb857..dbd494f 100644 --- a/app/models/concerns/triclops/resource/derivative_generation.rb +++ b/app/models/concerns/triclops/resource/derivative_generation.rb @@ -48,16 +48,17 @@ def generate_base_derivatives_if_not_exist! self.with_source_image_file do |source_image_file| # Use the original image to generate standard base if necessary unless File.exist?(standard_base_path) + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { + region: 'full', + size: 'full', + rotation: 0, + quality: Triclops::Iiif::Constants::BASE_QUALITY, + format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT + }) Triclops::Raster.generate( source_image_file.path, standard_base_path, - { - region: 'full', - size: 'full', - rotation: 0, - quality: Triclops::Iiif::Constants::BASE_QUALITY, - format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT - } + raster_opts ) end # Store standard base dimensions @@ -72,16 +73,17 @@ def generate_base_derivatives_if_not_exist! # has a long side that's smaller than LIMITED_BASE_SIZE. But that case will be rare, and # shouldn't cause any issues. unless base_exists?(limited_base_path) + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { + region: 'full', + size: "!#{Triclops::Iiif::Constants::LIMITED_BASE_SIZE},#{Triclops::Iiif::Constants::LIMITED_BASE_SIZE}", + rotation: 0, + quality: Triclops::Iiif::Constants::BASE_QUALITY, + format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT + }) Triclops::Raster.generate( source_image_file.path, limited_base_path, - { - region: 'full', - size: "!#{Triclops::Iiif::Constants::LIMITED_BASE_SIZE},#{Triclops::Iiif::Constants::LIMITED_BASE_SIZE}", - rotation: 0, - quality: Triclops::Iiif::Constants::BASE_QUALITY, - format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT - } + raster_opts ) end # Store limited base dimensions @@ -97,16 +99,17 @@ def generate_base_derivatives_if_not_exist! # shouldn't cause any issues. unless base_exists?(featured_base_path) + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { + region: self.featured_region, + size: "!#{Triclops::Iiif::Constants::FEATURED_BASE_SIZE},#{Triclops::Iiif::Constants::FEATURED_BASE_SIZE}", + rotation: 0, + quality: Triclops::Iiif::Constants::BASE_QUALITY, + format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT + }) Triclops::Raster.generate( source_image_file.path, featured_base_path, - { - region: self.featured_region, - size: "!#{Triclops::Iiif::Constants::FEATURED_BASE_SIZE},#{Triclops::Iiif::Constants::FEATURED_BASE_SIZE}", - rotation: 0, - quality: Triclops::Iiif::Constants::BASE_QUALITY, - format: Triclops::Iiif::Constants::BASE_IMAGE_FORMAT - } + raster_opts ) end @@ -142,13 +145,13 @@ def generate_commonly_requested_standard_derivatives # Generate scaled rasters at Triclops::Iiif::Constants::RECOMMENDED_SIZES. Triclops::Iiif::Constants::RECOMMENDED_SIZES.each do |size| - raster_opts = { + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { region: 'full', size: "!#{size},#{size}", rotation: 0, quality: Triclops::Iiif::Constants::BASE_QUALITY, format: Triclops::Iiif::Constants::DEFAULT_FORMAT - } + }) raster_path = Triclops::RasterCache.instance.iiif_cache_path_for_raster( Triclops::Iiif::Constants::BASE_TYPE_STANDARD, self.identifier, @@ -204,13 +207,13 @@ def generate_commonly_requested_limited_derivatives # Generate scaled rasters at Triclops::Iiif::Constants::RECOMMENDED_LIMITED_SIZES. Triclops::Iiif::Constants::RECOMMENDED_LIMITED_SIZES.each do |size| - raster_opts = { + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { region: 'full', size: "!#{size},#{size}", rotation: 0, quality: Triclops::Iiif::Constants::BASE_QUALITY, format: Triclops::Iiif::Constants::DEFAULT_FORMAT - } + }) raster_path = Triclops::RasterCache.instance.iiif_cache_path_for_raster( Triclops::Iiif::Constants::BASE_TYPE_LIMITED, self.identifier, @@ -251,13 +254,13 @@ def generate_commonly_requested_featured_derivatives # Generate recommended featured versions at PRE_GENERATED_SQUARE_SIZES Triclops::Iiif::Constants::PRE_GENERATED_SQUARE_SIZES.each do |size| - raster_opts = { + raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(self, { region: 'full', size: "!#{size},#{size}", rotation: 0, quality: Triclops::Iiif::Constants::BASE_QUALITY, format: Triclops::Iiif::Constants::DEFAULT_FORMAT - } + }) raster_path = Triclops::RasterCache.instance.iiif_cache_path_for_raster( Triclops::Iiif::Constants::BASE_TYPE_FEATURED, self.identifier, diff --git a/lib/triclops/iiif/raster_opt_normalizer.rb b/lib/triclops/iiif/raster_opt_normalizer.rb index 6f05c2b..806650b 100644 --- a/lib/triclops/iiif/raster_opt_normalizer.rb +++ b/lib/triclops/iiif/raster_opt_normalizer.rb @@ -17,6 +17,61 @@ def self.normalize_raster_opts(resource, raster_opts) # {quality: 'default'} should be converted into {quality: 'color'} because that is our default normalized_opts[:quality] = 'color' if normalized_opts[:quality] == 'default' + # There are multiple variations on the size parameter that could actualy resolve to the same cached image. + # For example, for a 6485x8690 (w x h) original image, the following three sizes are effectively the same: + # - 573,768 + # - 573, + # - ,768 + # - !768,768 + # We don't need to cache all of these variations, so we will instead convert requests for any of them into + # a request for one of them at a single cached location. + normalized_opts[:size] = self.normalize_raster_size(resource, raster_opts[:size]) + normalized_opts end + + def self.normalize_raster_size(resource, size_raster_opt) # rubocop:disable Metrics/AbcSize,Metrics/MethodLength + case size_raster_opt + when /^\d+,\d+$/ + # Example: "573,768" + size_raster_opt # Return unmodified original + when /^\d+,$/ + # Example: "573," + width = size_raster_opt[0...-1].to_i + # Calculate missing height value + height = (resource.standard_height.to_f / resource.standard_width) * width + "#{width.round},#{height.round}" + when /^,\d+$/ + # Example: ",768" + height = size_raster_opt[1..].to_i + # Calculate missing width value + width = (resource.standard_width.to_f / resource.standard_height) * height + "#{width.round},#{height.round}" + when /^!\d+,\d+$/ + # Example: "!768,768" or "!100,200" or "!800,600" + match_data = size_raster_opt.match(/!(\d+),(\d+)/) + max_width = match_data[1].to_i + max_height = match_data[2].to_i + + requested_dimensions_aspect_ratio = max_width.to_f / max_height + original_aspect_ratio = resource.standard_width.to_f / resource.standard_height # 1.5 for wide, 0.75 for tall + + # If the original aspect ratio is larger than the specified region aspect ratio, then we're width-limited and + # need to scale based on width as the long side. Otherwise we're height limited and need to scale based on height. + scale_based_on_width = original_aspect_ratio > requested_dimensions_aspect_ratio + + if scale_based_on_width + width = max_width + height = (resource.standard_height.to_f / resource.standard_width) * width + else + # We'll scale based on height + height = max_height + width = (resource.standard_width.to_f / resource.standard_height) * height + end + "#{width.round},#{height.round}" + else + # Return original value, since it's not a type that we would want to convert. + size_raster_opt + end + end end From d717528186d917763748844162d252826d1453c0 Mon Sep 17 00:00:00 2001 From: Eric O Date: Fri, 22 Aug 2025 18:26:26 -0400 Subject: [PATCH 04/15] Update capistrano --- Gemfile | 2 +- Gemfile.lock | 29 ++++++++++++++++++----------- config/deploy.rb | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Gemfile b/Gemfile index 7f6c1ff..f0368df 100644 --- a/Gemfile +++ b/Gemfile @@ -73,7 +73,7 @@ group :development, :test do end group :development do - gem 'capistrano', '~> 3.18.0', require: false + gem 'capistrano', '~> 3.19.0', require: false gem 'capistrano-cul', require: false gem 'capistrano-passenger', '~> 0.1', require: false gem 'capistrano-rails', '~> 1.4', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 09a040c..8ded997 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -85,7 +85,7 @@ GEM tzinfo (~> 2.0) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) - airbrussh (1.5.0) + airbrussh (1.5.3) sshkit (>= 1.6.1, != 1.7.0) ast (2.4.2) base64 (0.1.1) @@ -99,7 +99,7 @@ GEM msgpack (~> 1.2) builder (3.2.4) byebug (11.1.3) - capistrano (3.18.0) + capistrano (3.19.2) airbrussh (>= 1.0.0) i18n rake (>= 10.0.0) @@ -119,7 +119,7 @@ GEM capistrano-rails (1.6.3) capistrano (~> 3.1) capistrano-bundler (>= 1.1, < 3) - concurrent-ruby (1.2.3) + concurrent-ruby (1.3.5) connection_pool (2.4.1) crass (1.0.6) date (3.3.4) @@ -178,7 +178,7 @@ GEM globalid (1.2.1) activesupport (>= 6.1) hashie (5.0.0) - i18n (1.14.1) + i18n (1.14.7) concurrent-ruby (~> 1.0) io-console (0.7.2) io-wait (0.2.0) @@ -197,6 +197,7 @@ GEM listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) + logger (1.7.0) loofah (2.22.0) crass (~> 1.0.2) nokogiri (>= 1.12.0) @@ -217,7 +218,7 @@ GEM multi_json (1.15.0) mustermann (3.0.0) ruby2_keywords (~> 0.0.1) - mutex_m (0.2.0) + mutex_m (0.3.0) mysql2 (0.5.5) net-imap (0.4.10) date @@ -226,11 +227,13 @@ GEM net-protocol net-protocol (0.2.2) timeout - net-scp (4.0.0) + net-scp (4.1.0) net-ssh (>= 2.6.5, < 8.0.0) + net-sftp (4.0.0) + net-ssh (>= 5.0.0, < 8.0.0) net-smtp (0.4.0.1) net-protocol - net-ssh (7.2.1) + net-ssh (7.3.0) nio4r (2.7.0) nokogiri (1.16.2) mini_portile2 (~> 2.8.2) @@ -243,6 +246,7 @@ GEM devise (>= 4.9) omniauth (>= 2.0) orm_adapter (0.5.0) + ostruct (0.6.3) parallel (1.24.0) parser (3.3.0.2) ast (~> 2.4.1) @@ -296,7 +300,7 @@ GEM thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.1.0) + rake (13.3.0) rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) @@ -414,10 +418,13 @@ GEM sprockets (>= 3.0.0) sqlite3 (1.7.0) mini_portile2 (~> 2.8.0) - sshkit (1.21.7) - mutex_m + sshkit (1.24.0) + base64 + logger net-scp (>= 1.1.2) + net-sftp (>= 2.1.2) net-ssh (>= 2.8.0) + ostruct stringio (3.1.0) thor (1.3.0) tilt (2.3.0) @@ -454,7 +461,7 @@ DEPENDENCIES best_type (~> 0.0.10) bootsnap (>= 1.4.2) byebug - capistrano (~> 3.18.0) + capistrano (~> 3.19.0) capistrano-cul capistrano-passenger (~> 0.1) capistrano-rails (~> 1.4) diff --git a/config/deploy.rb b/config/deploy.rb index 4879af4..04a5086 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # config valid for current version and patch releases of Capistrano -lock '~> 3.18.0' +lock '~> 3.19.0' # Until we retire all old CentOS VMs, we need to set the rvm_custom_path because rvm is installed # in a non-standard location for our AlmaLinux VMs. This is because our service accounts need to From d433594fe007c226c77a15ed138b7872814c992b Mon Sep 17 00:00:00 2001 From: Eric O Date: Fri, 22 Aug 2025 18:35:54 -0400 Subject: [PATCH 05/15] In ImagesController, only generate normalized rasters opts for ready resources --- app/controllers/iiif/images_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index bfc953e..8a85dda 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -41,9 +41,8 @@ def raster original_raster_opts = params_validation_result.to_h original_raster_opts.delete(:identifier) # :identifier isn't part of our "raster opts" base_type = params[:base_type] # :base_type isn't part of our "raster opts" - normalized_raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(@resource, original_raster_opts) - handle_ready_resource_or_redirect(@resource, base_type, original_raster_opts, normalized_raster_opts) + handle_ready_resource_or_redirect(@resource, base_type, original_raster_opts) end def test_viewer @@ -73,7 +72,7 @@ def validate_image_request_token(token, base_type, resource_identifier, client_i Triclops::Utils::TokenUtils.token_is_valid?(token, base_type, resource_identifier, client_ip) end - def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts, normalized_raster_opts) + def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts) # Whenever a valid resource is requested, cache the Resource identifier in # our ResourceAccessStatCache. This cache will be periodically flushed to the # Resource database (by a separate process) so that many access time updates @@ -87,6 +86,7 @@ def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts, TRICLOPS[:raster_cache][:access_stats_enabled] if resource.ready? + normalized_raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(@resource, original_raster_opts) handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts) else Rails.logger.debug( From 3e1f50440dcac64e3d25d8bc4a69bc7148d10a80 Mon Sep 17 00:00:00 2001 From: Eric O Date: Fri, 22 Aug 2025 18:48:01 -0400 Subject: [PATCH 06/15] normalize_raster_opts should raise an exception when the resource is missing required standard_width or standard_height values --- lib/triclops/exceptions.rb | 1 + lib/triclops/iiif/raster_opt_normalizer.rb | 10 ++++++++ .../iiif/raster_opt_normalizer_spec.rb | 24 +++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/lib/triclops/exceptions.rb b/lib/triclops/exceptions.rb index dd6e0e0..6bf57b7 100644 --- a/lib/triclops/exceptions.rb +++ b/lib/triclops/exceptions.rb @@ -5,5 +5,6 @@ class TriclopsError < StandardError; end class RasterExists < TriclopsError; end class MissingBaseImageDependencyException < TriclopsError; end class UnknownBaseType < TriclopsError; end + class MissingWidthOrHeightInformation < TriclopsError; end end end diff --git a/lib/triclops/iiif/raster_opt_normalizer.rb b/lib/triclops/iiif/raster_opt_normalizer.rb index 806650b..4b25f29 100644 --- a/lib/triclops/iiif/raster_opt_normalizer.rb +++ b/lib/triclops/iiif/raster_opt_normalizer.rb @@ -3,11 +3,21 @@ module Triclops::Iiif::RasterOptNormalizer # Performs operations like converting a 'square' region into a numeric crop # region and aliasing 'color' quality as 'default' quality. # + # This method should only be called on a resource that has a known width and height. + # If it is called on a resource with a nil width or height it will raise a + # Triclops::Exceptions::MissingWidthOrHeightInformation exception. + # # @api private # @param raster_opts [Hash] # A hash of IIIF options (e.g. {identifier: '...', region: '...', size: '...', etc. }). # @return [Hash] the processed version of raster_opts def self.normalize_raster_opts(resource, raster_opts) + # The resource's standard_width and standard_height values are required for us to generate normalized raster opts. + if resource.standard_width.nil? || resource.standard_height.nil? + raise Triclops::Exceptions::MissingWidthOrHeightInformation, + "Resource #{resource.identifier} is missing width and/or height information, so it is not possible to determine normalized raster opts." + end + # duplicate raster_opts so we don't modify the incoming argument normalized_opts = raster_opts.dup diff --git a/spec/triclops/iiif/raster_opt_normalizer_spec.rb b/spec/triclops/iiif/raster_opt_normalizer_spec.rb index 2afbc42..bfdfb9d 100644 --- a/spec/triclops/iiif/raster_opt_normalizer_spec.rb +++ b/spec/triclops/iiif/raster_opt_normalizer_spec.rb @@ -42,6 +42,30 @@ expect(described_class.normalize_raster_opts(resource, raster_opts)).to eq(expected_output) end end + + context 'when the resource has a nil standard_width value' do + before do + resource.standard_width = nil + end + + it 'raises an exception' do + expect { + described_class.normalize_raster_opts(resource, raster_opts) + }.to raise_error(Triclops::Exceptions::MissingWidthOrHeightInformation) + end + end + + context 'when the resource has a nil standard_height value' do + before do + resource.standard_height = nil + end + + it 'raises an exception' do + expect { + described_class.normalize_raster_opts(resource, raster_opts) + }.to raise_error(Triclops::Exceptions::MissingWidthOrHeightInformation) + end + end end describe '.normalize_raster_size' do From bbf1fb4043cbd9a0d1e3b8a5eda3e0069e21b65d Mon Sep 17 00:00:00 2001 From: Eric O Date: Fri, 22 Aug 2025 18:50:12 -0400 Subject: [PATCH 07/15] Run apt-get update during CI --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88228a4..e1ec8ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,8 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Update apt + run: sudo apt-get update - name: Install vips run: sudo apt install -y libvips-tools - name: Check vips version From 2cfc6e64bb2dff63d7869310034e4557da7f98e6 Mon Sep 17 00:00:00 2001 From: Eric O Date: Sat, 23 Aug 2025 14:16:11 -0400 Subject: [PATCH 08/15] Slight modification to temporary raster-not-found fallback logic --- app/controllers/iiif/images_controller.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index 8a85dda..6424686 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -116,12 +116,11 @@ def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opt end else # TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR - # !!! THIS IS TEMPORARY. REMOVE THIS AFTER RASTER CACHE RESTRUCTURING IS COMPLETE. !!! - # If a raster is not found at the given path and the normalized_raster_opts != original_raster_opts, - # try checking the original_raster_opts path. - if normalized_raster_opts != original_raster_opts # rubocop:disable Style/IfInsideElse - # NOTE: Below, we are passing original_raster_opts to the normalized_raster_opts parameter - handle_ready_resource(base_type, original_raster_opts, original_raster_opts) + # !!! THIS IS TEMPORARY. AFTER RASTER CACHE RESTRUCTURING IS COMPLETE, REPLACE THE CODE BELOW. !!! + # If a raster is not found at the normalized opt location, + normalized_raster_opts_with_original_size_opt = normalized_raster_opts.merge(size: original_raster_opts[:size]) + if normalized_raster_opts != normalized_raster_opts_with_original_size_opt + handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts_with_original_size_opt) else render plain: 'not found', status: :not_found end From 0943bdbc80877c12a02c7684a0aa8f80009d2119 Mon Sep 17 00:00:00 2001 From: Eric O Date: Sat, 23 Aug 2025 23:39:52 -0400 Subject: [PATCH 09/15] Remove unsupported IIIF qualities --- lib/triclops/iiif/constants.rb | 2 +- spec/models/concerns/triclops/resource/iiif_info_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/triclops/iiif/constants.rb b/lib/triclops/iiif/constants.rb index cfb872a..8d099a9 100644 --- a/lib/triclops/iiif/constants.rb +++ b/lib/triclops/iiif/constants.rb @@ -13,7 +13,7 @@ module Triclops::Iiif::Constants DEFAULT_FORMAT = 'jpg' BASE_IMAGE_FORMAT = 'png' BASE_IMAGE_EXTNAME = ".#{BASE_IMAGE_FORMAT}".freeze - ALLOWED_QUALITIES = ['default', 'color', 'gray', 'bitonal'].freeze + ALLOWED_QUALITIES = ['default', 'color'].freeze # We don't currently offer ['gray', 'bitonal'], so they have been omitted BASE_QUALITY = 'color' ALLOWED_ROTATIONS = [0, 90, 180, 270].freeze LIMITED_BASE_SIZE = 768 diff --git a/spec/models/concerns/triclops/resource/iiif_info_spec.rb b/spec/models/concerns/triclops/resource/iiif_info_spec.rb index 29616bf..338073e 100644 --- a/spec/models/concerns/triclops/resource/iiif_info_spec.rb +++ b/spec/models/concerns/triclops/resource/iiif_info_spec.rb @@ -29,7 +29,7 @@ ] end let(:formats) { ['png', 'jpg'] } - let(:qualities) { ['default', 'color', 'gray', 'bitonal'] } + let(:qualities) { ['default', 'color'] } let(:tile_size) { 512 } let(:scale_factors) { [1, 2, 4, 8, 16] } it 'returns the expected hash' do From dfc8b8087ccf270330ce49c3a61c256320280472 Mon Sep 17 00:00:00 2001 From: Eric O Date: Sun, 24 Aug 2025 11:48:04 -0400 Subject: [PATCH 10/15] Fix for compliance level url in info response --- app/controllers/iiif/images_controller.rb | 17 ++++++++++------- .../concerns/triclops/resource/iiif_info.rb | 4 ++-- .../triclops/resource/iiif_info_spec.rb | 5 +++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index 6424686..ef4e5e1 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -192,13 +192,15 @@ def assign_headers_for_sent_file!(resp, raster_file, modification_time) resp['ETag'] = format('"%x"', modification_time) end + def compliance_level_url + if TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR + 'http://iiif.io/api/image/2/level0.json' + else + 'http://iiif.io/api/image/2/level1.json' + end + end + def assign_compliance_level_header!(resp) - compliance_level_url = - if TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR - 'http://iiif.io/api/image/2/level0.json' - else - 'http://iiif.io/api/image/2/level1.json' - end resp.set_header('Link', compliance_level_url) end @@ -212,7 +214,8 @@ def info_json_for_resource(resource, base_type) Triclops::Iiif::Constants::ALLOWED_FORMATS.keys, Triclops::Iiif::Constants::ALLOWED_QUALITIES, Triclops::Iiif::Constants::TILE_SIZE, - Imogen::Iiif::Tiles.scale_factors_for(width, height, Triclops::Iiif::Constants::TILE_SIZE) + Imogen::Iiif::Tiles.scale_factors_for(width, height, Triclops::Iiif::Constants::TILE_SIZE), + compliance_level_url ) end diff --git a/app/models/concerns/triclops/resource/iiif_info.rb b/app/models/concerns/triclops/resource/iiif_info.rb index ca7c6d4..61a05e8 100644 --- a/app/models/concerns/triclops/resource/iiif_info.rb +++ b/app/models/concerns/triclops/resource/iiif_info.rb @@ -6,7 +6,7 @@ module IiifInfo # Returns a IIIF 2.1 info hash about this image resource. # @param id_url [String] Identifying URL for this resource. # @return [Hash] IIIF 2.1 structured info response. - def iiif_info(id_url, width, height, sizes, formats, qualities, tile_size, scale_factors) + def iiif_info(id_url, width, height, sizes, formats, qualities, tile_size, scale_factors, compliance_level_url) { '@context': 'http://iiif.io/api/image/2/context.json', '@id': id_url, @@ -15,7 +15,7 @@ def iiif_info(id_url, width, height, sizes, formats, qualities, tile_size, scale 'height': height, 'sizes': sizes.map { |size| { 'width': size[0], 'height': size[1] } }, 'tiles': tile_info(tile_size, scale_factors), - 'profile': ['http://iiif.io/api/image/2/level2.json', { 'formats': formats, 'qualities': qualities }] + 'profile': [compliance_level_url, { 'formats': formats, 'qualities': qualities }] } end diff --git a/spec/models/concerns/triclops/resource/iiif_info_spec.rb b/spec/models/concerns/triclops/resource/iiif_info_spec.rb index 338073e..9c5efb3 100644 --- a/spec/models/concerns/triclops/resource/iiif_info_spec.rb +++ b/spec/models/concerns/triclops/resource/iiif_info_spec.rb @@ -32,6 +32,7 @@ let(:qualities) { ['default', 'color'] } let(:tile_size) { 512 } let(:scale_factors) { [1, 2, 4, 8, 16] } + let(:compliance_level_url) { 'http://iiif.io/api/image/2/level0.json' } it 'returns the expected hash' do expect( { @@ -47,9 +48,9 @@ { width: 786, height: 1280 } ], 'tiles': [{ 'width': tile_size, 'scaleFactors': scale_factors }], - 'profile': ['http://iiif.io/api/image/2/level2.json', { 'formats': formats, 'qualities': qualities }] + 'profile': [compliance_level_url, { 'formats': formats, 'qualities': qualities }] } - ).to eq(instance.iiif_info(id_url, standard_width, standard_height, sizes, formats, qualities, tile_size, scale_factors)) + ).to eq(instance.iiif_info(id_url, standard_width, standard_height, sizes, formats, qualities, tile_size, scale_factors, compliance_level_url)) end end end From 4b05365b43190593259c5cea427bbc5a8ed4fae6 Mon Sep 17 00:00:00 2001 From: Eric O Date: Sun, 24 Aug 2025 12:37:33 -0400 Subject: [PATCH 11/15] Ignore rake files named local.rake --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 6184690..40a540a 100644 --- a/.gitignore +++ b/.gitignore @@ -57,3 +57,5 @@ node_modules # https://vitejs.dev/guide/env-and-mode.html#env-files *.local +# Local rake file +local.rake From 51ffe6946e6949d0846ccacfe3f0e8207e7cb833 Mon Sep 17 00:00:00 2001 From: Eric O Date: Sun, 24 Aug 2025 13:15:55 -0400 Subject: [PATCH 12/15] Reference local variable instead of instance variable --- app/controllers/iiif/images_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index ef4e5e1..ff710f2 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -86,7 +86,7 @@ def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts) TRICLOPS[:raster_cache][:access_stats_enabled] if resource.ready? - normalized_raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(@resource, original_raster_opts) + normalized_raster_opts = Triclops::Iiif::RasterOptNormalizer.normalize_raster_opts(resource, original_raster_opts) handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts) else Rails.logger.debug( From 3092672acf9a9dbb04ac7475d9c0b1dda3c579b2 Mon Sep 17 00:00:00 2001 From: Eric O Date: Mon, 25 Aug 2025 21:20:25 -0400 Subject: [PATCH 13/15] Refactor temporary image fallback location behavior and add second fallback --- app/controllers/iiif/images_controller.rb | 49 +++++++++++++++++++---- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index ff710f2..18d280f 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -115,16 +115,51 @@ def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opt send_raster_file(raster_file, normalized_raster_opts, @resource.updated_at, delivery_method: :send_data) end else # TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR + # If we got here, that means we have a cache miss and we will treat this as an error (because we do not expect + # clients to request this resource with the given raster opts). We'll render a 404 response to the user. - # !!! THIS IS TEMPORARY. AFTER RASTER CACHE RESTRUCTURING IS COMPLETE, REPLACE THE CODE BELOW. !!! - # If a raster is not found at the normalized opt location, - normalized_raster_opts_with_original_size_opt = normalized_raster_opts.merge(size: original_raster_opts[:size]) - if normalized_raster_opts != normalized_raster_opts_with_original_size_opt - handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts_with_original_size_opt) - else - render plain: 'not found', status: :not_found + # TEMPORARY: Try checking a backup raster path + alternative_normalized_raster_opts = normalized_raster_opts.merge(size: original_raster_opts[:size]) + cache_hit = @resource.raster_exists?(base_type, alternative_normalized_raster_opts) + Rails.logger.error( + "[#{@resource.identifier}] "\ + "Second try: Cache #{cache_hit ? 'HIT' : 'MISS'} for alternative_normalized_raster_opts: #{alternative_normalized_raster_opts}" + ) + if cache_hit + @resource.yield_cached_raster(base_type, alternative_normalized_raster_opts) do |raster_file| + send_raster_file(raster_file, alternative_normalized_raster_opts, @resource.updated_at, delivery_method: :send_file) + end + return + end + + # TEMPORARY: If nothing was found at the backup raster path AND this a request for a 'full' region, + # try converting the size to a "!long_side,long_side" size value and see if that version exists in the cache. + if normalized_raster_opts[:region] == 'full' + # We expect normalized_raster_opts to have a size value of the format: "width,height" in most cases, + # but if it doesn't then we should skip the rest of this block. + size_opt = normalized_raster_opts[:size] + matches = /(\d+),(\d+)/.match(size_opt) + if matches + width = matches[1] + height = matches[2] + long_side = width > height ? width : height # rubocop:disable Metrics/BlockNesting + + alternative_normalized_raster_opts = normalized_raster_opts.merge(size: "!#{long_side},#{long_side}") + cache_hit = @resource.raster_exists?(base_type, alternative_normalized_raster_opts) + Rails.logger.error( + "[#{@resource.identifier}] "\ + "Third try: Cache #{cache_hit ? 'HIT' : 'MISS'} for alternative_normalized_raster_opts: #{alternative_normalized_raster_opts}" # rubocop:disable Metrics/BlockNesting + ) + if cache_hit # rubocop:disable Metrics/BlockNesting + @resource.yield_cached_raster(base_type, alternative_normalized_raster_opts) do |raster_file| + send_raster_file(raster_file, alternative_normalized_raster_opts, @resource.updated_at, delivery_method: :send_file) + end + return + end + end end + render plain: 'not found', status: :not_found end end # rubocop:enable Metrics/MethodLength, Metrics/AbcSize From eac6599c9eebd712e70b19fed7044a62496fbc8d Mon Sep 17 00:00:00 2001 From: Eric O Date: Tue, 23 Sep 2025 16:10:56 -0400 Subject: [PATCH 14/15] Updated iif size fallback logic; Minor update to triclops initializer to prevent accidental creation of same-name directory inside temp directories --- app/controllers/iiif/images_controller.rb | 95 +++++++++++------------ config/initializers/triclops.rb | 14 ++-- 2 files changed, 54 insertions(+), 55 deletions(-) diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index 18d280f..449d9e2 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -1,3 +1,4 @@ +# rubocop:disable Metrics/BlockNesting class Iiif::ImagesController < ApplicationController include ActionController::Live include Triclops::Iiif::ImagesController::Schemas @@ -98,71 +99,66 @@ def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opts) - cache_hit = @resource.raster_exists?(base_type, normalized_raster_opts) + raster_opts_to_try = normalized_raster_opts.dup + cache_hit = @resource.raster_exists?(base_type, raster_opts_to_try) + unless cache_hit Rails.logger.error( "[#{@resource.identifier}] "\ "Cache MISS: (original_raster_opts: #{original_raster_opts}) "\ - "(normalized_raster_opts: #{normalized_raster_opts.inspect})" + "(raster_opts_to_try: #{raster_opts_to_try.inspect})" ) + + # -- BEGIN temporary fallback code -- + # Try a backup raster path, using the original size opt + raster_opts_to_try = normalized_raster_opts.merge(size: original_raster_opts[:size]) + cache_hit = @resource.raster_exists?(base_type, raster_opts_to_try) + + unless cache_hit + Rails.logger.error( + "[#{@resource.identifier}] "\ + "Second try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}" + ) + + # If nothing was found at the backup raster path AND this a request for a 'full' region, + # try converting the size to a "!long_side,long_side" size value and see if that version exists in the cache. + if normalized_raster_opts[:region] == 'full' + # We expect normalized_raster_opts to have a size value of the format: "width,height" in most cases, + # but if it doesn't then we should skip the rest of this block. + size_opt = normalized_raster_opts[:size] + matches = /(\d+),(\d+)/.match(size_opt) + if matches + width = matches[1].to_i + height = matches[2].to_i + long_side = width > height ? width : height + + 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}" + ) + end + end + end + # -- END temporary fallback code -- end + if cache_hit || TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::GENERATE_AND_CACHE || @resource.source_uri_is_placeholder? - @resource.yield_cached_raster(base_type, normalized_raster_opts) do |raster_file| - send_raster_file(raster_file, normalized_raster_opts, @resource.updated_at, delivery_method: :send_file) + @resource.yield_cached_raster(base_type, raster_opts_to_try) do |raster_file| + send_raster_file(raster_file, raster_opts_to_try, @resource.updated_at, delivery_method: :send_file) end elsif TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::GENERATE_AND_DO_NOT_CACHE - @resource.yield_uncached_raster(base_type, normalized_raster_opts) do |raster_file| - send_raster_file(raster_file, normalized_raster_opts, @resource.updated_at, delivery_method: :send_data) + @resource.yield_uncached_raster(base_type, raster_opts_to_try) do |raster_file| + send_raster_file(raster_file, raster_opts_to_try, @resource.updated_at, delivery_method: :send_data) end else # TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::ERROR # If we got here, that means we have a cache miss and we will treat this as an error (because we do not expect # clients to request this resource with the given raster opts). We'll render a 404 response to the user. - - # TEMPORARY: Try checking a backup raster path - alternative_normalized_raster_opts = normalized_raster_opts.merge(size: original_raster_opts[:size]) - cache_hit = @resource.raster_exists?(base_type, alternative_normalized_raster_opts) - Rails.logger.error( - "[#{@resource.identifier}] "\ - "Second try: Cache #{cache_hit ? 'HIT' : 'MISS'} for alternative_normalized_raster_opts: #{alternative_normalized_raster_opts}" - ) - if cache_hit - @resource.yield_cached_raster(base_type, alternative_normalized_raster_opts) do |raster_file| - send_raster_file(raster_file, alternative_normalized_raster_opts, @resource.updated_at, delivery_method: :send_file) - end - return - end - - # TEMPORARY: If nothing was found at the backup raster path AND this a request for a 'full' region, - # try converting the size to a "!long_side,long_side" size value and see if that version exists in the cache. - if normalized_raster_opts[:region] == 'full' - # We expect normalized_raster_opts to have a size value of the format: "width,height" in most cases, - # but if it doesn't then we should skip the rest of this block. - size_opt = normalized_raster_opts[:size] - matches = /(\d+),(\d+)/.match(size_opt) - if matches - width = matches[1] - height = matches[2] - long_side = width > height ? width : height # rubocop:disable Metrics/BlockNesting - - alternative_normalized_raster_opts = normalized_raster_opts.merge(size: "!#{long_side},#{long_side}") - cache_hit = @resource.raster_exists?(base_type, alternative_normalized_raster_opts) - Rails.logger.error( - "[#{@resource.identifier}] "\ - "Third try: Cache #{cache_hit ? 'HIT' : 'MISS'} for alternative_normalized_raster_opts: #{alternative_normalized_raster_opts}" # rubocop:disable Metrics/BlockNesting - ) - if cache_hit # rubocop:disable Metrics/BlockNesting - @resource.yield_cached_raster(base_type, alternative_normalized_raster_opts) do |raster_file| - send_raster_file(raster_file, alternative_normalized_raster_opts, @resource.updated_at, delivery_method: :send_file) - end - return - end - end - end - render plain: 'not found', status: :not_found end end - # rubocop:enable Metrics/MethodLength, Metrics/AbcSize + # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, # Metrics/BlockNesting def error_response(errors) { result: false, errors: errors } @@ -267,3 +263,4 @@ def dimensions_for_base_type(resource, base_type) end end end +# rubocop:enable Metrics/BlockNesting diff --git a/config/initializers/triclops.rb b/config/initializers/triclops.rb index ee12d6f..85be692 100644 --- a/config/initializers/triclops.rb +++ b/config/initializers/triclops.rb @@ -41,19 +41,21 @@ def validate_triclops_config! # a constant from an auto-loaded file, and modules are only auto-loaded after initialization. validate_triclops_config! + # Set the TMPDIR ENV variable so that Vips (via Imogen) writes temp files to the vips_tmp_directory. + # This defaults to the OS temp directory if not otherwise set, which can be a + # problem if we're on a host that has limited local disk space. + # NOTE: This will also change the value of Ruby's Dir.tmpdir + ENV['TMPDIR'] = TRICLOPS[:vips_tmp_directory] if TRICLOPS[:vips_tmp_directory].present? + # If temp_directory is not set, default to ruby temp dir TRICLOPS[:tmp_directory] = File.join(Dir.tmpdir, Rails.application.class.module_parent_name.downcase) if TRICLOPS[:tmp_directory].blank? # Make temp_directory if it does not already exist - FileUtils.mkdir_p(TRICLOPS[:tmp_directory]) + FileUtils.mkdir_p(TRICLOPS[:tmp_directory]) unless File.exist?(TRICLOPS[:tmp_directory]) # If vips_tmp_directory is not set, default to ruby temp dir TRICLOPS[:vips_tmp_directory] = File.join(Dir.tmpdir, Rails.application.class.module_parent_name.downcase) if TRICLOPS[:vips_tmp_directory].blank? # Make vips_tmp_directory if it does not already exist - FileUtils.mkdir_p(TRICLOPS[:vips_tmp_directory]) - # Set the TMPDIR ENV variable so that Vips (via Imogen) writes temp files to the vips_tmp_directory. - # This defaults to the OS temp directory if not otherwise set, which can be a - # problem if we're on a host that has limited local disk space. - ENV['TMPDIR'] = TRICLOPS[:vips_tmp_directory] + FileUtils.mkdir_p(TRICLOPS[:vips_tmp_directory]) unless File.exist?(TRICLOPS[:vips_tmp_directory]) end Rails.application.config.active_job.queue_adapter = :inline if TRICLOPS['run_queued_jobs_inline'] From e71c936510986f3a13c7400fda4160fa856110df Mon Sep 17 00:00:00 2001 From: Eric O Date: Mon, 6 Oct 2025 12:24:52 -0400 Subject: [PATCH 15/15] Refactored raster opt fallback logic, plus tests --- .../raster_opt_fallback_logic.rb | 60 +++++++++ app/controllers/iiif/images_controller.rb | 52 +------- .../raster_opt_fallback_logic_spec.rb | 116 ++++++++++++++++++ 3 files changed, 180 insertions(+), 48 deletions(-) create mode 100644 app/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic.rb create mode 100644 spec/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic_spec.rb diff --git a/app/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic.rb b/app/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic.rb new file mode 100644 index 0000000..52284d0 --- /dev/null +++ b/app/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic.rb @@ -0,0 +1,60 @@ +module Triclops + module Iiif + module ImagesController + module RasterOptFallbackLogic + extend ActiveSupport::Concern + + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting + 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) + + unless cache_hit + Rails.logger.error( + "[#{resource.identifier}] "\ + "Cache MISS: (original_raster_opts: #{original_raster_opts}) "\ + "(raster_opts_to_try: #{raster_opts_to_try.inspect})" + ) + + # -- BEGIN temporary fallback code -- + # Try a backup raster path, using the original size opt + raster_opts_to_try = normalized_raster_opts.merge(size: original_raster_opts[:size]) + cache_hit = resource.raster_exists?(base_type, raster_opts_to_try) + + unless cache_hit + Rails.logger.error( + "[#{resource.identifier}] "\ + "Second try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}" + ) + + # If nothing was found at the backup raster path AND this a request for a 'full' region, + # try converting the size to a "!long_side,long_side" size value and see if that version exists in the cache. + if normalized_raster_opts[:region] == 'full' + # We expect normalized_raster_opts to have a size value of the format: "width,height" in most cases, + # but if it doesn't then we should skip the rest of this block. + size_opt = normalized_raster_opts[:size] + matches = /(\d+),(\d+)/.match(size_opt) + if matches + width = matches[1].to_i + height = matches[2].to_i + long_side = width > height ? width : height + + 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}" + ) + end + end + end + # -- END temporary fallback code -- + end + + [raster_opts_to_try, cache_hit] + end + # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, Metrics/BlockNesting + end + end + end +end diff --git a/app/controllers/iiif/images_controller.rb b/app/controllers/iiif/images_controller.rb index 449d9e2..92b917c 100644 --- a/app/controllers/iiif/images_controller.rb +++ b/app/controllers/iiif/images_controller.rb @@ -1,8 +1,8 @@ -# rubocop:disable Metrics/BlockNesting class Iiif::ImagesController < ApplicationController include ActionController::Live include Triclops::Iiif::ImagesController::Schemas include Triclops::Iiif::ImagesController::Sizing + include Triclops::Iiif::ImagesController::RasterOptFallbackLogic # skip_before_action :verify_authenticity_token, only: [:raster_preflight_check] before_action :add_cors_header!, only: [ @@ -97,52 +97,10 @@ def handle_ready_resource_or_redirect(resource, base_type, original_raster_opts) end end - # rubocop:disable Metrics/MethodLength, Metrics/AbcSize def handle_ready_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) - - unless cache_hit - Rails.logger.error( - "[#{@resource.identifier}] "\ - "Cache MISS: (original_raster_opts: #{original_raster_opts}) "\ - "(raster_opts_to_try: #{raster_opts_to_try.inspect})" - ) - - # -- BEGIN temporary fallback code -- - # Try a backup raster path, using the original size opt - raster_opts_to_try = normalized_raster_opts.merge(size: original_raster_opts[:size]) - cache_hit = @resource.raster_exists?(base_type, raster_opts_to_try) - - unless cache_hit - Rails.logger.error( - "[#{@resource.identifier}] "\ - "Second try: Cache #{cache_hit ? 'HIT' : 'MISS'} for raster_opts_to_try: #{raster_opts_to_try}" - ) - - # If nothing was found at the backup raster path AND this a request for a 'full' region, - # try converting the size to a "!long_side,long_side" size value and see if that version exists in the cache. - if normalized_raster_opts[:region] == 'full' - # We expect normalized_raster_opts to have a size value of the format: "width,height" in most cases, - # but if it doesn't then we should skip the rest of this block. - size_opt = normalized_raster_opts[:size] - matches = /(\d+),(\d+)/.match(size_opt) - if matches - width = matches[1].to_i - height = matches[2].to_i - long_side = width > height ? width : height - - 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}" - ) - end - end - end - # -- END temporary fallback code -- - end + raster_opts_to_try, cache_hit = raster_opts_for_ready_resource_with_fallback( + @resource, base_type, original_raster_opts, normalized_raster_opts + ) if cache_hit || TRICLOPS[:raster_cache][:on_miss] == Triclops::Iiif::Constants::CacheMissMode::GENERATE_AND_CACHE || @resource.source_uri_is_placeholder? @resource.yield_cached_raster(base_type, raster_opts_to_try) do |raster_file| @@ -158,7 +116,6 @@ def handle_ready_resource(base_type, original_raster_opts, normalized_raster_opt render plain: 'not found', status: :not_found end end - # rubocop:enable Metrics/MethodLength, Metrics/AbcSize, # Metrics/BlockNesting def error_response(errors) { result: false, errors: errors } @@ -263,4 +220,3 @@ def dimensions_for_base_type(resource, base_type) end end end -# rubocop:enable Metrics/BlockNesting diff --git a/spec/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic_spec.rb b/spec/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic_spec.rb new file mode 100644 index 0000000..6781a1c --- /dev/null +++ b/spec/controllers/concerns/triclops/iiif/images_controller/raster_opt_fallback_logic_spec.rb @@ -0,0 +1,116 @@ +require 'rails_helper' + +RSpec.describe Triclops::Iiif::ImagesController::RasterOptFallbackLogic do + let(:instance) do + outer_context = self + Class.new do + include outer_context.described_class + end.new + end + + let(:identifier) { 'example-identifier' } + let(:resource) { instance_double(Resource) } + let(:base_type) { Triclops::Iiif::Constants::BASE_TYPE_STANDARD } + let(:width) { 256 } + let(:height) { 190 } + let(:long_side_length) { width } + let(:original_raster_opts) do + { + region: 'full', + size: "#{width},", + rotation: '0', + quality: 'default', + format: 'jpg' + } + end + let(:normalized_raster_opts) do + { + region: 'full', + size: "#{width},#{height}", + rotation: '0', + quality: 'color', + format: 'jpg' + } + end + let(:normalized_raster_opts_with_original_size_opt) do + normalized_raster_opts.merge({ size: original_raster_opts[:size] }) + end + let(:normalized_raster_opts_with_exclamation_point_long_side_size_notation) do + normalized_raster_opts.merge({ size: "!#{long_side_length},#{long_side_length}" }) + end + + before do + allow(resource).to receive(:identifier).and_return(identifier) + end + + describe '#raster_opts_for_ready_resource_with_fallback' do + context 'when an associated file exists at the normalized_raster_opts path' do + before do + allow(resource).to receive(:raster_exists?).with(base_type, normalized_raster_opts).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, true]) + end + 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'\ + '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) + allow(resource).to receive(:raster_exists?).with( + base_type, normalized_raster_opts_with_original_size_opt + ).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_original_size_opt, 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, 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) + 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(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_size_notation, true]) + end + 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 + 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) + 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]) + end + end +end