Remove Paperclip and use just Active Storage

This commit is contained in:
Javi Martín
2021-07-28 01:55:54 +02:00
parent ca7f2bc9d5
commit 7212657c02
33 changed files with 128 additions and 355 deletions

View File

@@ -40,7 +40,6 @@ gem "omniauth-facebook", "~> 8.0.0"
gem "omniauth-google-oauth2", "~> 1.0.0" gem "omniauth-google-oauth2", "~> 1.0.0"
gem "omniauth-rails_csrf_protection", "~> 1.0.0" gem "omniauth-rails_csrf_protection", "~> 1.0.0"
gem "omniauth-twitter", "~> 1.4.0" gem "omniauth-twitter", "~> 1.4.0"
gem "paperclip", "~> 6.1.0"
gem "paranoia", "~> 2.4.3" gem "paranoia", "~> 2.4.3"
gem "pg", "~> 1.2.3" gem "pg", "~> 1.2.3"
gem "pg_search", "~> 2.3.5" gem "pg_search", "~> 2.3.5"

View File

@@ -373,9 +373,6 @@ GEM
mime-types (3.3.1) mime-types (3.3.1)
mime-types-data (~> 3.2015) mime-types-data (~> 3.2015)
mime-types-data (3.2021.0704) mime-types-data (3.2021.0704)
mimemagic (0.3.10)
nokogiri (~> 1)
rake
mini_magick (4.11.0) mini_magick (4.11.0)
mini_mime (1.1.0) mini_mime (1.1.0)
mini_portile2 (2.6.1) mini_portile2 (2.6.1)
@@ -430,12 +427,6 @@ GEM
omniauth-oauth (~> 1.1) omniauth-oauth (~> 1.1)
rack rack
orm_adapter (0.5.0) orm_adapter (0.5.0)
paperclip (6.1.0)
activemodel (>= 4.2.0)
activesupport (>= 4.2.0)
mime-types
mimemagic (~> 0.3.0)
terrapin (~> 0.6.0)
parallel (1.20.1) parallel (1.20.1)
paranoia (2.4.3) paranoia (2.4.3)
activerecord (>= 4.0, < 6.2) activerecord (>= 4.0, < 6.2)
@@ -753,7 +744,6 @@ DEPENDENCIES
omniauth-google-oauth2 (~> 1.0.0) omniauth-google-oauth2 (~> 1.0.0)
omniauth-rails_csrf_protection (~> 1.0.0) omniauth-rails_csrf_protection (~> 1.0.0)
omniauth-twitter (~> 1.4.0) omniauth-twitter (~> 1.4.0)
paperclip (~> 6.1.0)
paranoia (~> 2.4.3) paranoia (~> 2.4.3)
pg (~> 1.2.3) pg (~> 1.2.3)
pg_search (~> 2.3.5) pg_search (~> 2.3.5)

View File

@@ -7,7 +7,7 @@
<%= f.text_field :title, placeholder: t("#{plural_name}.form.title_placeholder") %> <%= f.text_field :title, placeholder: t("#{plural_name}.form.title_placeholder") %>
</div> </div>
<% if attachable.storage_attachment.attached? && attachable.storage_attachment.image? %> <% if attachable.attachment.attached? && attachable.attachment.image? %>
<%= render_image(attachable, :thumb, false) %> <%= render_image(attachable, :thumb, false) %>
<% end %> <% end %>

View File

@@ -24,7 +24,7 @@ class Attachable::FieldsComponent < ApplicationComponent
end end
def file_name def file_name
attachable.storage_attachment.filename if attachable.storage_attachment.attached? attachable.attachment_file_name
end end
def destroy_link def destroy_link

View File

@@ -15,9 +15,9 @@ class DirectUploadsController < ApplicationController
@direct_upload.relation.set_cached_attachment_from_attachment @direct_upload.relation.set_cached_attachment_from_attachment
render json: { cached_attachment: @direct_upload.relation.cached_attachment, render json: { cached_attachment: @direct_upload.relation.cached_attachment,
filename: @direct_upload.relation.storage_attachment.filename.to_s, filename: @direct_upload.relation.attachment_file_name,
destroy_link: render_destroy_upload_link(@direct_upload), destroy_link: render_destroy_upload_link(@direct_upload),
attachment_url: polymorphic_path(@direct_upload.relation.storage_attachment) } attachment_url: polymorphic_path(@direct_upload.relation.attachment) }
else else
render json: { errors: @direct_upload.errors[:attachment].join(", ") }, render json: { errors: @direct_upload.errors[:attachment].join(", ") },
status: :unprocessable_entity status: :unprocessable_entity

View File

@@ -3,7 +3,7 @@ module DocumentsHelper
info_text = "#{document.humanized_content_type} | #{number_to_human_size(document.attachment_file_size)}" info_text = "#{document.humanized_content_type} | #{number_to_human_size(document.attachment_file_size)}"
link_to safe_join([document.title, tag.small("(#{info_text})")], " "), link_to safe_join([document.title, tag.small("(#{info_text})")], " "),
document.storage_attachment, document.attachment,
target: "_blank", target: "_blank",
title: t("shared.target_blank") title: t("shared.target_blank")
end end

View File

@@ -25,7 +25,7 @@ module WelcomeHelper
def calculate_image_path(recommended, image_default) def calculate_image_path(recommended, image_default)
if recommended.respond_to?(:image) && recommended.image.present? && if recommended.respond_to?(:image) && recommended.image.present? &&
recommended.image.storage_attachment.attached? recommended.image.attachment.attached?
recommended.image.variant(:medium) recommended.image.variant(:medium)
elsif image_default.present? elsif image_default.present?
image_default image_default

View File

@@ -3,14 +3,12 @@ module Attachable
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
has_attachment :attachment
attr_accessor :cached_attachment attr_accessor :cached_attachment
# Disable paperclip security validation due to polymorphic configuration
# Paperclip do not allow to use Procs on valiations definition
do_not_validate_attachment_file_type :attachment
validate :attachment_presence validate :attachment_presence
validate :validate_attachment_content_type, if: -> { storage_attachment.attached? } validate :validate_attachment_content_type, if: -> { attachment.attached? }
validate :validate_attachment_size, if: -> { storage_attachment.attached? } validate :validate_attachment_size, if: -> { attachment.attached? }
before_validation :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } before_validation :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? }
end end
@@ -22,43 +20,35 @@ module Attachable
end end
def set_cached_attachment_from_attachment def set_cached_attachment_from_attachment
self.cached_attachment = storage_attachment.signed_id self.cached_attachment = attachment.signed_id
end end
def set_attachment_from_cached_attachment def set_attachment_from_cached_attachment
self.storage_attachment = cached_attachment self.attachment = cached_attachment
end
if filesystem_storage? def attachment_file_name
File.open(file_path) do |file| attachment.filename.to_s if attachment.attached?
self.paperclip_attachment = file
end
else
self.paperclip_attachment = URI.parse(cached_attachment).open
end
end end
def attachment_content_type def attachment_content_type
storage_attachment.blob.content_type if storage_attachment.attached? attachment.blob.content_type if attachment.attached?
end end
def attachment_file_size def attachment_file_size
if storage_attachment.attached? if attachment.attached?
storage_attachment.blob.byte_size attachment.blob.byte_size
else else
0 0
end end
end end
def file_path def file_path
ActiveStorage::Blob.service.path_for(storage_attachment.blob.key) ActiveStorage::Blob.service.path_for(attachment.blob.key)
end end
private private
def filesystem_storage?
Paperclip::Attachment.default_options[:storage] == :filesystem
end
def validate_attachment_size def validate_attachment_size
if association_class && attachment_file_size > max_file_size.megabytes if association_class && attachment_file_size > max_file_size.megabytes
errors.add(:attachment, I18n.t("#{model_name.plural}.errors.messages.in_between", errors.add(:attachment, I18n.t("#{model_name.plural}.errors.messages.in_between",
@@ -77,7 +67,7 @@ module Attachable
end end
def attachment_presence def attachment_presence
unless storage_attachment.attached? unless attachment.attached?
errors.add(:attachment, I18n.t("errors.messages.blank")) errors.add(:attachment, I18n.t("errors.messages.blank"))
end end
end end

View File

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

View File

@@ -34,7 +34,7 @@ class DirectUpload
end end
def save_attachment def save_attachment
@relation.storage_attachment.blob.save! @relation.attachment.blob.save!
end end
def persisted? def persisted?
@@ -53,7 +53,7 @@ class DirectUpload
def relation_attributtes def relation_attributtes
{ {
storage_attachment: @attachment, attachment: @attachment,
cached_attachment: @cached_attachment, cached_attachment: @cached_attachment,
user: @user user: @user
} }

View File

@@ -1,11 +1,6 @@
class Document < ApplicationRecord class Document < ApplicationRecord
include Attachable include Attachable
has_attachment :attachment, url: "/system/:class/:attachment/:id_partition/: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 :user
belongs_to :documentable, polymorphic: true, touch: true belongs_to :documentable, polymorphic: true, touch: true
@@ -16,23 +11,10 @@ class Document < ApplicationRecord
scope :admin, -> { where(admin: true) } scope :admin, -> { where(admin: true) }
Paperclip.interpolates :custom_hash_data do |attachment, _style|
attachment.instance.custom_hash_data(attachment)
end
def self.humanized_accepted_content_types def self.humanized_accepted_content_types
Setting.accepted_content_types_for("documents").join(", ") Setting.accepted_content_types_for("documents").join(", ")
end end
def custom_hash_data(attachment)
original_filename = if attachment.instance.persisted?
attachment.instance.title
else
attachment.instance.attachment_file_name
end
"#{attachment.instance.user_id}/#{original_filename}"
end
def humanized_content_type def humanized_content_type
attachment_content_type.split("/").last.upcase attachment_content_type.split("/").last.upcase
end end

View File

@@ -1,16 +1,6 @@
class Image < ApplicationRecord class Image < ApplicationRecord
include Attachable include Attachable
has_attachment :attachment, styles: {
large: "x#{Setting["uploads.images.min_height"]}",
medium: "300x300#",
thumb: "140x245#"
},
url: "/system/:class/:attachment/:id_partition/:style/:hash.:extension",
hash_data: ":class/:style",
use_timestamp: false,
hash_secret: Rails.application.secrets.secret_key_base
def self.styles def self.styles
{ {
large: { resize: "x#{Setting["uploads.images.min_height"]}" }, large: { resize: "x#{Setting["uploads.images.min_height"]}" },
@@ -27,7 +17,7 @@ class Image < ApplicationRecord
validates :user_id, presence: true validates :user_id, presence: true
validates :imageable_id, presence: true, if: -> { persisted? } validates :imageable_id, presence: true, if: -> { persisted? }
validates :imageable_type, presence: true, if: -> { persisted? } validates :imageable_type, presence: true, if: -> { persisted? }
validate :validate_image_dimensions, if: -> { storage_attachment.attached? && storage_attachment.new_record? } validate :validate_image_dimensions, if: -> { attachment.attached? && attachment.new_record? }
def self.max_file_size def self.max_file_size
Setting["uploads.images.max_size"].to_i Setting["uploads.images.max_size"].to_i
@@ -51,9 +41,9 @@ class Image < ApplicationRecord
def variant(style) def variant(style)
if style if style
storage_attachment.variant(self.class.styles[style]) attachment.variant(self.class.styles[style])
else else
storage_attachment attachment
end end
end end
@@ -71,10 +61,10 @@ class Image < ApplicationRecord
if accepted_content_types.include?(attachment_content_type) if accepted_content_types.include?(attachment_content_type)
return true if imageable_class == Widget::Card return true if imageable_class == Widget::Card
storage_attachment.analyze unless storage_attachment.analyzed? attachment.analyze unless attachment.analyzed?
width = storage_attachment.metadata[:width] width = attachment.metadata[:width]
height = storage_attachment.metadata[:height] height = attachment.metadata[:height]
min_width = Setting["uploads.images.min_width"].to_i min_width = Setting["uploads.images.min_width"].to_i
min_height = Setting["uploads.images.min_height"].to_i min_height = Setting["uploads.images.min_height"].to_i
errors.add(:attachment, :min_image_width, required_min_width: min_width) if width < min_width errors.add(:attachment, :min_image_width, required_min_width: min_width) if width < min_width

View File

@@ -14,9 +14,7 @@ class SiteCustomization::Image < ApplicationRecord
has_attachment :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 }
do_not_validate_attachment_file_type :image validates :image, file_content_type: { allow: ["image/png", "image/jpeg"], if: -> { image.attached? }}
validates :image, file_content_type: { allow: ["image/png", "image/jpeg"] }
validate :check_image validate :check_image
def self.all_images def self.all_images
@@ -40,21 +38,21 @@ class SiteCustomization::Image < ApplicationRecord
end end
def persisted_image def persisted_image
storage_image if persisted_attachment? image if persisted_attachment?
end end
def persisted_attachment? def persisted_attachment?
storage_image.attachment&.persisted? image.attachment&.persisted?
end end
private private
def check_image def check_image
return unless image? return unless image.attached?
storage_image.analyze unless storage_image.analyzed? image.analyze unless image.analyzed?
width = storage_image.metadata[:width] width = image.metadata[:width]
height = storage_image.metadata[:height] height = image.metadata[:height]
if name == "logo_header" if name == "logo_header"
errors.add(:image, :image_width, required_width: required_width) unless width <= required_width errors.add(:image, :image_width, required_width: required_width) unless width <= required_width

View File

@@ -47,7 +47,7 @@
<% if milestone.documents.present? %> <% if milestone.documents.present? %>
<% milestone.documents.each do |document| %> <% milestone.documents.each do |document| %>
<%= link_to document.title, <%= link_to document.title,
document.storage_attachment, document.attachment,
target: "_blank", target: "_blank",
rel: "nofollow" %><br> rel: "nofollow" %><br>
<% end %> <% end %>

View File

@@ -33,7 +33,7 @@
<% @answer.documents.each do |document| %> <% @answer.documents.each do |document| %>
<tr> <tr>
<td> <td>
<%= link_to document.title, document.storage_attachment %> <%= link_to document.title, document.attachment %>
</td> </td>
<td> <td>
<%= render Admin::TableActionsComponent.new(document, <%= render Admin::TableActionsComponent.new(document,
@@ -42,7 +42,7 @@
) do |actions| %> ) do |actions| %>
<%= actions.action(:download, <%= actions.action(:download,
text: t("documents.buttons.download_document"), text: t("documents.buttons.download_document"),
path: document.storage_attachment, path: document.attachment,
target: "_blank", target: "_blank",
rel: "nofollow") %> rel: "nofollow") %>

View File

@@ -21,9 +21,9 @@
<% @documents.each do |document| %> <% @documents.each do |document| %>
<tr id="<%= dom_id(document) %>" class="document"> <tr id="<%= dom_id(document) %>" class="document">
<td><%= document.title %></td> <td><%= document.title %></td>
<td><%= document.storage_attachment.content_type %></td> <td><%= document.attachment.content_type %></td>
<td><%= number_to_human_size(document.storage_attachment.byte_size) %></td> <td><%= number_to_human_size(document.attachment.byte_size) %></td>
<td><%= link_to document.title, document.storage_attachment, target: :blank %></td> <td><%= link_to document.title, document.attachment, target: :blank %></td>
<td> <td>
<div class="small-12 medium-6 column"> <div class="small-12 medium-6 column">
<%= render Admin::TableActionsComponent.new( <%= render Admin::TableActionsComponent.new(

View File

@@ -16,7 +16,7 @@
<td class="small-12 medium-8"> <td class="small-12 medium-8">
<%= form_for([:admin, image], html: { id: "edit_#{dom_id(image)}" }) do |f| %> <%= form_for([:admin, image], html: { id: "edit_#{dom_id(image)}" }) do |f| %>
<div class="small-12 medium-6 large-6 column"> <div class="small-12 medium-6 large-6 column">
<%= image_tag image.storage_image if image.persisted_attachment? %> <%= image_tag image.image if image.persisted_attachment? %>
<%= f.file_field :image, label: false %> <%= f.file_field :image, label: false %>
</div> </div>
<div class="small-12 medium-6 large-6 column"> <div class="small-12 medium-6 large-6 column">

View File

@@ -1,5 +1,5 @@
<p> <p>
<%= link_to document.storage_attachment, target: "_blank" do %> <%= link_to document.attachment, target: "_blank" do %>
<%= document.title %> <%= document.title %>
(<%= document.humanized_content_type %> (<%= document.humanized_content_type %>
&nbsp;|&nbsp; &nbsp;|&nbsp;

View File

@@ -1,6 +1,6 @@
<li id="<%= dom_id(document) %>"> <li id="<%= dom_id(document) %>">
<%= link_to t("documents.buttons.download_document"), <%= link_to t("documents.buttons.download_document"),
document.storage_attachment, target: "_blank", document.attachment, target: "_blank",
rel: "nofollow", class: "button hollow medium float-right" %> rel: "nofollow", class: "button hollow medium float-right" %>
<strong><%= document.title %></strong> <strong><%= document.title %></strong>

View File

@@ -37,7 +37,7 @@
<% milestone.documents.each do |document| %> <% milestone.documents.each do |document| %>
<%= link_to document.title, <%= link_to document.title,
document.storage_attachment, document.attachment,
target: "_blank", target: "_blank",
rel: "nofollow" %><br> rel: "nofollow" %><br>
<small> <small>

View File

@@ -18,8 +18,8 @@
<% answer.images.reverse.each_with_index do |image, index| %> <% answer.images.reverse.each_with_index do |image, index| %>
<li class="orbit-slide <%= is_active_class(index) %>"> <li class="orbit-slide <%= is_active_class(index) %>">
<%= link_to image.storage_attachment, target: "_blank" do %> <%= link_to image.attachment, target: "_blank" do %>
<%= image_tag image.storage_attachment, <%= image_tag image.attachment,
class: "orbit-image", class: "orbit-image",
alt: image.title.unicode_normalize %> alt: image.title.unicode_normalize %>
<% end %> <% end %>

View File

@@ -86,7 +86,7 @@
<% answer.documents.each do |document| %> <% answer.documents.each do |document| %>
<%= link_to document.title, <%= link_to document.title,
document.storage_attachment, document.attachment,
target: "_blank", target: "_blank",
rel: "nofollow" %><br> rel: "nofollow" %><br>
<% end %> <% end %>

View File

@@ -1,13 +0,0 @@
class Paperclip::UrlGenerator
private
# Code copied from the paperclip gem, only replacing
# `URI.escape` with `URI::DEFAULT_PARSER.escape`
def escape_url(url)
if url.respond_to?(:escape)
url.escape
else
URI::DEFAULT_PARSER.escape(url).gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" }
end
end
end

View File

@@ -1,98 +1,9 @@
# 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 namespace :active_storage do
desc "Copy paperclip's attachment database columns to active storage" desc "Updates ActiveStorage::Attachment old records created when we were also using paperclip"
task migrate_from_paperclip: :environment do task remove_paperclip_compatibility_in_existing_attachments: :environment do
logger = ApplicationLogger.new ApplicationLogger.new.info "Updating Active Storage attachment records"
connection = ActiveRecord::Base.connection.raw_connection ActiveStorage::Attachment.where("name ILIKE 'storage_%'")
statement_name = "active_storage_statement" .where.not(record_type: "Ckeditor::Asset")
.update_all("name = replace(name, 'storage_', '')")
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.parse(paperclip_attachment.url).open.read
end
logger.info "Copying #{paperclip_attachment.url} to active storage"
blob.service.upload(blob.key, source_file)
end
end end
end end

View File

@@ -2,10 +2,10 @@ namespace :consul do
desc "Runs tasks needed to upgrade to the latest version" desc "Runs tasks needed to upgrade to the latest version"
task execute_release_tasks: ["settings:rename_setting_keys", task execute_release_tasks: ["settings:rename_setting_keys",
"settings:add_new_settings", "settings:add_new_settings",
"execute_release_1.4.0_tasks"] "execute_release_1.5.0_tasks"]
desc "Runs tasks needed to upgrade from 1.3.0 to 1.4.0" desc "Runs tasks needed to upgrade from 1.4.0 to 1.5.0"
task "execute_release_1.4.0_tasks": [ task "execute_release_1.5.0_tasks": [
"active_storage:migrate_from_paperclip" "active_storage:remove_paperclip_compatibility_in_existing_attachments"
] ]
end end

View File

@@ -8,38 +8,4 @@ describe ProposalsController do
expect { get :index }.to raise_exception(FeatureFlags::FeatureDisabled) expect { get :index }.to raise_exception(FeatureFlags::FeatureDisabled)
end end
end end
describe "PUT update" do
let(:file) { Rails.root.join("spec/hacked") }
before do
File.delete(file) if File.exist?(file)
InvisibleCaptcha.timestamp_enabled = false
end
after { InvisibleCaptcha.timestamp_enabled = true }
it "ignores malicious cached attachments with remote storages" do
allow_any_instance_of(Image).to receive(:filesystem_storage?).and_return(false)
user = create(:user)
proposal = create(:proposal, author: user)
sign_in user
begin
put :update, params: {
id: proposal,
proposal: {
image_attributes: {
title: "Hacked!",
user_id: user.id,
cached_attachment: "| touch #{file}"
}
}
}
rescue StandardError
ensure
expect(file).not_to exist
end
end
end
end end

View File

@@ -1,90 +1,67 @@
require "rails_helper" require "rails_helper"
describe "active storage tasks" do describe "active storage tasks" do
describe "migrate_from_paperclip" do describe "remove_paperclip_compatibility_in_existing_attachments" do
let(:run_rake_task) do let(:run_rake_task) do
Rake::Task["active_storage:migrate_from_paperclip"].reenable Rake::Task["active_storage:remove_paperclip_compatibility_in_existing_attachments"].reenable
Rake.application.invoke_task("active_storage:migrate_from_paperclip") Rake.application.invoke_task("active_storage:remove_paperclip_compatibility_in_existing_attachments")
end end
let(:storage_root) { ActiveStorage::Blob.service.root } it "updates old regular attachments" do
before { FileUtils.rm_rf storage_root } document = create(:document)
image = create(:image)
site_customization_image = create(:site_customization_image)
it "migrates records and attachments" do document.attachment.attachment.update_column(:name, "storage_attachment")
document = build(:document, image.attachment.attachment.update_column(:name, "storage_attachment")
attachment: nil, site_customization_image.image.attachment.update_column(:name, "storage_image")
paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf"))
document.save(validate: false)
expect(ActiveStorage::Attachment.count).to eq 0 document.reload
expect(ActiveStorage::Blob.count).to eq 0 image.reload
expect(test_storage_file_paths.count).to eq 0 site_customization_image.reload
expect(document.attachment.attachment).to be nil
expect(image.attachment.attachment).to be nil
expect(site_customization_image.image.attachment).to be nil
run_rake_task run_rake_task
document.reload document.reload
image.reload
site_customization_image.reload
expect(ActiveStorage::Attachment.count).to eq 1 expect(document.attachment.attachment.name).to eq "attachment"
expect(ActiveStorage::Blob.count).to eq 1 expect(image.attachment.attachment.name).to eq "attachment"
expect(document.storage_attachment.filename).to eq "clippy.pdf" expect(site_customization_image.reload.image.attachment.name).to eq "image"
expect(test_storage_file_paths.count).to eq 1
expect(storage_file_path(document)).to eq test_storage_file_paths.first
end end
it "migrates records with deleted files ignoring the files" do it "does not modify old ckeditor attachments" do
document = build(:document, image = Ckeditor::Picture.create!(data: Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.png"))
attachment: nil,
paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf")) expect(image.storage_data.attachment.name).to eq "storage_data"
document.save(validate: false)
FileUtils.rm(document.attachment.path) run_rake_task
image.reload
expect(image.storage_data.attachment.name).to eq "storage_data"
end
it "does not modify new regular attachments" do
document = create(:document)
image = create(:image)
site_customization_image = create(:site_customization_image)
expect(document.attachment.attachment.name).to eq "attachment"
expect(image.attachment.attachment.name).to eq "attachment"
expect(site_customization_image.image.attachment.name).to eq "image"
run_rake_task run_rake_task
document.reload document.reload
image.reload
site_customization_image.reload
expect(ActiveStorage::Attachment.count).to eq 1 expect(document.attachment.attachment.name).to eq "attachment"
expect(ActiveStorage::Blob.count).to eq 1 expect(image.attachment.attachment.name).to eq "attachment"
expect(document.storage_attachment.filename).to eq "clippy.pdf" expect(site_customization_image.reload.image.attachment.name).to eq "image"
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 end
end end

View File

@@ -11,8 +11,8 @@ describe "files tasks" do
image = build(:image) image = build(:image)
document = build(:document) document = build(:document)
image.storage_attachment.blob.save! image.attachment.blob.save!
document.storage_attachment.blob.save! document.attachment.blob.save!
travel_to(2.days.from_now) { run_rake_task } travel_to(2.days.from_now) { run_rake_task }
@@ -24,8 +24,8 @@ describe "files tasks" do
image = build(:image) image = build(:image)
document = build(:document) document = build(:document)
image.storage_attachment.blob.save! image.attachment.blob.save!
document.storage_attachment.blob.save! document.attachment.blob.save!
travel_to(2.minutes.from_now) { run_rake_task } travel_to(2.minutes.from_now) { run_rake_task }

View File

@@ -40,8 +40,8 @@ describe DirectUpload do
direct_upload.save_attachment direct_upload.save_attachment
expect(File.exist?(direct_upload.relation.file_path)).to be true expect(File.exist?(direct_upload.relation.file_path)).to be true
expect(direct_upload.relation.storage_attachment.blob).to be_persisted expect(direct_upload.relation.attachment.blob).to be_persisted
expect(direct_upload.relation.storage_attachment.attachment).not_to be_persisted expect(direct_upload.relation.attachment.attachment).not_to be_persisted
end end
end end
end end

View File

@@ -4,14 +4,11 @@ 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 it "stores attachments with Active Storage" do
document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf")) document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
expect(document.attachment).to exist expect(document.attachment).to be_attached
expect(document.attachment_file_name).to eq "clippy.pdf" expect(document.attachment.filename).to eq "clippy.pdf"
expect(document.storage_attachment).to be_attached
expect(document.storage_attachment.filename).to eq "clippy.pdf"
end end
context "scopes" do context "scopes" do

View File

@@ -4,13 +4,10 @@ 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 it "stores attachments with Active Storage" do
image = create(:image, attachment: File.new("spec/fixtures/files/clippy.jpg")) image = create(:image, attachment: File.new("spec/fixtures/files/clippy.jpg"))
expect(image.attachment).to exist expect(image.attachment).to be_attached
expect(image.attachment_file_name).to eq "clippy.jpg" expect(image.attachment.filename).to eq "clippy.jpg"
expect(image.storage_attachment).to be_attached
expect(image.storage_attachment.filename).to eq "clippy.jpg"
end end
end end

View File

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

View File

@@ -18,7 +18,7 @@ describe "Documents", :admin do
1.times { create(:document) } 1.times { create(:document) }
document = Document.first document = Document.first
url = polymorphic_path(document.storage_attachment) url = polymorphic_path(document.attachment)
visit admin_site_customization_documents_path visit admin_site_customization_documents_path