Store files with both Paperclip and ActiveStorage

In order to migrate existing files from Paperclip to ActiveStorage, we
need Paperclip to find out the files associated to existing database
records. So we can't simply replace Paperclip with ActiveStorage.

That's why it's usually recommended [1] to first run the migration and
then replace Paperclip with ActiveStorage using two consecutive
deployments.

However, in our case we can't rely on two consecutive deployments
because we have to make an easy process so existing CONSUL installations
don't run into any issues. We can't just release version 1.4.0 and 1.5.0
and day and ask everyone to upgrade twice on the same day.

Instead, we're following a different plan:

* We're going to provide a Rake task (which will require Paperclip) to
  migrate existing files
* We still use Paperclip to generate link and image tags
* New files are handled using both Paperclip and ActiveStorage; that
  way, when we make the switch, we won't have to migrate them, and in
  the meantime they'll be accessible thanks to Paperclip
* After we make the switch, we'll update the `name` column in the active
  storage attachments tables in order to remove the `storage_` prefix

Regarding our handling of new files, the exception are cached
attachments. Since those attachments are temporary files used while
submitting a form and we have to delete them afterwards, we're only
handling them with Paperclip. We'll handle these ones in version 1.5.0.

Note the task creating the dev seeds was failing after these changes
with an `ActiveStorage::IntegrityError` exception because we were
opening some files without closing them. If the same file was attached
twice, it failed the second time.

We're solving it by closing the files with `File.open` and a block. Even
though we didn't get any errors, we're doing the same thing in the
`Attachable` concern because it's a good practice to close files after
we're done with them.

Also note we have to change the CKEditor Active Storage code so it's
compatible with Paperclip. In this case, I haven't been able to write a
test to confirm the attachment exists; I was getting the same
`ActiveStorage::IntegrityError` mentioned above.

Finally, we're updating the site customization image controller to use
`update` so the image and the attachment are updated within the same
transaction. This is also what we do in most controllers.

[1] https://www.youtube.com/watch?v=tZ_WNUytO9o
This commit is contained in:
Javi Martín
2021-07-26 01:21:00 +02:00
parent f412ab2c9a
commit 1290e2ecd3
15 changed files with 124 additions and 54 deletions

View File

@@ -26,8 +26,7 @@ class Admin::SiteCustomization::ImagesController < Admin::SiteCustomization::Bas
end end
def destroy def destroy
@image.image = nil if @image.update(image: nil)
if @image.save
notice = t("admin.site_customization.images.destroy.notice") notice = t("admin.site_customization.images.destroy.notice")
else else
notice = t("admin.site_customization.images.destroy.error") notice = t("admin.site_customization.images.destroy.error")

View File

@@ -1,4 +1,5 @@
class Ckeditor::Asset < ApplicationRecord class Ckeditor::Asset < ApplicationRecord
include Ckeditor::Orm::ActiveRecord::AssetBase include Ckeditor::Orm::ActiveRecord::AssetBase
include Ckeditor::Backend::ActiveStorage
include Ckeditor::Backend::Paperclip include Ckeditor::Backend::Paperclip
end end

View File

@@ -1,8 +1,10 @@
class Ckeditor::Picture < Ckeditor::Asset class Ckeditor::Picture < Ckeditor::Asset
has_attached_file :data, include HasAttachment
url: "/ckeditor_assets/pictures/:id/:style_:basename.:extension",
path: ":rails_root/public/ckeditor_assets/pictures/:id/:style_:basename.:extension", has_attachment :data,
styles: { content: "800>", thumb: "118x100#" } url: "/ckeditor_assets/pictures/:id/:style_:basename.:extension",
path: ":rails_root/public/ckeditor_assets/pictures/:id/:style_:basename.:extension",
styles: { content: "800>", thumb: "118x100#" }
validates_attachment_presence :data validates_attachment_presence :data
validates_attachment_size :data, less_than: 2.megabytes validates_attachment_size :data, less_than: 2.megabytes

View File

@@ -1,4 +1,5 @@
module Attachable module Attachable
include HasAttachment
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
@@ -33,11 +34,11 @@ module Attachable
end end
def set_attachment_from_cached_attachment def set_attachment_from_cached_attachment
self.attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem if Paperclip::Attachment.default_options[:storage] == :filesystem
File.open(cached_attachment) File.open(cached_attachment) { |file| self.attachment = file }
else else
URI.parse(cached_attachment) self.attachment = URI.parse(cached_attachment)
end end
end end
def prefix(attachment, _style) def prefix(attachment, _style)

View File

@@ -0,0 +1,24 @@
module HasAttachment
extend ActiveSupport::Concern
class_methods do
def has_attachment(attribute, paperclip_options = {})
has_one_attached :"storage_#{attribute}"
has_attached_file attribute, paperclip_options
alias_method :"paperclip_#{attribute}=", :"#{attribute}="
define_method :"#{attribute}=" do |file|
if file.is_a?(IO)
send(:"storage_#{attribute}").attach(io: file, filename: File.basename(file.path))
elsif file.nil?
send(:"storage_#{attribute}").detach
else
send(:"storage_#{attribute}").attach(file)
end
send(:"paperclip_#{attribute}=", file)
end
end
end
end

View File

@@ -53,7 +53,7 @@ class DirectUpload
def relation_attributtes def relation_attributtes
{ {
attachment: @attachment, paperclip_attachment: @attachment,
cached_attachment: @cached_attachment, cached_attachment: @cached_attachment,
user: @user user: @user
} }

View File

@@ -1,10 +1,10 @@
class Document < ApplicationRecord class Document < ApplicationRecord
include Attachable include Attachable
has_attached_file :attachment, url: "/system/:class/:prefix/:style/:hash.:extension", has_attachment :attachment, url: "/system/:class/:prefix/:style/:hash.:extension",
hash_data: ":class/:style/:custom_hash_data", hash_data: ":class/:style/:custom_hash_data",
use_timestamp: false, use_timestamp: false,
hash_secret: Rails.application.secrets.secret_key_base hash_secret: Rails.application.secrets.secret_key_base
belongs_to :user belongs_to :user
belongs_to :documentable, polymorphic: true, touch: true belongs_to :documentable, polymorphic: true, touch: true

View File

@@ -1,15 +1,15 @@
class Image < ApplicationRecord class Image < ApplicationRecord
include Attachable include Attachable
has_attached_file :attachment, styles: { has_attachment :attachment, styles: {
large: "x#{Setting["uploads.images.min_height"]}", large: "x#{Setting["uploads.images.min_height"]}",
medium: "300x300#", medium: "300x300#",
thumb: "140x245#" thumb: "140x245#"
}, },
url: "/system/:class/:prefix/:style/:hash.:extension", url: "/system/:class/:prefix/:style/:hash.:extension",
hash_data: ":class/:style", hash_data: ":class/:style",
use_timestamp: false, use_timestamp: false,
hash_secret: Rails.application.secrets.secret_key_base hash_secret: Rails.application.secrets.secret_key_base
belongs_to :user belongs_to :user
belongs_to :imageable, polymorphic: true, touch: true belongs_to :imageable, polymorphic: true, touch: true

View File

@@ -1,4 +1,6 @@
class SiteCustomization::Image < ApplicationRecord class SiteCustomization::Image < ApplicationRecord
include HasAttachment
VALID_IMAGES = { VALID_IMAGES = {
"logo_header" => [260, 80], "logo_header" => [260, 80],
"social_media_icon" => [470, 246], "social_media_icon" => [470, 246],
@@ -9,7 +11,7 @@ class SiteCustomization::Image < ApplicationRecord
"logo_email" => [400, 80] "logo_email" => [400, 80]
}.freeze }.freeze
has_attached_file :image has_attachment :image
validates :name, presence: true, uniqueness: true, inclusion: { in: VALID_IMAGES.keys } validates :name, presence: true, uniqueness: true, inclusion: { in: VALID_IMAGES.keys }
validates_attachment_content_type :image, content_type: ["image/png", "image/jpeg"] validates_attachment_content_type :image, content_type: ["image/png", "image/jpeg"]

View File

@@ -6,21 +6,23 @@ INVESTMENT_IMAGE_FILES = %w[
olesya-grichina-218176-unsplash_713x475.jpg olesya-grichina-218176-unsplash_713x475.jpg
sole-d-alessandro-340443-unsplash_713x475.jpg sole-d-alessandro-340443-unsplash_713x475.jpg
].map do |filename| ].map do |filename|
File.new(Rails.root.join("db", Rails.root.join("db",
"dev_seeds", "dev_seeds",
"images", "images",
"budget", "budget",
"investments", filename)) "investments", filename)
end end
def add_image_to(imageable) def add_image_to(imageable)
# imageable should respond to #title & #author # imageable should respond to #title & #author
imageable.image = Image.create!({ File.open(INVESTMENT_IMAGE_FILES.sample) do |file|
imageable: imageable, imageable.image = Image.create!({
title: imageable.title, imageable: imageable,
attachment: INVESTMENT_IMAGE_FILES.sample, title: imageable.title,
user: imageable.author attachment: file,
}) user: imageable.author
})
end
imageable.save! imageable.save!
end end

View File

@@ -4,20 +4,22 @@ IMAGE_FILES = %w[
steve-harvey-597760-unsplash_713x475.jpg steve-harvey-597760-unsplash_713x475.jpg
tim-mossholder-302931-unsplash_713x475.jpg tim-mossholder-302931-unsplash_713x475.jpg
].map do |filename| ].map do |filename|
File.new(Rails.root.join("db", Rails.root.join("db",
"dev_seeds", "dev_seeds",
"images", "images",
"proposals", filename)) "proposals", filename)
end end
def add_image_to(imageable) def add_image_to(imageable)
# imageable should respond to #title & #author # imageable should respond to #title & #author
imageable.image = Image.create!({ File.open(IMAGE_FILES.sample) do |file|
imageable: imageable, imageable.image = Image.create!({
title: imageable.title, imageable: imageable,
attachment: IMAGE_FILES.sample, title: imageable.title,
user: imageable.author attachment: file,
}) user: imageable.author
})
end
imageable.save! imageable.save!
end end

View File

@@ -16,7 +16,7 @@ module Ckeditor
base.class_eval do base.class_eval do
before_save :apply_data before_save :apply_data
validate do validate do
if data.nil? || file.nil? if data.nil? || storage_file.nil?
errors.add(:data, :not_data_present, message: "data must be present") errors.add(:data, :not_data_present, message: "data must be present")
end end
end end
@@ -46,19 +46,25 @@ module Ckeditor
protected protected
def file def storage_file
@file ||= storage_data @storage_file ||= storage_data
end end
def blob def blob
@blob ||= ::ActiveStorage::Blob.find(file.attachment.blob_id) @blob ||= ::ActiveStorage::Blob.find(storage_file.attachment.blob_id)
end end
def apply_data def apply_data
if data.is_a?(Ckeditor::Http::QqFile) non_paperclip_data = if data.is_a?(::Paperclip::Attachment)
storage_data.attach(io: data, filename: data.original_filename) file.instance_variable_get("@target")
else
data
end
if non_paperclip_data.is_a?(Ckeditor::Http::QqFile)
storage_data.attach(io: non_paperclip_data, filename: non_paperclip_data.original_filename)
else else
storage_data.attach(data) storage_data.attach(non_paperclip_data)
end end
self.data_file_name = storage_data.blob.filename self.data_file_name = storage_data.blob.filename

View File

@@ -4,6 +4,16 @@ describe Document do
it_behaves_like "document validations", "budget_investment_document" it_behaves_like "document validations", "budget_investment_document"
it_behaves_like "document validations", "proposal_document" it_behaves_like "document validations", "proposal_document"
it "stores attachments with both Paperclip and Active Storage" do
document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
expect(document.attachment).to exist
expect(document.attachment_file_name).to eq "clippy.pdf"
expect(document.storage_attachment).to be_attached
expect(document.storage_attachment.filename).to eq "clippy.pdf"
end
context "scopes" do context "scopes" do
describe "#admin" do describe "#admin" do
it "returns admin documents" do it "returns admin documents" do

View File

@@ -3,4 +3,14 @@ require "rails_helper"
describe Image do describe Image do
it_behaves_like "image validations", "budget_investment_image" it_behaves_like "image validations", "budget_investment_image"
it_behaves_like "image validations", "proposal_image" it_behaves_like "image validations", "proposal_image"
it "stores attachments with both Paperclip and Active Storage" do
image = create(:image, attachment: File.new("spec/fixtures/files/clippy.jpg"))
expect(image.attachment).to exist
expect(image.attachment_file_name).to eq "clippy.jpg"
expect(image.storage_attachment).to be_attached
expect(image.storage_attachment.filename).to eq "clippy.jpg"
end
end end

View File

@@ -1,6 +1,17 @@
require "rails_helper" require "rails_helper"
describe SiteCustomization::Image do describe SiteCustomization::Image do
it "stores images with both Paperclip and Active Storage" do
image = create(:site_customization_image, name: "map",
image: File.new("spec/fixtures/files/custom_map.jpg"))
expect(image.image).to exist
expect(image.image_file_name).to eq "custom_map.jpg"
expect(image.storage_image).to be_attached
expect(image.storage_image.filename).to eq "custom_map.jpg"
end
describe "logo" do describe "logo" do
it "is valid with a 260x80 image" do it "is valid with a 260x80 image" do
image = build(:site_customization_image, image = build(:site_customization_image,