Include fcr:version in IIIF image URI

[dlpierce]

Fixes #2910

Include fcr:version in IIIF image URI so the cached file's hashed filename created by riiif is version specific.
Use Rails' URI escape library (instead of CGI) in the spec because colon is handled differently.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

• Create a work with an image file.
• Add a new visually distinct version of the image file.
• Verify the universal viewer displays the new version on the work show page.

@samvera/hyrax-code-reviewers

[RudyOnRails]

Hi @dlpierce - we've been anticipating this one!

What are your thoughts on this patch also being applied to Sufia while using the Universal Viewer? Would it work the same way?

[cjcolvar]

This is a creative approach! I think this could work, but am still wondering about IIIF manifests that are already in the wild. It seems like some guidance might be useful in the release notes. I think expiring the RIIIF cache after upgrading with this patch would temporarily solve this issue for both new and existing manifests. I'm guessing that if new versions are added for a file then it will still affect old manifests unless local RIIIF cache expiration happens.

[RudyOnRails]

@cjcolvar - I agree, I believe the final solution will involve two parts. The iiif_endpoint change (which @dlpierce did above), and a change to the HTTPFileResolver.

Like you said above, only newly cached network_files will work okay. As soon as a new version is ingested, and this runs again:

      def fetch
        download_file unless ::File.exist?(file_name)
        file_name
      end

the previous version's image will be displayed since unless ::File.exist?(file_name) will return true. As @jcoyne and you mentioned we'd need to improve the resolver like you suggested. I'm toying around with a url + timestamp idea now.

[dlpierce]

Using the file's parent FileSet timestamp from Solr might be a better performing solution since it should be able to avoid some requests to Fedora to determine available versions. There is still the preexisting original_file request, however.

Perhaps the riiif resover could compare its cached file's filesystem mtime to the FileSet timestamp?

[cjcolvar]

Looking at this again I have a small concern about manifests being downloaded after this change is applied and then being stuck with fedora specific path elements in the uri. Upgrading to a valkyrie-based Hyrax would probably break these uris unless special routes were added to continue to support them. I'm not sure if that should block this PR though which makes real progress in fixing a bug.

@dlpierce Maybe the timestamp approach you mention would deal with this concern?
@no-reply @vantuyls @chrisdaaz What are your thoughts?

[jcoyne]

Another way this could be fixed would be to create a listener to the Fedora messages and invalidate the cache entries when an appropriate message is received.

[vantuyls]

@cjcolvar i'd be interested to hear @no-reply and others thoughts on whether we should be predictively flagging PRs based on what we think valkyrie based hyrax is going to look like. should we, instead, be flagging this type of thing as work we need to do on the valkyrie - hyrax sprints to ensure that this doesn't break?

[RudyOnRails]

@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.

[RudyOnRails]

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

[dlpierce]

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.

[RudyOnRails]

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

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[no-reply]

@dlpierce how can we unblock finishing this work?

[dlpierce]

@no-reply I believe I can focus on this now that CircleCI is usable.

[ghost]

@dlpierce any updates on this? tx

[dlpierce]

I've been working on an alternative approach that doesn't modify the URIs, but instead compares timestamps. I plan to put that work in its own PR, so this one could be closed unless people think it should be used after all.

[cjcolvar]

Closed per suggestion from @dlpierce