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

Removes FileSet related objects when work deleted (#6334). #6357

Merged
merged 16 commits into from
Oct 17, 2023

Conversation

bwatson78
Copy link
Contributor

@bwatson78 bwatson78 commented Oct 10, 2023

Fixes

Fixes #6334

Summary

Adds transaction triggers that removes FileSets' Hyrax::FileMetadata and AccessControl objects.

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

Type of change (for release notes)

notes-valkyrie Valkyrie Progress

Detailed Description

Adds transaction triggers that removes FileSets' Hyrax::FileMetadata and AccessControl objects.

Changes proposed in this pull request:

  • Override lib/hyrax/transactions/work_destroy.rb's #call to process private methods that find the Work's associated FileSets and remove the previously orphaned Hyrax::FileMetadata and AccessControl objects tie to them.

@samvera/hyrax-code-reviewers

@bwatson78 bwatson78 self-assigned this Oct 10, 2023
@bwatson78 bwatson78 added notes-valkyrie Release Notes: Valkyrie specific valkyrie-fedora labels Oct 10, 2023
@dlpierce
Copy link
Contributor

The pattern for transactions/steps has been to put all of the actual work in a step and keep transactions contain only a list of steps to take, so this should be moved to a step.

I wonder if UpdateWorkMembers could be of use?

@bwatson78
Copy link
Contributor Author

@dlpierce I've added my best attempt of rspec here. I tried to get a work with a fileset that contained a file with an associated FileMetadata object, but I couldn't get one going.

@bwatson78 bwatson78 marked this pull request as ready for review October 11, 2023 19:04
@dlpierce
Copy link
Contributor

I tried this out on clean koppie and sirenia.

  • I created a generic work with one file.
  • Sirenia: Everything related to the work is cleared from fedora, although solr still has records for two FileMetadata objects.
  • Koppie: Postgres still contains one FileSet object, solr has records for two FileMetadata objects and one FileSet

It looks like the steps do not actually remove the FileSet as it is now, so I'm curious how it was removed from Fedora

@bwatson78
Copy link
Contributor Author

I forgot to check Koppie--apologies for that. When we last discussed the work deletion behavior, I commented how it appears the object created by the resource_factory that is sent to Fedora to delete the work included references to the associated FileSet. Looking for that now...

@bwatson78
Copy link
Contributor Author

For the Solr objects--should I just kick off a full reindex?

@bwatson78
Copy link
Contributor Author

The resource factory here creates a graph that references the FileSet when forming the object to send to the Fedora API. Apparently, the API takes that as a deletion dependency...I guess. But that's my best guess.

@bwatson78
Copy link
Contributor Author

Should I reverse tactics and experiment with explictly deleting all FileSets attached to a Work before deleting the Work itself? See if that takes care of all of the FileSets' children without having to call all of the other steps?

@bwatson78 bwatson78 force-pushed the remove_file_set_relatives_work_del_6334 branch from d96404a to 81fc50f Compare October 12, 2023 19:24
@dlpierce
Copy link
Contributor

The deletion dependency seems possible. The way I see it working is the destroy work steps should include a call to a step to destroy the FileSet, which then destroys its FileMetadata objects. I think the generic delete resource step can be used at each level to carry out the actual deletion? That way the number of places in code concerned with doing a given action is kept to a minimum.

@bwatson78
Copy link
Contributor Author

Alrighty, I'll see how making a call to delete all FileSets before deleting the Work goes.

@bwatson78
Copy link
Contributor Author

@dlpierce When attempting to call the Hyrax::Transactions::Container['file_set.destroy'] from within the same step I've been working on, I received an LDP::Gone error from this file: app/services/hyrax/listeners/member_cleanup_listener.rb. I realized that the transaction wasn't set up there, like it should be. That's why we still have orphans (and why work_resource.destroy deletes the FileSet--that is, at least for Fedora. I don't know why this doesn't work for PG yet.)

This "fix" doesn't fix everything, though. An ACL for the FileSet is still left behind. And, for some reason, Solr is left with the AdminSet and 2 FileMetadata objects (which are gone from Fedora.) I think I'll add a full reindex call into the same method I've already altered.

@bwatson78
Copy link
Contributor Author

@dlpierce I've removed all of the errant objects (Solr and Fedora) on the Sirenia side. I've run out of time for today, so I couldn't get to Koppie testing or rspec. See you on Monday the 16th.

@bwatson78
Copy link
Contributor Author

This should be ready for review. The Koppie side of thing is affected by the Work being removed before queried for its child members.

If the list of member file sets is no longer available after a work resource is deleted, it will not be possible to perform the cleanup.
Therefore file sets (and their file metadata) must be deleted prior to the work's deletion during the transaction call.
The member cleanup listener is no long concerned with deleting file sets.
@dlpierce dlpierce merged commit 4f60343 into main Oct 17, 2023
@dlpierce dlpierce deleted the remove_file_set_relatives_work_del_6334 branch October 17, 2023 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-valkyrie Release Notes: Valkyrie specific valkyrie-fedora
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In valkyrie/fedora deleting a work leaves orphan FileMetadata objects
2 participants