scientist-softserv / iiif_print

A gem for Hyrax/Samvera for displaying PDF pages in a IIIF Compliant viewer
Apache License 2.0
4 stars 1 forks source link

🎁 Adding DerivativeRodeoService#cleanup_derivatives ♻️ #271

Closed jeremyf closed 1 year ago

jeremyf commented 1 year ago

Apologies in advance, this commit conflates two things, but I'll explain.

This commit is in service of completing the DerivativeService interface; namely the #cleanup_derivatives method.

Originally, I was thinking I would only delete the derivatives generated by this service. So I began refactoring to reduce duplicated knowledge. That refactor meant extracting #named_derivatives_and_generators, and as a matter of hygiene and legibility, I moved the method closer to the configuration. The hope being that if one thing changes the other might.

This then involved rethinking the #create_derivatives and #cleanup_derivatives to use this new method. I was looking for symmetry in method implementation (e.g. loop over the named derivatives and either create them or delete them).

However, as I looked at the other reference implementations I noticed that I could get all of the derivatives by calling Hyrax::DerivativePath.derivatives_for_reference (see code). I spent a bit of time thinking, as the comments indicate, as to which approach to take: delete all derivatives OR only those that would be created by the present configuration.

It makes sense to me to delete all of them, in part due to the implementation details of finding the correct valid? derivative service but also the fact that any valid? service is subject to configuration, which might change over time, and thus leave orphaned derivatives dangling in the file system.

Closes #270

Related to: