From bd33ddaf710f94fafff1854ed4b4311e497e8492 Mon Sep 17 00:00:00 2001 From: Alex Ghiculescu Date: Wed, 10 Mar 2021 18:04:00 -0600 Subject: [PATCH] Active Storage: `Blob` creation shouldn't crash if no service selected https://github.com/rails/rails/issues/41653 noted an issue where if there's service configured (`config.active_storage.service` is commented out), Blob creation via direct upload crashes: ``` Started POST "/rails/active_storage/direct_uploads" for ::1 at 2021-03-09 13:02:57 -0800 Processing by ActiveStorage::DirectUploadsController#create as JSON Parameters: {"blob"=>{"filename"=>"banana.jpg", "content_type"=>"image/jpeg", "byte_size"=>577085, "checksum"=>"W/vo/JqBNmJHMCaL+PRlBQ=="}, "direct_upload"=>{"blob"=>{"filename"=>"banana.jpg", "content_type"=>"image/jpeg", "byte_size"=>577085, "checksum"=>"W/vo/JqBNmJHMCaL+PRlBQ=="}}} Completed 500 Internal Server Error in 12ms (ActiveRecord: 3.3ms | Allocations: 5864) NoMethodError (undefined method `name' for nil:NilClass): activestorage (6.1.3) app/models/active_storage/blob.rb:52:in `block in ' activesupport (6.1.3) lib/active_support/callbacks.rb:427:in `instance_exec' ``` This PR fixes that crash. Blob creation will still fail, but with a more informative error about a `service_name` being required. --- activestorage/app/models/active_storage/blob.rb | 2 +- activestorage/test/models/blob_test.rb | 8 ++++++++ activestorage/test/test_helper.rb | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 303dcd59ae525..761408e571d4f 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -49,7 +49,7 @@ class ActiveStorage::Blob < ActiveStorage::Record scope :unattached, -> { where.missing(:attachments) } after_initialize do - self.service_name ||= self.class.service.name + self.service_name ||= self.class.service&.name end after_update_commit :update_service_metadata, if: :content_type_previously_changed? diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 10a4c69478ba5..54588fc27a12f 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -252,6 +252,14 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end + test "doesn't create a valid blob if service setting is nil" do + with_service(nil) do + assert_raises(ActiveRecord::RecordInvalid) do + create_blob(filename: "funky.jpg") + end + end + end + test "invalidates record when provided service_name is invalid" do blob = create_blob(filename: "funky.jpg") blob.update(service_name: :unknown) diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index e85c82ae99a31..a9e918653866e 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -102,7 +102,7 @@ def fixture_file_upload(filename) def with_service(service_name) previous_service = ActiveStorage::Blob.service - ActiveStorage::Blob.service = ActiveStorage::Blob.services.fetch(service_name) + ActiveStorage::Blob.service = service_name ? ActiveStorage::Blob.services.fetch(service_name) : nil yield ensure