From 98abeea7b6cae376533b7c0176d139f561a9e347 Mon Sep 17 00:00:00 2001
From: Shouichi Kamiya <shouichi.kamiya@gmail.com>
Date: Thu, 26 Jan 2023 10:35:39 +0900
Subject: [PATCH] Allow destroying active storage variant

Background:

When creating active storage variants, `ActiveStorage::VariantRecord` is
inserted, then a file is uploaded. Because upload can be failed, the
file can be missing even though `ActiveStorage::VariantRecord` exists.

When a file is missing, we need to delete the corresponding
`ActiveStorage::VariantRecord` but there's no API to delete just one
variant e.g., `blob.variant(resize_to_limit: [100, 100]).destroy`.

Co-authored-by: Yuichiro NAKAGAWA <ii.hsif.drows@gmail.com>
Co-authored-by: Ryohei UEDA <ueda@anipos.co.jp>
---
 activestorage/CHANGELOG.md                        |  8 ++++++++
 .../app/models/active_storage/variant.rb          |  5 +++++
 .../models/active_storage/variant_with_record.rb  |  5 +++++
 activestorage/test/models/variant_test.rb         |  9 +++++++++
 .../test/models/variant_with_record_test.rb       | 15 +++++++++++++++
 5 files changed, 42 insertions(+)

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 74d14ffa90..32728d051e 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Allow destroying active storage variants
+
+    ```ruby
+    User.first.avatar.variant(resize_to_limit: [100, 100]).destroy
+    ```
+
+    *Shouichi Kamiya*, *Yuichiro NAKAGAWA*, *Ryohei UEDA*
+
 *   Add missing preview event to `ActiveStorage::LogSubscriber`
 
     A `preview` event is being instrumented in `ActiveStorage::Previewer`.
diff --git a/activestorage/app/models/active_storage/variant.rb b/activestorage/app/models/active_storage/variant.rb
index 0066e601d5..826e8ae3e1 100644
--- a/activestorage/app/models/active_storage/variant.rb
+++ b/activestorage/app/models/active_storage/variant.rb
@@ -100,6 +100,11 @@ def image
     self
   end
 
+  # Deletes variant file from service.
+  def destroy
+    service.delete(key)
+  end
+
   private
     def processed?
       service.exist?(key)
diff --git a/activestorage/app/models/active_storage/variant_with_record.rb b/activestorage/app/models/active_storage/variant_with_record.rb
index 9955ba0292..0f1da393d6 100644
--- a/activestorage/app/models/active_storage/variant_with_record.rb
+++ b/activestorage/app/models/active_storage/variant_with_record.rb
@@ -27,6 +27,11 @@ def image
     record&.image
   end
 
+  # Destroys record and deletes file from service.
+  def destroy
+    record&.destroy
+  end
+
   delegate :key, :url, :download, to: :image, allow_nil: true
 
   private
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 246a3c1779..a4b57962ba 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -275,6 +275,15 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     end
   end
 
+  test "destroy deletes file from service" do
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = blob.variant(resize_to_limit: [100, 100]).processed
+
+    assert_changes -> { blob.service.exist?(variant.key) }, from: true, to: false do
+      variant.destroy
+    end
+  end
+
   private
     def process_variants_with(processor)
       previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, processor
diff --git a/activestorage/test/models/variant_with_record_test.rb b/activestorage/test/models/variant_with_record_test.rb
index adc0f23d03..b77bdac7db 100644
--- a/activestorage/test/models/variant_with_record_test.rb
+++ b/activestorage/test/models/variant_with_record_test.rb
@@ -4,6 +4,8 @@
 require "database/setup"
 
 class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
   setup do
     @was_tracking, ActiveStorage.track_variants = ActiveStorage.track_variants, true
   end
@@ -204,4 +206,17 @@ class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
       end
     end
   end
+
+  test "destroy deletes file from service" do
+    blob = create_file_blob(filename: "racecar.jpg")
+    variant = blob.variant(resize_to_limit: [100, 100]).processed
+
+    assert_equal 1, ActiveStorage::VariantRecord.count
+    assert blob.service.exist?(variant.key)
+
+    variant.destroy
+
+    assert_equal 0, ActiveStorage::VariantRecord.count
+    assert_enqueued_with(job: ActiveStorage::PurgeJob, args: [variant.image.blob])
+  end
 end