Fix unexpected behavior for dependent: :purge

activestorage/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Correct unexpected behavior resulting from dependent: :purge when using
+    has_one_attached or has_many_attached. Fixes #36423. 
+
+    *Mark Oveson*
+
 ## Rails 5.2.3 (March 27, 2019) ##
 
 *   No changes.

activestorage/lib/active_storage/attached/macros.rb

@@ -25,12 +25,16 @@ module Attached::Macros
     # The system has been designed to having you go through the ActiveStorage::Attached::One
     # proxy that provides the dynamic proxy to the associations and factory methods, like +attach+.
     #
-    # If the +:dependent+ option isn't set, the attachment will be purged
-    # (i.e. destroyed) whenever the record is destroyed.
+    # The +:dependent:+ option may be set to :purge, :purge_later, or :detach.
+    # If the +:dependent+ option isn't set, +:dependent:+ will default to :purge_later,
+    # meaning that the attachment will be purged (i.e. destroyed) in a background job
+    # whenever the record is destroyed.
+    DEPENDENT_OPERATIONS = [:purge, :purge_later]
+
     def has_one_attached(name, dependent: :purge_later)
       class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}
-          @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
+          @active_storage_attached_#{name} ||= ActiveStorage::Attached::One.new("#{name}", self, dependent: #{dependent.in?(DEPENDENT_OPERATIONS) ? ":#{dependent}" : "false"})
         end
 
         def #{name}=(attachable)
@@ -43,8 +47,8 @@ def #{name}=(attachable)
 
       scope :"with_attached_#{name}", -> { includes("#{name}_attachment": :blob) }
 
-      if dependent == :purge_later
-        after_destroy_commit { public_send(name).purge_later }
+      if dependent.in?(DEPENDENT_OPERATIONS)
+        after_destroy_commit { public_send(name).public_send(dependent) }
       else
         before_destroy { public_send(name).detach }
       end
@@ -72,12 +76,14 @@ def #{name}=(attachable)
     # The system has been designed to having you go through the ActiveStorage::Attached::Many
     # proxy that provides the dynamic proxy to the associations and factory methods, like +#attach+.
     #
-    # If the +:dependent+ option isn't set, all the attachments will be purged
-    # (i.e. destroyed) whenever the record is destroyed.
+    # The +:dependent:+ option may be set to :purge, :purge_later, or :detach.
+    # If the +:dependent+ option isn't set, +:dependent:+ will default to :purge_later,
+    # meaning that all the attachments will be purged (i.e. destroyed) in a background job
+    # whenever the record is destroyed.
     def has_many_attached(name, dependent: :purge_later)
       class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}
-          @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent == :purge_later ? ":purge_later" : "false"})
+          @active_storage_attached_#{name} ||= ActiveStorage::Attached::Many.new("#{name}", self, dependent: #{dependent.in?(DEPENDENT_OPERATIONS) ? ":#{dependent}" : "false"})
         end
 
         def #{name}=(attachables)
@@ -100,8 +106,8 @@ def purge_later
 
       scope :"with_attached_#{name}", -> { includes("#{name}_attachments": :blob) }
 
-      if dependent == :purge_later
-        after_destroy_commit { public_send(name).purge_later }
+      if dependent.in?(DEPENDENT_OPERATIONS)
+        after_destroy_commit { public_send(name).public_send(dependent) }
       else
         before_destroy { public_send(name).detach }
       end

activestorage/test/models/attachments_test.rb

@@ -234,6 +234,16 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     end
   end
 
+  test "purge attached blob now when the record is destroyed" do
+    @user.icon.attach create_blob(filename: "funky.jpg")
+    icon_key = @user.icon.key
+
+    @user.reload.destroy
+
+    assert_nil ActiveStorage::Blob.find_by(key: icon_key)
+    assert_not ActiveStorage::Blob.service.exist?(icon_key)
+  end
+
   test "delete attachment for independent blob when record is destroyed" do
     @user.cover_photo.attach create_blob(filename: "funky.jpg")
 
@@ -422,6 +432,19 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
     end
   end
 
+  test "purge attached blobs now when the record is destroyed" do
+    @user.favorites.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
+    favorite_keys = @user.favorites.collect(&:key)
+
+    @user.reload.destroy
+
+    assert_nil ActiveStorage::Blob.find_by(key: favorite_keys.first)
+    assert_not ActiveStorage::Blob.service.exist?(favorite_keys.first)
+
+    assert_nil ActiveStorage::Blob.find_by(key: favorite_keys.second)
+    assert_not ActiveStorage::Blob.service.exist?(favorite_keys.second)
+  end
+
   test "delete attachments for independent blobs when the record is destroyed" do
     @user.vlogs.attach create_blob(filename: "funky.mp4"), create_blob(filename: "wonky.mp4")
 

activestorage/test/test_helper.rb

@@ -83,8 +83,10 @@ def read_image(blob_or_variant)
 
 class User < ActiveRecord::Base
   has_one_attached :avatar
+  has_one_attached :icon, dependent: :purge
   has_one_attached :cover_photo, dependent: false
 
   has_many_attached :highlights
+  has_many_attached :favorites, dependent: :purge
   has_many_attached :vlogs, dependent: false
 end