Skip to content

Redirects normalize on write#7435

Open
laritakr wants to merge 3 commits intomainfrom
redirects/normalize-on-write
Open

Redirects normalize on write#7435
laritakr wants to merge 3 commits intomainfrom
redirects/normalize-on-write

Conversation

@laritakr
Copy link
Copy Markdown
Contributor

@laritakr laritakr commented May 6, 2026

Summary

Details

Until now, every consumer of the redirects feature had its own normalization call: the form populator, the indexer, the sync step, the resolver, and the lookup service all ran the same RedirectPathNormalizer independently. The duplication worked, but it was fragile — anyone adding a new code path that wrote redirects had to remember to normalize, and any console write or non-form importer that bypassed the populator could quietly persist non-canonical data.

This change makes the resource itself the source of truth for canonical form. A new Hyrax::RedirectsNormalization concern is included on Hyrax::Work and Hyrax::PcdmCollection and normalizes redirect path entries on assignment, working under both flexible-metadata modes. Read-side consumers (indexer, sync step) no longer re-normalize and trust the persisted shape.

Normalization still runs at the points where unfamiliar input arrives from outside the resource — the form populator (so a user pasting a DSpace URL gets a friendly form experience), the redirect resolver (so /foo/ and /foo both match), and the redirect lookup service (so callers can pass any reasonable path form).

Testing

  • With the redirects feature enabled, save a work with a non-canonical path via the Rails console (e.g. work.redirects = [{path: '/foo/'}]; persister.save(resource: work)) and confirm the persisted entry is normalized to /foo.
  • Save a work via the form with a pasted DSpace-style URL (https://old.example.edu/handle/123?utm=email) and confirm it persists as /handle/123 and resolves correctly when visited.
  • Visit /foo/ (with trailing slash) for a redirect persisted as /foo and confirm the resolver matches it.
  • Run the redirects feature under both HYRAX_FLEXIBLE=false and HYRAX_FLEXIBLE=true and confirm the form, console-write, and resolver behaviors above hold in each mode.

laritakr added 2 commits May 6, 2026 16:24
Previously, if the redirects feature was enabled but the redirects database
table couldn't be queried (most often because the migration hadn't been run
yet), saving or deleting a work produced a 500 — even though the work
itself had already been saved successfully.

This commit catches that error and surfaces it as a clean failure instead.
The work save still succeeds; only the redirect bookkeeping is skipped
for that request. Redirects are a sidecar to the work, not load-bearing
for it, so a soft fail is the right outcome.

Also renames "ledger" terminology to "redirects table" throughout the
related code and docs for clearer language.
Centralizes redirect path normalization on the work and collection
resources themselves so any save — form, console, importer, change-set
apply — produces canonical paths without callers having to remember.

The read-side consumers (indexer, sync step) no longer re-normalize and
trust the persisted shape.

Normalization still runs at the points where unfamiliar input arrives
from outside the resource: the form populator (so a user pasting a
DSpace URL gets a friendly form experience), the redirect resolver
(so /foo/ and /foo both match), and the redirect lookup service (so
callers can pass any reasonable path form).
@laritakr laritakr added the notes-minor Release Notes: Non-breaking features label May 6, 2026
@laritakr laritakr changed the base branch from main to redirects/rescue-statement-invalid May 6, 2026 21:56
@laritakr laritakr requested a review from Copilot May 6, 2026 21:57

This comment was marked as resolved.

@laritakr laritakr requested a review from orangewolf May 6, 2026 22:07
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Test Results

    17 files  ± 0      17 suites  ±0   3h 25m 21s ⏱️ - 4m 33s
 7 348 tests +12   7 042 ✅ +12  306 💤 ±0  0 ❌ ±0 
24 732 runs  +46  24 141 ✅ +46  591 💤 ±0  0 ❌ ±0 

Results for commit a9c9524. ± Comparison against base commit cae6920.

This pull request removes 436 and adds 448 tests. Note that renamed tests count towards both.
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007efd87f90160>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f43517afc88>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f940c3e3d50>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007fb14017d498>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007efd880cb2c8>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f43517bc1e0>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f940c403218>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007fb14085e258>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: 03bcd766-44a7-4c85-a114-cc0a95c5566b
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 7913a7b4-338a-4103-bd3e-5d961108ea33
…
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f72d3bb3710>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f7f8476bb90>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f8cf4b0f288>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007ff05abedfc8>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f72d3bd0158>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f7f847aec60>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f8cf3bc5160>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007ff05c86e8a0>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: a78620e9-9e10-4b42-a688-377489855e7c
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 345a1a59-d524-44cd-9877-b6e7d710e3c9
…

Base automatically changed from redirects/rescue-statement-invalid to main May 8, 2026 01:57
orangewolf
orangewolf previously approved these changes May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes-minor Release Notes: Non-breaking features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants