Merge pull request #4598 from consul/active_storage_dual

Store files with both Paperclip and ActiveStorage
This commit is contained in:
Javi Martín
2021-09-24 16:31:56 +02:00
committed by GitHub
30 changed files with 518 additions and 61 deletions

1
.gitignore vendored
View File

@@ -40,3 +40,4 @@ public/assets/
public/machine_learning/data/
public/system/
/public/ckeditor_assets/
storage/

View File

@@ -8,6 +8,7 @@ AllCops:
DisplayStyleGuide: true
Exclude:
- "db/schema.rb"
- "lib/ckeditor/backend/active_storage.rb"
DisabledByDefault: true
Bundler/DuplicatedGem:

View File

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

View File

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

View File

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

View File

@@ -1,4 +1,5 @@
module Attachable
include HasAttachment
extend ActiveSupport::Concern
included do
@@ -33,11 +34,11 @@ module Attachable
end
def set_attachment_from_cached_attachment
self.attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem
File.open(cached_attachment)
else
URI.parse(cached_attachment)
end
if Paperclip::Attachment.default_options[:storage] == :filesystem
File.open(cached_attachment) { |file| self.attachment = file }
else
self.attachment = URI.open(cached_attachment)
end
end
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) || file.is_a?(Tempfile) && !file.is_a?(Ckeditor::Http::QqFile)
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
{
attachment: @attachment,
paperclip_attachment: @attachment,
cached_attachment: @cached_attachment,
user: @user
}

View File

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

View File

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

View File

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

View File

@@ -1,17 +1,6 @@
require_relative "boot"
require "rails"
# Pick the frameworks you want:
require "active_model/railtie"
require "active_job/railtie"
require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_mailer/railtie"
require "action_view/railtie"
require "action_cable/engine"
require "sprockets/railtie"
require "rails/test_unit/railtie"
require "rails/all"
# Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production.
@@ -35,6 +24,9 @@ module Consul
# Handle custom exceptions
config.action_dispatch.rescue_responses["FeatureFlags::FeatureDisabled"] = :forbidden
# Store files locally.
config.active_storage.service = :local
# Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
# Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
# config.time_zone = 'Central Time (US & Canada)'

View File

@@ -22,7 +22,7 @@ set :pty, true
set :use_sudo, false
set :linked_files, %w[config/database.yml config/secrets.yml]
set :linked_dirs, %w[.bundle log tmp public/system public/assets public/ckeditor_assets public/machine_learning/data]
set :linked_dirs, %w[.bundle log tmp public/system public/assets public/ckeditor_assets public/machine_learning/data storage]
set :keep_releases, 5

View File

@@ -47,6 +47,9 @@ Rails.application.configure do
# Raises error for missing translations
# config.action_view.raise_on_missing_translations = true
# Store files in tmp folders.
config.active_storage.service = :test
config.cache_store = :null_store
config.after_initialize do

View File

@@ -0,0 +1,11 @@
ActiveStorage::DirectUploadsController.class_eval do
def create
head :unauthorized
end
end
ActiveStorage::DiskController.class_eval do
def update
head :unauthorized
end
end

32
config/storage.yml Normal file
View File

@@ -0,0 +1,32 @@
local:
service: Disk
root: <%= Rails.root.join("storage") %>
test:
service: Disk
root: <%= Rails.root.join("tmp/storage") %>
# s3:
# service: S3
# access_key_id: <%= Rails.application.secrets.dig(:s3, :access_key_id) %>
# secret_access_key: <%= Rails.application.secrets.dig(:s3, :secret_access_key) %>
# region: <%= Rails.application.secrets.dig(:s3, :region) %>
# bucket: <%= Rails.application.secrets.dig(:s3, :bucket) %>
# Remember not to checkin your GCS keyfile to a repository
# gcs:
# service: GCS
# project: <%= Rails.application.secrets.dig(:gcs, :project) %>
# credentials: <%= Rails.root.join(Rails.application.secrets.dig(:gcs, :credentials).to_s) %>
# bucket: <%= Rails.application.secrets.dig(:gcs, :bucket) %>
# azure:
# service: AzureStorage
# storage_account_name: <%= Rails.application.secrets.dig(:azure, :storage_account_name) %>
# storage_access_key: <%= Rails.application.secrets.dig(:azure, :storage_access_key) %>
# container: <%= Rails.application.secrets.dig(:azure, :container) %>
# mirror:
# service: Mirror
# primary: local
# mirrors: [ s3, gcs, azure ]

View File

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

View File

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

View File

@@ -0,0 +1,27 @@
# 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.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
end
end

View File

@@ -32,6 +32,27 @@ ActiveRecord::Schema.define(version: 2021_08_11_195800) do
t.datetime "updated_at", null: false
end
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.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 "activities", id: :serial, force: :cascade do |t|
t.integer "user_id"
t.string "action"
@@ -1726,6 +1747,7 @@ ActiveRecord::Schema.define(version: 2021_08_11_195800) do
t.datetime "updated_at", null: false
end
add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
add_foreign_key "administrators", "users"
add_foreign_key "budget_administrators", "administrators"
add_foreign_key "budget_administrators", "budgets"

View File

@@ -0,0 +1,79 @@
# frozen_string_literal: true
# Code copied from the ckeditor gem:
# https://github.com/galetahub/ckeditor/pull/853
module Ckeditor
module Backend
module ActiveStorage
def self.included(base)
base.send(:include, Rails.application.routes.url_helpers)
base.send(:include, InstanceMethods)
base.send(:extend, ClassMethods)
end
module ClassMethods
def self.extended(base)
base.class_eval do
before_save :apply_data
validate do
if data.nil? || storage_file.nil?
errors.add(:data, :not_data_present, message: "data must be present")
end
end
end
end
end
module InstanceMethods
def url
rails_blob_path(self.storage_data, only_path: true)
end
def path
rails_blob_path(self.storage_data, only_path: true)
end
def styles
end
def content_type
self.storage_data.content_type
end
def content_type=(_content_type)
self.storage_data.content_type = _content_type
end
protected
def storage_file
@storage_file ||= storage_data
end
def blob
@blob ||= ::ActiveStorage::Blob.find(storage_file.attachment.blob_id)
end
def apply_data
non_paperclip_data = if data.is_a?(::Paperclip::Attachment)
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
storage_data.attach(non_paperclip_data)
end
self.data_file_name = storage_data.blob.filename
self.data_content_type = storage_data.blob.content_type
self.data_file_size = storage_data.blob.byte_size
end
end
end
autoload :ActiveStorage, "ckeditor/backend/active_storage"
end
end

View File

@@ -0,0 +1,98 @@
# This code is based on Thoughtbot's guide to migrating from Paperclip
# to Active Storage:
# https://github.com/thoughtbot/paperclip/blob/master/MIGRATING.md
namespace :active_storage do
desc "Copy paperclip's attachment database columns to active storage"
task migrate_from_paperclip: :environment do
logger = ApplicationLogger.new
connection = ActiveRecord::Base.connection.raw_connection
statement_name = "active_storage_statement"
unless connection.exec("SELECT COUNT(*) FROM pg_prepared_statements WHERE name='#{statement_name}'").each_row.to_a[0][0] > 0
connection.prepare(statement_name, <<-SQL)
with rows as(
INSERT INTO active_storage_blobs (
key, filename, content_type, metadata, byte_size, checksum, created_at
) VALUES ($1, $2, $3, '{}', $4, $5, $6) RETURNING id
)
INSERT INTO active_storage_attachments (
name, record_type, record_id, blob_id, created_at
) VALUES ($7, $8, $9, (SELECT id FROM rows), $10)
SQL
end
Rails.application.eager_load!
models = ActiveRecord::Base.descendants.reject(&:abstract_class?)
paperclip_storage = Paperclip::Attachment.default_options[:storage]
ActiveRecord::Base.transaction do
models.each do |model|
next if model.name == "OldPassword" && !model.table_exists?
attachments = model.column_names.map do |c|
if c =~ /(.+)_file_name$/
$1
end
end.compact
if attachments.blank?
next
end
model.find_each.each do |instance|
attachments.each do |attachment|
next if instance.send(:"storage_#{attachment}").attached?
source = if paperclip_storage == :filesystem
instance.send(attachment).path
else
instance.send(attachment).url
end
next if source.blank? || source == "/images/original/missing.png"
file = if paperclip_storage == :filesystem
File.read(source) if File.exist?(source)
else
Net::HTTP.get(URI(source))
end
connection.exec_prepared(
statement_name, [
SecureRandom.uuid, # Alternatively instance.send("#{attachment}_file_name"),
instance.send("#{attachment}_file_name"),
instance.send("#{attachment}_content_type"),
instance.send("#{attachment}_file_size"),
file && Digest::MD5.base64digest(file) || SecureRandom.hex(32),
instance.updated_at.iso8601,
"storage_#{attachment}",
model.name,
instance.id,
instance.updated_at.iso8601
])
end
end
end
end
ActiveStorage::Attachment.find_each do |attachment|
blob = attachment.blob
next if blob.service.exist?(blob.key) || !attachment.record
name = attachment.name.delete_prefix("storage_")
paperclip_attachment = attachment.record.send(name)
next unless paperclip_attachment.exists?
source_file = if paperclip_storage == :filesystem
paperclip_attachment.path
else
URI.open(paperclip_attachment.url, &:read)
end
logger.info "Copying #{paperclip_attachment.url} to active storage"
blob.service.upload(blob.key, source_file)
end
end
end

View File

@@ -1,7 +1,13 @@
namespace :consul do
desc "Runs tasks needed to upgrade to the latest version"
task execute_release_tasks: ["settings:rename_setting_keys",
"settings:add_new_settings"]
"settings:add_new_settings",
"execute_release_1.4.0_tasks"]
desc "Runs tasks needed to upgrade from 1.3.0 to 1.4.0"
task "execute_release_1.4.0_tasks": [
"active_storage:migrate_from_paperclip"
]
desc "Runs tasks needed to upgrade from 1.2.0 to 1.3.0"
task "execute_release_1.3.0_tasks": [

View File

@@ -0,0 +1,14 @@
require "rails_helper"
describe ActiveStorage::DirectUploadsController do
describe "POST create" do
it "doesn't allow anonymous users to upload files" do
blob_attributes = { filename: "logo.pdf", byte_size: 30000, checksum: SecureRandom.hex(32) }
post :create, params: { blob: blob_attributes }
expect(ActiveStorage::Blob.count).to eq 0
expect(response).to be_unauthorized
end
end
end

View File

@@ -0,0 +1,13 @@
require "rails_helper"
describe ActiveStorage::DiskController do
describe "PUT update" do
it "doesn't allow anonymous users to upload files" do
blob = create(:active_storage_blob)
put :update, params: { encoded_token: blob.signed_id }
expect(response).to be_unauthorized
end
end
end

View File

@@ -55,4 +55,10 @@ FactoryBot.define do
end
initialize_with { new(attributes) }
end
factory :active_storage_blob, class: "ActiveStorage::Blob" do
filename { "sample.pdf" }
byte_size { 3000 }
checksum { SecureRandom.hex(32) }
end
end

View File

@@ -0,0 +1,88 @@
require "rails_helper"
describe "active storage tasks" do
describe "migrate_from_paperclip" do
let(:run_rake_task) do
Rake::Task["active_storage:migrate_from_paperclip"].reenable
Rake.application.invoke_task("active_storage:migrate_from_paperclip")
end
let(:storage_root) { ActiveStorage::Blob.service.root }
before { FileUtils.rm_rf storage_root }
it "migrates records and attachments" do
document = create(:document,
attachment: nil,
paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf"))
expect(ActiveStorage::Attachment.count).to eq 0
expect(ActiveStorage::Blob.count).to eq 0
expect(test_storage_file_paths.count).to eq 0
run_rake_task
document.reload
expect(ActiveStorage::Attachment.count).to eq 1
expect(ActiveStorage::Blob.count).to eq 1
expect(document.storage_attachment.filename).to eq "clippy.pdf"
expect(test_storage_file_paths.count).to eq 1
expect(storage_file_path(document)).to eq test_storage_file_paths.first
end
it "migrates records with deleted files ignoring the files" do
document = create(:document,
attachment: nil,
paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf"))
FileUtils.rm(document.attachment.path)
run_rake_task
document.reload
expect(ActiveStorage::Attachment.count).to eq 1
expect(ActiveStorage::Blob.count).to eq 1
expect(document.storage_attachment.filename).to eq "clippy.pdf"
expect(test_storage_file_paths.count).to eq 0
end
it "does not migrate already migrated records" do
document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
migrated_file = test_storage_file_paths.first
attachment_id = document.storage_attachment.attachment.id
blob_id = document.storage_attachment.blob.id
run_rake_task
document.reload
expect(ActiveStorage::Attachment.count).to eq 1
expect(ActiveStorage::Blob.count).to eq 1
expect(document.storage_attachment.attachment.id).to eq attachment_id
expect(document.storage_attachment.blob.id).to eq blob_id
expect(test_storage_file_paths.count).to eq 1
expect(storage_file_path(document)).to eq migrated_file
expect(test_storage_file_paths.first).to eq migrated_file
end
it "does not migrate files for deleted records" do
document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
FileUtils.rm storage_file_path(document)
Document.delete_all
run_rake_task
expect(ActiveStorage::Attachment.count).to eq 1
expect(ActiveStorage::Blob.count).to eq 1
expect(document.storage_attachment.filename).to eq "clippy.pdf"
expect(test_storage_file_paths.count).to eq 0
end
def test_storage_file_paths
Dir.glob("#{storage_root}/**/*").select { |file_or_folder| File.file?(file_or_folder) }
end
def storage_file_path(record)
ActiveStorage::Blob.service.path_for(record.storage_attachment.blob.key)
end
end
end

View File

@@ -4,6 +4,16 @@ describe Document do
it_behaves_like "document validations", "budget_investment_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
describe "#admin" do
it "returns admin documents" do

View File

@@ -3,4 +3,14 @@ require "rails_helper"
describe Image do
it_behaves_like "image validations", "budget_investment_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

View File

@@ -1,6 +1,17 @@
require "rails_helper"
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
it "is valid with a 260x80 image" do
image = build(:site_customization_image,