Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Include fcr:version in IIIF image URI #3165

Closed
wants to merge 1 commit into from
Closed

Include fcr:version in IIIF image URI #3165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/presenters/hyrax/displays_image.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@ def display_image
return nil unless ::FileSet.exists?(id) && solr_document.image? && current_ability.can?(:read, id)
# @todo this is slow, find a better way (perhaps index iiif url):
original_file = ::FileSet.find(id).original_file
latest_file_id = original_file.has_versions? ? ActiveFedora::File.uri_to_id(original_file.versions.last.uri) : original_file.id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlpierce - I was just wondering - does this need to be encoded? So that Apache or the like doesn't receive a decoded path? I am learning a bit here and in my Suifa application, I had to apply your spec's ActionDispatch::Journey::Router::Utils.escape_segment(uri) above so that the image requests went through ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, original_file.has_versions? always seems to return true even when a file_set has only one version, so our iiif_endpoints changed for all our manifests, invalidating Rails.cache entries. Perhaps something like this is needed?:

latest_file_id = original_file.versions.count > 1 ? ActiveFedora::File.uri_to_id(original_file.versions.last.uri) : original_file.id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of has_versions? was meant as a safeguard in case a file did not have support for versioning, although I do not know if that is really an issue in any application.

For now I'm working on the potential timestamp based solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the knowledge. And thanks for getting this solution going so we could begin applying it to Sufia.

Not sure if it helps to share, but we are experiencing some older version Riiif tiles still when a new file_set version derivative is not completed yet. FileSetPresenter#display_image returns the new iiif_endpoint using the new Fedora file version id, but when the images requests are sent to Riiif, the new file id invalidates the Rails.cache keys, sparking the file resolver to check it's cache for a matching hashed file name, but it's checking the derivative path and the timestamp of the old version. That new file can sometimes not be ready yet, since it is created in a background job.

What i plan to do is control the iiif_endpoint change from some sort of after_perform on the CreateDerivativesJob - that way the derivative file will be ready when the HTTPFileResolver fetches.

What I'm not 100% sure of yet is how I will control the versioning. I may just increment some solr field on the file_set that acts as the version.

Any thoughts from the community are welcome, even those that correct my thinking. All of this is new to me and I may be a smidgeon off.

Best,
Kevin


url = Hyrax.config.iiif_image_url_builder.call(
original_file.id,
latest_file_id,
request.base_url,
Hyrax.config.iiif_image_size_default
)
# @see https://github.com/samvera-labs/iiif_manifest
IIIFManifest::DisplayImage.new(url,
width: 640,
height: 480,
iiif_endpoint: iiif_endpoint(original_file.id))
iiif_endpoint: iiif_endpoint(latest_file_id))
end

private
Expand Down
14 changes: 9 additions & 5 deletions spec/presenters/hyrax/file_set_presenter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,15 @@
end

describe 'IIIF integration' do
def uri_segment_escape(uri)
ActionDispatch::Journey::Router::Utils.escape_segment(uri)
end

let(:file_set) { create(:file_set) }
let(:solr_document) { SolrDocument.new(file_set.to_solr) }
let(:request) { double(base_url: 'http://test.host') }
let(:presenter) { described_class.new(solr_document, ability, request) }
let(:id) { CGI.escape(file_set.original_file.id) }
let(:id) { ActiveFedora::File.uri_to_id(file_set.original_file.versions.last.uri) }
let(:read_permission) { true }

before do
Expand Down Expand Up @@ -345,7 +349,7 @@
end

it { is_expected.to be_instance_of IIIFManifest::DisplayImage }
its(:url) { is_expected.to eq "http://test.host/images/#{id}/full/600,/0/default.jpg" }
its(:url) { is_expected.to eq "http://test.host/images/#{uri_segment_escape(id)}/full/600,/0/default.jpg" }

context 'with custom image size default' do
let(:custom_image_size) { '666,' }
Expand All @@ -358,7 +362,7 @@
end

it { is_expected.to be_instance_of IIIFManifest::DisplayImage }
its(:url) { is_expected.to eq "http://test.host/images/#{id}/full/#{custom_image_size}/0/default.jpg" }
its(:url) { is_expected.to eq "http://test.host/images/#{uri_segment_escape(id)}/full/#{custom_image_size}/0/default.jpg" }
end

context 'with custom image url builder' do
Expand Down Expand Up @@ -388,7 +392,7 @@
end

describe "#iiif_endpoint" do
subject { presenter.send(:iiif_endpoint, file_set.original_file.id) }
subject { presenter.send(:iiif_endpoint, id) }

before do
allow(Hyrax.config).to receive(:iiif_image_server?).and_return(riiif_enabled)
Expand All @@ -399,7 +403,7 @@
context 'with iiif_image_server enabled' do
let(:riiif_enabled) { true }

its(:url) { is_expected.to eq "http://test.host/images/#{id}" }
its(:url) { is_expected.to eq "http://test.host/images/#{uri_segment_escape(id)}" }
its(:profile) { is_expected.to eq 'http://iiif.io/api/image/2/level2.json' }

context 'with a custom iiif image profile' do
Expand Down