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/Gemfile b/Gemfile index c7fe245943..a6ec2e5a78 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,11 @@ 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" + gem 'activemerchant', '>= 1.78.0' gem 'rexml' gem 'angular-rails-templates', '>= 0.3.0' diff --git a/Gemfile.lock b/Gemfile.lock index a3a6b9e440..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) @@ -158,11 +163,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 +352,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 +398,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 +586,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) @@ -683,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 @@ -697,6 +726,7 @@ DEPENDENCIES awesome_nested_set awesome_print aws-sdk (= 1.67.0) + aws-sdk-s3 bigdecimal (= 3.0.2) bootsnap bugsnag @@ -736,6 +766,7 @@ DEPENDENCIES hiredis i18n i18n-js (~> 3.9.0) + image_processing immigrant jquery-rails (= 4.4.0) jquery-ui-rails (~> 4.2) diff --git a/app/models/concerns/has_migrating_file.rb b/app/models/concerns/has_migrating_file.rb new file mode 100644 index 0000000000..923ee59ba3 --- /dev/null +++ b/app/models/concerns/has_migrating_file.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +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 + 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. + 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 + 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/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/spree/image.rb b/app/models/spree/image.rb index e508395729..996067ec6f 100644 --- a/app/models/spree/image.rb +++ b/app/models/spree/image.rb @@ -4,6 +4,8 @@ require 'spree/core/s3_support' module Spree class Image < Asset + include HasMigratingFile + validates_attachment_presence :attachment validate :no_attachment_errors @@ -13,7 +15,7 @@ module Spree # - 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, 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/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb index 471bdb8ca3..d652c37f7e 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 @@ -19,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 diff --git a/config/application.rb b/config/application.rb index 89b71e9994..7d88b45db8 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", @@ -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/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." diff --git a/config/storage.yml b/config/storage.yml new file mode 100644 index 0000000000..255c8298d9 --- /dev/null +++ b/config/storage.yml @@ -0,0 +1,21 @@ +local: + service: Disk + root: <%= Rails.root.join("storage") %> + +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"] %> + secret_access_key: <%= ENV["S3_SECRET"] %> + bucket: <%= ENV["S3_BUCKET"] %> + region: <%= ENV.fetch("S3_REGION", "us-east-1") %> 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" 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..8859c7bed4 --- /dev/null +++ b/lib/tasks/from_paperclip_to_active_storage.rake @@ -0,0 +1,130 @@ +# 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 + + # 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) + 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 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 + + 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..e13cfc2a18 --- /dev/null +++ b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb @@ -0,0 +1,110 @@ +# 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 + 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 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) + ) + 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 + + 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) + end +end 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