diff options
8 files changed, 25 insertions, 6 deletions
diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index fa15e0451d..b021b5f2d0 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -2,7 +2,7 @@ # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveStorage::BaseJob - discard_on ActiveRecord::RecordNotFound + discard_on ActiveRecord::RecordNotFound, ActiveRecord::InvalidForeignKey def perform(blob) blob.purge diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 5d36990fb9..1c4dc25094 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -19,14 +19,14 @@ class ActiveStorage::Attachment < ActiveRecord::Base # Synchronously deletes the attachment and {purges the blob}[rdoc-ref:ActiveStorage::Blob#purge]. def purge - blob.purge delete + blob.purge end # Deletes the attachment and {enqueues a background job}[rdoc-ref:ActiveStorage::Blob#purge_later] to purge the blob. def purge_later - blob.purge_later delete + blob.purge_later end private diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index b72f2e796d..bf87598a66 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -205,8 +205,8 @@ class ActiveStorage::Blob < ActiveRecord::Base # blobs. Note, though, that deleting the file off the service will initiate a HTTP connection to the service, which may # be slow or prevented, so you should not use this method inside a transaction or in callbacks. Use #purge_later instead. def purge - delete destroy + delete end # Enqueues an ActiveStorage::PurgeJob to call #purge. This is the recommended way to purge blobs from a transaction, diff --git a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb index 9e31e3966a..cfaf01cd5e 100644 --- a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb +++ b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb @@ -20,6 +20,7 @@ class CreateActiveStorageTables < ActiveRecord::Migration[5.2] t.datetime :created_at, null: false t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true + t.foreign_key :active_storage_blobs, column: :blob_id end end end diff --git a/activestorage/test/jobs/purge_job_test.rb b/activestorage/test/jobs/purge_job_test.rb index 251022a96f..ed4100b78d 100644 --- a/activestorage/test/jobs/purge_job_test.rb +++ b/activestorage/test/jobs/purge_job_test.rb @@ -24,4 +24,14 @@ class ActiveStorage::PurgeJobTest < ActiveJob::TestCase end end end + + test "ignores attached blob" do + User.create! name: "DHH", avatar: @blob + + perform_enqueued_jobs do + assert_nothing_raised do + ActiveStorage::PurgeJob.perform_later @blob + end + end + end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 82be88af08..334254d099 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -10,7 +10,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase @user = User.create!(name: "Josh") end - teardown { ActiveStorage::Blob.all.each(&:purge) } + teardown { ActiveStorage::Blob.all.each(&:delete) } test "attaching existing blobs to an existing record" do @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index 01caaf0b55..3333fd9323 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -10,7 +10,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase @user = User.create!(name: "Josh") end - teardown { ActiveStorage::Blob.all.each(&:purge) } + teardown { ActiveStorage::Blob.all.each(&:delete) } test "attaching an existing blob to an existing record" do @user.avatar.attach create_blob(filename: "funky.jpg") diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index a0e207642a..c2e7aae13a 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -174,6 +174,14 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase assert_not ActiveStorage::Blob.service.exist?(variant.key) end + test "purge fails when attachments exist" do + create_blob.tap do |blob| + User.create! name: "DHH", avatar: blob + assert_raises(ActiveRecord::InvalidForeignKey) { blob.purge } + assert ActiveStorage::Blob.service.exist?(blob.key) + end + end + private def expected_url_for(blob, disposition: :inline, filename: nil) filename ||= blob.filename |