[ActiveStorage for All] Optional Active Storage backend for staged uploads#7555
Draft
orangewolf wants to merge 3 commits into
Draft
[ActiveStorage for All] Optional Active Storage backend for staged uploads#7555orangewolf wants to merge 3 commits into
orangewolf wants to merge 3 commits into
Conversation
Introduces filename, content_type, byte_size, stored?, with_io, and with_local_path on Hyrax::UploadedFile as the supported interface for reading staged upload content, and adopts it in the storage-agnostic consumers: ValkyrieIngestJob, WorkUploadsHandler, the uploads JSON response, the collection branding transaction steps, and the dashboard collections controller. These call sites were reaching through the CarrierWave uploader (uploader.file.to_file, uploader.filename, file_url-as-local-path) and so were coupled to CarrierWave storing files at absolute local paths. Routing them through one narrow API prepares for an alternative Active Storage staging backend without behavior change today; with_io also deterministically closes the IO that ValkyrieIngestJob previously leaked. ActiveFedora-specific consumers (JobIoWrapper, FileActor, FileSetActor) intentionally keep using the CarrierWave interface unchanged. Tests: spec/models/hyrax/uploaded_file_spec.rb (new API coverage), spec/controllers/hyrax/uploads_controller_spec.rb, spec/services/hyrax/work_uploads_handler_spec.rb, spec/jobs/valkyrie_ingest_job_spec.rb, spec/hyrax/transactions/steps/save_collection_banner_spec.rb, spec/hyrax/transactions/steps/save_collection_logo_spec.rb, spec/controllers/hyrax/dashboard/collections_controller_spec.rb — all green locally against the koppie (Valkyrie) test app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the canonical Rails create_active_storage_tables migration to the .dassie and .koppie test applications and regenerates their schemas, teaches the hyrax:models generator to run active_storage:install so host applications get the active_storage_* tables alongside Hyrax's own migrations, and has the hyrax:config generator install a default config/storage.yml (local disk services plus commented S3/GCS/Azure examples) when the application does not already have one. The .dassie sample app's storage.yml is aligned with that default (.koppie's already matches). Both test apps already ship config.active_storage.service settings per environment; this fills in the missing database tables and default service declarations so upcoming work can stage uploads and store repository files through Active Storage, where switching local disk vs S3 is a single Rails storage configuration change. No behavior change: nothing in Hyrax attaches or reads Active Storage data yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds config.uploaded_file_storage_backend (:carrierwave by default, :active_storage opt-in) selecting where Hyrax::UploadedFile writes new content, and implements the Active Storage backend end to end while leaving the CarrierWave behavior byte-for-byte unchanged. Model: Hyrax::UploadedFile gains a file_attachment Active Storage attachment. The storage-neutral read API now dispatches on what each record actually carries (attachment vs CarrierWave file), so records created under either backend stay readable whatever the setting; a bare String assignment records the intended filename ahead of content, which the chunked upload protocol sends first. store_file routes writes to the configured backend. Virus scanning covers newly attached content once, from its local source, instead of CarrierWave's rescan-on-every-save. Controller: Hyrax::UploadsController keeps the existing two-step, CONTENT-RANGE chunk protocol (Cloudflare-safe requests) for both backends. Under :active_storage, chunks assemble in a local staging file under config.cache_path and the completed file is attached on the final chunk, sending the bytes to whatever Active Storage service is configured; switching that service between local disk and S3 is a Rails storage.yml change only. Destroying an upload purges its attachment. The ActiveFedora ingest path intentionally still reads staged files through CarrierWave; AF applications should keep the default backend (documented in the generated initializer). Tests: spec/models/hyrax/uploaded_file_spec.rb (shared examples run the neutral API against both backends, plus Active Storage-specific coverage), spec/controllers/hyrax/uploads_controller_spec.rb (pre-create, single-shot, sequential chunk assembly + finalize, mismatch restart, staging cleanup, purge on destroy; legacy CarrierWave examples untouched and passing), spec/lib/hyrax/configuration_spec.rb, spec/controllers/hyrax/file_sets_controller_spec.rb, spec/jobs/valkyrie_ingest_job_spec.rb, spec/services/hyrax/work_uploads_handler_spec.rb — all green locally against the koppie (Valkyrie) test app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 18, 2026
Test Results 17 files ± 0 17 suites ±0 1h 21m 56s ⏱️ - 2h 7m 43s For more details on these failures, see this check. Results for commit 2779d3d. ± Comparison against base commit 5cc41d6. This pull request removes 2220 and adds 328 tests. Note that renamed tests count towards both.This pull request skips 21 tests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in Active Storage backend for staged uploads (
Hyrax::UploadedFile), preserving the existing chunked upload protocol. Part of the [ActiveStorage for All] chain; stacked on #7553 and #7554 (their commits appear in this diff until they merge).New config:
:carrierwave(default): byte-for-byte the existing behavior. ActiveFedora apps should stay here — the AF ingest path reads staged files through CarrierWave (documented in the generated initializer).:active_storage: staged bytes are written through Rails Active Storage, soconfig/storage.yml/config.active_storage.servicedecide where they live — switching staging from local disk to S3 becomes that one Rails config change.Design points:
Hyrax::UploadedFilegainshas_one_attached :file_attachment. Reads dispatch per record (attachment present → Active Storage; CarrierWave file → CarrierWave), so records created under either backend remain readable whatever the setting — no migration required to flip.CONTENT-RANGEchunks, Cloudflare-size-safe requests; the upload widget needs no changes). Under Active Storage,Hyrax::UploadsControllerassembles chunks in a local staging file underconfig.cache_pathand attaches the completed file when the final chunk arrives (last byte + 1 == total); a chunk that doesn't continue the pending assembly restarts it, mirroring the CarrierWave replacement behavior. Assembly assumes sequential chunks on one node or a shared staging path — the same constraint CarrierWave append has always had.file = "name.png", the protocol's pre-create step) records the intended filename ahead of content.ActiveStorage::PurgeJob).FileSetsController#uploaded_file_from_pathpasses the raw multipart upload straight through (both backends accept it; theCarrierWave::SanitizedFilewrap was redundant).Dependencies
Supersedes #7549 (same change, moved from a fork branch to an in-repo branch and retitled).
Tests
Run locally against the koppie (Valkyrie) test app — dockerized Postgres/Solr/Redis:
spec/models/hyrax/uploaded_file_spec.rb: the storage-neutral API examples now run as shared examples against both backends, plus AS-specific coverage (intended filename,store_file, virus scan on attach, no rescan on unchanged saves, purge on destroy) — 39 examples, 0 failuresspec/controllers/hyrax/uploads_controller_spec.rb: legacy CarrierWave examples untouched and passing; new AS context covers pre-create, single-shot attach, sequential chunk assembly + finalize with content verification, staging-file cleanup, mismatched-chunk restart, purge on destroy — 140 examples total, 0 failuresspec/lib/hyrax/configuration_spec.rb(backend validation),spec/controllers/hyrax/file_sets_controller_spec.rb,spec/jobs/valkyrie_ingest_job_spec.rb,spec/services/hyrax/work_uploads_handler_spec.rb— greenAF-side (dassie) suite portions not run locally; CI covers them. RuboCop clean on touched files.
Notes for reviewers
AnalyzeJob(image metadata); harmless for staging records, called out for awareness.🤖 Generated with Claude Code