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.

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

Filehash Fixes and improve checksum view #912

Merged
merged 3 commits into from
Dec 14, 2022

Conversation

rosiel
Copy link
Member

@rosiel rosiel commented Nov 30, 2022

GitHub Issue: Islandora/documentation#1931

Islandora requires the filehash module, and provides (in the core features) settings(!!) and a view that use it.

Since the update to filehash 2, the view has been broken. (Filehash 2 stores data in fields on files, so the plugin handler was different)

Also! Filehash 2 provides some great features:

  • store the "original" checksum of the file (on the creation of the file entity)
  • re-hash the checksum any time the file changes.

Is it the be-all-and-end-all of preservation fixity for a repository? No! lol, but we can do a lot more with it now than we could before.

What does this Pull Request do?

Updates the settings and view to work with the new filehash module.

What's new?

  • Turn on features to re-hash files when changed, and store the hash of the original.
  • For the view that exposes checksums at /checksum/{file_id}, include the original hash, and lock it down so that you need 'View checksums' permission to see the view (That permission had been defined, but seemingly never used?)
  • Does this change add any new dependencies? Changes dependencies - filehash 2.
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? Upgrade will involve regenerating filehashes.
  • Could this change impact execution of existing code? I can't tell if the view's contents are used anywhere. Even if they are, I added an additional element and left what was in place.

How should this be tested?

Load the starter site. (or try to update filehash to 2.x on the install profile?)
Go to the View "File checksums" and see it's missing a handler.
Load this PR.
Go to the Islandora Core Feature, in Features.
Select views.view.file_checksum and filehash.settings, and import the changes.
Create some test files.
Go to the View. It should not be broken. Test at /checksums/1 (replace 1 with your file id) and see a JSON array containing checksums, including 'sha1andoriginal_sha1`

Screen Shot 2022-11-30 at 11 03 36 AM

Note: Derivatives will have empty original_sha1. The derivative code creates the file empty, then updates it with the content.

Documentation Status

  • Does this change existing behaviour that's currently documented? lol, not documented
  • Does this change require new pages or sections of documentation? yeah, maybe
  • Who does this need to be documented for? site builders
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@rosiel rosiel marked this pull request as draft December 13, 2022 19:01
@rosiel
Copy link
Member Author

rosiel commented Dec 13, 2022

Found another thing to add to this.

  • the new structure of filehash is causing our check in hook_update_file() to fail, meaning we create double derivatives.
  • Filehash is failing during the derivative creation process, when we create a file object that has no actual file in it. we could maybe fix?

@rosiel
Copy link
Member Author

rosiel commented Dec 13, 2022

OK, the latest commit 7df45a0 should fix the check in hook_file_update() and the double derivative thing.

there's kind of a larger issue here that we could use filehash more effectively/reliably but I think that's a separate issue

@nigelgbanks nigelgbanks self-requested a review December 14, 2022 09:32
@nigelgbanks nigelgbanks marked this pull request as ready for review December 14, 2022 10:40
Copy link
Member

@nigelgbanks nigelgbanks left a comment

Choose a reason for hiding this comment

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

Reviewed and tested. Working as intended.

@nigelgbanks nigelgbanks merged commit f63dce6 into Islandora:2.x Dec 14, 2022
@rosiel rosiel mentioned this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants