diff --git a/.circleci/config.yml b/.circleci/config.yml index 66bff32a87c..f91e700d706 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -59,6 +59,12 @@ jobs: parallelism: *parallelism steps: *steps + postgres_activestorage: + <<: *postgres + environment: + <<: *environment + ACTIVE_STORAGE: 'true' + postgres_rails51: <<: *postgres environment: @@ -79,3 +85,4 @@ workflows: - mysql - postgres_rails51 - mysql_rails51 + - postgres_activestorage diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index e95ae12bf18..e1090c1a817 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -23,6 +23,11 @@ class BaseController < ActionController::Base before_action :authorize_for_order, if: proc { order_token.present? } before_action :authenticate_user before_action :load_user_roles + if defined? ActiveStorage + before_action do + ActiveStorage::Current.host = request.base_url + end + end rescue_from ActionController::ParameterMissing, with: :parameter_missing_error rescue_from ActiveRecord::RecordNotFound, with: :not_found diff --git a/api/app/views/spree/api/images/_image.json.jbuilder b/api/app/views/spree/api/images/_image.json.jbuilder index 4e798175163..0311e99686c 100644 --- a/api/app/views/spree/api/images/_image.json.jbuilder +++ b/api/app/views/spree/api/images/_image.json.jbuilder @@ -3,5 +3,5 @@ json.(image, *image_attributes) json.(image, :viewable_type, :viewable_id) Spree::Image.attachment_definitions[:attachment][:styles].each do |k, _v| - json.set! "#{k}_url", image.attachment.url(k) + json.set! "#{k}_url", image.url(k) end diff --git a/backend/app/views/spree/admin/taxons/_form.html.erb b/backend/app/views/spree/admin/taxons/_form.html.erb index f3baebcd403..6c93d998402 100644 --- a/backend/app/views/spree/admin/taxons/_form.html.erb +++ b/backend/app/views/spree/admin/taxons/_form.html.erb @@ -23,7 +23,7 @@ <%= f.field_container :icon do %> <%= f.label :icon %>
<%= f.file_field :icon %> - <%= image_tag f.object.icon(:mini) if f.object.icon.present? %> + <%= image_tag f.object.icon(:mini) if f.object.icon_present? %> <% end %> diff --git a/core/app/controllers/spree/base_controller.rb b/core/app/controllers/spree/base_controller.rb index 7af1d0a13f3..21ab4d034b8 100644 --- a/core/app/controllers/spree/base_controller.rb +++ b/core/app/controllers/spree/base_controller.rb @@ -11,5 +11,11 @@ class Spree::BaseController < ApplicationController include Spree::Core::ControllerHelpers::Store include Spree::Core::ControllerHelpers::StrongParameters + if defined? ActiveStorage + before_action do + ActiveStorage::Current.host = request.base_url + end + end + respond_to :html end diff --git a/core/app/models/concerns/spree/active_storage_attachment.rb b/core/app/models/concerns/spree/active_storage_attachment.rb new file mode 100644 index 00000000000..bf9790134f9 --- /dev/null +++ b/core/app/models/concerns/spree/active_storage_attachment.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Spree + module ActiveStorageAttachment + extend ActiveSupport::Concern + + # @private + def self.attachment_variant(attachment, style:, default_style:, styles:) + return unless attachment && attachment.attachment + + if style.nil? || style == default_style + attachment_variant = attachment + else + attachment_variant = attachment.variant( + resize: styles[style.to_sym], + strip: true, + 'auto-orient': true, + colorspace: 'sRGB', + ).processed + end + + attachment_variant + end + + class_methods do + def redefine_attachment_writer_with_legacy_io_support(name) + define_method :"#{name}=" do |attachable| + attachment = public_send(name) + + case attachable + when ActiveStorage::Blob, ActionDispatch::Http::UploadedFile, + Rack::Test::UploadedFile, Hash, String + attachment.attach(attachable) + when ActiveStorage::Attached + attachment.attach(attachable.blob) + else # assume it's an IO + if attachable.respond_to?(:to_path) + filename = attachable.to_path + else + filename = SecureRandom.uuid + end + attachable.rewind + + attachment.attach( + io: attachable, + filename: filename + ) + end + end + end + + def validate_attachment_to_be_an_image(name) + method_name = :"attached_#{name}_is_an_image" + + define_method method_name do + attachment = public_send(name) + next if attachment.nil? || attachment.attachment.nil? + + errors.add name, 'is not an image' unless attachment.attachment.image? + end + + validate method_name + end + end + end +end diff --git a/core/app/models/spree/image.rb b/core/app/models/spree/image.rb index 8478257758e..728a81925ec 100644 --- a/core/app/models/spree/image.rb +++ b/core/app/models/spree/image.rb @@ -2,55 +2,33 @@ module Spree class Image < Asset - validate :no_attachment_errors - - has_attached_file :attachment, - styles: { mini: '48x48>', small: '100x100>', product: '240x240>', large: '600x600>' }, - default_style: :product, - default_url: 'noimage/:style.png', - url: '/spree/products/:id/:style/:basename.:extension', - path: ':rails_root/public/spree/products/:id/:style/:basename.:extension', - convert_options: { all: '-strip -auto-orient -colorspace sRGB' } - validates_attachment :attachment, - presence: true, - content_type: { content_type: %w(image/jpeg image/jpg image/png image/gif) } + if ::Spree::Config.image_attachment_module.blank? + Spree::Deprecation.warn <<-MESSAGE.strip_heredoc + "\n\n" + Using Paperclip as image_attachment_module for Solidus. + + Please configure Spree::Config.image_attachment_module in your store + initializer. + + To use the ActiveStorage adapter (recommended): + Spree.config do |config| + config.image_attachment_module = 'Spree::Image::ActiveStorageAttachment' + end + + To use the Paperclip adapter (legacy, deprecated): + Spree.config do |config| + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + end + MESSAGE + ::Spree::Config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + end - # save the w,h of the original image (from which others can be calculated) - # we need to look at the write-queue for images which have not been saved yet - after_post_process :find_dimensions, if: :valid? + include ::Spree::Config.image_attachment_module.to_s.constantize def mini_url Spree::Deprecation.warn( 'Spree::Image#mini_url is DEPRECATED. Use Spree::Image#url(:mini) instead.' ) - attachment.url(:mini, false) - end - - def url(size) - attachment.url(size) - end - - def filename - attachment_file_name - end - - def find_dimensions - temporary = attachment.queued_for_write[:original] - filename = temporary.path unless temporary.nil? - filename = attachment.path if filename.blank? - geometry = Paperclip::Geometry.from_file(filename) - self.attachment_width = geometry.width - self.attachment_height = geometry.height - end - - # if there are errors from the plugin, then add a more meaningful message - def no_attachment_errors - unless attachment.errors.empty? - # uncomment this to get rid of the less-than-useful interim messages - # errors.clear - errors.add :attachment, "Paperclip returned errors for file '#{attachment_file_name}' - check ImageMagick installation or image source file." - false - end + url(:mini, false) end end end diff --git a/core/app/models/spree/image/active_storage_attachment.rb b/core/app/models/spree/image/active_storage_attachment.rb new file mode 100644 index 00000000000..b68c368950d --- /dev/null +++ b/core/app/models/spree/image/active_storage_attachment.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'active_storage' + +module Spree::Image::ActiveStorageAttachment + extend ActiveSupport::Concern + include Spree::ActiveStorageAttachment + + module DeprecatedPaperclipAPI + def attachment(*args) + if args.size == 1 + # TODO: deprecation + style = args.first + Spree::ActiveStorageAttachment.attachment_variant( + super(), + style: style, + default_style: default_style, + styles: ATTACHMENT_STYLES + ) + else + # With 0 args will be ok, otherwise will raise an ArgumentError + super + end + end + end + + included do + has_one_attached :attachment + redefine_attachment_writer_with_legacy_io_support :attachment + validate_attachment_to_be_an_image :attachment + validates :attachment, presence: true + prepend DeprecatedPaperclipAPI + end + + class_methods do + def attachment_definitions + { attachment: { styles: ATTACHMENT_STYLES } } + end + end + + ATTACHMENT_STYLES = { + mini: '48x48>', + small: '100x100>', + product: '240x240>', + large: '600x600>', + } + + def default_style + :original + end + + def url(style = default_style, options = {}) + options = normalize_url_options(options) + + Spree::ActiveStorageAttachment.attachment_variant( + attachment, + style: style, + default_style: default_style, + styles: ATTACHMENT_STYLES + )&.service_url(options) + end + + def filename + attachment.blob.filename.to_s + end + + def attachment_width + attachment.metadata[:width] + end + + def attachment_height + attachment.metadata[:height] + end + + def attachment_present? + attachment.attached? + end + + private + + def normalize_url_options(options) + if [true, false].include? options # Paperclip backwards compatibility. + Spree::Deprecation.warn( + "Using #{self.class}#url with true/false as second parameter is deprecated, if you "\ + "want to enable/disable timestamps pass `timestamps: true` (or `false`)." + ) + options = { timestamp: options } + end + + options + end +end diff --git a/core/app/models/spree/image/paperclip_attachment.rb b/core/app/models/spree/image/paperclip_attachment.rb new file mode 100644 index 00000000000..b716824d8b6 --- /dev/null +++ b/core/app/models/spree/image/paperclip_attachment.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Spree::Image::PaperclipAttachment + extend ActiveSupport::Concern + + included do + validate :no_attachment_errors + + has_attached_file :attachment, + styles: { mini: '48x48>', small: '100x100>', product: '240x240>', large: '600x600>' }, + default_style: :product, + default_url: 'noimage/:style.png', + url: '/spree/products/:id/:style/:basename.:extension', + path: ':rails_root/public/spree/products/:id/:style/:basename.:extension', + convert_options: { all: '-strip -auto-orient -colorspace sRGB' } + validates_attachment :attachment, + presence: true, + content_type: { content_type: %w[image/jpeg image/jpg image/png image/gif] } + + # save the w,h of the original image (from which others can be calculated) + # we need to look at the write-queue for images which have not been saved yet + after_post_process :find_dimensions, if: :valid? + end + + def url(size) + attachment.url(size) + end + + def filename + attachment_file_name + end + + def attachment_present? + attachment.present? + end + + def find_dimensions + temporary = attachment.queued_for_write[:original] + filename = temporary.path unless temporary.nil? + filename = attachment.path if filename.blank? + geometry = Paperclip::Geometry.from_file(filename) + self.attachment_width = geometry.width + self.attachment_height = geometry.height + end + + # if there are errors from the plugin, then add a more meaningful message + def no_attachment_errors + unless attachment.errors.empty? + # uncomment this to get rid of the less-than-useful interim messages + # errors.clear + errors.add :attachment, "Paperclip returned errors for file '#{attachment_file_name}' - check ImageMagick installation or image source file." + false + end + end +end diff --git a/core/app/models/spree/taxon.rb b/core/app/models/spree/taxon.rb index ecd4c475a1e..2b36163ffc8 100644 --- a/core/app/models/spree/taxon.rb +++ b/core/app/models/spree/taxon.rb @@ -25,15 +25,27 @@ class Taxon < Spree::Base after_save :touch_ancestors_and_taxonomy after_touch :touch_ancestors_and_taxonomy - has_attached_file :icon, - styles: { mini: '32x32>', normal: '128x128>' }, - default_style: :mini, - url: '/spree/taxons/:id/:style/:basename.:extension', - path: ':rails_root/public/spree/taxons/:id/:style/:basename.:extension', - default_url: '/assets/default_taxon.png' - - validates_attachment :icon, - content_type: { content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif"] } + if ::Spree::Config.taxon_attachment_module.blank? + Spree::Deprecation.warn <<-MESSAGE.strip_heredoc + "\n\n" + Using Paperclip as taxon_attachment_module for Taxon. + + Please configure Spree::Config.taxon_attachment_module in your store + initializer. + + To use the ActiveStorage adapter (recommended): + Spree.config do |config| + config.image_attachment_module = 'Spree::Taxon::ActiveStorageAttachment' + end + + To use the Paperclip adapter (legacy, deprecated): + Spree.config do |config| + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' + end + MESSAGE + ::Spree::Config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' + end + + include ::Spree::Config.taxon_attachment_module.to_s.constantize self.whitelisted_ransackable_attributes = %w[name] diff --git a/core/app/models/spree/taxon/active_storage_attachment.rb b/core/app/models/spree/taxon/active_storage_attachment.rb new file mode 100644 index 00000000000..892e0007bb7 --- /dev/null +++ b/core/app/models/spree/taxon/active_storage_attachment.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'active_storage' + +module Spree::Taxon::ActiveStorageAttachment + extend ActiveSupport::Concern + include Spree::ActiveStorageAttachment + + included do + has_one_attached :icon + redefine_attachment_writer_with_legacy_io_support :icon + validate_attachment_to_be_an_image :icon + end + + def icon_present? + icon.attached? + end +end diff --git a/core/app/models/spree/taxon/paperclip_attachment.rb b/core/app/models/spree/taxon/paperclip_attachment.rb new file mode 100644 index 00000000000..cb4a83437cc --- /dev/null +++ b/core/app/models/spree/taxon/paperclip_attachment.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Spree::Taxon::PaperclipAttachment + extend ActiveSupport::Concern + + included do + has_attached_file :icon, + styles: { mini: '32x32>', normal: '128x128>' }, + default_style: :mini, + url: '/spree/taxons/:id/:style/:basename.:extension', + path: ':rails_root/public/spree/taxons/:id/:style/:basename.:extension', + default_url: '/assets/default_taxon.png' + + validates_attachment :icon, + content_type: { content_type: %w[image/jpg image/jpeg image/png image/gif] } + end + + def icon_present? + icon.present? + end +end diff --git a/core/config/routes.rb b/core/config/routes.rb new file mode 100644 index 00000000000..e5f7de1e070 --- /dev/null +++ b/core/config/routes.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Spree::Core::Engine.routes.draw do + resolve("ActiveStorage::Variant") { |object, options| Rails.application.routes.url_helpers.url_for(object, options.merge(only_path: true)) } + resolve("ActiveStorage::Preview") { |object, options| Rails.application.routes.url_helpers.url_for(object, options.merge(only_path: true)) } + resolve("ActiveStorage::Blob") { |object, options| Rails.application.routes.url_helpers.url_for(object, options.merge(only_path: true)) } + resolve("ActiveStorage::Attachment") { |object, options| Rails.application.routes.url_helpers.url_for(object, options.merge(only_path: true)) } +end diff --git a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt index e4d4a91d76d..5039d31dc43 100644 --- a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt @@ -18,6 +18,9 @@ Spree.config do |config| # any inventory changes. # config.inventory_cache_threshold = 3 + # Enable Paperclip adapter for attachments on images and taxons + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' # Frontend: diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 02d1249390e..6e2d248f5b4 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -423,6 +423,34 @@ def payment_canceller # Enumerable of images adhering to the present_image_class interface class_name_attribute :product_gallery_class, default: 'Spree::Gallery::ProductGallery' + # Allows switching attachment library for Image + # + # `Spree::Image::PaperclipAttachment` + # is the default and provides the classic Paperclip implementation. + # + # `Spree::Image::ActiveStorageAttachment` + # Is the new ActiveStorage implementation, requires `bin/rails active_storage:install` in + # order to work. + # + # @!attribute [rw] image_attachment_module + # @return [Module] a module that can be included into Spree::Image to allow attachments + # Enumerable of images adhering to the present_image_class interface + preference :image_attachment_module, default: nil + + # Allows switching attachment library for Taxon + # + # `Spree::Taxon::PaperclipAttachment` + # is the default and provides the classic Paperclip implementation. + # + # `Spree::Taxon::ActiveStorageAttachment` + # Is the new ActiveStorage implementation, requires `bin/rails active_storage:install` in + # order to work. + # + # @!attribute [rw] taxon_attachment_module + # @return [Module] a module that can be included into Spree::Taxon to allow attachments + # Enumerable of taxons adhering to the present_taxon_class interface + preference :taxon_attachment_module, default: nil + # Allows providing your own class instance for generating order numbers. # # @!attribute [rw] order_number_generator diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index ffb4d327935..29131f125ba 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -3,23 +3,17 @@ ENV['RAILS_ENV'] = 'test' ENV['DISABLE_DATABASE_ENVIRONMENT_CHECK'] = '1' -require 'rails' -require 'active_record/railtie' -require 'action_controller/railtie' -require 'action_mailer/railtie' +require 'rails/all' Rails.env = 'test' require 'solidus_core' -# @private -def forgery_protected_by_default? - Gem::Version.new(Rails.version) >= Gem::Version.new('5.2') -end +RAILS_52_OR_ABOVE = Gem::Version.new(Rails.version) >= Gem::Version.new('5.2') # @private class ApplicationController < ActionController::Base - if !forgery_protected_by_default? + unless RAILS_52_OR_ABOVE protect_from_forgery with: :exception end end @@ -65,13 +59,17 @@ class Application < ::Rails::Application config.active_support.deprecation = :stderr config.secret_key_base = 'SECRET_TOKEN' - if forgery_protected_by_default? + if RAILS_52_OR_ABOVE config.action_controller.default_protect_from_forgery = true - end - - if config.active_record.sqlite3 - # Rails >= 5.2 config.active_record.sqlite3.represent_boolean_as_integer = true + + initializer 'solidus.active_storage' do + require 'active_storage' + require 'active_storage/service/disk_service' + ::ActiveStorage::Blob.service = ::ActiveStorage::Service::DiskService.new( + root: Dir.mktmpdir('activestorage-test') + ) + end end # Avoid issues if an old spec/dummy still exists @@ -112,6 +110,14 @@ class Application < ::Rails::Application Spree.user_class = 'Spree::LegacyUser' Spree.config do |config| config.mails_from = "store@example.com" + + if ENV['ACTIVE_STORAGE'] + config.image_attachment_module = 'Spree::Image::ActiveStorageAttachment' + config.taxon_attachment_module = 'Spree::Taxon::ActiveStorageAttachment' + else + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' + end end # Raise on deprecation warnings diff --git a/core/lib/spree/testing_support/dummy_app/migrations.rb b/core/lib/spree/testing_support/dummy_app/migrations.rb index c0ab0c6125d..1b3d7bc1c69 100644 --- a/core/lib/spree/testing_support/dummy_app/migrations.rb +++ b/core/lib/spree/testing_support/dummy_app/migrations.rb @@ -32,6 +32,10 @@ def auto_migrate sh 'rake db:reset VERBOSE=false' + if ENV['ACTIVE_STORAGE'] + sh 'rake active_storage:install db:migrate VERBOSE=false' + end + # We have a brand new database, so we must re-establish our connection ActiveRecord::Base.establish_connection end diff --git a/core/lib/spree/testing_support/preferences.rb b/core/lib/spree/testing_support/preferences.rb index 61a1938ce6f..ba2e6a5f547 100644 --- a/core/lib/spree/testing_support/preferences.rb +++ b/core/lib/spree/testing_support/preferences.rb @@ -18,6 +18,16 @@ def reset_spree_preferences(&config_block) Rails.application.config.spree = Spree::Config.environment end + ::Spree.config do |config| + if ENV['ACTIVE_STORAGE'] + config.image_attachment_module = 'Spree::Image::ActiveStorageAttachment' + config.taxon_attachment_module = 'Spree::Taxon::ActiveStorageAttachment' + else + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' + end + end + configure_spree_preferences(&config_block) if block_given? end diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index 12fd56a8d15..044b164c683 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -34,6 +34,7 @@ Gem::Specification.new do |s| s.add_dependency 'carmen', '~> 1.1.0' s.add_dependency 'discard', '~> 1.0' s.add_dependency 'friendly_id', '~> 5.0' + s.add_dependency 'image_processing', '~> 1.2' s.add_dependency 'kaminari-activerecord', '~> 1.1' s.add_dependency 'monetize', '~> 1.8' s.add_dependency 'paperclip', ['>= 4.2', '< 6'] diff --git a/guides/source/developers/upgrades/migrate-from-paperclip-to-active_storage.html.md b/guides/source/developers/upgrades/migrate-from-paperclip-to-active_storage.html.md new file mode 100644 index 00000000000..dcb3d41be5c --- /dev/null +++ b/guides/source/developers/upgrades/migrate-from-paperclip-to-active_storage.html.md @@ -0,0 +1,8 @@ +# Migrate from Paperclip to ActiveStorage + +## Gotchas + +- Load order, the spree initializer must be loaded first, use `01_spree.rb` or similar alphabetization +- A catch-all route can prevent disk service from working +- Until [rails/rails#34581](https://github.com/rails/rails/pull/34581) is merged there's no reliable way to have cachable, non-expiring, public URLs, using the ActiveStorage adapter will generate URLs that will expire (by default) in 5 minutes +- Application code can raise errors like `Can’t resolve image into URL: undefined method 'attachment_url’ for #<#:0x00007f84c22bdc68>` around code like `<%= link_to image_tag(line_item.product.display_image.attachment, itemprop: "image"), spree.product_path(line_item.product) %>`; this is solvable changing it to `<%= link_to image_tag(line_item.product.display_image.url, itemprop: "image"), spree.product_path(line_item.product) %>`