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

Fix sipity with migrations #2471

Merged
merged 5 commits into from
Feb 27, 2025
Merged

Fix sipity with migrations #2471

merged 5 commits into from
Feb 27, 2025

Conversation

laritakr
Copy link
Collaborator

@laritakr laritakr commented Feb 24, 2025

Summary

Refs

Workflow management is broken between pre-valkyrie works and valkyrie resources. Due to how the proxy_for_global_id was being built, the Entity could not be found, resulting in no valid workflows being found either. This resulted in numerous errors.

f3feb43 begins to work on the fix, by reworking how we find the entity for a work/resource. There may still be remaining issues.

This work should be moved into Hyrax once it is fully fixed.

The referenced PR is merged and override is not neded.
@laritakr laritakr added patch-ver for release notes bug labels Feb 24, 2025
Copy link

github-actions bot commented Feb 24, 2025

Test Results

    3 files  ±0      3 suites  ±0   17m 40s ⏱️ +7s
2 091 tests +4  2 035 ✅ +4  56 💤 ±0  0 ❌ ±0 
2 118 runs  +4  2 060 ✅ +4  58 💤 ±0  0 ❌ ±0 

Results for commit 7be09b7. ± Comparison against base commit 3189d29.

This pull request removes 42 and adds 46 tests. Note that renamed tests count towards both.
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 61b51952-ff7d-496b-9697-8f95d9ecdf3d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit a6a7c84c-9a8a-4d92-b852-4b2152e38f8b
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 16ca0d91-58fc-4b1e-b771-16d0e3e80141
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update ec2d9140-d06b-49bf-a8ce-bd15c86a1862
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 72f143d1-d762-4aeb-a32f-1cbccb2a75dd
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit e80d3d18-633e-4156-b45d-3bd288c841a3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 9ece69c5-af73-484d-aa83-2eabb1b90dac
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update bd57a63a-0331-4037-b37b-99f34c6cb54d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy ed6cf8c9-3b50-4fb9-86ce-7cc82adbdef5
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit a71502ca-59a5-4c26-aa58-b9cb21767d10
…
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 5be61cec-513e-4e7c-baf0-28a1f7290e73
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit 3fce331b-98ff-4d99-ac4f-a82e6749765a
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read c8a7d059-f0b1-41a5-a0aa-fbb3111c5522
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update 4e5d8fda-d3ce-4a65-82c4-51f398513cfc
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 2c4844a4-46b0-4914-abbe-026566eab1ea
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit f0369288-f0c6-4233-bca6-7324d8d1e5bf
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read c5954e54-9dbf-4aeb-bf45-d9289abe5b6b
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 72a49f93-17e8-440b-afac-144a098dc3fc
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy 99bf1912-0e34-42fb-b909-89fbeaac501c
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit e6f49dba-55d5-4454-a39d-13944d771a87
…

♻️ This comment has been updated with latest results.

LaRita Robinson added 2 commits February 24, 2025 13:21
Sipity is unable to find the entities due to migrations. Errors manifest
in several situations, including:
- a fedora work searches for the entity by solr document. Due to the
lazy migration option, it can find the resource model rather than the
original model, resulting in an incorrect key proxy gid when trying
to find the entity.
- a migrated work may result in a new proxy gid when attempting to find
the sipity entity, resulting in the entity not being found. Sipity
entities need to be migrated as works are migrated.
@laritakr laritakr force-pushed the fix-sipity-with-migrations branch from f3feb43 to f67534d Compare February 24, 2025 18:21
@laritakr laritakr marked this pull request as draft February 24, 2025 19:17
@laritakr laritakr force-pushed the fix-sipity-with-migrations branch from b59bcf6 to 7be09b7 Compare February 27, 2025 04:52
@laritakr laritakr marked this pull request as ready for review February 27, 2025 04:53
Copy link
Member

@orangewolf orangewolf left a comment

Choose a reason for hiding this comment

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

I don't think this covers all possible cases, but it definitely covers some cases. going to approve it so I don't hold up the train, but I'll revisit it with a fresh brain tomorrow.

@laritakr laritakr merged commit 0546192 into main Feb 27, 2025
8 checks passed
@laritakr laritakr deleted the fix-sipity-with-migrations branch February 27, 2025 16:16
@laritakr laritakr restored the fix-sipity-with-migrations branch February 27, 2025 16:31
laritakr added a commit that referenced this pull request Feb 27, 2025
* Remove unnecessary override

The referenced PR is merged and override is not neded.

* Override Sipity for Migrating Works

Sipity is unable to find the entities due to migrations. Errors manifest
in several situations, including:
- a fedora work searches for the entity by solr document. Due to the
lazy migration option, it can find the resource model rather than the
original model, resulting in an incorrect key proxy gid when trying
to find the entity.
- a migrated work may result in a new proxy gid when attempting to find
the sipity entity, resulting in the entity not being found. Sipity
entities need to be migrated as works are migrated.

* Fix broken specs
laritakr pushed a commit to samvera/hyrax that referenced this pull request Mar 6, 2025
When lazy migration is used, the sipity entity cannot be found for works
which are not yet migrated. Valkyrie resources have a `proxy_for_global_id`
with the format:
`gid://hyku/Hyrax::ValkyrieGlobalIdProxy/2f6da5dd-9314-421f-b0dd-fda84fec9ee3`
while prior works used the model name such as:
`gid://hyku/GenericWork/2f6da5dd-9314-421f-b0dd-fda84fec9ee3`.

This commit introduces a lazy migration of the sipity entity via a job
which is submitted after a work is migrated to valkyrie via the Freyja
persister.

Prior work in pull request samvera/hyku#2471
added some overrides in Hyku to resolve the sipity entity issues. These
changes still needs to be backported.

The prior work includes these changes:
The MigrateResourceService introduced a migration of the entity along
with the work's migration. This service is needed for direct migration
calls, but is not adequate for lazy migration, as the Freyja persister
does not call it for the work itself that was just migrated.

Hyku overrides to sipity.rb support cases where the entity is not yet migrated
and lazy migration is in use. This is necessary because the solr_document
is used to find the Sipity::Entity. Due to the model mapping found in the
solr document, the Entity method searches for the entity with the model
name. This is problematic when the entity is not yet migrated and the
entity is searched with a GenericWorkResource model rather than the
model.
laritakr pushed a commit to samvera/hyrax that referenced this pull request Mar 6, 2025
When lazy migration is used, the sipity entity cannot be found for works
which are not yet migrated. Valkyrie resources have a `proxy_for_global_id`
with the format:
`gid://hyku/Hyrax::ValkyrieGlobalIdProxy/2f6da5dd-9314-421f-b0dd-fda84fec9ee3`
while prior works used the model name such as:
`gid://hyku/GenericWork/2f6da5dd-9314-421f-b0dd-fda84fec9ee3`.

This commit introduces a lazy migration of the sipity entity via a job
which is submitted after a work is migrated to valkyrie via the Freyja
persister.

Prior work in pull request samvera/hyku#2471
added some overrides in Hyku to resolve the sipity entity issues. These
changes still needs to be backported.

The prior work includes these changes:
The MigrateResourceService introduced a migration of the entity along
with the work's migration. This service is needed for direct migration
calls, but is not adequate for lazy migration, as the Freyja persister
does not call it for the work itself that was just migrated.

Hyku overrides to sipity.rb support cases where the entity is not yet migrated
and lazy migration is in use. This is necessary because the solr_document
is used to find the Sipity::Entity. Due to the model mapping found in the
solr document, the Entity method searches for the entity with the model
name. This is problematic when the entity is not yet migrated and the
entity is searched with a GenericWorkResource model rather than the
model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants