From d89af8f8b6e2d1e65e3e173f1786d0259f048f42 Mon Sep 17 00:00:00 2001 From: Adam Ploshay Date: Thu, 23 Jan 2025 11:27:22 -0500 Subject: [PATCH] [ESSI-2062] fix iiif image lookup cases being unaware of external_storage --- app/controllers/purl_controller.rb | 2 +- app/helpers/catalog_helper.rb | 8 +- app/models/collection_branding_info.rb | 14 +-- app/models/file_set.rb | 16 +++ app/presenters/hyrax/displays_image.rb | 47 ++------ app/services/essi/generate_pdf_service.rb | 4 +- .../iiif_collection_thumbnail_path_service.rb | 2 +- app/services/iiif_file_set_path_service.rb | 88 ++++++++++++++ app/services/iiif_thumbnail_path_service.rb | 4 +- lib/extensions/extensions.rb | 3 + .../file_set_presenter/content_location.rb | 9 ++ .../iiif_file_set_path_service_spec.rb | 111 ++++++++++++++++++ 12 files changed, 250 insertions(+), 58 deletions(-) create mode 100644 app/services/iiif_file_set_path_service.rb create mode 100644 lib/extensions/hyrax/file_set_presenter/content_location.rb create mode 100644 spec/services/iiif_file_set_path_service_spec.rb diff --git a/app/controllers/purl_controller.rb b/app/controllers/purl_controller.rb index 3a6dbf5a5..99c51bdb7 100644 --- a/app/controllers/purl_controller.rb +++ b/app/controllers/purl_controller.rb @@ -76,7 +76,7 @@ def rescue_url def jp2_url(solr_hit) begin - Hyrax.config.iiif_image_url_builder.call(solr_hit['original_file_id_ssi'], nil, '!600,600') + IIIFFileSetPathService.new(solr_hit).iiif_image_url(size: '!600,600') rescue StandardError nil end diff --git a/app/helpers/catalog_helper.rb b/app/helpers/catalog_helper.rb index 61b83d4ef..5eea666ab 100644 --- a/app/helpers/catalog_helper.rb +++ b/app/helpers/catalog_helper.rb @@ -15,11 +15,9 @@ def thumbnail_url document else representative_document = ::SolrDocument.find(document.thumbnail_id) end - - thumbnail_file_id = representative_document&.content_location - thumbnail_file_id ||= representative_document.original_file_id - if thumbnail_file_id - Hyrax.config.iiif_image_url_builder.call(thumbnail_file_id, nil, '250,') + iiif_path_service = IIIFFileSetPathService.new(representative_document) + if iiif_path_service.lookup_id + iiif_path_service.iiif_image_url(size: '250,') else raise 'thumbnail_file_id is nil' end diff --git a/app/models/collection_branding_info.rb b/app/models/collection_branding_info.rb index c4d5fec25..bf541f21c 100644 --- a/app/models/collection_branding_info.rb +++ b/app/models/collection_branding_info.rb @@ -95,16 +95,10 @@ def find_local_dir_name(collection_id, role) File.join(Hyrax.config.branding_path, collection_id.to_s, role.to_s) end - # this passes a nil value for request base_url, as our custom url builder - # does not use that argument, and the model also doesn't have a request def generate_image_path! if image_path.blank? && file_set_versions.any? - original_uri = file_set_versions.all.last.uri - uri_to_id = ActiveFedora::File.uri_to_id(original_uri.sub(/\/fcr.versions.*/,'')) - self.image_path = \ - Hyrax.config.iiif_image_url_builder.call(uri_to_id, - nil, - ESSI.config.dig(:essi, :collection_banner_size) || Hyrax.config.iiif_image_size_default) + return unless iiif_path_service.lookup_id + self.image_path = iiif_path_service.iiif_image_url(size: ESSI.config.dig(:essi, :collection_banner_size)) save end end @@ -117,4 +111,8 @@ def uploaded_files(uploaded_file_ids) return [] if uploaded_file_ids.empty? Hyrax::UploadedFile.find(uploaded_file_ids) end + + def iiif_path_service + @iiif_path_service ||= IIIFFileSetPathService.new(file_set) + end end diff --git a/app/models/file_set.rb b/app/models/file_set.rb index bb3b679e3..e9f22d8d7 100644 --- a/app/models/file_set.rb +++ b/app/models/file_set.rb @@ -22,4 +22,20 @@ def ocr_language ESSI.config.dig(:essi, :ocr_language), 'eng'].map { |l| Tesseract.try_languages(l) }.select(&:present?).first end + + # @todo revisit after Hyrax 3.x upgrade + # copied from Hyrax::FileSetIndexer to provide a common interface across: + # - FileSet + # - SolrDocument + # - Hyrax::FileSetPresenter, initialized with either of above + def original_file_id + @original_file_id ||= begin + return unless self.original_file + if self.original_file.versions.present? + ActiveFedora::File.uri_to_id(self.current_content_version_uri) + else + self.original_file.id + end + end + end end diff --git a/app/presenters/hyrax/displays_image.rb b/app/presenters/hyrax/displays_image.rb index 4edc2f7f5..5e9967033 100644 --- a/app/presenters/hyrax/displays_image.rb +++ b/app/presenters/hyrax/displays_image.rb @@ -8,62 +8,31 @@ module Hyrax module DisplaysImage extend ActiveSupport::Concern - delegate :content_location, to: :solr_document - # Creates a display image only where FileSet is an image. # # @return [IIIFManifest::DisplayImage] the display image required by the manifest builder. def display_image return nil unless solr_document.image? && current_ability.can?(:read, id) - - latest_file_id = lookup_original_file_id - - return nil unless latest_file_id - - url = Hyrax.config.iiif_image_url_builder.call( - latest_file_id, - request.base_url, - Hyrax.config.iiif_image_size_default - ) + return nil unless iiif_path_service.lookup_id # @see https://github.com/samvera-labs/iiif_manifest - IIIFManifest::DisplayImage.new(url, + IIIFManifest::DisplayImage.new(iiif_path_service.iiif_image_url, width: width, height: height, - iiif_endpoint: iiif_endpoint(latest_file_id)) - end - - def lookup_original_file_id - return content_location if content_location&.start_with?('s3://') - result = original_file_id - if result.blank? - Rails.logger.warn "original_file_id for #{id} not found, falling back to Fedora." - # result = Hyrax::VersioningService.versioned_file_id ::FileSet.find(id).original_file - result = versioned_file_id ::FileSet.find(id).original_file - end - result + iiif_endpoint: iiif_endpoint) end private + def iiif_path_service + @iiif_path_service ||= IIIFFileSetPathService.new(solr_document, versioned_lookup: true) + end - def iiif_endpoint(file_id) + def iiif_endpoint return unless Hyrax.config.iiif_image_server? IIIFManifest::IIIFEndpoint.new( - Hyrax.config.iiif_info_url_builder.call(file_id, request.base_url), + iiif_path_service.iiif_info_url(request.base_url), profile: Hyrax.config.iiif_image_compliance_level_uri ) end - - # @todo remove after upgrade to Hyrax 3.x - # cherry-picked from Hyrax 3.x VersioningService - # @param [ActiveFedora::File | Hyrax::FileMetadata] content - def versioned_file_id(file) - versions = file.versions.all - if versions.present? - ActiveFedora::File.uri_to_id versions.last.uri - else - file.id - end - end end end diff --git a/app/services/essi/generate_pdf_service.rb b/app/services/essi/generate_pdf_service.rb index 898017f26..466058a94 100644 --- a/app/services/essi/generate_pdf_service.rb +++ b/app/services/essi/generate_pdf_service.rb @@ -44,7 +44,9 @@ def create_tmp_files(pdf) fs_solr = SolrDocument.find(fs) image_width = get_image_width(fs_solr).to_i raise StandardError, 'Image width unavailable' unless image_width > 0 # IIIF server call requires a positive integer value - uri = Hyrax.config.iiif_image_url_builder.call(fs_solr.original_file_id, nil, render_dimensions(image_width)) + iiif_path_service = IIIFFileSetPathService.new(fs_solr) + raise StandardError, 'Source image file unavailable' unless iiif_path_service.lookup_id + uri = iiif_path_service.iiif_image_url(size: render_dimensions(image_width)) URI.open(uri) do |file| page_size = [CoverPageGenerator::LETTER_WIDTH, CoverPageGenerator::LETTER_HEIGHT] file.binmode diff --git a/app/services/iiif_collection_thumbnail_path_service.rb b/app/services/iiif_collection_thumbnail_path_service.rb index 9fddc1413..b0b34e711 100644 --- a/app/services/iiif_collection_thumbnail_path_service.rb +++ b/app/services/iiif_collection_thumbnail_path_service.rb @@ -3,7 +3,7 @@ class << self # @return the network path to the thumbnail # @param [FileSet] thumbnail the object that is the thumbnail def thumbnail_path(thumbnail) - Hyrax.config.iiif_image_url_builder.call(thumbnail.original_file.id, nil, '250,') + IIIFFileSetPathService.new(thumbnail).iiif_image_url(size: '250,') # Hyrax::Engine.routes.url_helpers.download_path(thumbnail.id, # file: 'thumbnail') end diff --git a/app/services/iiif_file_set_path_service.rb b/app/services/iiif_file_set_path_service.rb new file mode 100644 index 000000000..f12d176ac --- /dev/null +++ b/app/services/iiif_file_set_path_service.rb @@ -0,0 +1,88 @@ +class IIIFFileSetPathService + attr_reader :file_set, :versioned_lookup + + # @param [ActiveFedora::SolrHit, FileSet, Hash, SolrDocument, Hyrax::FileSetPresenter] file_set + # @param [TrueClass, FalseClass, NilClass] versioned_lookup: whether to use versioned file lookup if original_file_id fails + def initialize(file_set, versioned_lookup: false) + @versioned_lookup = versioned_lookup + file_set = SolrDocument.new(file_set) if file_set.class.in? [ActiveFedora::SolrHit, Hash] + if [:id, :content_location, :original_file_id].map { |m| file_set.respond_to?(m) }.all? + @file_set = file_set + else + raise StandardError, 'Provided file_set does not meet interface requirements' + end + end + + def lookup_id + @lookup_id ||= (@versioned_lookup ? versioned_lookup_id : basic_lookup_id) + end + + # @return [String] a URL that resolves to an image provided by a IIIF image server + def iiif_image_url(base_url: nil, size: nil) + return unless lookup_id + Hyrax.config.iiif_image_url_builder.call(lookup_id, base_url, size || Hyrax.config.iiif_image_size_default) + end + + # @return [String] a URL that resolves to an info.json file provided by a IIIF image server + def iiif_info_url(base_url) + return unless lookup_id + Hyrax.config.iiif_info_url_builder.call(lookup_id, base_url) + end + + private + # imported logic from IIIFThumbnailPathService, etc + def basic_lookup_id + @basic_lookup_id ||= (file_set.content_location || file_set.original_file_id) + end + + # imported from Hyrax::DisplaysImage + def versioned_lookup_id + @versioned_lookup_id ||= begin + return file_set.content_location if file_set.content_location&.start_with?('s3://') + result = file_set.original_file_id + if result.blank? + Rails.logger.warn "original_file_id for #{file_set.id} not found, falling back to Fedora." + # result = Hyrax::VersioningService.versioned_file_id(original_file) + result = versioned_file_id(original_file) if original_file + + end + if result.blank? + Rails.logger.warn "original_file for #{file_set.id} not found, versioned_lookup_id failed." + nil + else + result + end + end + end + + # @return [Hydra::PCDM::File, nil] + def original_file + @original_file ||= + case file_set + when FileSet + file_set.original_file + else + begin + FileSet.find(file_set.id).original_file + rescue => error + Rails.logger.error "original_file lookup for #{file_set.id} raised error: #{error.inspect}" + nil + end + end + end + + # @todo remove after upgrade to Hyrax 3.x + # cherry-picked from Hyrax 3.x VersioningService + # @param [ActiveFedora::File | Hyrax::FileMetadata] content + def versioned_file_id(file) + @versioned_file_id ||= begin + raise StandardError, 'No original_file available for versioning' if file.nil? + versions = file.versions.all + if versions.present? + ActiveFedora::File.uri_to_id versions.last.uri + else + file.id + end + end + end +end diff --git a/app/services/iiif_thumbnail_path_service.rb b/app/services/iiif_thumbnail_path_service.rb index f33cb74db..029f81e97 100644 --- a/app/services/iiif_thumbnail_path_service.rb +++ b/app/services/iiif_thumbnail_path_service.rb @@ -3,9 +3,7 @@ class << self # @return the network path to the thumbnail # @param [FileSet] thumbnail the object that is the thumbnail def thumbnail_path(thumbnail) - return unless thumbnail.original_file - id = thumbnail.content_location || thumbnail.original_file.id - Hyrax.config.iiif_image_url_builder.call(id, nil, '250,') + IIIFFileSetPathService.new(thumbnail).iiif_image_url(size: '250,') # Hyrax::Engine.routes.url_helpers.download_path(thumbnail.id, # file: 'thumbnail') end diff --git a/lib/extensions/extensions.rb b/lib/extensions/extensions.rb index b0b7b1038..433772c8c 100644 --- a/lib/extensions/extensions.rb +++ b/lib/extensions/extensions.rb @@ -187,3 +187,6 @@ def attribute_will_change!(attr) # support for rendering an orphan FileSet Hyrax::FileSetsController.prepend Extensions::Hyrax::FileSetsController::RenderOrphanFileSet + +# support for FileSetPresenter#content_location +Hyrax::FileSetPresenter.include Extensions::Hyrax::FileSetPresenter::ContentLocation diff --git a/lib/extensions/hyrax/file_set_presenter/content_location.rb b/lib/extensions/hyrax/file_set_presenter/content_location.rb new file mode 100644 index 000000000..f270a21ad --- /dev/null +++ b/lib/extensions/hyrax/file_set_presenter/content_location.rb @@ -0,0 +1,9 @@ +module Extensions + module Hyrax + module FileSetPresenter + module ContentLocation + delegate :content_location, to: :solr_document + end + end + end +end diff --git a/spec/services/iiif_file_set_path_service_spec.rb b/spec/services/iiif_file_set_path_service_spec.rb new file mode 100644 index 000000000..af80df329 --- /dev/null +++ b/spec/services/iiif_file_set_path_service_spec.rb @@ -0,0 +1,111 @@ +require 'rails_helper' + +RSpec.describe IIIFFileSetPathService do + let(:base_url) { 'http://localhost:3000/' } + let(:content_location) { 's3://localhost:9000/essi-test/ext-store/12/34/ab/cd/1234abcd-original_file.ptif' } + let(:local_file) { File.open(RSpec.configuration.fixture_path + '/world.png') } + # must explicitly define :file_set, normally as one of three following options + let(:empty_file_set) { FactoryBot.create(:file_set) } + let(:local_file_set) { FactoryBot.create(:file_set, content: local_file) } + let(:remote_file_set) { FactoryBot.create(:file_set, content_location: content_location) } + let(:solr_hit) { FileSet.search_with_conditions(id: file_set.id).first } + let(:solr_document) { SolrDocument.new(solr_hit) } + # must explicitly define :source if using file_set_presenter + let(:file_set_presenter) { Hyrax::FileSetPresenter.new(source, Ability.new(FactoryBot.build(:user))) } + let(:versioned_lookup) { false } # default, override when desired + # must explicitly define :resource + let(:service) { described_class.new(resource, versioned_lookup: versioned_lookup) } + + # requires defining lookup_id, url + shared_examples "url examples" do + context "without a lookup_id value" do + let(:file_set) { empty_file_set } + it "returns nil" do + expect(lookup_id).to be_nil + expect(url).to be_nil + end + end + context "with an original_file_id lookup_id value" do + let(:file_set) { local_file_set } + it "returns a url" do + expect(lookup_id).not_to match /^s3/ + expect(url).to match /^http/ + end + end + context "with a content_location lookup_id value" do + let(:file_set) { remote_file_set } + it "returns a url" do + expect(lookup_id).to match /^s3/ + expect(url).to match /^http/ + end + end + context "with versioned_file_id lookup" do + let(:file_set) { empty_file_set } + let(:versioned_lookup) { true } + context "that fails" do + it "returns nil" do + expect(lookup_id).to be_nil + expect(url).to be_nil + end + end + context "that succeeds" do + before do + allow(service).to receive(:original_file).and_return(local_file_set.original_file) + end + it "returns a url" do + expect(lookup_id).not_to match /^s3/ + expect(url).to match /^http/ + # below is proxy check that #versioned_file_id was called + expect(service.instance_variable_get(:@versioned_file_id)).not_to be_nil + end + end + end + end + + # requires defining resource + shared_examples "#iiif_(image|info)_url examples" do + let(:lookup_id) { service.lookup_id } + describe "#iiif_image_url" do + let(:url) { service.iiif_image_url } + include_examples "url examples" + end + describe "#iiif_info_url" do + let(:url) { service.iiif_info_url(base_url) } + include_examples "url examples" + end + end + + context "for an ActiveFedora::SolrHit" do + let(:resource) { solr_hit } + include_examples "#iiif_(image|info)_url examples" + end + + context "for a FileSet" do + context "directly" do + let(:resource) { file_set } + include_examples "#iiif_(image|info)_url examples" + end + context "sourcing a Hyrax::FileSetPresenter" do + let(:source) { file_set } + let(:resource) { file_set_presenter } + include_examples "#iiif_(image|info)_url examples" + end + end + + context "for a SolrDocument" do + context "directly" do + let(:resource) { solr_document } + include_examples "#iiif_(image|info)_url examples" + end + context "sourcing a Hyrax::FileSetPresenter" do + let(:source) { solr_document } + let(:resource) { file_set_presenter } + include_examples "#iiif_(image|info)_url examples" + end + end + + context "for a Hash" do + let(:resource) { solr_document.to_h } + include_examples "#iiif_(image|info)_url examples" + end +end