[ShanaLMoore]

Refs #570

When several file set rows for one work import at once, some file sets drop off the work. ValkyrieObjectFactory#create_file_set loads the work outside any lock and runs Hyrax's change_set.update_work, whose first step saves the work's member_ids as loaded, erasing file sets other rows attached meanwhile. samvera/hyrax#7240 fixed only the attach step, on 5.0-flexible, and that step runs after the stale save.

🐛 Return the saved work when its child is already a member

d250437

CreateRelationshipsJob takes add_child_to_parent_work's return value as the work for the rest of the pass and saves it at the end. Both object factories returned true for a child already a member, as on a re-import or a rescheduled pass, and ActiveFedora's returned the ordered_members proxy after adding one. Both now return the work, and re-adding a member does nothing. Adds a shared spec context that runs the Valkyrie path on Valkyrie's in-memory adapter.

🐛 Relate a work's children in their stored order

8ac6276

.ordered.find_each batches by primary key and ignores the order scope, so a work's children were attached in the order their rows finished. The work pass now uses .ordered.order(:id).each: row order, with children named in one cell keeping their listed order. This changes the ActiveFedora path too. The collection pass keeps find_each, since collection membership has no order.

🐛 Reschedule a work's relationship pass that fails

a3677d9

An exception from a work's pass (saving, indexing or publishing the work, or acquiring its lock) escaped perform, which most adapters never retry, so relationships stayed pending. The pass now records it as a failure, and perform reschedules it like any failed relationship. Lock errors are included on purpose, which covers the work side of #1076. Each relationship is now destroyed only once the work is saved, indexed and published, so a failed pass leaves them all for the rescheduled one rather than finding nothing to do and reporting success. They are destroyed while the pass still holds the work's lock, so a second pass for the same work cannot finish them too. A failed pass marks its parent's pending relationships with the error, so a stalled ScheduleRelationshipsJob leaves them to this reschedule instead of retrying them hourly, which counted as progress and kept it from ever reaching GIVE_UP_LIMIT. Those relationships are counted as failed while they wait, so the pass that links them recounts and sets the importer back to Complete. Applies to both object factories.

🐛 Attach imported file sets in one pass

fe6609b

Valkyrie path only:

• create_file_set creates the file set without touching the work or queueing its ingest, with the work's permissions and the row's visibility, or else the work's visibility, embargo and lease. It builds the file set as Hyrax 5.3's WorkUploadsHandler does instead of calling it, since Hyrax 4's handler queues the ingest itself. A row whose parent is not found, or is a collection or file set rather than a work, fails before anything is saved.
• A re-import of a row with no visibility gives the file set its work's current visibility, embargo and lease, as creating it does, unless the file set's own embargo or lease is still running. An ended embargo or lease on the file set is cleared first, so Hyrax's expiry jobs cannot later release it to its after-visibility, and the file set is reindexed so its indexed embargo and lease dates match. (On Hyrax 5.x an update of a file set carrying an ended embargo or lease already fails in Hyrax's Steps::Save future-date check; Hyrax 4.0's Valkyrie save has no such check, so this is where it applies.) A row that names a visibility, including embargo or lease with its dates, is updated by the file set form as before.
• The file set row records a pending relationship to its first parent, skipping a parent the import deletes (deleted_identifier?, from 🐛 Validate delete rows the way import runs them #1279).
• CreateRelationshipsJob adds every file set in one pass under the work's lock and sets the representative and thumbnail as Hyrax did on attach. Once the work is saved, still under its lock, it queues each upload once and then destroys its relationship. A pass that is killed, fails, or has an enqueue refused leaves the relationships for the retry, which queues the ingest of a file set already attached without its file.

ActiveFedora is unchanged here: its file sets are still attached when created.

Behaviour changes

• File sets appear on their work after the relationship pass, not row by row.
• A file set row without a visibility takes its work's, not the importer default, on import and on re-import.
• On Hyrax before 5.3, a file set row's metadata (title and so on) now reaches the file set, as it already did on 5.3+.

Known trade-offs / for reviewers

• Errors from a work's relationship pass now go through Bulkrax's own reschedule (max_failure_count, 5 tries, 10 minutes apart) rather than the adapter's retries or dead set. They show in the parent entry's status and the run's failed_relationships.
• If the last try fails, a file set can be left on its work with no file. It is reported as a failed relationship; re-running that entry queues the ingest again.
• A file can be ingested twice when an importer re-run, or a second relationship job naming the same work by another identifier, runs before an earlier ingest finishes, as can passes that overlap past the lock's TTL or with locking off (no REDIS_HOST). Preventing it needs a claim shared across jobs.
• A file set row that fails and then succeeds on retry is attached after the rows that attached in the meantime, so it lands out of CSV order. Holding later rows back would let one bad row block every later page of the work.
• file.set.attached is no longer published for imported file sets, so FileSetAttachedEventJob activity-stream entries stop for them.
• A file set row must have exactly one file. A row with a file plus thumbnail_url or remote_files now fails, since its one identifier can relate only one file set.
• An embargo or lease a file set inherited from its work is not lifted when the work's is; it ends on its own dates.
• When a re-import copies the work's embargo or lease onto a file set whose own has ended, the file set's old embargo or lease record is left orphaned rather than deleted.
• On a re-import that replaces the file, a derivative finishing between replace_original_file and the visibility save could lose a file_id, the same window detach_files already has.
• A pass-level failure marks the parent's pending relationships with the error before the final attempt and counts them in failed_relationships, so while a rescheduled pass is still to run, the importer can show Complete (with failures). The pass that links them recounts and sets it back to Complete.
• On Hyrax 5.x, re-importing a file set row whose file set carries an ended embargo or lease fails in Hyrax's own future-date check; that is upstream behaviour and unchanged here.
• try(:deleted_identifier?, ...) works before and after 🐛 Validate delete rows the way import runs them #1279; it relies on 🐛 Validate delete rows the way import runs them #1279 keeping deleted_identifier? public. Whichever merges second can make it a plain call.

Testing

• Import a CSV with one work and 8 file set rows. Once relationships run, the work lists all 8 in row order, has a representative and thumbnail, and each file set has its file.
• Re-run the importer: nothing attaches twice.
• Specs run on Hyrax 5.2 / Rails 7.2 and Hyrax 4.0 / Rails 6.1.

Refs

• move fileset relationship creation into the CreateRelationshipsJob? #570
• Automatically retry on Hyrax::LockManager::UnableToAcquireLockError #1076
• 🐛 Validate delete rows the way import runs them #1279
• Reload work when attaching FileSets hyrax#7240
• 🐛 Attach imported file sets through relationships notch8/utk_knapsack#435
• Upload FileSets via Bulkrax using Valkyrie notch8/hykuup_knapsack#567

🤖 Assisted with Claude Code

[Copilot]

🟡 Changes recommended

Critical failure paths can discard relationships needed for retries or leave orphaned file sets and uploads, while re-imports can retain visibility inconsistent with the parent work’s access policy.

3 open issues

• Reject unresolved parents before saving file sets or uploads · New
• Retain relationships until work save and indexing succeed · New
• Apply parent visibility fallback when updating file sets · New

What changed in this PR

This PR moves Valkyrie file-set attachment into Bulkrax’s relationship pass to prevent concurrent imports from overwriting work memberships.

Changes:

• Creates unattached file sets, then queues ingest after the relationship pass saves the work.
• Preserves stored child order and reschedules failed work relationship passes.
• Adds coverage for permissions, visibility inheritance, concurrent creation, and retry behavior.

File	Description
spec/​support/​valkyrie_memory_adapter.rb	Adds shared in-memory Valkyrie test setup.
spec/​models/​bulkrax/​valkyrie_object_factory_spec.rb	Tests unattached creation, access policies, membership, and ingest enqueueing.
spec/​models/​bulkrax/​csv_file_set_entry_spec.rb	Tests pending relationships and visibility handling.
spec/​jobs/​bulkrax/​create_relationships_job_spec.rb	Tests attachment order, deferred ingest, and failure recovery.
app/​models/​concerns/​bulkrax/​file_set_entry_behavior.rb	Records Valkyrie parent relationships and leaves blank visibility for inheritance.
app/​jobs/​bulkrax/​create_relationships_job.rb	Processes children in order, defers ingest, and catches work-pass failures.
app/​factories/​bulkrax/​valkyrie_object_factory.rb	Creates unattached file sets and supplies attachment and ingest helpers.
app/​factories/​bulkrax/​object_factory_interface.rb	Defines factory hooks for deferred ingest.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Re-import can leave an expired embargo attached to a file set, allowing the expiry job to restore open visibility despite its parent being restricted.

4 open issues

• Retire expired target restrictions before inheriting parent policy · New
• Reject non-work parents before creating file sets · New
• Reindex file set after inheriting embargo or lease · New
• Keep relationship cleanup inside the work lock · New

3 resolved since last review

• Retain relationships until work save and indexing succeed
• Reject unresolved parents before saving file sets or uploads
• Apply parent visibility fallback when updating file sets

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

A re-import can attach a file set to a new work without removing its existing membership, violating Hyrax’s one-work-per-file-set contract and potentially changing access through visibility inheritance.

1 open issue

• Prevent file sets from being linked to multiple works · New

4 resolved since last review

• Retire expired target restrictions before inheriting parent policy
• Keep relationship cleanup inside the work lock
• Reindex file set after inheriting embargo or lease
• Reject non-work parents before creating file sets

_{Review effort: Balanced}