diff --git a/.circleci/config.yml b/.circleci/config.yml index 04428f47bea..785b4deb528 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -96,11 +96,12 @@ jobs: - setup - test - postgres_rails_master: + postgres_rails_master_activestorage: executor: postgres parallelism: *parallelism environment: RAILS_VERSION: 'master' + ENABLE_ACTIVE_STORAGE: true steps: - setup - test @@ -141,7 +142,7 @@ workflows: - mysql - postgres_rails52 - mysql_rails52 - - postgres_rails_master + - postgres_rails_master_activestorage - stoplight/push: project: solidus/solidus-api git_token: $STOPLIGHT_GIT_TOKEN diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index 77d6df42885..13a8028e30c 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -13,6 +13,7 @@ class BaseController < ActionController::Base include Spree::Core::ControllerHelpers::Store include Spree::Core::ControllerHelpers::Pricing include Spree::Core::ControllerHelpers::StrongParameters + include ActiveStorage::SetCurrent if Spree::Config.active_storage_enabled? class_attribute :admin_line_item_attributes self.admin_line_item_attributes = [:price, :variant_id, :sku] diff --git a/core/app/controllers/spree/base_controller.rb b/core/app/controllers/spree/base_controller.rb index 7af1d0a13f3..85f6c290096 100644 --- a/core/app/controllers/spree/base_controller.rb +++ b/core/app/controllers/spree/base_controller.rb @@ -10,6 +10,7 @@ class Spree::BaseController < ApplicationController include Spree::Core::ControllerHelpers::Search include Spree::Core::ControllerHelpers::Store include Spree::Core::ControllerHelpers::StrongParameters + include ActiveStorage::SetCurrent if Spree::Config.active_storage_enabled? respond_to :html end diff --git a/core/app/models/concerns/spree/active_storage_adapter.rb b/core/app/models/concerns/spree/active_storage_adapter.rb new file mode 100644 index 00000000000..bd396579dd3 --- /dev/null +++ b/core/app/models/concerns/spree/active_storage_adapter.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module Spree + # Adapts ActiveStorage interface to make it compliant with Solidus' + # Paperclip-oriented attachment support. + module ActiveStorageAdapter + extend ActiveSupport::Concern + include Spree::ActiveStorageAdapter::Normalization + + included do + next if Rails.gem_version >= Gem::Version.new('6.1.0.alpha') + + abort <<~MESSAGE + Configuration Error: Solidus ActiveStorage attachment adpater requires Rails >= 6.1.0. + + Spree::Config.image_attachment_module preference is set to #{Spree::Config.image_attachment_module} + Spree::Config.taxon_attachment_module preference is set to #{Spree::Config.taxon_attachment_module} + Rails version is #{Rails.gem_version} + + To solve the problem you can upgrade to a Rails version greater than or equal to 6.1.0 + or use legacy Paperclip attachment adapter by editing `config/initialiers/spree/rb`: + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' + MESSAGE + end + + class_methods do + attr_reader :attachment_name + attr_reader :attachment_definition + + # Specifies the relation between a single attachment and the model + def has_attachment(name, definition) + @attachment_name = name.to_sym + @attachment_definition = definition + + has_one_attached attachment_name + + override_reader + override_writer + define_image_validation + define_presence_reader + end + + def attachment_definitions + { attachment_name => attachment_definition } + end + + private + + def override_reader + method_name = attachment_name + override = Module.new do + define_method method_name do |*args| + attachment = Attachment.new(super(), styles: styles) + if args.empty? + attachment + else + style = args.first || default_style + attachment.url(style) + end + end + end + prepend override + + alias_method :attachment, method_name if method_name != :attachment + end + + def override_writer + method_name = :"#{attachment_name}=" + override = Module.new do + define_method method_name do |attachable| + no_other_changes = persisted? && !changed? + super(normalize_attachable(attachable)) + save if no_other_changes + end + end + prepend override + end + + def define_image_validation + define_method :"#{attachment_name}_is_an_image" do + return unless attachment.attached? + return if attachment.image? + + errors.add(self.class.attachment_name, 'is not an image') + end + end + + def define_presence_reader + define_method :"#{attachment_name}_present?" do + attachment.attached? + end + end + end + + def styles + self.class.attachment_definition[:styles] + end + + def default_style + self.class.attachment_definition[:default_style] + end + + def filename + attachment.filename + end + + def url(style = default_style) + attachment.url(style) + end + + def destroy_attachment(_name) + attachment.destroy + end + end +end diff --git a/core/app/models/concerns/spree/active_storage_adapter/attachment.rb b/core/app/models/concerns/spree/active_storage_adapter/attachment.rb new file mode 100644 index 00000000000..2b9f6b16d9a --- /dev/null +++ b/core/app/models/concerns/spree/active_storage_adapter/attachment.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'mini_magick' + +module Spree + module ActiveStorageAdapter + # Decorares AtiveStorage attachment to add methods exptected by Solidus' + # Paperclip-oriented attachment support. + class Attachment + delegate_missing_to :@attachment + + DEFAULT_SIZE = '100%' + + def initialize(attachment, styles: {}) + @attachment = attachment + @styles = styles + end + + def exists? + attached? + end + + def filename + blob.filename.to_s + end + + def url(style = nil) + variant(style).url + end + + def variant(style = nil) + size = style_to_size(style&.to_sym) + @attachment.variant( + resize: size, + strip: true, + 'auto-orient': true, + colorspace: 'sRGB', + ).processed + end + + def height + metadata[:height] + end + + def width + metadata[:width] + end + + def destroy + return false unless attached? + + purge + true + end + + private + + def metadata + analyze unless analyzed? + + @attachment.metadata + end + + def style_to_size(style) + @styles.fetch(style) { DEFAULT_SIZE } + end + end + end +end diff --git a/core/app/models/concerns/spree/active_storage_adapter/normalization.rb b/core/app/models/concerns/spree/active_storage_adapter/normalization.rb new file mode 100644 index 00000000000..9ac95fbef08 --- /dev/null +++ b/core/app/models/concerns/spree/active_storage_adapter/normalization.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Spree + module ActiveStorageAdapter + # Contains normalization methods to make objects compliant with + # ActiveStorage API. + module Normalization + # Normalizes an attachable + def normalize_attachable(attachable) + case attachable + when ActiveStorage::Blob, ActionDispatch::Http::UploadedFile, + Rack::Test::UploadedFile, Hash, String + attachable + when Attachment, ActiveStorage::Attached + attachable_blob(attachable) + else # assume it's an IO + attachable_io(attachable) + end + end + + private + + def attachable_blob(attachable) + attachable.blob + end + + def attachable_io(attachable) + filename = if attachable.respond_to?(:to_path) + attachable.to_path + else + SecureRandom.uuid + end + attachable.rewind + + { io: attachable, filename: filename } + end + 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..c4f87d09e08 --- /dev/null +++ b/core/app/models/spree/image/active_storage_attachment.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Spree::Image::ActiveStorageAttachment + extend ActiveSupport::Concern + include Spree::ActiveStorageAdapter + + delegate :width, :height, to: :attachment, prefix: true + + included do + has_attachment :attachment, + styles: { + mini: '48x48>', + small: '400x400>', + product: '680x680>', + large: '1200x1200>' + }, + default_style: :product + validates :attachment, presence: true + validate :attachment_is_an_image + end +end 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..fd8b220389e --- /dev/null +++ b/core/app/models/spree/taxon/active_storage_attachment.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Spree::Taxon::ActiveStorageAttachment + extend ActiveSupport::Concern + include Spree::ActiveStorageAdapter + + included do + has_attachment :icon, + styles: { mini: '32x32>', normal: '128x128>' }, + default_style: :mini + validate :icon_is_an_image + + + end + + def attachment_partial_name + 'paperclip' + end +end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index d543fd0c14b..a97ac827877 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -585,5 +585,11 @@ def admin_vat_location country: Spree::Country.find_by(iso: admin_vat_country_iso) ) end + + def active_storage_enabled? + @active_storage_enabled ||= + Spree::Config.image_attachment_module == Spree::Image::ActiveStorageAttachment || + Spree::Config.taxon_attachment_module == Spree::Taxon::ActiveStorageAttachment + end end end diff --git a/core/lib/spree/awesome_nested_set_override.rb b/core/lib/spree/awesome_nested_set_override.rb index f88b533e091..3cf540d840b 100644 --- a/core/lib/spree/awesome_nested_set_override.rb +++ b/core/lib/spree/awesome_nested_set_override.rb @@ -1,26 +1,44 @@ # frozen_string_literal: true module AwesomeNestedSetOvveride - # Add :polimorphic key option only when used to make it work with Rails 6.1+ - # https://github.com/osmaelo/rails/commit/2c008d9f6311d92b3660193285230505e74a114f + # Add :polimorphic key option only when used to make it work with Rails 6.1+, + # required since rails/rails@2c008d9 # This can be removed when upgrading to an awesome_nested_set version - # compliant with Rails 6.1+. - def acts_as_nested_set_relate_parent! - # Disable Rubocop to keep original code for diffs - # rubocop:disable - options = { - :class_name => self.base_class.to_s, - :foreign_key => parent_column_name, - :primary_key => primary_column_name, - :counter_cache => acts_as_nested_set_options[:counter_cache], - :inverse_of => (:children unless acts_as_nested_set_options[:polymorphic]), - :touch => acts_as_nested_set_options[:touch] - } - options[:polymorphic] = true if acts_as_nested_set_options[:polymorphic] - options[:optional] = true if ActiveRecord::VERSION::MAJOR >= 5 - belongs_to :parent, options - # rubocop:disable + # compliant with Rails 6.1+, already addressed in + # collectiveidea/awesome_nested_set#421 + module RelateParent + def acts_as_nested_set_relate_parent! + # Disable Rubocop to keep original code for diffs + # rubocop:disable + options = { + :class_name => self.base_class.to_s, + :foreign_key => parent_column_name, + :primary_key => primary_column_name, + :counter_cache => acts_as_nested_set_options[:counter_cache], + :inverse_of => (:children unless acts_as_nested_set_options[:polymorphic]), + :touch => acts_as_nested_set_options[:touch] + } + options[:polymorphic] = true if acts_as_nested_set_options[:polymorphic] + options[:optional] = true if ActiveRecord::VERSION::MAJOR >= 5 + belongs_to :parent, options + # rubocop:enable + end + + CollectiveIdea::Acts::NestedSet.prepend self end - CollectiveIdea::Acts::NestedSet.prepend self + # Skip breaking model reload before update depth. Already addressed in + # collectiveidea/awesome_nested_set#413 + # This can be removed when a new version of awesome_nested_set is released. + module Model + def set_depth! + return unless has_depth_column? + + in_tenacious_transaction do + update_depth(level) + end + end + + CollectiveIdea::Acts::NestedSet::Model.prepend self + end end diff --git a/core/lib/spree/core.rb b/core/lib/spree/core.rb index 99ebd500434..cdf3a001249 100644 --- a/core/lib/spree/core.rb +++ b/core/lib/spree/core.rb @@ -94,6 +94,6 @@ class GatewayError < RuntimeError; end require 'spree/preferences/static_model_preferences' require 'spree/preferences/scoped_store' -if Gem::Version.new(Rails.version) >= Gem::Version.new('6.1.0.alpha') +if Rails.gem_version >= Gem::Version.new('6.1.0.alpha') require 'spree/awesome_nested_set_override' end diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 541884c973d..25bc2180ace 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -12,7 +12,7 @@ require 'solidus_core' -RAILS_6_OR_ABOVE = Gem::Version.new(Rails.version) >= Gem::Version.new('6.0') +RAILS_6_OR_ABOVE = Rails.gem_version >= Gem::Version.new('6.0') # @private class ApplicationController < ActionController::Base @@ -64,6 +64,20 @@ class Application < ::Rails::Application config.active_record.sqlite3.represent_boolean_as_integer = true end + config.storage_path = Rails.root.join('tmp', 'storage') + + if ENV['ENABLE_ACTIVE_STORAGE'] + initializer 'solidus.active_storage' do + config.active_storage.service_configurations = { + test: { + service: 'Disk', + root: config.storage_path + } + } + config.active_storage.service = :test + end + end + # Avoid issues if an old spec/dummy still exists config.paths['config/initializers'] = [] config.paths['config/environments'] = [] @@ -104,6 +118,11 @@ class Application < ::Rails::Application config.mails_from = "store@example.com" config.raise_with_invalid_currency = false config.use_combined_first_and_last_name_in_address = true + + if ENV['ENABLE_ACTIVE_STORAGE'] + config.image_attachment_module = 'Spree::Image::ActiveStorageAttachment' + config.taxon_attachment_module = 'Spree::Taxon::ActiveStorageAttachment' + 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 79d0e3d0c74..37aa5888903 100644 --- a/core/lib/spree/testing_support/dummy_app/migrations.rb +++ b/core/lib/spree/testing_support/dummy_app/migrations.rb @@ -27,6 +27,9 @@ def auto_migrate ActiveRecord::Base.remove_connection sh 'rake db:reset VERBOSE=false' + if ENV['ENABLE_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 diff --git a/core/lib/spree/testing_support/factories/image_factory.rb b/core/lib/spree/testing_support/factories/image_factory.rb index 67355b6ee66..6d276ca9e73 100644 --- a/core/lib/spree/testing_support/factories/image_factory.rb +++ b/core/lib/spree/testing_support/factories/image_factory.rb @@ -2,6 +2,6 @@ FactoryBot.define do factory :image, class: 'Spree::Image' do - attachment { File.new(Spree::Core::Engine.root + 'spec/fixtures/thinking-cat.jpg') } + attachment { Spree::Core::Engine.root.join('spec', 'fixtures', 'thinking-cat.jpg').open } end end diff --git a/core/lib/spree/testing_support/factories/taxon_factory.rb b/core/lib/spree/testing_support/factories/taxon_factory.rb index 525bd03b983..4850a4aa267 100644 --- a/core/lib/spree/testing_support/factories/taxon_factory.rb +++ b/core/lib/spree/testing_support/factories/taxon_factory.rb @@ -7,5 +7,9 @@ name { 'Ruby on Rails' } taxonomy parent_id { nil } + + trait :with_icon do + icon { Spree::Core::Engine.root.join('spec', 'fixtures', 'thinking-cat.jpg').open } + end end end diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index 8a0ca94ea18..595629bce49 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -35,7 +35,9 @@ 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.10' s.add_dependency 'kaminari-activerecord', '~> 1.1' + s.add_dependency 'mini_magick', '~> 4.10' s.add_dependency 'monetize', '~> 1.8' s.add_dependency 'paperclip', '>= 4.2' s.add_dependency 'paranoia', '~> 2.4' diff --git a/core/spec/models/spree/image_spec.rb b/core/spec/models/spree/image_spec.rb index bfa27a4d0db..69fc69f5a65 100644 --- a/core/spec/models/spree/image_spec.rb +++ b/core/spec/models/spree/image_spec.rb @@ -3,23 +3,34 @@ require 'rails_helper' RSpec.describe Spree::Image, type: :model do - context '#save' do - context 'invalid attachment' do - let(:invalid_image) { File.open(__FILE__) } - subject { described_class.new(attachment: invalid_image) } - - it 'returns false' do - expect(subject.save).to be false - end + it_behaves_like 'an attachment' do + subject { create(:image) } + let(:attachment_name) { :attachment } + let(:default_style) { :product } + end + + describe 'attachment details' do + let(:image_file) { File.open(File.join('spec', 'fixtures', 'thinking-cat.jpg')) } + subject { create(:image, attachment: image_file) } + + it 'returns if attachment is present' do + expect(subject.attachment_present?).to be_truthy + end + + it 'returns attachment filename' do + expect(subject.filename).to end_with('thinking-cat.jpg') + end + + it 'returns attachment url' do + expect(subject.url(:product)).to include('thinking-cat.jpg') end - context 'valid attachment' do - let(:valid_image) { File.open(File.join('spec', 'fixtures', 'thinking-cat.jpg')) } - subject { described_class.new(attachment: valid_image) } + it 'computes attachment width' do + expect(subject.attachment_width).to eq(489) + end - it 'returns true' do - expect(subject.save).to be true - end + it 'computes attachment height' do + expect(subject.attachment_height).to eq(490) end end end diff --git a/core/spec/models/spree/promotion/rules/taxon_spec.rb b/core/spec/models/spree/promotion/rules/taxon_spec.rb index 2fbb0e2a8ed..9ab462e449a 100644 --- a/core/spec/models/spree/promotion/rules/taxon_spec.rb +++ b/core/spec/models/spree/promotion/rules/taxon_spec.rb @@ -101,6 +101,8 @@ before do taxon.children << taxon2 taxon.save! + taxon.reload + product.taxons = [taxon2, taxon3] rule.taxons = [taxon, taxon3] end diff --git a/core/spec/models/spree/taxon_spec.rb b/core/spec/models/spree/taxon_spec.rb index 29de339855c..960e9d5b080 100644 --- a/core/spec/models/spree/taxon_spec.rb +++ b/core/spec/models/spree/taxon_spec.rb @@ -3,6 +3,12 @@ require 'rails_helper' RSpec.describe Spree::Taxon, type: :model do + it_behaves_like 'an attachment' do + subject { create(:taxon) } + let(:attachment_name) { :icon } + let(:default_style) { :mini } + end + context "#destroy" do subject(:nested_set_options) { described_class.acts_as_nested_set_options } @@ -11,6 +17,34 @@ end end + describe "#destroy_attachment" do + context "when trying to destroy a valid attachment definition" do + context "and taxon has a file attached " do + it "removes the attachment" do + taxon = create(:taxon, :with_icon) + + expect(taxon.destroy_attachment(:icon)).to be_truthy + end + end + + context "and the taxon does not have any file attached yet" do + it "returns false" do + taxon = create(:taxon) + + expect(taxon.destroy_attachment(:icon)).to be_falsey + end + end + end + + context "when trying to destroy an invalid attachment" do + it 'returns false' do + taxon = create(:taxon) + + expect(taxon.destroy_attachment(:foo)).to be_falsey + end + end + end + describe '#to_param' do let(:taxon) { FactoryBot.build(:taxon, name: "Ruby on Rails") } diff --git a/core/spec/models/spree/taxons/paperclip_attachment_spec.rb b/core/spec/models/spree/taxons/paperclip_attachment_spec.rb deleted file mode 100644 index 013bf8047bc..00000000000 --- a/core/spec/models/spree/taxons/paperclip_attachment_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe "Spree::Taxon::PaperclipAttachment", type: :model do - describe "#destroy_attachment" do - let(:taxon) { create(:taxon) } - - context "when trying to destroy a valid attachment definition" do - context "and taxon has a file attached " do - it "removes the attachment" do - taxon.update(icon: File.new(Rails.root.join('..', '..', 'spec', 'fixtures', 'thinking-cat.jpg'))) - expect(taxon.destroy_attachment(:icon)).to be_truthy - end - end - context "and the taxon does not have any file attached yet" do - it "returns false" do - expect(taxon.destroy_attachment(:icon)).to be_falsey - end - end - end - - context "when trying to destroy an invalid attachment" do - it 'returns false' do - expect(taxon.destroy_attachment(:foo)).to be_falsey - end - end - end -end diff --git a/core/spec/rails_helper.rb b/core/spec/rails_helper.rb index 4898f9641ec..586cba446ec 100644 --- a/core/spec/rails_helper.rb +++ b/core/spec/rails_helper.rb @@ -34,10 +34,12 @@ config.use_transactional_fixtures = true config.before :suite do + FileUtils.rm_rf(Rails.configuration.storage_path) DatabaseCleaner.clean_with :truncation end config.before :each do + ActiveStorage::Current.host = 'https://www.example.com' Rails.cache.clear end diff --git a/core/spec/support/shared_examples/attachment.rb b/core/spec/support/shared_examples/attachment.rb new file mode 100644 index 00000000000..41a5808b2a5 --- /dev/null +++ b/core/spec/support/shared_examples/attachment.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'an attachment' do + context 'valid attachment' do + before do + subject.send( + :"#{attachment_name}=", + File.open(File.join('spec', 'fixtures', 'thinking-cat.jpg')) + ) + end + + it 'passes validations' do + expect(subject).to be_valid + end + + it 'returns definition' do + expect(subject.class.attachment_definitions[attachment_name]) + .to include(default_style: default_style) + end + + it 'returns if present' do + expect(subject.send(:"#{attachment_name}_present?")).to be_truthy + end + + it 'returns an actual attachment' do + expect(subject.send(attachment_name)).to respond_to( + :url, + :exists? + ) + end + end + + context 'invalid attachment' do + it 'fails validation' do + invalid_file = File.open(__FILE__) + subject.send(:"#{attachment_name}=", invalid_file) + + expect(subject).not_to be_valid + expect(subject.errors).to include(attachment_name) + end + end +end