From 4eb550431e4afe444e596045cc55521c641e46c2 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 16 Mar 2022 16:52:22 +1100
Subject: [PATCH 01/12] Add gems required by Active Storage

---
 Gemfile      |  4 ++++
 Gemfile.lock | 25 +++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/Gemfile b/Gemfile
index c7fe245943..c45d1a6972 100644
--- a/Gemfile
+++ b/Gemfile
@@ -8,6 +8,10 @@ gem 'dotenv-rails', require: 'dotenv/rails-now' # Load ENV vars before other gem
 
 gem 'rails', '>= 6.1.4'
 
+# Active Storage
+gem "aws-sdk-s3", require: false
+gem "image_processing"
+
 gem 'activemerchant', '>= 1.78.0'
 gem 'rexml'
 gem 'angular-rails-templates', '>= 0.3.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index a3a6b9e440..ee36cee834 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -158,11 +158,27 @@ GEM
     awesome_nested_set (3.4.0)
       activerecord (>= 4.0.0, < 7.0)
     awesome_print (1.9.2)
+    aws-eventstream (1.2.0)
+    aws-partitions (1.570.0)
     aws-sdk (1.67.0)
       aws-sdk-v1 (= 1.67.0)
+    aws-sdk-core (3.130.0)
+      aws-eventstream (~> 1, >= 1.0.2)
+      aws-partitions (~> 1, >= 1.525.0)
+      aws-sigv4 (~> 1.1)
+      jmespath (~> 1.0)
+    aws-sdk-kms (1.55.0)
+      aws-sdk-core (~> 3, >= 3.127.0)
+      aws-sigv4 (~> 1.1)
+    aws-sdk-s3 (1.113.0)
+      aws-sdk-core (~> 3, >= 3.127.0)
+      aws-sdk-kms (~> 1)
+      aws-sigv4 (~> 1.4)
     aws-sdk-v1 (1.67.0)
       json (~> 1.4)
       nokogiri (~> 1)
+    aws-sigv4 (1.4.0)
+      aws-eventstream (~> 1, >= 1.0.2)
     axlsx_styler (1.1.0)
       activesupport (>= 3.1)
       caxlsx (>= 2.0.2)
@@ -331,9 +347,13 @@ GEM
       concurrent-ruby (~> 1.0)
     i18n-js (3.9.0)
       i18n (>= 0.6.6)
+    image_processing (1.12.2)
+      mini_magick (>= 4.9.5, < 5)
+      ruby-vips (>= 2.0.17, < 3)
     immigrant (0.3.6)
       activerecord (>= 3.0)
     ipaddress (0.8.3)
+    jmespath (1.6.1)
     jquery-rails (4.4.0)
       rails-dom-testing (>= 1, < 3)
       railties (>= 4.2.0)
@@ -373,6 +393,7 @@ GEM
     mimemagic (0.4.3)
       nokogiri (~> 1)
       rake
+    mini_magick (4.11.0)
     mini_mime (1.1.2)
     mini_portile2 (2.8.0)
     mini_racer (0.4.0)
@@ -560,6 +581,8 @@ GEM
       rubocop (>= 1.7.0, < 2.0)
     ruby-progressbar (1.11.0)
     ruby-rc4 (0.1.5)
+    ruby-vips (2.1.4)
+      ffi (~> 1.12)
     ruby2_keywords (0.0.4)
     rubyzip (2.3.2)
     rufus-scheduler (3.7.0)
@@ -697,6 +720,7 @@ DEPENDENCIES
   awesome_nested_set
   awesome_print
   aws-sdk (= 1.67.0)
+  aws-sdk-s3
   bigdecimal (= 3.0.2)
   bootsnap
   bugsnag
@@ -736,6 +760,7 @@ DEPENDENCIES
   hiredis
   i18n
   i18n-js (~> 3.9.0)
+  image_processing
   immigrant
   jquery-rails (= 4.4.0)
   jquery-ui-rails (~> 4.2)

From 92bb23d914e56bc2cf1ca3f1595de4ab05bec92e Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 16 Mar 2022 16:52:22 +1100
Subject: [PATCH 02/12] Add validation gem for Active Storage

We used validations with Paperclip and it would be nice to keep them.
---
 Gemfile               |  1 +
 Gemfile.lock          |  6 ++++++
 config/locales/en.yml | 24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/Gemfile b/Gemfile
index c45d1a6972..a6ec2e5a78 100644
--- a/Gemfile
+++ b/Gemfile
@@ -9,6 +9,7 @@ gem 'dotenv-rails', require: 'dotenv/rails-now' # Load ENV vars before other gem
 gem 'rails', '>= 6.1.4'
 
 # Active Storage
+gem "active_storage_validations"
 gem "aws-sdk-s3", require: false
 gem "image_processing"
 
diff --git a/Gemfile.lock b/Gemfile.lock
index ee36cee834..13fb10770e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -101,6 +101,11 @@ GEM
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
     active_model_serializers (0.8.4)
       activemodel (>= 3.0)
+    active_storage_validations (0.9.7)
+      activejob (>= 5.2.0)
+      activemodel (>= 5.2.0)
+      activestorage (>= 5.2.0)
+      activesupport (>= 5.2.0)
     activejob (6.1.4.4)
       activesupport (= 6.1.4.4)
       globalid (>= 0.3.6)
@@ -706,6 +711,7 @@ PLATFORMS
 DEPENDENCIES
   actionpack-action_caching
   active_model_serializers (= 0.8.4)
+  active_storage_validations
   activemerchant (>= 1.78.0)
   activerecord-import
   activerecord-postgresql-adapter
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 78a009b389..854406c54d 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -82,6 +82,30 @@ en:
             using_producer_stock_settings_but_count_on_hand_set: "must be blank because using producer stock settings"
             on_demand_but_count_on_hand_set: "must be blank if on demand"
             limited_stock_but_no_count_on_hand: "must be specified because forcing limited stock"
+
+  # Used by active_storage_validations
+  errors:
+    messages:
+      content_type_invalid: "has an invalid content type"
+      file_size_out_of_range: "size %{file_size} is not between required range"
+      limit_out_of_range: "total number is out of range"
+      image_metadata_missing: "is not a valid image"
+      dimension_min_inclusion: "must be greater than or equal to %{width} x %{height} pixel."
+      dimension_max_inclusion: "must be less than or equal to %{width} x %{height} pixel."
+      dimension_width_inclusion: "width is not included between %{min} and %{max} pixel."
+      dimension_height_inclusion: "height is not included between %{min} and %{max} pixel."
+      dimension_width_greater_than_or_equal_to: "width must be greater than or equal to %{length} pixel."
+      dimension_height_greater_than_or_equal_to: "height must be greater than or equal to %{length} pixel."
+      dimension_width_less_than_or_equal_to: "width must be less than or equal to %{length} pixel."
+      dimension_height_less_than_or_equal_to: "height must be less than or equal to %{length} pixel."
+      dimension_width_equal_to: "width must be equal to %{length} pixel."
+      dimension_height_equal_to: "height must be equal to %{length} pixel."
+      aspect_ratio_not_square: "must be a square image"
+      aspect_ratio_not_portrait: "must be a portrait image"
+      aspect_ratio_not_landscape: "must be a landscape image"
+      aspect_ratio_is_not: "must have an aspect ratio of %{aspect_ratio}"
+      aspect_ratio_unknown: "has an unknown aspect ratio"
+
   stripe:
     error_code:
       incorrect_number: "The card number is incorrect."

From b6cdb04a27c715449d0c3afbe0b5d77a7bc9995a Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 16 Mar 2022 16:53:44 +1100
Subject: [PATCH 03/12] Activate Active Storage

---
 config/application.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/application.rb b/config/application.rb
index 89b71e9994..05d695c191 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -3,7 +3,7 @@ require_relative 'boot'
 require "rails"
 [
   "active_record/railtie",
-  #"active_storage/engine",
+  "active_storage/engine",
   "action_controller/railtie",
   "action_view/railtie",
   "action_mailer/railtie",

From ce0e33fffa9fc068c4300a047acd79ca8054671d Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 16 Mar 2022 16:57:23 +1100
Subject: [PATCH 04/12] Install Active Storage database tables

---
 ...te_active_storage_tables.active_storage.rb | 36 +++++++++++++++++++
 db/schema.rb                                  | 30 ++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 db/migrate/20220316055458_create_active_storage_tables.active_storage.rb

diff --git a/db/migrate/20220316055458_create_active_storage_tables.active_storage.rb b/db/migrate/20220316055458_create_active_storage_tables.active_storage.rb
new file mode 100644
index 0000000000..87798267b4
--- /dev/null
+++ b/db/migrate/20220316055458_create_active_storage_tables.active_storage.rb
@@ -0,0 +1,36 @@
+# This migration comes from active_storage (originally 20170806125915)
+class CreateActiveStorageTables < ActiveRecord::Migration[5.2]
+  def change
+    create_table :active_storage_blobs do |t|
+      t.string   :key,          null: false
+      t.string   :filename,     null: false
+      t.string   :content_type
+      t.text     :metadata
+      t.string   :service_name, null: false
+      t.bigint   :byte_size,    null: false
+      t.string   :checksum,     null: false
+      t.datetime :created_at,   null: false
+
+      t.index [ :key ], unique: true
+    end
+
+    create_table :active_storage_attachments do |t|
+      t.string     :name,     null: false
+      t.references :record,   null: false, polymorphic: true, index: false
+      t.references :blob,     null: false
+
+      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
+
+    create_table :active_storage_variant_records do |t|
+      t.belongs_to :blob, null: false, index: false
+      t.string :variation_digest, null: false
+
+      t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 6ce6c4dc61..0ad1f12db3 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -15,6 +15,34 @@ ActiveRecord::Schema.define(version: 2022_04_10_162955) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
+  create_table "active_storage_attachments", force: :cascade do |t|
+    t.string "name", null: false
+    t.string "record_type", null: false
+    t.bigint "record_id", null: false
+    t.bigint "blob_id", null: false
+    t.datetime "created_at", null: false
+    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
+    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
+  end
+
+  create_table "active_storage_blobs", force: :cascade do |t|
+    t.string "key", null: false
+    t.string "filename", null: false
+    t.string "content_type"
+    t.text "metadata"
+    t.string "service_name", null: false
+    t.bigint "byte_size", null: false
+    t.string "checksum", null: false
+    t.datetime "created_at", null: false
+    t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
+  end
+
+  create_table "active_storage_variant_records", force: :cascade do |t|
+    t.bigint "blob_id", null: false
+    t.string "variation_digest", null: false
+    t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true
+  end
+
   create_table "adjustment_metadata", force: :cascade do |t|
     t.integer "adjustment_id"
     t.integer "enterprise_id"
@@ -1183,6 +1211,8 @@ ActiveRecord::Schema.define(version: 2022_04_10_162955) do
     t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id"
   end
 
+  add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
+  add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
   add_foreign_key "adjustment_metadata", "enterprises", name: "adjustment_metadata_enterprise_id_fk"
   add_foreign_key "adjustment_metadata", "spree_adjustments", column: "adjustment_id", name: "adjustment_metadata_adjustment_id_fk", on_delete: :cascade
   add_foreign_key "coordinator_fees", "enterprise_fees", name: "coordinator_fees_enterprise_fee_id_fk"

From 95cb6e93e72d0cd8c7531ec87e78e92e9f8ece39 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Thu, 17 Mar 2022 11:20:28 +1100
Subject: [PATCH 05/12] Configure Active Storage

We are re-using the same config used for Paperclip except for disk
storage. Active Storage uses directory sharding on the local disk which
means that we can't create blob entries that point to the existing
Paperclip files. We will just copy them to the standard `storage/`
directory.
---
 .gitignore                  |  1 +
 config/application.rb       |  2 ++
 config/environments/test.rb |  2 ++
 config/storage.yml          | 14 ++++++++++++++
 4 files changed, 19 insertions(+)
 create mode 100644 config/storage.yml

diff --git a/.gitignore b/.gitignore
index 6ec58bba6a..cba45b438d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@ db/*.csv
 log/*.log
 log/*.log.lck
 log/*.log.*
+/storage
 tmp/
 .idea/*
 \#*
diff --git a/config/application.rb b/config/application.rb
index 05d695c191..7d88b45db8 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -238,5 +238,7 @@ module Openfoodnetwork
     Rails.application.routes.default_url_options[:host] = ENV["SITE_URL"]
 
     Rails.autoloaders.main.ignore(Rails.root.join('app/webpacker'))
+
+    config.active_storage.service = ENV["S3_BUCKET"].present? ? :amazon : :local
   end
 end
diff --git a/config/environments/test.rb b/config/environments/test.rb
index f85c7aab9e..71228c5e30 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -51,4 +51,6 @@ Openfoodnetwork::Application.configure do
   config.active_support.deprecation = :stderr
 
   config.active_job.queue_adapter = :test
+
+  config.active_storage.service = :test
 end
diff --git a/config/storage.yml b/config/storage.yml
new file mode 100644
index 0000000000..fbac2cd79c
--- /dev/null
+++ b/config/storage.yml
@@ -0,0 +1,14 @@
+local:
+  service: Disk
+  root: <%= Rails.root.join("storage") %>
+
+test:
+  service: Disk
+  root: <%= Rails.root.join("tmp/storage") %>
+
+amazon:
+  service: S3
+  access_key_id: <%= ENV["S3_ACCESS_KEY"] %>
+  secret_access_key: <%= ENV["S3_SECRET"] %>
+  bucket: <%= ENV["S3_BUCKET"] %>
+  region: <%= ENV.fetch("S3_REGION", "us-east-1") %>

From ec64e5c8f729d76389910bca78c0bb4ba3eed30e Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Thu, 17 Mar 2022 14:54:38 +1100
Subject: [PATCH 06/12] Store images with Active Storage as well

While we migrate from Paperclip to Active Storage, we need to use both
at the same time to avoid any downtime or lost images.

Once the migration is complete, we want to use the same name for
attachment as before. Using Paperclip and Active Storage at the same
time creates a name conflict on a couple of methods.

I'm using alias_method as a temporary solution to access Active Storage
methods. We will remove that after the migration. I declare Paperclip
afterwards so that we have those methods declarations for backwards
compatibility now.
---
 app/models/spree/image.rb       | 23 +++++++++++++++++++++++
 config/storage.yml              |  7 +++++++
 spec/models/spree/image_spec.rb | 27 +++++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index e508395729..3d835de7b2 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -7,6 +7,17 @@ module Spree
     validates_attachment_presence :attachment
     validate :no_attachment_errors
 
+    # Active Storage declaration
+    has_one_attached :attachment
+
+    # Backup Active Storage methods before they get overridden by Paperclip.
+    alias_method :active_storage_attachment, :attachment
+    alias_method :active_storage_attachment=, :attachment=
+
+    # Paperclip declaration
+    #
+    # This will define the `name` and `name=` methods as well.
+    #
     # This is where the styles are used in the app:
     # - mini: used in the BackOffice: Bulk Product Edit page and Order Cycle edit page
     # - small: used in the FrontOffice: Product List page
@@ -21,6 +32,18 @@ module Spree
                       path: ':rails_root/public/spree/products/:id/:style/:basename.:extension',
                       convert_options: { all: '-strip -auto-orient -colorspace sRGB' }
 
+    after_post_process do
+      if attachment.errors.blank?
+        attachable = {
+          io: File.open(local_filename_of_original),
+          filename: attachment_file_name,
+          content_type: attachment_content_type,
+          identify: false,
+        }
+        self.active_storage_attachment = attachable
+      end
+    end
+
     # save the w,h of the original image (from which others can be calculated)
     # we need to look at the write-queue for images which have not been saved yet
     after_post_process :find_dimensions
diff --git a/config/storage.yml b/config/storage.yml
index fbac2cd79c..255c8298d9 100644
--- a/config/storage.yml
+++ b/config/storage.yml
@@ -6,6 +6,13 @@ test:
   service: Disk
   root: <%= Rails.root.join("tmp/storage") %>
 
+test_amazon:
+  service: S3
+  access_key_id: "A...A"
+  secret_access_key: "H...H"
+  bucket: "ofn"
+  region: "us-east-1"
+
 amazon:
   service: S3
   access_key_id: <%= ENV["S3_ACCESS_KEY"] %>
diff --git a/spec/models/spree/image_spec.rb b/spec/models/spree/image_spec.rb
index eba817048b..06de9c955b 100644
--- a/spec/models/spree/image_spec.rb
+++ b/spec/models/spree/image_spec.rb
@@ -21,6 +21,18 @@ module Spree
         expect(attachment.file?).to eq true
         expect(attachment.url).to match %r"^/spree/products/[0-9]+/product/logo-black\.png\?[0-9]+$"
       end
+
+      it "duplicates the image with Active Storage" do
+        image = Spree::Image.create!(
+          attachment: file,
+          viewable: product.master,
+        )
+
+        attachment = image.active_storage_attachment
+        url = Rails.application.routes.url_helpers.url_for(attachment)
+
+        expect(url).to match %r|^http://test\.host/rails/active_storage/blobs/redirect/[[:alnum:]-]+/logo-black\.png$|
+      end
     end
 
     describe "using AWS S3" do
@@ -47,9 +59,12 @@ module Spree
         allow(Spree::Image).to receive(:attachment_definitions).and_return(
           attachment: attachment_definition.merge(s3_config)
         )
+        allow(Rails.application.config.active_storage).
+          to receive(:service).and_return(:test_amazon)
       end
 
       it "saves a new image when none is present" do
+        # Paperclip requests
         upload_pattern = %r"^https://ofn.s3.amazonaws.com/[0-9]+/(original|mini|small|product|large)/logo-black.png$"
         download_pattern = %r"^https://ofn.s3.amazonaws.com/[0-9]+/product/logo-black.png$"
         public_url_pattern = %r"^https://ofn.s3.us-east-1.amazonaws.com/[0-9]+/product/logo-black.png\?[0-9]+$"
@@ -57,15 +72,27 @@ module Spree
         stub_request(:put, upload_pattern).to_return(status: 200, body: "", headers: {})
         stub_request(:head, download_pattern).to_return(status: 200, body: "", headers: {})
 
+        # Active Storage requests
+        as_upload_pattern = %r"^https://ofn.s3.amazonaws.com/[[:alnum:]]+$"
+
+        stub_request(:put, as_upload_pattern).to_return(status: 200, body: "", headers: {})
+
         image = Spree::Image.create!(
           attachment: file,
           viewable: product.master,
         )
 
+        # Paperclip
         attachment = image.attachment
         expect(attachment.exists?).to eq true
         expect(attachment.file?).to eq true
         expect(attachment.url).to match public_url_pattern
+
+        # Active Storage
+        attachment = image.active_storage_attachment
+        expect(attachment).to be_attached
+        expect(Rails.application.routes.url_helpers.url_for(attachment)).
+          to match %r"^http://test\.host/rails/active_storage/blobs/redirect/[[:alnum:]-]+/logo-black\.png"
       end
     end
   end

From c36ad96accad770690fde29dca4f477fb6f83531 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Mon, 28 Mar 2022 17:16:02 +1100
Subject: [PATCH 07/12] Move file duplication code to concern to share

I chose `has_one_migrating` as method name for two reasons:

 1. It reflects Active Storage's method `has_one_attached`.
 2. And it has the same length as Paperclip's `has_attached_file`.
    Therefore the commits don't need any whitespace changes.

When we change it to `has_one_attached`, we will also remove the
Paperclip options which then don't need whitespace changes either.
---
 app/models/concerns/has_migrating_file.rb | 57 +++++++++++++++++++++++
 app/models/spree/image.rb                 | 27 ++---------
 2 files changed, 60 insertions(+), 24 deletions(-)
 create mode 100644 app/models/concerns/has_migrating_file.rb

diff --git a/app/models/concerns/has_migrating_file.rb b/app/models/concerns/has_migrating_file.rb
new file mode 100644
index 0000000000..d82528f38c
--- /dev/null
+++ b/app/models/concerns/has_migrating_file.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+module HasMigratingFile
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def has_one_migrating(name, paperclip_options = {})
+      # Active Storage declaration
+      has_one_attached name
+
+      # Backup Active Storage methods before they get overridden by Paperclip.
+      alias_method "active_storage_#{name}", name
+      alias_method "active_storage_#{name}=", "#{name}="
+
+      # Paperclip declaration
+      #
+      # This will define the `name` and `name=` methods as well.
+      has_attached_file name, paperclip_options
+
+      # Paperclip callback to duplicate file with Active Storage
+      #
+      # We store files with Paperclip *and* Active Storage while we migrate
+      # old Paperclip files to Active Storage. This enables availability
+      # during the migration.
+      after_post_process do
+        if public_send(name).errors.blank?
+          file = File.open(processed_local_file_path)
+          attach_file(name, file)
+        end
+      end
+    end
+  end
+
+  def attach_file(name, io)
+    attachable = {
+      io: io,
+      filename: public_send("#{name}_file_name"),
+      content_type: public_send("#{name}_content_type"),
+      identify: false,
+    }
+    public_send("active_storage_#{name}=", attachable)
+  end
+
+  private
+
+  def processed_local_file_path(name)
+    attachment = public_send(name)
+
+    temporary = attachment.queued_for_write[:original]
+
+    if temporary&.path.present?
+      temporary.path
+    else
+      attachment.path
+    end
+  end
+end
diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index 3d835de7b2..996067ec6f 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -4,27 +4,18 @@ require 'spree/core/s3_support'
 
 module Spree
   class Image < Asset
+    include HasMigratingFile
+
     validates_attachment_presence :attachment
     validate :no_attachment_errors
 
-    # Active Storage declaration
-    has_one_attached :attachment
-
-    # Backup Active Storage methods before they get overridden by Paperclip.
-    alias_method :active_storage_attachment, :attachment
-    alias_method :active_storage_attachment=, :attachment=
-
-    # Paperclip declaration
-    #
-    # This will define the `name` and `name=` methods as well.
-    #
     # This is where the styles are used in the app:
     # - mini: used in the BackOffice: Bulk Product Edit page and Order Cycle edit page
     # - small: used in the FrontOffice: Product List page
     # - product: used in the BackOffice: Product Image upload modal in the Bulk Product Edit page
     #                                      and Product image edit page
     # - large: used in the FrontOffice: product modal
-    has_attached_file :attachment,
+    has_one_migrating :attachment,
                       styles: { mini: "48x48#", small: "227x227#",
                                 product: "240x240>", large: "600x600>" },
                       default_style: :product,
@@ -32,18 +23,6 @@ module Spree
                       path: ':rails_root/public/spree/products/:id/:style/:basename.:extension',
                       convert_options: { all: '-strip -auto-orient -colorspace sRGB' }
 
-    after_post_process do
-      if attachment.errors.blank?
-        attachable = {
-          io: File.open(local_filename_of_original),
-          filename: attachment_file_name,
-          content_type: attachment_content_type,
-          identify: false,
-        }
-        self.active_storage_attachment = attachable
-      end
-    end
-
     # save the w,h of the original image (from which others can be calculated)
     # we need to look at the write-queue for images which have not been saved yet
     after_post_process :find_dimensions

From 92bbcbb7ceb4aa206b3d98f33cac18ce03646b61 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 5 Apr 2022 15:48:14 +1000
Subject: [PATCH 08/12] Process correct attachment when model has several

Luckily Paperclip has designated callbacks for processing each
attachment separately. We can just hook into that.
---
 app/models/concerns/has_migrating_file.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/concerns/has_migrating_file.rb b/app/models/concerns/has_migrating_file.rb
index d82528f38c..83d736ab93 100644
--- a/app/models/concerns/has_migrating_file.rb
+++ b/app/models/concerns/has_migrating_file.rb
@@ -22,10 +22,10 @@ module HasMigratingFile
       # We store files with Paperclip *and* Active Storage while we migrate
       # old Paperclip files to Active Storage. This enables availability
       # during the migration.
-      after_post_process do
-        if public_send(name).errors.blank?
-          file = File.open(processed_local_file_path)
-          attach_file(name, file)
+      public_send("after_#{name}_post_process") do
+        path = processed_local_file_path(name)
+        if public_send(name).errors.blank? && path.present?
+          attach_file(name, File.open(path))
         end
       end
     end

From 7bcfda0a527d6e226b36a38834e27f9827c62404 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Mon, 4 Apr 2022 15:01:52 +1000
Subject: [PATCH 09/12] Duplicate all new Paperclip files to ActiveStorage

We do this for all models in the code base. There's one special case,
the ConentConfiguration which is not a model and we can't use the same
approach there. We will have to deal with that separately later.
---
 app/models/enterprise.rb            | 7 ++++---
 app/models/enterprise_group.rb      | 5 +++--
 app/models/terms_of_service_file.rb | 4 +++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index 1e73ae448f..9b9e587bed 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -3,6 +3,7 @@
 require 'spree/core/s3_support'
 
 class Enterprise < ApplicationRecord
+  include HasMigratingFile
   include Spree::Core::S3Support
 
   SELLS = %w(unspecified none own any).freeze
@@ -72,12 +73,12 @@ class Enterprise < ApplicationRecord
                                               tag_rule[:preferred_customer_tags].blank?
                                             }
 
-  has_attached_file :logo,
+  has_one_migrating :logo,
                     styles: { medium: "300x300>", small: "180x180>", thumb: "100x100>" },
                     url: '/images/enterprises/logos/:id/:style/:basename.:extension',
                     path: 'public/images/enterprises/logos/:id/:style/:basename.:extension'
 
-  has_attached_file :promo_image,
+  has_one_migrating :promo_image,
                     styles: {
                       large: ["1200x260#", :jpg],
                       medium: ["720x156#", :jpg],
@@ -88,7 +89,7 @@ class Enterprise < ApplicationRecord
   validates_attachment_content_type :logo, content_type: %r{\Aimage/.*\Z}
   validates_attachment_content_type :promo_image, content_type: %r{\Aimage/.*\Z}
 
-  has_attached_file :terms_and_conditions,
+  has_one_migrating :terms_and_conditions,
                     url: '/files/enterprises/terms_and_conditions/:id/:basename.:extension',
                     path: 'public/files/enterprises/terms_and_conditions/:id/:basename.:extension'
   validates_attachment_content_type :terms_and_conditions,
diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index 6054043bf0..8b2c908006 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -4,6 +4,7 @@ require 'open_food_network/locking'
 require 'spree/core/s3_support'
 
 class EnterpriseGroup < ApplicationRecord
+  include HasMigratingFile
   include PermalinkGenerator
   include Spree::Core::S3Support
 
@@ -27,12 +28,12 @@ class EnterpriseGroup < ApplicationRecord
 
   delegate :phone, :address1, :address2, :city, :zipcode, :state, :country, to: :address
 
-  has_attached_file :logo,
+  has_one_migrating :logo,
                     styles: { medium: "100x100" },
                     url: '/images/enterprise_groups/logos/:id/:style/:basename.:extension',
                     path: 'public/images/enterprise_groups/logos/:id/:style/:basename.:extension'
 
-  has_attached_file :promo_image,
+  has_one_migrating :promo_image,
                     styles: { large: ["1200x260#", :jpg] },
                     url: '/images/enterprise_groups/promo_images/:id/:style/:basename.:extension',
                     path: 'public/images/enterprise_groups/promo_images/:id/:style/:basename.:extension'
diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb
index 471bdb8ca3..add728403b 100644
--- a/app/models/terms_of_service_file.rb
+++ b/app/models/terms_of_service_file.rb
@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 class TermsOfServiceFile < ApplicationRecord
-  has_attached_file :attachment
+  include HasMigratingFile
+
+  has_one_migrating :attachment
 
   validates :attachment, presence: true
 

From 0b885b3954b51ae4aaf3de94fa866b3c72b8953c Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 5 Apr 2022 14:58:08 +1000
Subject: [PATCH 10/12] Protect terms of service from migration updates

Active Storage always touches associated records when attachments are
changed. But for the Terms of Service it's important to keep the
updated_at date because that's how we find out how new it is and if a
customer accepted those terms already.

And while we migrate files, the content of the files will stay the same
and we don't want customers to be asked to accept the same terms again.
---
 app/models/terms_of_service_file.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb
index add728403b..d652c37f7e 100644
--- a/app/models/terms_of_service_file.rb
+++ b/app/models/terms_of_service_file.rb
@@ -21,4 +21,9 @@ class TermsOfServiceFile < ApplicationRecord
   def self.updated_at
     current&.updated_at || Time.zone.now
   end
+
+  def touch(_)
+    # Ignore Active Storage changing the timestamp during migrations.
+    # This can be removed once we got rid of Paperclip.
+  end
 end

From 1c1f9d73a3c055264f03e21a54cb5df33f129115 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Mon, 4 Apr 2022 16:25:48 +1000
Subject: [PATCH 11/12] Add task to migrate existing files to Active Storage

Common migrations look for all models with *_file_name attributes but I
found that unreliable in our code base. It finds too many model classes
and doesn't allow us to be more selective in the migration. So I used
our own migration declaration to migrate exactly those attachments
specified.
---
 app/models/concerns/has_migrating_file.rb     | 10 ++
 .../from_paperclip_to_active_storage.rake     | 94 +++++++++++++++++++
 ...m_paperclip_to_active_storage_rake_spec.rb | 81 ++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 lib/tasks/from_paperclip_to_active_storage.rake
 create mode 100644 spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb

diff --git a/app/models/concerns/has_migrating_file.rb b/app/models/concerns/has_migrating_file.rb
index 83d736ab93..923ee59ba3 100644
--- a/app/models/concerns/has_migrating_file.rb
+++ b/app/models/concerns/has_migrating_file.rb
@@ -3,6 +3,16 @@
 module HasMigratingFile
   extend ActiveSupport::Concern
 
+  @migrating_models = []
+
+  def self.migrating_models
+    @migrating_models
+  end
+
+  included do
+    HasMigratingFile.migrating_models.push(name)
+  end
+
   class_methods do
     def has_one_migrating(name, paperclip_options = {})
       # Active Storage declaration
diff --git a/lib/tasks/from_paperclip_to_active_storage.rake b/lib/tasks/from_paperclip_to_active_storage.rake
new file mode 100644
index 0000000000..1d59f65f12
--- /dev/null
+++ b/lib/tasks/from_paperclip_to_active_storage.rake
@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+namespace :from_paperclip_to_active_storage do
+  # This migration can't be a pure database migration because we need to know
+  # the location of current files which is computed by Paperclip depending on
+  # the `url` option.
+  desc "Copy data to Active Storage tables referencing Paperclip files"
+  task migrate: :environment do
+    Rails.application.eager_load!
+
+    HasMigratingFile.migrating_models.each do |model_name|
+      puts "Migrating #{model_name}"
+      migrate_model(model_name.constantize)
+    end
+  end
+
+  def migrate_model(model)
+    duplicated_attachment_names(model).each do |name|
+      migrate_attachment(model, name)
+    end
+  end
+
+  def migrate_attachment(model, name)
+    records_to_migrate = missing_active_storage_attachment(model, name)
+
+    print " - #{name} (#{records_to_migrate.count}) "
+
+    records_to_migrate.find_each do |record|
+      attach_paperclip(name, record)
+    end
+
+    puts ""
+  end
+
+  def attach_paperclip(name, record)
+    paperclip = record.public_send(name)
+
+    if paperclip.respond_to?(:s3_object)
+      attachment = storage_record_for(name, paperclip)
+      record.public_send("#{name}_attachment=", attachment)
+      print "."
+    elsif File.exist?(paperclip.path)
+      record.attach_file(name, File.open(paperclip.path))
+      record.save!
+      print "."
+    else
+      print "x"
+    end
+  rescue StandardError => e
+    puts "x"
+    puts e.message
+  end
+
+  # Creates an Active Storage record pointing to the same file Paperclip
+  # stored on AWS S3. Getting the checksum requires a HEAD request.
+  # In my tests, I could process 100 records per minute this way.
+  def storage_record_for(name, paperclip)
+    blob = ActiveStorage::Blob.new(
+      key: paperclip.path(:original),
+      filename: paperclip.original_filename,
+      content_type: paperclip.content_type,
+      metadata: {},
+      byte_size: paperclip.size,
+      checksum: paperclip.s3_object.etag,
+      created_at: paperclip.updated_at,
+    )
+    ActiveStorage::Attachment.new(
+      name: name,
+      blob: blob,
+      created_at: paperclip.updated_at,
+    )
+  end
+
+  def duplicated_attachment_names(model)
+    paperclip_attachments = model.attachment_definitions.keys.map(&:to_s)
+    active_storage_attachments = model.attachment_reflections.keys
+
+    only_paperclip = paperclip_attachments - active_storage_attachments
+    only_active_storage = active_storage_attachments - paperclip_attachments
+    both = paperclip_attachments & active_storage_attachments
+
+    puts "WARNING: not migrating #{only_paperclip}" if only_paperclip.present?
+    puts "WARNING: no source for #{only_active_storage}" if only_active_storage.present?
+
+    both
+  end
+
+  # Records with Paperclip but without an Active storage attachment yet
+  def missing_active_storage_attachment(model, attachment)
+    model.where.not("#{attachment}_file_name" => [nil, ""]).
+      left_outer_joins("#{attachment}_attachment".to_sym).
+      where(active_storage_attachments: { id: nil })
+  end
+end
diff --git a/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb
new file mode 100644
index 0000000000..8f3641ba16
--- /dev/null
+++ b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "rake"
+
+describe "from_paperclip_to_active_storage.rake" do
+  include FileHelper
+
+  let(:file) { Rack::Test::UploadedFile.new(black_logo_file, 'image/png') }
+  let(:s3_config) {
+    {
+      url: ":s3_alias_url",
+      storage: :s3,
+      s3_credentials: {
+        access_key_id: "A...A",
+        secret_access_key: "H...H",
+      },
+      s3_headers: { "Cache-Control" => "max-age=31557600" },
+      bucket: "ofn",
+      s3_protocol: "https",
+      s3_host_alias: "ofn.s3.us-east-1.amazonaws.com",
+
+      # This is for easier testing:
+      path: "/:id/:style/:basename.:extension",
+    }
+  }
+
+  before(:all) do
+    Rake.application.rake_require "tasks/from_paperclip_to_active_storage"
+    Rake::Task.define_task(:environment)
+  end
+
+  describe ":migrate" do
+    it "creates Active Storage records for existing images on disk" do |example|
+      image = Spree::Image.create!(attachment: file)
+      image.attachment_attachment.delete
+      image.attachment_blob.delete
+
+      expect {
+        run_task "from_paperclip_to_active_storage:migrate"
+      }.to change {
+        image.reload.active_storage_attachment.attached?
+      }.to(true)
+    end
+
+    it "creates Active Storage records for existing images on disk" do |example|
+      attachment_definition = Spree::Image.attachment_definitions[:attachment]
+      allow(Spree::Image).to receive(:attachment_definitions).and_return(
+        attachment: attachment_definition.merge(s3_config)
+      )
+      allow(Rails.application.config.active_storage).
+        to receive(:service).and_return(:test_amazon)
+
+      stub_request(:put, /amazonaws/).to_return(status: 200, body: "", headers: {})
+      stub_request(:head, /amazonaws/).to_return(
+        status: 200, body: "",
+        headers: {
+          "ETag" => "md5sum000test000example"
+        }
+      )
+      stub_request(:put, /amazonaws/).to_return(status: 200, body: "", headers: {})
+
+      image = Spree::Image.create!(attachment: file)
+      image.attachment_attachment.delete
+      image.attachment_blob.delete
+
+      expect {
+        run_task "from_paperclip_to_active_storage:migrate"
+      }.to change {
+        image.reload.active_storage_attachment.attached?
+      }.to(true)
+
+      expect(image.attachment_blob.checksum).to eq "md5sum000test000example"
+    end
+  end
+
+  def run_task(name)
+    Rake::Task[name].reenable
+    Rake.application.invoke_task(name)
+  end
+end

From 9dc3c5a06c903a1a9ce7634f86fe75cde7bec2d9 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 5 Apr 2022 13:22:48 +1000
Subject: [PATCH 12/12] Add task for ContentConfig image migration

---
 .../spree/preferences/file_configuration.rb   | 15 +++++++-
 .../from_paperclip_to_active_storage.rake     | 36 +++++++++++++++++++
 ...m_paperclip_to_active_storage_rake_spec.rb | 33 +++++++++++++++--
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/preferences/file_configuration.rb b/app/models/spree/preferences/file_configuration.rb
index 7015e60296..421484993f 100644
--- a/app/models/spree/preferences/file_configuration.rb
+++ b/app/models/spree/preferences/file_configuration.rb
@@ -5,6 +5,10 @@ module Spree
     class FileConfiguration < Configuration
       def self.preference(name, type, *args)
         if type == :file
+          # Active Storage blob id:
+          super "#{name}_blob_id", :integer, *args
+
+          # Paperclip attachment attributes:
           super "#{name}_file_name",    :string,  *args
           super "#{name}_content_type", :string,  *args
           super "#{name}_file_size",    :integer, *args
@@ -17,7 +21,12 @@ module Spree
 
       def get_preference(key)
         if !has_preference?(key) && has_attachment?(key)
+          # Call Paperclip's attachment method:
           public_send key
+        elsif key.ends_with?("_blob")
+          # Find referenced Active Storage blob:
+          blob_id = super("#{key}_id")
+          ActiveStorage::Blob.find_by(id: blob_id)
         else
           super key
         end
@@ -37,7 +46,11 @@ module Spree
       # errors if respond_to? isn't correct, so we override it here.
       def respond_to?(method, include_all = false)
         name = method.to_s.delete('=')
-        super(self.class.preference_getter_method(name), include_all) || super(method, include_all)
+        reference_name = "#{name}_id"
+
+        super(self.class.preference_getter_method(name), include_all) ||
+          super(reference_name, include_all) ||
+          super(method, include_all)
       end
 
       def has_attachment?(name)
diff --git a/lib/tasks/from_paperclip_to_active_storage.rake b/lib/tasks/from_paperclip_to_active_storage.rake
index 1d59f65f12..8859c7bed4 100644
--- a/lib/tasks/from_paperclip_to_active_storage.rake
+++ b/lib/tasks/from_paperclip_to_active_storage.rake
@@ -14,6 +14,25 @@ namespace :from_paperclip_to_active_storage do
     end
   end
 
+  # We have a special class called ContentConfiguration which is not a model
+  # and therfore can't use the normal Active Storage magic.
+  #
+  # It uses `Spree::Preference`s to store all the Paperclip attributes. These
+  # files are stored locally and we can replace them with preferences pointing
+  # to an Active Storage blob.
+  desc "Associate ContentConfig to ActiveStorage blobs"
+  task copy_content_config: :environment do
+    [
+      :logo,
+      :logo_mobile,
+      :logo_mobile_svg,
+      :home_hero,
+      :footer_logo,
+    ].each do |name|
+      migrate_content_config_file(name)
+    end
+  end
+
   def migrate_model(model)
     duplicated_attachment_names(model).each do |name|
       migrate_attachment(model, name)
@@ -71,6 +90,23 @@ namespace :from_paperclip_to_active_storage do
     )
   end
 
+  def migrate_content_config_file(name)
+    paperclip = ContentConfig.public_send(name)
+
+    return if ContentConfig.public_send("#{name}_blob_id")
+    return if paperclip.path.blank? || !paperclip.exists?
+
+    blob = ActiveStorage::Blob.create_and_upload!(
+      io: File.open(paperclip.path),
+      filename: paperclip.original_filename,
+      content_type: paperclip.content_type,
+      identify: false,
+    )
+
+    ContentConfig.public_send("#{name}_blob_id=", blob.id)
+    puts "Copied #{name}"
+  end
+
   def duplicated_attachment_names(model)
     paperclip_attachments = model.attachment_definitions.keys.map(&:to_s)
     active_storage_attachments = model.attachment_reflections.keys
diff --git a/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb
index 8f3641ba16..e13cfc2a18 100644
--- a/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb
+++ b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb
@@ -31,7 +31,7 @@ describe "from_paperclip_to_active_storage.rake" do
   end
 
   describe ":migrate" do
-    it "creates Active Storage records for existing images on disk" do |example|
+    it "creates Active Storage records for existing images on disk" do
       image = Spree::Image.create!(attachment: file)
       image.attachment_attachment.delete
       image.attachment_blob.delete
@@ -43,7 +43,7 @@ describe "from_paperclip_to_active_storage.rake" do
       }.to(true)
     end
 
-    it "creates Active Storage records for existing images on disk" do |example|
+    it "creates Active Storage records for images on AWS S3" do
       attachment_definition = Spree::Image.attachment_definitions[:attachment]
       allow(Spree::Image).to receive(:attachment_definitions).and_return(
         attachment: attachment_definition.merge(s3_config)
@@ -74,6 +74,35 @@ describe "from_paperclip_to_active_storage.rake" do
     end
   end
 
+  describe ":copy_content_config" do
+    it "doesn't copy default images" do
+      run_task "from_paperclip_to_active_storage:copy_content_config"
+
+      expect(ContentConfig.logo_blob).to eq nil
+    end
+
+    it "copies uploaded images" do
+      ContentConfig.logo = file
+      ContentConfig.logo.save
+
+      run_task "from_paperclip_to_active_storage:copy_content_config"
+
+      expect(ContentConfig.logo_blob).to be_a ActiveStorage::Blob
+    end
+
+    it "doesn't copy twice" do
+      ContentConfig.logo = file
+      ContentConfig.logo.save
+
+      expect {
+        run_task "from_paperclip_to_active_storage:copy_content_config"
+        run_task "from_paperclip_to_active_storage:copy_content_config"
+      }.to change {
+        ActiveStorage::Blob.count
+      }.by(1)
+    end
+  end
+
   def run_task(name)
     Rake::Task[name].reenable
     Rake.application.invoke_task(name)