From e3ade5fd2dc76235ecc98999f44217c470eb72e1 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <david@loudthinking.com>
Date: Wed, 5 Jul 2017 18:31:49 +0200
Subject: [PATCH] Default to purging later when the owning record is destroyed

---
 lib/active_vault/attached/macros.rb | 12 ++++++++--
 lib/active_vault/attachment.rb      |  4 ++++
 lib/active_vault/blob.rb            |  1 +
 lib/active_vault/purge_job.rb       | 11 ++++++----
 test/attachments_test.rb            | 34 +++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/lib/active_vault/attached/macros.rb b/lib/active_vault/attached/macros.rb
index 8cc84a95a9..1b95c14c9c 100644
--- a/lib/active_vault/attached/macros.rb
+++ b/lib/active_vault/attached/macros.rb
@@ -1,15 +1,23 @@
 module ActiveVault::Attached::Macros
-  def has_one_attached(name)
+  def has_one_attached(name, dependent: :purge_later)
     define_method(name) do
       instance_variable_get("@active_vault_attached_#{name}") || 
         instance_variable_set("@active_vault_attached_#{name}", ActiveVault::Attached::One.new(name, self))
     end
+
+    if dependent == :purge_later
+      before_destroy { public_send(name).purge_later }
+    end
   end
 
-  def has_many_attached(name)
+  def has_many_attached(name, dependent: :purge_later)
     define_method(name) do
       instance_variable_get("@active_vault_attached_#{name}") || 
         instance_variable_set("@active_vault_attached_#{name}", ActiveVault::Attached::Many.new(name, self))
     end
+
+    if dependent == :purge_later
+      before_destroy { public_send(name).purge_later }
+    end
   end
 end
diff --git a/lib/active_vault/attachment.rb b/lib/active_vault/attachment.rb
index 1c96dabe31..549a734d68 100644
--- a/lib/active_vault/attachment.rb
+++ b/lib/active_vault/attachment.rb
@@ -23,4 +23,8 @@ class ActiveVault::Attachment < ActiveRecord::Base
     blob.purge
     destroy
   end
+
+  def purge_later
+    ActiveVault::PurgeJob.perform_later(self)
+  end
 end
diff --git a/lib/active_vault/blob.rb b/lib/active_vault/blob.rb
index e2a6582e9f..a232ca5c1a 100644
--- a/lib/active_vault/blob.rb
+++ b/lib/active_vault/blob.rb
@@ -1,5 +1,6 @@
 require "active_vault/site"
 require "active_vault/filename"
+require "active_vault/purge_job"
 
 # Schema: id, key, filename, content_type, metadata, byte_size, checksum, created_at
 class ActiveVault::Blob < ActiveRecord::Base
diff --git a/lib/active_vault/purge_job.rb b/lib/active_vault/purge_job.rb
index d7634af2bb..b68eb370bb 100644
--- a/lib/active_vault/purge_job.rb
+++ b/lib/active_vault/purge_job.rb
@@ -1,7 +1,10 @@
-class ActiveVault::PurgeJob < ActiveJob::Base
-  retry_on ActiveVault::StorageException
+require "active_job"
 
-  def perform(blob)
-    blob.purge
+class ActiveVault::PurgeJob < ActiveJob::Base
+  # FIXME: Limit this to a custom ActiveVault error
+  retry_on StandardError
+
+  def perform(attachment_or_blob)
+    attachment_or_blob.purge
   end
 end
diff --git a/test/attachments_test.rb b/test/attachments_test.rb
index 2e7e5d1a79..1b784b50c1 100644
--- a/test/attachments_test.rb
+++ b/test/attachments_test.rb
@@ -2,6 +2,10 @@ require "test_helper"
 require "database/setup"
 require "active_vault/blob"
 
+require "active_job"
+ActiveJob::Base.queue_adapter = :test
+ActiveJob::Base.logger = nil
+
 # ActiveRecord::Base.logger = Logger.new(STDOUT)
 
 class User < ActiveRecord::Base
@@ -10,6 +14,8 @@ class User < ActiveRecord::Base
 end
 
 class ActiveVault::AttachmentsTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
   setup { @user = User.create!(name: "DHH") }
 
   teardown { ActiveVault::Blob.all.each(&:purge) }
@@ -33,6 +39,19 @@ class ActiveVault::AttachmentsTest < ActiveSupport::TestCase
     assert_not ActiveVault::Blob.site.exist?(avatar_key)
   end
 
+  test "purge attached blob later when the record is destroyed" do
+    @user.avatar.attach create_blob(filename: "funky.jpg")
+    avatar_key = @user.avatar.key
+
+    perform_enqueued_jobs do
+      @user.destroy
+
+      assert_nil ActiveVault::Blob.find_by(key: avatar_key)
+      assert_not ActiveVault::Blob.site.exist?(avatar_key)
+    end
+  end
+
+
   test "attach existing blobs" do
     @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
 
@@ -58,4 +77,19 @@ class ActiveVault::AttachmentsTest < ActiveSupport::TestCase
     assert_not ActiveVault::Blob.site.exist?(highlight_keys.first)
     assert_not ActiveVault::Blob.site.exist?(highlight_keys.second)
   end
+
+  test "purge attached blobs later when the record is destroyed" do
+    @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
+    highlight_keys = @user.highlights.collect(&:key)
+
+    perform_enqueued_jobs do
+      @user.destroy
+
+      assert_nil ActiveVault::Blob.find_by(key: highlight_keys.first)
+      assert_not ActiveVault::Blob.site.exist?(highlight_keys.first)
+
+      assert_nil ActiveVault::Blob.find_by(key: highlight_keys.second)
+      assert_not ActiveVault::Blob.site.exist?(highlight_keys.second)
+    end
+  end
 end