From 93c8704611c5a393ba11e58115ee68dbe136840c Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:28 +0200 Subject: [PATCH 01/27] Add activestorage --- alchemy_cms.gemspec | 1 + lib/alchemy_cms.rb | 1 + spec/dummy/config/application.rb | 1 + spec/dummy/config/environments/development.rb | 2 + spec/dummy/config/environments/production.rb | 2 + spec/dummy/config/environments/test.rb | 2 + spec/dummy/config/storage.yml | 34 +++++++++++ ...te_active_storage_tables.active_storage.rb | 57 +++++++++++++++++++ spec/dummy/db/schema.rb | 30 ++++++++++ 9 files changed, 130 insertions(+) create mode 100644 spec/dummy/config/storage.yml create mode 100644 spec/dummy/db/migrate/20240611151253_create_active_storage_tables.active_storage.rb diff --git a/alchemy_cms.gemspec b/alchemy_cms.gemspec index a8273c018f..657871ddab 100644 --- a/alchemy_cms.gemspec +++ b/alchemy_cms.gemspec @@ -30,6 +30,7 @@ Gem::Specification.new do |gem| activejob activemodel activerecord + activestorage activesupport railties ].each do |rails_gem| diff --git a/lib/alchemy_cms.rb b/lib/alchemy_cms.rb index 89a56fa9de..41aba2db6f 100644 --- a/lib/alchemy_cms.rb +++ b/lib/alchemy_cms.rb @@ -7,6 +7,7 @@ require "acts_as_list" require "action_view/dependency_tracker" require "active_model_serializers" +require "active_storage/engine" require "awesome_nested_set" require "cancan" require "dragonfly" diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb index c7b10e8b29..cc2040f3f0 100644 --- a/spec/dummy/config/application.rb +++ b/spec/dummy/config/application.rb @@ -10,6 +10,7 @@ require "action_mailer/railtie" require "action_view/railtie" require "active_job/railtie" +require "active_storage/engine" # require "action_cable/engine" # require "rails/test_unit/railtie" diff --git a/spec/dummy/config/environments/development.rb b/spec/dummy/config/environments/development.rb index 37a90fd90f..be8fb69b55 100644 --- a/spec/dummy/config/environments/development.rb +++ b/spec/dummy/config/environments/development.rb @@ -79,4 +79,6 @@ # Uncomment if you wish to allow Action Cable access from any origin. # config.action_cable.disable_request_forgery_protection = true + + config.active_storage.service = :local end diff --git a/spec/dummy/config/environments/production.rb b/spec/dummy/config/environments/production.rb index 63991a8eb9..d6e630909a 100644 --- a/spec/dummy/config/environments/production.rb +++ b/spec/dummy/config/environments/production.rb @@ -82,4 +82,6 @@ # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false + + config.active_storage.service = :local end diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb index acb102b502..b2ce241857 100644 --- a/spec/dummy/config/environments/test.rb +++ b/spec/dummy/config/environments/test.rb @@ -61,4 +61,6 @@ # Annotate rendered view with file names. # config.action_view.annotate_rendered_view_with_filenames = true + + config.active_storage.service = :test end diff --git a/spec/dummy/config/storage.yml b/spec/dummy/config/storage.yml new file mode 100644 index 0000000000..d32f76e8fb --- /dev/null +++ b/spec/dummy/config/storage.yml @@ -0,0 +1,34 @@ +test: + service: Disk + root: <%= Rails.root.join("tmp/storage") %> + +local: + service: Disk + root: <%= Rails.root.join("storage") %> + +# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key) +# amazon: +# service: S3 +# access_key_id: <%= Rails.application.credentials.dig(:aws, :access_key_id) %> +# secret_access_key: <%= Rails.application.credentials.dig(:aws, :secret_access_key) %> +# region: us-east-1 +# bucket: your_own_bucket + +# Remember not to checkin your GCS keyfile to a repository +# google: +# service: GCS +# project: your_project +# credentials: <%= Rails.root.join("path/to/gcs.keyfile") %> +# bucket: your_own_bucket + +# Use rails credentials:edit to set the Azure Storage secret (as azure_storage:storage_access_key) +# microsoft: +# service: AzureStorage +# storage_account_name: your_account_name +# storage_access_key: <%= Rails.application.credentials.dig(:azure_storage, :storage_access_key) %> +# container: your_container_name + +# mirror: +# service: Mirror +# primary: local +# mirrors: [ amazon, google, microsoft ] diff --git a/spec/dummy/db/migrate/20240611151253_create_active_storage_tables.active_storage.rb b/spec/dummy/db/migrate/20240611151253_create_active_storage_tables.active_storage.rb new file mode 100644 index 0000000000..e4706aa21c --- /dev/null +++ b/spec/dummy/db/migrate/20240611151253_create_active_storage_tables.active_storage.rb @@ -0,0 +1,57 @@ +# This migration comes from active_storage (originally 20170806125915) +class CreateActiveStorageTables < ActiveRecord::Migration[7.0] + def change + # Use Active Record's configured type for primary and foreign keys + primary_key_type, foreign_key_type = primary_and_foreign_key_types + + create_table :active_storage_blobs, id: primary_key_type 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 + + if connection.supports_datetime_with_precision? + t.datetime :created_at, precision: 6, null: false + else + t.datetime :created_at, null: false + end + + t.index [ :key ], unique: true + end + + create_table :active_storage_attachments, id: primary_key_type do |t| + t.string :name, null: false + t.references :record, null: false, polymorphic: true, index: false, type: foreign_key_type + t.references :blob, null: false, type: foreign_key_type + + if connection.supports_datetime_with_precision? + t.datetime :created_at, precision: 6, null: false + else + t.datetime :created_at, null: false + end + + 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, id: primary_key_type do |t| + t.belongs_to :blob, null: false, index: false, type: foreign_key_type + t.string :variation_digest, null: false + + t.index [ :blob_id, :variation_digest ], name: :index_active_storage_variant_records_uniqueness, unique: true + t.foreign_key :active_storage_blobs, column: :blob_id + end + end + + private + def primary_and_foreign_key_types + config = Rails.configuration.generators + setting = config.options[config.orm][:primary_key_type] + primary_key_type = setting || :primary_key + foreign_key_type = setting || :bigint + [primary_key_type, foreign_key_type] + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 676ff4a484..2728ed3600 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -11,6 +11,34 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema[7.2].define(version: 2024_04_11_155901) do + 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" + 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 "alchemy_attachments", force: :cascade do |t| t.string "name" t.string "file_name" @@ -294,6 +322,8 @@ t.string "name" 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 "alchemy_elements", "alchemy_page_versions", column: "page_version_id", on_delete: :cascade add_foreign_key "alchemy_ingredients", "alchemy_elements", column: "element_id", on_delete: :cascade add_foreign_key "alchemy_languages", "alchemy_sites", column: "site_id" From a1e2065b61b0a7df0c9719069b73a20b2012c0d0 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:29 +0200 Subject: [PATCH 02/27] Add image_processing Necessary for active storage image transforms --- alchemy_cms.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/alchemy_cms.gemspec b/alchemy_cms.gemspec index 657871ddab..59f1c02f3b 100644 --- a/alchemy_cms.gemspec +++ b/alchemy_cms.gemspec @@ -46,6 +46,7 @@ Gem::Specification.new do |gem| gem.add_runtime_dependency "dragonfly", ["~> 1.4"] gem.add_runtime_dependency "dragonfly_svg", ["~> 0.0.4"] gem.add_runtime_dependency "gutentag", ["~> 2.2", ">= 2.2.1"] + gem.add_runtime_dependency "image_processing", ["~> 1.13"] gem.add_runtime_dependency "importmap-rails", ["~> 2.0"] gem.add_runtime_dependency "kaminari", ["~> 1.1"] gem.add_runtime_dependency "originator", ["~> 3.1"] From a09ac6add13bcd9c4918766ce9d427697783abe9 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:31 +0200 Subject: [PATCH 03/27] Add dragonfly to image processing converter ActiveStorage uses the image_processing gem to process images. This uses a hash to describe the conversion instead of a string --- .../alchemy/dragonfly_to_image_processing.rb | 100 +++++++++ .../dragonfly_to_image_processing_spec.rb | 192 ++++++++++++++++++ 2 files changed, 292 insertions(+) create mode 100644 app/services/alchemy/dragonfly_to_image_processing.rb create mode 100644 spec/services/alchemy/dragonfly_to_image_processing_spec.rb diff --git a/app/services/alchemy/dragonfly_to_image_processing.rb b/app/services/alchemy/dragonfly_to_image_processing.rb new file mode 100644 index 0000000000..750b69000d --- /dev/null +++ b/app/services/alchemy/dragonfly_to_image_processing.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +module Alchemy + module DragonflyToImageProcessing + RESIZE_TO_LIMIT = />$/ + RESIZE_TO_FILL = /\#$/ + RESIZE_TO_FIT = /\^$/ + NOSHARPEN = {sharpen: false}.freeze + + class << self + def call(options = {}) + opts = crop_options(options).presence || resize_options(options) + opts.merge!(format_options(options)) + opts.merge!(quality_options(options)) + opts + end + + private + + def crop_options(options) + return {} unless options[:crop] && options[:crop_from] && options[:crop_size] && options[:size] + + crop_from = options[:crop_from].split("x", 2).map(&:to_i) + crop_size = options[:crop_size].split("x", 2).map(&:to_i) + { + crop: crop_from + crop_size + }.merge(resize_options(options.except(:crop))) + end + + def resize_options(options) + return {} unless options[:size] + + size_string = image_magick_string(options) + width, height = size_string.split("x", 2).map(&:to_i) + case size_string + when RESIZE_TO_FIT + resize_to_fit_options(width, height) + when RESIZE_TO_FILL + resize_to_fill_options(width, height) + else + resize_to_limit_options(width, height) + end.transform_values! do |value| + value.push(sharpen_option(options)) + end + end + + def quality_options(options) + quality = options[:quality] || Alchemy::Config.get(:output_image_quality) + {saver: {quality: quality}} + end + + def format_options(options) + format = options[:format] || default_output_format + return {} if format.nil? + + {format: format} + end + + def image_magick_string(options) + if options[:crop] == true + "#{options[:size]}#" + else + options[:size] + end + end + + def resize_to_fit_options(width, height) + { + resize_to_fit: [width, height] + } + end + + def resize_to_fill_options(width, height) + { + resize_to_fill: [width, height] + } + end + + def resize_to_limit_options(width, height) + { + resize_to_limit: [width, height] + } + end + + def sharpen_option(options) + sharpen_value(options) ? {} : NOSHARPEN + end + + def sharpen_value(options) + options.key?(:sharpen) ? options[:sharpen] : Alchemy::Config.get(:sharpen_images) + end + + def default_output_format + return nil if Alchemy::Config.get(:image_output_format) == "original" + + Alchemy::Config.get(:image_output_format) + end + end + end +end diff --git a/spec/services/alchemy/dragonfly_to_image_processing_spec.rb b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb new file mode 100644 index 0000000000..c6e8f763de --- /dev/null +++ b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb @@ -0,0 +1,192 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Alchemy::DragonflyToImageProcessing do + subject { described_class.call(options) } + + context "if crop size options are given" do + let(:options) do + { + crop: true, + crop_from: "0x0", + size: "200x100", + crop_size: "2000x1000" + } + end + + it "crops then resizes" do + is_expected.to eq( + { + crop: [0, 0, 2000, 1000], + resize_to_limit: [200, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + + context "if sharpen is enabled" do + let(:options) do + { + crop: true, + crop_from: "0x0", + size: "200x100", + crop_size: "2000x1000", + sharpen: true + } + end + + it "enables sharpen" do + is_expected.to eq( + { + crop: [0, 0, 2000, 1000], + resize_to_limit: [200, 100, {}], + saver: {quality: 85} + } + ) + end + end + end + + context "if no crop size options are given" do + context "Size option contains trailing >" do + let(:options) { {size: "100x100>"} } + + it "uses resize_to_limit" do + is_expected.to eq( + { + resize_to_limit: [100, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + end + + context "Size option contains trailing ^" do + let(:options) { {size: "100x100^"} } + + it "uses resize_to_fit" do + is_expected.to eq( + { + resize_to_fit: [100, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + end + + context "Size option contains trailing #" do + let(:options) { {size: "100x100#"} } + + it "uses resize_to_fill with a center gravity" do + is_expected.to eq( + { + resize_to_fill: [100, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + end + + context "Size option contains no operator" do + let(:options) { {size: "100x100"} } + + it "uses resize_to_limit" do + is_expected.to eq( + { + resize_to_limit: [100, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + + context "but options[:crop] is true" do + let(:options) { {size: "100x100", crop: true} } + + it "uses resize_to_fill with a center gravity" do + is_expected.to eq( + { + resize_to_fill: [100, 100, {sharpen: false}], + saver: {quality: 85} + } + ) + end + end + end + + context "if sharpen is enabled" do + let(:options) { {size: "100x100", sharpen: true} } + + it "enables sharpen" do + is_expected.to eq( + { + resize_to_limit: [100, 100, {}], + saver: {quality: 85} + } + ) + end + end + + context "Size option is nil" do + let(:options) { {} } + + it "just contains default quality option" do + is_expected.to eq({saver: {quality: 85}}) + end + + context "if quality is given" do + let(:options) { {quality: 15} } + + it "contains given quality option" do + is_expected.to eq({saver: {quality: 15}}) + end + end + end + + context "Format option is given" do + let(:options) { {format: "webp"} } + + it "contains the format option" do + is_expected.to eq( + { + format: "webp", + saver: {quality: 85} + } + ) + end + end + + context "Format option is not given" do + let(:options) { {} } + + context "and the image output format is configured to be original" do + before do + stub_alchemy_config(:image_output_format, "original") + end + + it "does not contain the format option" do + is_expected.to eq( + { + saver: {quality: 85} + } + ) + end + end + + context "and the image output format is configured to webp" do + before do + stub_alchemy_config(:image_output_format, "webp") + end + + it "does not contain the format option" do + is_expected.to eq( + { + saver: {quality: 85}, + format: "webp" + } + ) + end + end + end + end +end From eafe65fbd18f9e3071db040fcca0a8c2cd0b6f23 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:32 +0200 Subject: [PATCH 04/27] Use vips as image processor in dummy app --- Gemfile | 2 ++ spec/dummy/config/application.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index d7ead989b0..a3d4aab67d 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,8 @@ gem "pg", "~> 1.0" if ENV["DB"] == "postgresql" gem "alchemy_i18n", github: "AlchemyCMS/alchemy_i18n", branch: "download-flatpickr-locales" +gem "ruby-vips" + group :development, :test do gem "execjs", "~> 2.9.1" gem "rubocop", require: false diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb index cc2040f3f0..69578724d8 100644 --- a/spec/dummy/config/application.rb +++ b/spec/dummy/config/application.rb @@ -30,5 +30,7 @@ class Application < Rails::Application # Application configuration can go into files in config/initializers # -- all .rb files in that directory are automatically loaded after loading # the framework and any gems in your application. + # config.active_storage.variant_processor = :mini_magick + config.active_storage.variant_processor = :vips end end From 42c9250ad170d771c25dd0f74066db641c01a115 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:33 +0200 Subject: [PATCH 05/27] [ci] install libvips --- .github/workflows/build_test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 0c5c5605d6..c870971c58 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -130,6 +130,10 @@ jobs: sudo apt update -qq sudo apt install -qq --fix-missing libmysqlclient-dev -o dir::cache::archives="/home/runner/apt/cache" sudo chown -R runner /home/runner/apt/cache + - name: Install libvips + env: + DEBIAN_FRONTEND: noninteractive + run: sudo apt install --fix-missing libvips -o dir::cache::archives="/home/runner/apt/cache" - uses: actions/download-artifact@v4 if: needs.check_bun_lock.outputs.bun_lock_changed == 'true' with: From 5ad0360bf7f4cc3b064759821f84044200eecc22 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:34 +0200 Subject: [PATCH 06/27] Ignore active storage files in dummy app --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c3f71e6055..c6d1a67404 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,4 @@ yarn-debug.log* /spec/dummy/public/pictures .byebug_history .vscode/ +/spec/dummy/storage From 00dee771b7fe61e35e90d1680b5636ffb6b047ff Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:35 +0200 Subject: [PATCH 07/27] Use custom validations for picture size and format Instead of using the Dragonfly Model validations we write our own. That way we can use another image attachment library more easily --- app/models/alchemy/picture.rb | 33 ++++++++++++++++++++------------- config/locales/alchemy.en.yml | 1 + 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 11b6185904..25cc53fa18 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -97,20 +97,9 @@ def self.preprocessor_class=(klass) # Create important thumbnails upfront after_create -> { PictureThumb.generate_thumbs!(self) if has_convertible_format? } - # We need to define this method here to have it available in the validations below. - class << self - def allowed_filetypes - Config.get(:uploader).fetch("allowed_filetypes", {}).fetch("alchemy/pictures", []) - end - end - validates_presence_of :image_file - validates_size_of :image_file, maximum: Config.get(:uploader)["file_size_limit"].megabytes - validates_property :format, - of: :image_file, - in: allowed_filetypes, - case_sensitive: false, - message: Alchemy.t("not a valid image") + validate :image_file_type_allowed, :image_file_not_too_big, + if: -> { image_file.present? } stampable stamper_class_name: Alchemy.user_class.name @@ -310,5 +299,23 @@ def deletable? def image_file_dimensions "#{image_file_width}x#{image_file_height}" end + + private + + def image_file_type_allowed + allowed_filetypes = Config + .get(:uploader) + .dig("allowed_filetypes", "alchemy/pictures") || [] + unless MiniMime.lookup_by_content_type(image_file.content_type)&.extension&.in? allowed_filetypes + errors.add(:image_file, Alchemy.t("not a valid image")) + end + end + + def image_file_not_too_big + maximum = Config.get(:uploader)["file_size_limit"].megabytes + if image_file.byte_size > maximum + errors.add(:image_file, :too_big) + end + end end end diff --git a/config/locales/alchemy.en.yml b/config/locales/alchemy.en.yml index 6904fe2f06..de5f81dcd6 100644 --- a/config/locales/alchemy.en.yml +++ b/config/locales/alchemy.en.yml @@ -719,6 +719,7 @@ en: attributes: image_file: blank: "Please attach a picture." + too_big: must not be larger than %{maximum}MB alchemy/user: attributes: email: From add712326b5c62ee6c248de74402c243675cbb6e Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:37 +0200 Subject: [PATCH 08/27] Use activestorage for picture file handling --- app/models/alchemy/picture.rb | 42 +++---- app/models/alchemy/picture/url.rb | 54 ++++---- .../concerns/alchemy/picture_thumbnails.rb | 2 +- .../test_support/factories/picture_factory.rb | 17 ++- .../having_picture_thumbnails_examples.rb | 40 +++--- .../alchemy/ingredients/picture_view_spec.rb | 13 +- .../alchemy/admin/tags_controller_spec.rb | 3 +- spec/models/alchemy/picture/url_spec.rb | 46 +++---- spec/models/alchemy/picture_spec.rb | 118 +++--------------- .../alchemy/admin/pictures_controller_spec.rb | 2 +- 10 files changed, 124 insertions(+), 213 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 25cc53fa18..99f2c4f4aa 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -84,18 +84,8 @@ def self.preprocessor_class=(klass) @_preprocessor_class = klass end - # Enables Dragonfly image processing - dragonfly_accessor :image_file, app: :alchemy_pictures do - # Preprocess after uploading the picture - after_assign do |image| - if has_convertible_format? - self.class.preprocessor_class.new(image).call - end - end - end - - # Create important thumbnails upfront - after_create -> { PictureThumb.generate_thumbs!(self) if has_convertible_format? } + # Use ActiveStorage image processing + has_one_attached :image_file, service: :alchemy_cms validates_presence_of :image_file validate :image_file_type_allowed, :image_file_not_too_big, @@ -130,7 +120,10 @@ def url_class=(klass) end def alchemy_resource_filters - @_file_formats ||= distinct.pluck(:image_file_format).compact.presence || [] + @_file_formats ||= file_formats.map do |format| + MiniMime.lookup_by_content_type(format)&.extension + end + [ { name: :by_file_format, @@ -153,33 +146,30 @@ def last_upload Picture.where(upload_hash: last_picture.upload_hash) end + + private + + def file_formats + ActiveStorage::Blob.joins(:attachments).merge( + ActiveStorage::Attachment.where(record_type: name) + ).distinct.pluck(:content_type) + end end # Instance methods # Returns an url (or relative path) to a processed image for use inside an image_tag helper. # - # Any additional options are passed to the url method, so you can add params to your url. - # # Example: # # <%= image_tag picture.url(size: '320x200', format: 'png') %> # - # @see Alchemy::PictureVariant#call for transformation options - # @see Alchemy::Picture::Url#call for url options # @return [String|Nil] def url(options = {}) return unless image_file - variant = PictureVariant.new(self, options.slice(*TRANSFORMATION_OPTIONS)) - self.class.url_class.new(variant).call( - options.except(*TRANSFORMATION_OPTIONS).merge( - basename: name, - ext: variant.render_format, - name: name - ) - ) - rescue ::Dragonfly::Job::Fetch::NotFound => e + self.class.url_class.new(self).call(options) + rescue ::ActiveStorage::Error => e log_warning(e.message) nil end diff --git a/app/models/alchemy/picture/url.rb b/app/models/alchemy/picture/url.rb index a4e7cdc46b..1d1edf888d 100644 --- a/app/models/alchemy/picture/url.rb +++ b/app/models/alchemy/picture/url.rb @@ -3,50 +3,42 @@ module Alchemy class Picture < BaseRecord class Url - attr_reader :variant, :thumb + attr_reader :picture, :image_file - # @param [Alchemy::PictureVariant] + # @param [Alchemy::Picture] # - def initialize(variant) - raise ArgumentError, "Variant missing!" if variant.nil? - - @variant = variant + def initialize(picture) + @picture = picture + @image_file = picture.image_file end # The URL to a variant of a picture # # @return [String] # - def call(params = {}) - return variant.image.url(params) unless processible_image? - - "/#{uid}" + def call(options = {}) + variant_options = DragonflyToImageProcessing.call(options) + variant = image_file&.variant(variant_options) + return unless variant + + Rails.application.routes.url_helpers.rails_storage_proxy_url( + variant, + { + filename: filename(options), + only_path: true, + format: nil + } + ) end private - def processible_image? - variant.image.is_a?(::Dragonfly::Job) - end - - def uid - signature = PictureThumb::Signature.call(variant) - if find_thumb_by(signature) - thumb.uid - else - uid = PictureThumb::Uid.call(signature, variant) - ActiveRecord::Base.connected_to(role: ActiveRecord.writing_role) do - PictureThumb::Create.call(variant, signature, uid) - end - uid - end - end - - def find_thumb_by(signature) - @thumb = if variant.picture.thumbs.loaded? - variant.picture.thumbs.find { |t| t.signature == signature } + def filename(options = {}) + format = options[:format] || picture.image_file_extension + if picture.name.presence + "#{picture.name.to_param}.#{format}" else - variant.picture.thumbs.find_by(signature: signature) + picture.image_file_name end end end diff --git a/app/models/concerns/alchemy/picture_thumbnails.rb b/app/models/concerns/alchemy/picture_thumbnails.rb index aa4e907233..0de7a518c7 100644 --- a/app/models/concerns/alchemy/picture_thumbnails.rb +++ b/app/models/concerns/alchemy/picture_thumbnails.rb @@ -105,7 +105,7 @@ def allow_image_cropping? settings[:crop] && picture&.can_be_cropped_to?( settings[:size], settings[:upsample] - ) && !!picture.image_file + ) && !!picture.image_file.attached? end private diff --git a/lib/alchemy/test_support/factories/picture_factory.rb b/lib/alchemy/test_support/factories/picture_factory.rb index 08aeffdee7..63adcc7cc7 100644 --- a/lib/alchemy/test_support/factories/picture_factory.rb +++ b/lib/alchemy/test_support/factories/picture_factory.rb @@ -2,8 +2,21 @@ FactoryBot.define do factory :alchemy_picture, class: "Alchemy::Picture" do - image_file do - File.new(Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png")) + transient do + image_file do + Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png") + end + end + + after(:build) do |picture, acc| + if acc.image_file + picture.image_file.attach( + io: File.open(acc.image_file), + filename: File.basename(acc.image_file), + content_type: MiniMime.lookup_by_extension(File.extname(acc.image_file).remove("."))&.content_type || "application/octet-stream", + identify: false + ) + end end name { "image" } upload_hash { Time.current.hash } diff --git a/lib/alchemy/test_support/having_picture_thumbnails_examples.rb b/lib/alchemy/test_support/having_picture_thumbnails_examples.rb index af9550af68..b23407f7b5 100644 --- a/lib/alchemy/test_support/having_picture_thumbnails_examples.rb +++ b/lib/alchemy/test_support/having_picture_thumbnails_examples.rb @@ -78,20 +78,6 @@ end end - context "with other options" do - let(:options) { {foo: "baz"} } - - context "and the image does not need to be processed" do - before do - allow(record).to receive(:settings) { {} } - end - - it "adds them to the url" do - expect(picture_url).to match(/\?foo=baz/) - end - end - end - context "without picture assigned" do let(:picture) { nil } @@ -432,8 +418,8 @@ let(:settings) { {} } before do - picture.image_file_width = 300 - picture.image_file_height = 250 + allow(picture).to receive(:image_file_width) { 300 } + allow(picture).to receive(:image_file_height) { 250 } allow(record).to receive(:settings) { settings } end @@ -555,8 +541,8 @@ let(:settings) { {crop: true, size: size} } before do - picture.image_file_width = 200 - picture.image_file_height = 100 + allow(picture).to receive(:image_file_width) { 200 } + allow(picture).to receive(:image_file_height) { 100 } end context "size 200x50" do @@ -619,8 +605,13 @@ end describe "#allow_image_cropping?" do - let(:picture) do - stub_model(Alchemy::Picture, image_file_width: 400, image_file_height: 300) + let(:picture) { Alchemy::Picture.new } + let(:image_file_width) { 400 } + let(:image_file_height) { 300 } + + before do + allow(picture).to receive(:image_file_width) { image_file_width } + allow(picture).to receive(:image_file_height) { image_file_height } end subject { record.allow_image_cropping? } @@ -639,6 +630,9 @@ allow(picture).to receive(:can_be_cropped_to?) { true } end + let(:image_file_width) { 1201 } + let(:image_file_height) { 481 } + it { is_expected.to be_falsy } context "with crop set to true" do @@ -648,14 +642,16 @@ context "if picture.image_file is nil" do before do - expect(picture).to receive(:image_file) { nil } + expect(picture.image_file).to receive(:attached?) { false } end it { is_expected.to be_falsy } end context "if picture.image_file is present" do - let(:picture) { build_stubbed(:alchemy_picture) } + before do + expect(picture.image_file).to receive(:attached?) { true } + end it { is_expected.to be(true) } end diff --git a/spec/components/alchemy/ingredients/picture_view_spec.rb b/spec/components/alchemy/ingredients/picture_view_spec.rb index 22d3c901a8..e9f7cbffb6 100644 --- a/spec/components/alchemy/ingredients/picture_view_spec.rb +++ b/spec/components/alchemy/ingredients/picture_view_spec.rb @@ -8,9 +8,7 @@ end let(:picture) do - stub_model Alchemy::Picture, - image_file_format: "png", - image_file: image + build_stubbed(:alchemy_picture, image_file: image) end let(:ingredient) do @@ -320,8 +318,7 @@ context "and not passed as html option" do context "with name on the picture" do let(:picture) do - stub_model Alchemy::Picture, - image_file_format: "png", + build_stubbed :alchemy_picture, image_file: image, name: "cute_kitty-cat" end @@ -332,6 +329,12 @@ end context "and no name on the picture" do + let(:picture) do + build_stubbed :alchemy_picture, + image_file: image, + name: nil + end + it "has no alt text" do expect(page).to_not have_selector("img[alt]") end diff --git a/spec/controllers/alchemy/admin/tags_controller_spec.rb b/spec/controllers/alchemy/admin/tags_controller_spec.rb index 4fd7ebc25a..9974e80557 100644 --- a/spec/controllers/alchemy/admin/tags_controller_spec.rb +++ b/spec/controllers/alchemy/admin/tags_controller_spec.rb @@ -21,8 +21,7 @@ module Admin context "with taggable missing" do before do - picture.thumbs.destroy_all - picture.delete + picture.destroy end it "does not raise error" do diff --git a/spec/models/alchemy/picture/url_spec.rb b/spec/models/alchemy/picture/url_spec.rb index 3ba99e4a1d..e06504af7f 100644 --- a/spec/models/alchemy/picture/url_spec.rb +++ b/spec/models/alchemy/picture/url_spec.rb @@ -5,40 +5,42 @@ RSpec.describe Alchemy::Picture::Url do let(:image) { File.new(File.expand_path("../../../fixtures/image.png", __dir__)) } let(:picture) { create(:alchemy_picture, image_file: image) } - let(:variant) { Alchemy::PictureVariant.new(picture) } - subject { described_class.new(variant).call(params) } + subject { described_class.new(picture).call(options) } - let(:params) { {} } + let(:options) { {} } - it "returns the url to the image" do - is_expected.to match(/\/pictures\/.+\/image\.png\?sha=.+/) + it "returns the proxy url to the image" do + is_expected.to match(/\/rails\/active_storage\/representations\/proxy\/.+\/image\.png/) end - context "when params are passed" do - let(:params) do - { - page: 1, - per_page: 10 - } + it "adds image name and format to url" do + is_expected.to match(/\/image\.png$/) + end + + context "with a processed variant" do + let(:options) do + {size: "10x10"} end - it "passes them to the URL" do - is_expected.to match(/page=1/) + it "uses converted options for image_processing" do + expect(picture.image_file).to receive(:variant).with( + { + resize_to_limit: [10, 10, {sharpen: false}], + saver: {quality: 85} + } + ) + subject end end - context "with a processed variant" do - let(:variant) { Alchemy::PictureVariant.new(picture, {size: "10x10"}) } - - it "returns the url to the thumbnail" do - is_expected.to match(/\/pictures\/\d+\/.+\/image\.png/) + context "with format in options" do + let(:options) do + {format: "webp"} end - it "connects to writing database" do - writing_role = ActiveRecord.writing_role - expect(ActiveRecord::Base).to receive(:connected_to).with(role: writing_role) - subject + it "adds format to url" do + is_expected.to match(/\/image\.webp$/) end end end diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 7804699299..6d70332eaf 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -5,105 +5,34 @@ module Alchemy describe Picture do let :image_file do - File.new(File.expand_path("../../fixtures/image.png", __dir__)) + Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png") end - let(:picture) { Picture.new } + let(:picture) { build(:alchemy_picture, image_file: image_file) } it_behaves_like "has image calculations" - it { is_expected.to have_many(:thumbs).class_name("Alchemy::PictureThumb") } - - context "with a png file" do - it "generates thumbnails after create" do - expect { - create(:alchemy_picture) - }.to change { Alchemy::PictureThumb.count }.by(3) - end - end - - context "with a svg file" do - let :image_file do - File.new(File.expand_path("../../fixtures/icon.svg", __dir__)) - end - - it "does not generate any thumbnails" do - expect { - create(:alchemy_picture, image_file: image_file) - }.to_not change { Alchemy::PictureThumb.count } - end - end - - context "with a webp file" do - let :image_file do - File.new(File.expand_path("../../fixtures/image5.webp", __dir__)) - end - - it "generates thumbnails after create" do - expect { - create(:alchemy_picture) - }.to change { Alchemy::PictureThumb.count }.by(3) - end - end - it "is valid with valid attributes" do - picture = Picture.new(image_file: image_file) expect(picture).to be_valid end it "is not valid without image file" do - picture = Picture.new + picture = build(:alchemy_picture, image_file: nil) expect(picture).not_to be_valid end it "is valid with capitalized image file extension" do - image_file = File.new(File.expand_path("../../fixtures/image2.PNG", __dir__)) - picture = Picture.new(image_file: image_file) + image_file = File.open(File.expand_path("../../fixtures/image2.PNG", __dir__)) + picture = build(:alchemy_picture, image_file: image_file) expect(picture).to be_valid end it "is valid with jpeg image file extension" do - image_file = File.new(File.expand_path("../../fixtures/image3.jpeg", __dir__)) - picture = Picture.new(image_file: image_file) + image_file = File.open(File.expand_path("../../fixtures/image3.jpeg", __dir__)) + picture = build(:alchemy_picture, image_file: image_file) expect(picture).to be_valid end - context "with enabled preprocess_image_resize config option" do - let(:image_file) do - File.new(File.expand_path("../../fixtures/80x60.png", __dir__)) - end - - context "with > geometry string" do - before do - allow(Config).to receive(:get) do |arg| - if arg == :preprocess_image_resize - "10x10>" - end - end - end - - it "it resizes the image after upload" do - picture = Picture.new(image_file: image_file) - expect(picture.image_file.data[0x10..0x18].unpack("NN")).to eq([10, 8]) - end - end - - context "without > geometry string" do - before do - allow(Config).to receive(:get) do |arg| - if arg == :preprocess_image_resize - "10x10" - end - end - end - - it "it resizes the image after upload" do - picture = Picture.new(image_file: image_file) - expect(picture.image_file.data[0x10..0x18].unpack("NN")).to eq([10, 8]) - end - end - end - describe "#suffix" do it "should return the suffix of original filename" do pic = stub_model(Picture, image_file_name: "kitten.JPG") @@ -164,9 +93,9 @@ module Alchemy describe ".last_upload" do it "should return all pictures that have the same upload-hash as the most recent picture" do - other_upload = Picture.create!(image_file: image_file, upload_hash: "456") - same_upload = Picture.create!(image_file: image_file, upload_hash: "123") - most_recent = Picture.create!(image_file: image_file, upload_hash: "123") + other_upload = create(:alchemy_picture, image_file: image_file, upload_hash: "456") + same_upload = create(:alchemy_picture, image_file: image_file, upload_hash: "123") + most_recent = create(:alchemy_picture, image_file: image_file, upload_hash: "123") expect(Picture.last_upload).to include(most_recent) expect(Picture.last_upload).to include(same_upload) @@ -179,8 +108,8 @@ module Alchemy describe ".recent" do before do now = Time.current - @recent = Picture.create!(image_file: image_file) - @old_picture = Picture.create!(image_file: image_file) + @recent = create(:alchemy_picture, image_file: image_file) + @old_picture = create(:alchemy_picture, image_file: image_file) @recent.update_column(:created_at, now - 23.hours) @old_picture.update_column(:created_at, now - 10.days) end @@ -213,7 +142,7 @@ module Alchemy end describe "#update_name_and_tag_list!" do - let(:picture) { Picture.new(image_file: image_file) } + let(:picture) { build(:alchemy_picture, image_file: image_file) } before { allow(picture).to receive(:save!).and_return(true) } @@ -248,13 +177,13 @@ module Alchemy end let(:picture) do - create(:alchemy_picture, image_file: image) + create(:alchemy_picture, name: "square", image_file: image) end let(:options) { {} } it "includes the name and render format" do - expect(url).to match(/\/#{picture.name}\.#{picture.default_render_format}/) + expect(url).to match(/\/square\.png/) end context "when no image is present" do @@ -270,7 +199,7 @@ module Alchemy context "when the image can not be fetched" do before do expect_any_instance_of(described_class.url_class).to receive(:call) do - raise(::Dragonfly::Job::Fetch::NotFound) + raise(::ActiveStorage::FileNotFoundError) end end @@ -291,20 +220,7 @@ module Alchemy end it "returns the url to the thumbnail" do - is_expected.to match(/\/pictures\/\d+\/.+\/500x500\.png/) - end - end - - context "that are params" do - let(:options) do - { - page: 1, - per_page: 10 - } - end - - it "passes them to the URL" do - expect(url).to match(/page=1/) + is_expected.to match(/\/rails\/active_storage\/representations\/proxy\/.+\/square\.png/) end end end diff --git a/spec/requests/alchemy/admin/pictures_controller_spec.rb b/spec/requests/alchemy/admin/pictures_controller_spec.rb index 663011eebe..7849ada9c2 100644 --- a/spec/requests/alchemy/admin/pictures_controller_spec.rb +++ b/spec/requests/alchemy/admin/pictures_controller_spec.rb @@ -23,7 +23,7 @@ get alchemy.url_admin_picture_path(picture) json = JSON.parse(response.body) expect(json).to match({ - "url" => /\/pictures\/.+\/image\.png/, + "url" => /\/rails\/active_storage\/representations\/proxy\/.+\/image\.png/, "alt" => picture.name, "title" => Alchemy.t(:image_name, name: picture.name) }) From 007da401c46d0b4d2616fcd889dec0b1c8cee275 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:38 +0200 Subject: [PATCH 09/27] Do not sharpen images by default The vips image_processing processor sharpens images by default. This disables this behavior. --- config/alchemy/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/alchemy/config.yml b/config/alchemy/config.yml index c810a2b6cb..ac64cef978 100644 --- a/config/alchemy/config.yml +++ b/config/alchemy/config.yml @@ -87,6 +87,7 @@ items_per_page: 15 output_image_quality: 85 preprocess_image_resize: image_output_format: original +sharpen_images: false # This is used by the seeder to create the default site. default_site: From 46865f9646477beb32a036633c80ec86abdc708f Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:39 +0200 Subject: [PATCH 10/27] Add image_file_extension method --- app/models/alchemy/picture.rb | 17 ++--- spec/models/alchemy/picture_spec.rb | 106 +++++++++++++++------------- 2 files changed, 65 insertions(+), 58 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 99f2c4f4aa..b15a75d590 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -218,18 +218,12 @@ def urlname end end - # Returns the suffix of the filename. - # - def suffix - image_file.ext - end - # Returns a humanized, readable name from image filename. # def humanized_name return "" if image_file_name.blank? - convert_to_humanized_name(image_file_name, suffix) + convert_to_humanized_name(image_file_name, image_file_extension) end # Returns the format the image should be rendered with @@ -241,7 +235,7 @@ def default_render_format if convertible? Config.get(:image_output_format) else - image_file_format + image_file_extension end end @@ -280,6 +274,13 @@ def deletable? picture_ingredients.empty? end + def image_file_extension + image_file&.filename&.extension + end + + alias_method :suffix, :image_file_extension + deprecate suffix: :image_file_extension, deprecator: Alchemy::Deprecation + # A size String from original image file values. # # == Example diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 6d70332eaf..1063ec201e 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -35,39 +35,31 @@ module Alchemy describe "#suffix" do it "should return the suffix of original filename" do - pic = stub_model(Picture, image_file_name: "kitten.JPG") - allow(pic).to receive(:image_file).and_return(OpenStruct.new({ext: "jpg"})) - expect(pic.suffix).to eq("jpg") - end - - context "image has no suffix" do - it "should return empty string" do - pic = stub_model(Picture, image_file_name: "kitten") - allow(pic).to receive(:image_file).and_return(OpenStruct.new({ext: ""})) - expect(pic.suffix).to eq("") + Alchemy::Deprecation.silenced do + pic = build(:alchemy_picture) + expect(pic.suffix).to eq("png") end end end describe "#humanized_name" do it "should return a humanized version of original filename" do - pic = stub_model(Picture, image_file_name: "cute_kitten.JPG") - allow(pic).to receive(:image_file).and_return(OpenStruct.new({ext: "jpg"})) - expect(pic.humanized_name).to eq("cute kitten") + allow(picture).to receive(:image_file_name).and_return("cute_kitten.JPG") + allow(picture).to receive(:image_file_extension).and_return("jpg") + expect(picture.humanized_name).to eq("cute kitten") end it "should not remove incidents of suffix from filename" do - pic = stub_model(Picture, image_file_name: "cute_kitten_mo.jpgi.JPG") - allow(pic).to receive(:image_file).and_return(OpenStruct.new({ext: "jpg"})) - expect(pic.humanized_name).to eq("cute kitten mo.jpgi") - expect(pic.humanized_name).not_to eq("cute kitten moi") + allow(picture).to receive(:image_file_name).and_return("cute_kitten_mo.jpgi.JPG") + allow(picture).to receive(:image_file_extension).and_return("jpg") + expect(picture.humanized_name).to eq("cute kitten mo.jpgi") end context "image has no suffix" do it "should return humanized name" do - pic = stub_model(Picture, image_file_name: "cute_kitten") - allow(pic).to receive(:suffix).and_return("") - expect(pic.humanized_name).to eq("cute kitten") + allow(picture).to receive(:image_file_name).and_return("cute_kitten") + allow(picture).to receive(:image_file_extension).and_return("") + expect(picture.humanized_name).to eq("cute kitten") end end end @@ -355,67 +347,81 @@ module Alchemy end describe "#default_render_format" do - let(:picture) do - Picture.new(image_file_format: "png") - end + let(:picture) { build(:alchemy_picture) } subject { picture.default_render_format } - context "when `image_output_format` is configured to `original`" do + context "when image is convertible" do before do - stub_alchemy_config(:image_output_format, "original") + expect(picture).to receive(:convertible?) { true } + stub_alchemy_config(:image_output_format, "jpg") end - it "returns the image file format" do - is_expected.to eq("png") + it "returns the configured image output format" do + is_expected.to eq("jpg") end end - context "when `image_output_format` is configured to jpg" do + context "when image is not convertible" do before do - stub_alchemy_config(:image_output_format, "jpg") + expect(picture).to receive(:convertible?) { false } + stub_alchemy_config(:image_output_format, "original") end - context "and the format is a convertible format" do - it "returns the configured file format." do - is_expected.to eq("jpg") - end + it "returns the original file format." do + is_expected.to eq("png") end + end + end - context "but the format is not a convertible format" do - before do - allow(picture).to receive(:image_file_format) { "svg" } - end + describe "#convertible?" do + let(:picture) do + Picture.new(image_file_format: "image/png") + end - it "returns the original file format." do - is_expected.to eq("svg") - end + subject { picture.convertible? } + + context "when `image_output_format` is configured to `original`" do + before do + stub_alchemy_config(:image_output_format, "original") end + + it { is_expected.to be(false) } end - context "when `image_output_format` is configured to webp" do + context "when `image_output_format` is configured to jpg" do before do - stub_alchemy_config(:image_output_format, "webp") + stub_alchemy_config(:image_output_format, "jpg") end - context "and the format is a convertible format" do - it "returns the configured file format." do - is_expected.to eq("webp") + context "and the image has a convertible format" do + before do + expect(picture).to receive(:has_convertible_format?) { true } end + + it { is_expected.to be(true) } end - context "but the format is not a convertible format" do + context "but the image has no convertible format" do before do - allow(picture).to receive(:image_file_format) { "svg" } + expect(picture).to receive(:has_convertible_format?) { false } end - it "returns the original file format." do - is_expected.to eq("svg") - end + it { is_expected.to be(false) } end end end + describe "#image_file_extension" do + let(:picture) { build(:alchemy_picture) } + + subject { picture.image_file_extension } + + it "returns file extension by file format" do + is_expected.to eq("png") + end + end + describe "after update" do context "assigned to ingredient" do let(:picture) { create(:alchemy_picture) } From 20ed1bdfe51fc59d04b7282034636bebe3540eff Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:40 +0200 Subject: [PATCH 11/27] Delegate convertible format check to activestorage Active storage already knows if a file is convertible --- app/models/alchemy/picture.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index b15a75d590..985262bf43 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -28,8 +28,6 @@ class Picture < BaseRecord large: "240x180" }.with_indifferent_access.freeze - CONVERTIBLE_FILE_FORMATS = %w[gif jpg jpeg png webp].freeze - TRANSFORMATION_OPTIONS = [ :crop, :crop_from, @@ -253,7 +251,7 @@ def convertible? # Returns true if the image can be converted into other formats # def has_convertible_format? - image_file_format.in?(CONVERTIBLE_FILE_FORMATS) + image_file&.variable? end # Checks if the picture is restricted. From 91aac5acb980dcb681ee75d4bb0a32526d357e1d Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:41 +0200 Subject: [PATCH 12/27] Eager load attachments in admin pictures controller This avoids N+1 in the image library --- app/controllers/alchemy/admin/pictures_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/alchemy/admin/pictures_controller.rb b/app/controllers/alchemy/admin/pictures_controller.rb index 1d94097d7f..4301e4077c 100644 --- a/app/controllers/alchemy/admin/pictures_controller.rb +++ b/app/controllers/alchemy/admin/pictures_controller.rb @@ -22,7 +22,7 @@ class PicturesController < Alchemy::Admin::ResourcesController def index @query = Picture.ransack(search_filter_params[:q]) - @pictures = filtered_pictures.includes(:thumbs) + @pictures = filtered_pictures.with_attached_image_file if in_overlay? archive_overlay From 2197a1646702f4a5f98ff9987c620e6e76553cec Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:43 +0200 Subject: [PATCH 13/27] Delegate image_file_* methods to attached file Using activestorage methods to read the values. Ideally we would cache these values, but active storage asynchronously sets these values and provides no callback we could use. --- app/models/alchemy/picture.rb | 20 +++++++++++++++++++ .../having_crop_action_examples.rb | 4 ++-- spec/models/alchemy/picture_spec.rb | 7 ++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 985262bf43..d0eb21e8ba 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -272,6 +272,26 @@ def deletable? picture_ingredients.empty? end + def image_file_name + image_file&.filename&.to_s + end + + def image_file_format + image_file&.content_type + end + + def image_file_size + image_file&.byte_size + end + + def image_file_width + image_file&.metadata&.fetch(:width, nil) + end + + def image_file_height + image_file&.metadata&.fetch(:height, nil) + end + def image_file_extension image_file&.filename&.extension end diff --git a/lib/alchemy/test_support/having_crop_action_examples.rb b/lib/alchemy/test_support/having_crop_action_examples.rb index 8bba272c7b..9e4afc4540 100644 --- a/lib/alchemy/test_support/having_crop_action_examples.rb +++ b/lib/alchemy/test_support/having_crop_action_examples.rb @@ -30,8 +30,8 @@ let(:settings) { {} } before do - picture.image_file_width = 300 - picture.image_file_height = 250 + allow(picture).to receive(:image_file_width) { 300 } + allow(picture).to receive(:image_file_height) { 250 } allow(croppable_resource).to receive(:settings) { settings } expect(Alchemy::Picture).to receive(:find_by) { picture } end diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 1063ec201e..71718a7ec1 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -127,8 +127,13 @@ module Alchemy end describe "#image_file_dimensions" do + before do + expect(picture.image_file).to receive(:metadata).twice do + {width: 1, height: 1} + end + end + it "should return the width and height in the format of '1024x768'" do - picture.image_file = image_file expect(picture.image_file_dimensions).to eq("1x1") end end From 72808eb6515c04ddfe8ce0bea65d8dbbcd2118c8 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:45 +0200 Subject: [PATCH 14/27] Remove custom picture variant classes Now that we use ActiveStorage we can remove our custom picture variant handling --- .../alchemy/ingredients/picture_view.rb | 4 +- app/models/alchemy/picture.rb | 1 - app/models/alchemy/picture/transformations.rb | 115 ----- app/models/alchemy/picture_thumb.rb | 57 --- app/models/alchemy/picture_thumb/create.rb | 39 -- .../alchemy/picture_thumb/file_store.rb | 33 -- app/models/alchemy/picture_thumb/signature.rb | 23 - app/models/alchemy/picture_thumb/uid.rb | 22 - app/models/alchemy/picture_variant.rb | 124 ------ .../20230121212637_alchemy_six_point_one.rb | 10 - lib/alchemy/dragonfly/processors/thumbnail.rb | 27 -- .../factories/picture_thumb_factory.rb | 9 - lib/tasks/alchemy/thumbnails.rake | 53 --- .../dragonfly/processors/thumbnail_spec.rb | 46 -- .../alchemy/picture_thumb/create_spec.rb | 74 ---- .../alchemy/picture_thumb/file_store_spec.rb | 27 -- spec/models/alchemy/picture_thumb/uid_spec.rb | 39 -- spec/models/alchemy/picture_thumb_spec.rb | 9 - spec/models/alchemy/picture_variant_spec.rb | 418 ------------------ 19 files changed, 2 insertions(+), 1128 deletions(-) delete mode 100644 app/models/alchemy/picture/transformations.rb delete mode 100644 app/models/alchemy/picture_thumb.rb delete mode 100644 app/models/alchemy/picture_thumb/create.rb delete mode 100644 app/models/alchemy/picture_thumb/file_store.rb delete mode 100644 app/models/alchemy/picture_thumb/signature.rb delete mode 100644 app/models/alchemy/picture_thumb/uid.rb delete mode 100644 app/models/alchemy/picture_variant.rb delete mode 100644 lib/alchemy/dragonfly/processors/thumbnail.rb delete mode 100644 lib/alchemy/test_support/factories/picture_thumb_factory.rb delete mode 100644 lib/tasks/alchemy/thumbnails.rake delete mode 100644 spec/libraries/dragonfly/processors/thumbnail_spec.rb delete mode 100644 spec/models/alchemy/picture_thumb/create_spec.rb delete mode 100644 spec/models/alchemy/picture_thumb/file_store_spec.rb delete mode 100644 spec/models/alchemy/picture_thumb/uid_spec.rb delete mode 100644 spec/models/alchemy/picture_thumb_spec.rb delete mode 100644 spec/models/alchemy/picture_variant_spec.rb diff --git a/app/components/alchemy/ingredients/picture_view.rb b/app/components/alchemy/ingredients/picture_view.rb index d32ab0ab99..c97427cdaa 100644 --- a/app/components/alchemy/ingredients/picture_view.rb +++ b/app/components/alchemy/ingredients/picture_view.rb @@ -18,9 +18,9 @@ class PictureView < BaseView # @param disable_link [Boolean] (false) Whether to disable the link even if the picture has a link. # @param srcset [Array] An array of srcset sizes that will generate variants of the picture. # @param sizes [Array] An array of sizes that will be passed to the img tag. - # @param picture_options [Hash] Options that will be passed to the picture url. See {Alchemy::PictureVariant} for options. + # @param picture_options [Hash] Options that will be passed to the picture url. See {Alchemy::Picture#url} for options. # @param html_options [Hash] Options that will be passed to the img tag. - # @see Alchemy::PictureVariant + # @see Alchemy::Picture#url def initialize( ingredient, show_caption: nil, diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index d0eb21e8ba..4251226fdf 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -52,7 +52,6 @@ class Picture < BaseRecord has_many :elements, through: :picture_ingredients has_many :pages, through: :elements - has_many :thumbs, class_name: "Alchemy::PictureThumb", dependent: :destroy has_many :descriptions, class_name: "Alchemy::PictureDescription", dependent: :destroy accepts_nested_attributes_for :descriptions, allow_destroy: true, reject_if: ->(attr) { attr[:text].blank? } diff --git a/app/models/alchemy/picture/transformations.rb b/app/models/alchemy/picture/transformations.rb deleted file mode 100644 index 7779e8acae..0000000000 --- a/app/models/alchemy/picture/transformations.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - # This concern can extend classes that expose image_file, image_file_width and image_file_height. - # It provides methods for cropping and resizing. - # - module Picture::Transformations - extend ActiveSupport::Concern - - included do - include Alchemy::Picture::Calculations - end - - # Returns the rendered cropped image. Tries to use the crop_from and crop_size - # parameters. When they can't be parsed, it just crops from the center. - # - def crop(size, crop_from = nil, crop_size = nil, upsample = false) - raise "No size given!" if size.empty? - - render_to = inferred_sizes_from_string(size) - if crop_from && crop_size - top_left = point_from_string(crop_from) - crop_dimensions = inferred_sizes_from_string(crop_size) - xy_crop_resize(render_to, top_left, crop_dimensions, upsample) - else - center_crop(render_to, upsample) - end - end - - # Returns the rendered resized image using imagemagick directly. - # - def resize(size, upsample = false) - image_file.thumbnail(upsample ? size : "#{size}>") - end - - # Returns true if the class we're included in has a meaningful render_size attribute - # - def render_size? - respond_to?(:render_size) && render_size.present? - end - - # Returns true if the class we're included in has a meaningful crop_size attribute - # - def crop_size? - respond_to?(:crop_size) && !crop_size.nil? && !crop_size.empty? - end - - private - - # Given a string with an x, this function return a Hash with key :x and :y - # - def point_from_string(string = "0x0") - string = "0x0" if string.empty? - raise ArgumentError if !string.match(/(\d*x)|(x\d*)/) - - x, y = string.scan(/(\d*)x(\d*)/)[0].map(&:to_i) - - x = 0 if x.nil? - y = 0 if y.nil? - { - x: x, - y: y - } - end - - def inferred_sizes_from_string(string) - sizes = sizes_from_string(string) - ratio = image_file_width.to_f / image_file_height - - if sizes[:width].zero? - sizes[:width] = (sizes[:height] * ratio).round.to_i - end - if sizes[:height].zero? - sizes[:height] = (sizes[:width] / ratio).round.to_i - end - - sizes - end - - # Converts a dimensions hash to a string of from "20x20" - # - def dimensions_to_string(dimensions) - "#{dimensions[:width]}x#{dimensions[:height]}" - end - - # Uses imagemagick to make a centercropped thumbnail. Does not scale the image up. - # - def center_crop(dimensions, upsample) - if is_smaller_than?(dimensions) && upsample == false - dimensions = reduce_to_image(dimensions) - end - image_file.thumbnail("#{dimensions_to_string(dimensions)}#") - end - - # Use imagemagick to custom crop an image. Uses -thumbnail for better performance when resizing. - # - def xy_crop_resize(dimensions, top_left, crop_dimensions, upsample) - crop_argument = dimensions_to_string(crop_dimensions) - crop_argument += "+#{top_left[:x]}+#{top_left[:y]}" - - resize_argument = dimensions_to_string(dimensions) - resize_argument += ">" unless upsample - image_file.crop_resize(crop_argument, resize_argument) - end - - # Used when centercropping. - # - def reduce_to_image(dimensions) - { - width: [dimensions[:width].to_i, image_file_width.to_i].min, - height: [dimensions[:height].to_i, image_file_height.to_i].min - } - end - end -end diff --git a/app/models/alchemy/picture_thumb.rb b/app/models/alchemy/picture_thumb.rb deleted file mode 100644 index 5aead4616f..0000000000 --- a/app/models/alchemy/picture_thumb.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - # The persisted version of a rendered picture variant - # - # You can configure the generator class to implement a - # different thumbnail store (ie. a remote file storage). - # - # config/initializers/alchemy.rb - # Alchemy::PictureThumb.storage_class = My::ThumbnailStore - # - class PictureThumb < BaseRecord - belongs_to :picture, class_name: "Alchemy::Picture" - - validates :signature, presence: true - validates :uid, presence: true - - class << self - # Thumbnail storage class - # - # @see Alchemy::PictureThumb::FileStore - def storage_class - @_storage_class ||= Alchemy::PictureThumb::FileStore - end - - # Set a thumbnail storage class - # - # @see Alchemy::PictureThumb::FileStore - def storage_class=(klass) - @_storage_class = klass - end - - # Upfront generation of picture thumbnails - # - # Called after a Alchemy::Picture has been created (after an image has been uploaded) - # - # Generates three types of thumbnails that are used by Alchemys picture archive and - # persists them in the configures file store (Default Dragonfly::FileDataStore). - # - # @see Picture::THUMBNAIL_SIZES - def generate_thumbs!(picture) - Alchemy::Picture::THUMBNAIL_SIZES.values.each do |size| - variant = Alchemy::PictureVariant.new(picture, { - size: size, - flatten: true - }) - signature = Alchemy::PictureThumb::Signature.call(variant) - thumb = find_by(signature: signature) - next if thumb - - uid = Alchemy::PictureThumb::Uid.call(signature, variant) - Alchemy::PictureThumb::Create.call(variant, signature, uid) - end - end - end - end -end diff --git a/app/models/alchemy/picture_thumb/create.rb b/app/models/alchemy/picture_thumb/create.rb deleted file mode 100644 index f9408788b2..0000000000 --- a/app/models/alchemy/picture_thumb/create.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - class PictureThumb < BaseRecord - # Creates a Alchemy::PictureThumb - # - # Stores the processes result of a Alchemy::PictureVariant - # in the configured +Alchemy::PictureThumb.storage_class+ - # (Default: {Alchemy::PictureThumb::FileStore}) - # - class Create - class << self - # @param [Alchemy::PictureVariant] variant the to be rendered image - # @param [String] signature A unique hashed version of the rendering options - # @param [String] uid The Unique Image Identifier the image is stored at - # - # @return [Alchemy::PictureThumb] The persisted thumbnail record - # - def call(variant, signature, uid) - return if !variant.picture.valid? - - # create the thumb before storing - # to prevent db race conditions - @thumb = Alchemy::PictureThumb.create_or_find_by!(signature: signature) do |thumb| - thumb.picture = variant.picture - thumb.uid = uid - end - begin - Alchemy::PictureThumb.storage_class.call(variant, uid) - rescue => e - ErrorTracking.notification_handler.call(e) - # destroy the thumb if processing or storing fails - @thumb&.destroy - end - end - end - end - end -end diff --git a/app/models/alchemy/picture_thumb/file_store.rb b/app/models/alchemy/picture_thumb/file_store.rb deleted file mode 100644 index cfe4d313f1..0000000000 --- a/app/models/alchemy/picture_thumb/file_store.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - class PictureThumb < BaseRecord - # Stores the render result of a Alchemy::PictureVariant - # in the configured Dragonfly datastore - # (Default: Dragonfly::FileDataStore) - # - class FileStore - class << self - # @param [Alchemy::PictureVariant] variant the to be rendered image - # @param [String] uid The Unique Image Identifier the image is stored at - # - def call(variant, uid) - # process the image - image = variant.image - # store the processed image - image.to_file(server_path(uid)).close - end - - private - - # Alchemys dragonfly datastore config seperates the storage path from the public server - # path for security reasons. The Dragonfly FileDataStorage does not support that, - # so we need to build the path on our own. - def server_path(uid) - dragonfly_app = ::Dragonfly.app(:alchemy_pictures) - "#{dragonfly_app.datastore.server_root}/#{uid}" - end - end - end - end -end diff --git a/app/models/alchemy/picture_thumb/signature.rb b/app/models/alchemy/picture_thumb/signature.rb deleted file mode 100644 index 75ff6dde52..0000000000 --- a/app/models/alchemy/picture_thumb/signature.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - class PictureThumb < BaseRecord - class Signature - # Returns a unique image process signature - # - # @param [Alchemy::PictureVariant] - # - # @return [String] - def self.call(variant) - steps_without_fetch = variant.image.steps.reject do |step| - step.is_a?(::Dragonfly::Job::Fetch) - end - - steps_with_id = [[variant.picture.id]] + steps_without_fetch - job_string = steps_with_id.map(&:to_a).to_dragonfly_unique_s - - Digest::SHA1.hexdigest(job_string) - end - end - end -end diff --git a/app/models/alchemy/picture_thumb/uid.rb b/app/models/alchemy/picture_thumb/uid.rb deleted file mode 100644 index 6b7f48ffb4..0000000000 --- a/app/models/alchemy/picture_thumb/uid.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - class PictureThumb < BaseRecord - class Uid - # Returns a image variant uid for storage - # - # @param [String] - # @param [Alchemy::PictureVariant] - # - # @return [String] - def self.call(signature, variant) - picture = variant.picture - filename = variant.image_file_name || "image" - name = File.basename(filename, ".*").gsub(/[^\w.]+/, "_") - ext = variant.render_format - - "pictures/#{picture.id}/#{signature}/#{name}.#{ext}" - end - end - end -end diff --git a/app/models/alchemy/picture_variant.rb b/app/models/alchemy/picture_variant.rb deleted file mode 100644 index 8e2f291d75..0000000000 --- a/app/models/alchemy/picture_variant.rb +++ /dev/null @@ -1,124 +0,0 @@ -# frozen_string_literal: true - -require "forwardable" - -module Alchemy - # Represents a rendered picture - # - # Resizes, crops and encodes the image with imagemagick - # - class PictureVariant - extend Forwardable - - include Alchemy::Logger - include Alchemy::Picture::Transformations - - ANIMATED_IMAGE_FORMATS = %w[gif webp] - TRANSPARENT_IMAGE_FORMATS = %w[gif webp png] - ENCODABLE_IMAGE_FORMATS = %w[jpg jpeg webp] - - attr_reader :picture, :render_format - - def_delegators :@picture, - :image_file, - :image_file_width, - :image_file_height, - :image_file_name, - :image_file_size - - # @param [Alchemy::Picture] - # - # @param [Hash] options passed to the image processor - # @option options [Boolean] :crop Pass true to enable cropping - # @option options [String] :crop_from Coordinates to start cropping from - # @option options [String] :crop_size Size of the cropping area - # @option options [Boolean] :flatten Pass true to flatten GIFs - # @option options [String|Symbol] :format Image format to encode the image in - # @option options [Integer] :quality JPEG compress quality - # @option options [String] :size Size of resulting image in WxH - # @option options [Boolean] :upsample Pass true to upsample (grow) an image if the original size is lower than the resulting size - # - def initialize(picture, options = {}) - raise ArgumentError, "Picture missing!" if picture.nil? - - @picture = picture - @options = options - @render_format = (options[:format] || picture.default_render_format).to_s - end - - # Process a variant of picture - # - # @return [Dragonfly::Attachment|Dragonfly::Job] The processed image variant - # - def image - image = image_file - - raise MissingImageFileError, "Missing image file for #{picture.inspect}" if image.nil? - - image = processed_image(image, @options) - encoded_image(image, @options) - rescue MissingImageFileError, WrongImageFormatError => e - log_warning(e.message) - nil - end - - private - - # Returns the processed image dependent of size and cropping parameters - def processed_image(image, options = {}) - size = options[:size] - upsample = !!options[:upsample] - - return image unless size.present? && picture.has_convertible_format? - - if options[:crop] - crop(size, options[:crop_from], options[:crop_size], upsample) - else - resize(size, upsample) - end - end - - # Returns the encoded image - # - # Flatten animated gifs, only if converting to a different format. - # Can be overwritten via +options[:flatten]+. - # - def encoded_image(image, options = {}) - unless render_format.in?(Alchemy::Picture.allowed_filetypes) - raise WrongImageFormatError.new(picture, @render_format) - end - - options = { - flatten: !render_format.in?(ANIMATED_IMAGE_FORMATS) && picture.image_file_format == "gif" - }.with_indifferent_access.merge(options) - - encoding_options = [] - - convert_format = render_format.sub("jpeg", "jpg") != picture.image_file_format.sub("jpeg", "jpg") - - if encodable_image? && (convert_format || options[:quality]) - quality = options[:quality] || Config.get(:output_image_quality) - encoding_options << "-quality #{quality}" - end - - if options[:flatten] - if render_format.in?(TRANSPARENT_IMAGE_FORMATS) && picture.image_file_format.in?(TRANSPARENT_IMAGE_FORMATS) - encoding_options << "-background transparent" - end - encoding_options << "-flatten" - end - - convertion_needed = convert_format || encoding_options.present? - - if picture.has_convertible_format? && convertion_needed - image = image.encode(render_format, encoding_options.join(" ")) - end - - image - end - - def encodable_image? - render_format.in?(ENCODABLE_IMAGE_FORMATS) - end - end -end diff --git a/db/migrate/20230121212637_alchemy_six_point_one.rb b/db/migrate/20230121212637_alchemy_six_point_one.rb index f5aa627175..6ba3d46f26 100644 --- a/db/migrate/20230121212637_alchemy_six_point_one.rb +++ b/db/migrate/20230121212637_alchemy_six_point_one.rb @@ -219,15 +219,6 @@ def up t.index ["updater_id"], name: "index_alchemy_pictures_on_updater_id" end end - - unless table_exists?("alchemy_picture_thumbs") - create_table "alchemy_picture_thumbs" do |t| - t.references "picture", null: false, foreign_key: {to_table: :alchemy_pictures} - t.string "signature", null: false - t.text "uid", null: false - t.index ["signature"], name: "index_alchemy_picture_thumbs_on_signature", unique: true - end - end end def down @@ -241,7 +232,6 @@ def down drop_table "alchemy_nodes" if table_exists?("alchemy_nodes") drop_table "alchemy_page_versions" if table_exists?("alchemy_page_versions") drop_table "alchemy_pages" if table_exists?("alchemy_pages") - drop_table "alchemy_picture_thumbs" if table_exists?("alchemy_picture_thumbs") drop_table "alchemy_pictures" if table_exists?("alchemy_pictures") drop_table "alchemy_sites" if table_exists?("alchemy_sites") end diff --git a/lib/alchemy/dragonfly/processors/thumbnail.rb b/lib/alchemy/dragonfly/processors/thumbnail.rb deleted file mode 100644 index fcf0ab7076..0000000000 --- a/lib/alchemy/dragonfly/processors/thumbnail.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require "dragonfly/image_magick/processors/thumb" - -module Alchemy - module Dragonfly - module Processors - class Thumbnail < ::Dragonfly::ImageMagick::Processors::Thumb - def call(content, geometry, opts = {}) - # store content into an instance variable to use it in args_for_geometry - method - @content = content - super - end - - ## - # due to a missing ImageMagick parameter animated GIFs were broken with the default - # Dragonfly Thumb processor - def args_for_geometry(geometry) - # resize all frames in a GIF - # @link https://imagemagick.org/script/command-line-options.php#coalesce - # @link https://imagemagick.org/script/command-line-options.php#deconstruct - (@content&.mime_type == "image/gif") ? "-coalesce #{super} -deconstruct" : super - end - end - end - end -end diff --git a/lib/alchemy/test_support/factories/picture_thumb_factory.rb b/lib/alchemy/test_support/factories/picture_thumb_factory.rb deleted file mode 100644 index ca6fc779ad..0000000000 --- a/lib/alchemy/test_support/factories/picture_thumb_factory.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :alchemy_picture_thumb, class: "Alchemy::PictureThumb" do - picture { create(:alchemy_picture) } - signature { SecureRandom.hex(16) } - sequence(:uid) { |n| "#{Time.now.strftime("%Y/%m/%d")}/#{n}.jpg" } - end -end diff --git a/lib/tasks/alchemy/thumbnails.rake b/lib/tasks/alchemy/thumbnails.rake deleted file mode 100644 index 7b39b47eeb..0000000000 --- a/lib/tasks/alchemy/thumbnails.rake +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -namespace :alchemy do - namespace :generate do - desc "Generates all thumbnails for Alchemy Pictures and Picture Ingredients." - task thumbnails: [ - "alchemy:generate:picture_thumbnails", - "alchemy:generate:ingredient_picture_thumbnails" - ] - - desc "Generates thumbnails for Alchemy Pictures." - task picture_thumbnails: :environment do - puts "Regenerate #{Alchemy::Picture.count} picture thumbnails." - puts "Please wait..." - - Alchemy::Picture.find_each do |picture| - next unless picture.has_convertible_format? - - puts Alchemy::PictureThumb.generate_thumbs!(picture) - end - - puts "Done!" - end - - desc "Generates thumbnails for Alchemy Picture Ingredients (set ELEMENTS=element1,element2 to only generate thumbnails for a subset of elements)." - task ingredient_picture_thumbnails: :environment do - ingredient_pictures = Alchemy::Ingredients::Picture - .joins(:element) - .preload({related_object: :thumbs}) - .merge(Alchemy::Element.published) - - if ENV["ELEMENTS"].present? - ingredient_pictures = ingredient_pictures.merge( - Alchemy::Element.named(ENV["ELEMENTS"].split(",")) - ) - end - - puts "Regenerate #{ingredient_pictures.count} ingredient picture thumbnails." - puts "Please wait..." - - ingredient_pictures.find_each do |ingredient_picture| - puts ingredient_picture.picture_url - puts ingredient_picture.thumbnail_url - - ingredient_picture.settings.fetch(:srcset, []).each do |src| - puts ingredient_picture.picture_url(src) - end - end - - puts "Done!" - end - end -end diff --git a/spec/libraries/dragonfly/processors/thumbnail_spec.rb b/spec/libraries/dragonfly/processors/thumbnail_spec.rb deleted file mode 100644 index 3fccfeb66e..0000000000 --- a/spec/libraries/dragonfly/processors/thumbnail_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" -require_relative "../../../support/dragonfly_test_app" - -RSpec.describe Alchemy::Dragonfly::Processors::Thumbnail do - let(:app) { dragonfly_test_app } - let(:file) { Pathname.new(File.expand_path("../../../fixtures/80x60.png", __dir__)) } - let(:image) { Dragonfly::Content.new(app, file) } - let(:processor) { described_class.new } - let(:geometry) { "40x30#" } - - describe "validation" do - it "works with a valid argument" do - expect { - processor.call(image, geometry) - }.to_not raise_error - end - - it "validates with invalid argument" do - expect { - processor.call(image, "foo") - }.to raise_error(ArgumentError) - end - end - - describe "args_for_geometry" do - before do - processor.call(image, geometry) - end - - context "PNG" do - it "should not have the coalesce and deconstruct argument" do - expect(processor.args_for_geometry(geometry)).not_to include("coalesce", "deconstruct") - end - end - - context "GIF" do - let(:file) { Pathname.new(File.expand_path("../../../fixtures/animated.gif", __dir__)) } - - it "should have the coalesce and deconstruct argument" do - expect(processor.args_for_geometry(geometry)).to include("coalesce", "deconstruct") - end - end - end -end diff --git a/spec/models/alchemy/picture_thumb/create_spec.rb b/spec/models/alchemy/picture_thumb/create_spec.rb deleted file mode 100644 index db298d3f87..0000000000 --- a/spec/models/alchemy/picture_thumb/create_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Alchemy::PictureThumb::Create do - let(:image) { File.new(File.expand_path("../../../fixtures/image.png", __dir__)) } - let(:picture) { FactoryBot.create(:alchemy_picture, image_file: image) } - let(:variant) { Alchemy::PictureVariant.new(picture, {size: "1x1"}) } - - subject(:create) do - Alchemy::PictureThumb::Create.call(variant, "1234", "/pictures/#{picture.id}/1234/image.png") - end - - it "creates thumb on picture thumbs collection" do - expect { create }.to change { variant.picture.thumbs.reload.length }.by(1) - end - - context "with a thumb already existing" do - let!(:thumb) do - Alchemy::PictureThumb.create!( - picture: picture, - signature: "1234", - uid: "/pictures/#{picture.id}/1234/image.png" - ) - end - - it "does not create a new thumb" do - expect { create }.to_not change { picture.thumbs.reload.length } - end - end - - context "with an invalid picture" do - let(:picture) { FactoryBot.build(:alchemy_picture) } - - before do - expect(picture).to receive(:valid?) { false } - end - - it "does not create a thumb" do - expect { create }.not_to change { variant.picture.thumbs.reload.length } - end - - it "does not process the image" do - expect(variant).to_not receive(:image) - create - end - end - - context "on processing errors" do - before do - variant - expect(variant).to receive(:image) do - raise(Dragonfly::Job::Fetch::NotFound) - end - end - - it "destroys thumbnail" do - expect { subject }.to_not change { variant.picture.thumbs.reload.length } - end - end - - context "on file errors" do - before do - variant - expect_any_instance_of(Dragonfly::Content).to receive(:to_file) do - raise("Bam!") - end - end - - it "destroys thumbnail" do - expect { subject }.to_not change { variant.picture.thumbs.reload.length } - end - end -end diff --git a/spec/models/alchemy/picture_thumb/file_store_spec.rb b/spec/models/alchemy/picture_thumb/file_store_spec.rb deleted file mode 100644 index deaee026d2..0000000000 --- a/spec/models/alchemy/picture_thumb/file_store_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Alchemy::PictureThumb::FileStore do - let(:image) { File.new(File.expand_path("../../../fixtures/image.png", __dir__)) } - let(:picture) { FactoryBot.create(:alchemy_picture, image_file: image) } - let!(:variant) { Alchemy::PictureVariant.new(picture, {size: "1x1"}) } - let(:uid_path) { "pictures/#{picture.id}/1234" } - - let(:root_path) do - datastore = Dragonfly.app(:alchemy_pictures).datastore - datastore.server_root - end - - subject(:store) do - Alchemy::PictureThumb::FileStore.call(variant, "/#{uid_path}/image.png") - end - - before do - FileUtils.rm_rf("#{root_path}/#{uid_path}") - end - - it "stores thumb on the disk" do - expect { store }.to change { Dir.glob("#{root_path}/#{uid_path}").length }.by(1) - end -end diff --git a/spec/models/alchemy/picture_thumb/uid_spec.rb b/spec/models/alchemy/picture_thumb/uid_spec.rb deleted file mode 100644 index 60844037a4..0000000000 --- a/spec/models/alchemy/picture_thumb/uid_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Alchemy::PictureThumb::Uid do - let(:image) { File.new(File.expand_path("../../../fixtures/image2.PNG", __dir__)) } - let(:picture) { build_stubbed(:alchemy_picture, image_file: image) } - let(:variant) { Alchemy::PictureVariant.new(picture) } - - subject { described_class.call("12345", variant) } - - it { - is_expected.to eq "pictures/#{picture.id}/12345/image2.png" - } - - context "with format options" do - let(:variant) { Alchemy::PictureVariant.new(picture, {format: "jpg"}) } - - it "uses this as extension" do - is_expected.to eq "pictures/#{picture.id}/12345/image2.jpg" - end - end - - context "with non word characters in filename" do - let(:picture) { build_stubbed(:alchemy_picture, image_file: image, image_file_name: "The +*&image).png") } - - it "replaces them with underscore" do - is_expected.to eq "pictures/#{picture.id}/12345/The_image_.png" - end - end - - context "with no image_file_name" do - let(:picture) { build_stubbed(:alchemy_picture, image_file: image, image_file_name: nil) } - - it "uses 'image' as default" do - is_expected.to eq "pictures/#{picture.id}/12345/image.png" - end - end -end diff --git a/spec/models/alchemy/picture_thumb_spec.rb b/spec/models/alchemy/picture_thumb_spec.rb deleted file mode 100644 index 38660462e0..0000000000 --- a/spec/models/alchemy/picture_thumb_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Alchemy::PictureThumb do - it { should belong_to(:picture).class_name("Alchemy::Picture") } - it { should validate_presence_of(:signature) } - it { should validate_presence_of(:uid) } -end diff --git a/spec/models/alchemy/picture_variant_spec.rb b/spec/models/alchemy/picture_variant_spec.rb deleted file mode 100644 index 679c6164c5..0000000000 --- a/spec/models/alchemy/picture_variant_spec.rb +++ /dev/null @@ -1,418 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe Alchemy::PictureVariant do - let(:image_file) do - File.new(File.expand_path("../../fixtures/500x500.png", __dir__)) - end - - let(:alchemy_picture) { build_stubbed(:alchemy_picture, image_file: image_file) } - - subject { described_class.new(alchemy_picture, options).image } - - let(:options) { {} } - - context "when no image is present" do - let(:alchemy_picture) { nil } - - it "raises ArgumentError" do - expect { subject }.to raise_error(ArgumentError) - end - end - - context "when a size is passed in" do - let(:options) do - {size: "120x160"} - end - - it "resizes the image without upsampling it" do - expect(subject.steps[0].arguments).to eq(["120x160>"]) - end - - context "but upsample set to true" do - let(:options) do - { - size: "1600x1200", - upsample: true - } - end - - it "resizes the image with upsampling it" do - expect(subject.steps[0].arguments).to eq(["1600x1200"]) - end - end - - context "and crop is set to true" do - let(:options) do - { - size: "160x120", - crop: true - } - end - - it "crops from center and resizes the picture" do - expect(subject.steps[0].arguments).to eq(["160x120#"]) - end - - context "and crop_from and crop_size is passed in" do - let(:options) do - { - crop_size: "123x44", - crop_from: "0x0", - size: "160x120", - crop: true - } - end - - it "crops and resizes the picture" do - expect(subject.steps[0].arguments).to eq(["123x44+0+0", "160x120>"]) - end - end - end - - context "and crop is set to false" do - let(:options) do - { - size: "160x120", - crop: false - } - end - - it "does not crop the picture" do - expect(subject.steps[0].arguments).to eq(["160x120>"]) - end - - context "and crop_from and crop_size is passed in" do - let(:options) do - { - crop_size: "123x44", - crop_from: "0x0", - size: "160x120", - crop: false - } - end - - it "does not crop the picture" do - expect(subject.steps[0].arguments).to eq(["160x120>"]) - end - end - end - - context "with no height given" do - let(:options) do - {size: "40"} - end - - it "resizes the image inferring the height" do - expect(subject.steps[0].arguments).to eq(["40>"]) - end - - context "and crop set to true" do - let(:image_file) do - File.new(File.expand_path("../../fixtures/80x60.png", __dir__)) - end - let(:options) do - {size: "17x", crop: true} - end - - it "resizes the image inferring the height" do - expect(subject.steps[0].arguments).to eq(["17x13#"]) - end - end - end - - context "with no width given" do - let(:options) do - {size: "x30"} - end - - it "resizes the image inferring the width" do - expect(subject.steps[0].arguments).to eq(["x30>"]) - end - end - end - - context "when no size is passed in" do - it "does not process the image" do - expect(subject.job.steps).to be_empty - end - end - - context "when a different format is requested" do - let(:options) do - {format: "gif"} - end - - it "converts the format" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to include("gif") - end - - context "but image has not a convertible format (svg)" do - let(:image_file) do - fixture_file_upload( - File.expand_path("../../fixtures/icon.svg", __dir__), - "image/svg+xml" - ) - end - - it "does not convert the picture format" do - expect(subject.job.steps.size).to eq(0) - end - end - - context "for an animated gif" do - let(:options) do - {format: "png"} - end - - let(:image_file) do - fixture_file_upload( - File.expand_path("../../fixtures/animated.gif", __dir__), - "image/gif" - ) - end - - it "flattens the image." do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["png", "-background transparent -flatten"]) - end - - context "converted to non transparent format" do - let(:options) do - {format: "jpg"} - end - - it "does not add transparent background." do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["jpg", "-quality 85 -flatten"]) - end - end - - context "converted from non transparent format" do - let(:options) do - {format: "png", flatten: true} - end - - let(:image_file) do - fixture_file_upload( - File.expand_path("../../fixtures/image4.jpg", __dir__), - "image/jpeg" - ) - end - - it "does not add transparent background." do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["png", "-flatten"]) - end - end - - context "converted to webp" do - let(:options) do - {format: "webp"} - end - - let(:image_file) do - fixture_file_upload( - File.expand_path("../../fixtures/animated.gif", __dir__), - "image/gif" - ) - end - - it "does not flatten the image." do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["webp", "-quality 85"]) - end - end - end - - context "passed as symbol" do - let(:options) do - {format: :gif} - end - - it "converts the format" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to include("gif") - end - end - end - - context "requesting a not allowed format" do - let(:options) do - {format: "zip"} - end - - it "returns nil" do - expect(subject).to be_nil - end - - it "logs warning" do - expect(Alchemy::Logger).to receive(:warn) - subject - end - end - - %w[jpg jpeg].each do |format| - context "when #{format} format is requested" do - let(:options) do - {format: format} - end - - context "and the image file format is not JPG" do - it "sets the default quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 85"]) - end - - context "and quality is passed" do - let(:options) do - {format: format, quality: "30"} - end - - it "sets the quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 30"]) - end - end - end - - context "and image has jpg format" do - let(:alchemy_picture) do - build_stubbed(:alchemy_picture, image_file: image_file, image_file_format: "jpg") - end - - it "does not convert the picture format" do - expect(subject).to_not respond_to(:steps) - end - - context "and quality is passed in options" do - let(:options) do - {format: format, quality: "30"} - end - - it "sets the quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 30"]) - end - end - end - - context "and image has jpeg format" do - let(:alchemy_picture) do - build_stubbed(:alchemy_picture, image_file: image_file, image_file_format: "jpeg") - end - - it "does not convert the picture format" do - expect(subject).to_not respond_to(:steps) - end - - context "and quality is passed in options" do - let(:options) do - {format: format, quality: "30"} - end - - it "sets the quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 30"]) - end - end - end - - context "and image has webp format" do - let(:image_file) do - File.new(File.expand_path("../../fixtures/image5.webp", __dir__)) - end - - let(:alchemy_picture) do - build_stubbed(:alchemy_picture, image_file: image_file, image_file_format: "webp") - end - - let(:options) do - {format: format} - end - - it "converts the picture into #{format}" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 85"]) - end - - context "and quality is passed in options" do - let(:options) do - {format: format, quality: "30"} - end - - it "sets the quality as well" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq([format, "-quality 30"]) - end - end - end - end - end - - context "when webp format is requested" do - let(:options) do - {format: "webp"} - end - - context "and the image file format is not WebP" do - it "converts image into webp and sets the default quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["webp", "-quality 85"]) - end - - context "but quality is passed" do - let(:options) do - {format: "webp", quality: "30"} - end - - it "converts with given quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["webp", "-quality 30"]) - end - end - end - - context "and image already has webp format" do - let(:image_file) do - File.new(File.expand_path("../../fixtures/image5.webp", __dir__)) - end - - let(:alchemy_picture) do - build_stubbed(:alchemy_picture, image_file: image_file, image_file_format: "webp") - end - - it "does not convert the picture format" do - expect(subject).to_not respond_to(:steps) - end - - context "and quality is passed in options" do - let(:options) do - {format: "webp", quality: "30"} - end - - it "converts to given quality" do - step = subject.steps[0] - expect(step.name).to eq(:encode) - expect(step.arguments).to eq(["webp", "-quality 30"]) - end - end - end - end -end From 3199621d7c1f4cb164b8619e965b4b4343a54514 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:46 +0200 Subject: [PATCH 15/27] Move can_be_cropped_to? into picture_thumbnails This method is the only one left after the PictureVariant and PictureThumb removal. --- app/models/alchemy/picture.rb | 1 - app/models/alchemy/picture/calculations.rb | 49 ------------------- .../concerns/alchemy/picture_thumbnails.rb | 17 +++++-- .../having_picture_thumbnails_examples.rb | 35 ++++++++++--- spec/models/alchemy/picture_spec.rb | 2 - spec/rails_helper.rb | 1 - spec/support/calculation_examples.rb | 36 -------------- 7 files changed, 40 insertions(+), 101 deletions(-) delete mode 100644 app/models/alchemy/picture/calculations.rb delete mode 100644 spec/support/calculation_examples.rb diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 4251226fdf..b4ee4b3769 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -43,7 +43,6 @@ class Picture < BaseRecord include Alchemy::NameConversions include Alchemy::Taggable include Alchemy::TouchElements - include Calculations has_many :picture_ingredients, class_name: "Alchemy::Ingredients::Picture", diff --git a/app/models/alchemy/picture/calculations.rb b/app/models/alchemy/picture/calculations.rb deleted file mode 100644 index 5e1e3fd3e5..0000000000 --- a/app/models/alchemy/picture/calculations.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -module Alchemy - class Picture < BaseRecord - module Calculations - # An Image smaller than dimensions - # can not be cropped to given size - unless upsample is true. - # - def can_be_cropped_to?(string, upsample = false) - return true if upsample - - is_bigger_than? sizes_from_string(string) - end - - # Returns true if both dimensions of the base image are bigger than the dimensions hash. - # - def is_bigger_than?(dimensions) - image_file_width > dimensions[:width] && image_file_height > dimensions[:height] - end - - # Returns true is one dimension of the base image is smaller than the dimensions hash. - # - def is_smaller_than?(dimensions) - !is_bigger_than?(dimensions) - end - - # Given a string with an x, this function returns a Hash with point - # :width and :height. - # - def sizes_from_string(string) - width, height = string.to_s.split("x", 2).map(&:to_i) - - { - width: width, - height: height - } - end - - # This function returns the :width and :height of the image file - # as a Hash - def image_size - { - width: image_file_width, - height: image_file_height - } - end - end - end -end diff --git a/app/models/concerns/alchemy/picture_thumbnails.rb b/app/models/concerns/alchemy/picture_thumbnails.rb index 0de7a518c7..8edb17738d 100644 --- a/app/models/concerns/alchemy/picture_thumbnails.rb +++ b/app/models/concerns/alchemy/picture_thumbnails.rb @@ -102,14 +102,23 @@ def image_cropper_settings # Show image cropping link for ingredient def allow_image_cropping? - settings[:crop] && picture&.can_be_cropped_to?( - settings[:size], - settings[:upsample] - ) && !!picture.image_file.attached? + settings[:crop] && + picture&.image_file&.attached? && + can_be_cropped_to? end private + # An Image smaller than dimensions + # can not be cropped to given size - unless upsample is true. + # + def can_be_cropped_to? + return true if settings[:upsample] + + dimensions = inferred_dimensions_from_string(settings[:size]) + picture.image_file_width > dimensions[0] && picture.image_file_height > dimensions[1] + end + def default_crop_size return nil unless settings[:crop] && settings[:size] diff --git a/lib/alchemy/test_support/having_picture_thumbnails_examples.rb b/lib/alchemy/test_support/having_picture_thumbnails_examples.rb index b23407f7b5..51c6e1c7bc 100644 --- a/lib/alchemy/test_support/having_picture_thumbnails_examples.rb +++ b/lib/alchemy/test_support/having_picture_thumbnails_examples.rb @@ -608,10 +608,15 @@ let(:picture) { Alchemy::Picture.new } let(:image_file_width) { 400 } let(:image_file_height) { 300 } + let(:crop_size) { "400x300" } + let(:upsample) { false } before do allow(picture).to receive(:image_file_width) { image_file_width } allow(picture).to receive(:image_file_height) { image_file_height } + allow(record).to receive(:settings) do + {crop: true, size: crop_size, upsample: upsample} + end end subject { record.allow_image_cropping? } @@ -623,23 +628,37 @@ allow(record).to receive(:picture) { picture } end - it { is_expected.to be_falsy } + context "and image smaller or equal to crop size" do + context "if picture.image_file is nil" do + before do + expect(picture.image_file).to receive(:attached?) { false } + end - context "and with image larger than crop size" do - before do - allow(picture).to receive(:can_be_cropped_to?) { true } + it { is_expected.to be_falsy } end + context "if picture.image_file is present" do + before do + expect(picture.image_file).to receive(:attached?) { true } + end + + it { is_expected.to be_falsy } + + context "but with upsample set to true" do + let(:upsample) { true } + + it { is_expected.to be(true) } + end + end + end + + context "and with image larger than crop size" do let(:image_file_width) { 1201 } let(:image_file_height) { 481 } it { is_expected.to be_falsy } context "with crop set to true" do - before do - allow(record).to receive(:settings) { {crop: true} } - end - context "if picture.image_file is nil" do before do expect(picture.image_file).to receive(:attached?) { false } diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 71718a7ec1..4ba5bfb8fc 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -10,8 +10,6 @@ module Alchemy let(:picture) { build(:alchemy_picture, image_file: image_file) } - it_behaves_like "has image calculations" - it "is valid with valid attributes" do expect(picture).to be_valid end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 32ca5e3228..6ec5ab593b 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -33,7 +33,6 @@ require "alchemy/test_support/shared_uploader_examples" require "alchemy/test_support/current_language_shared_examples" -require_relative "support/calculation_examples" require_relative "support/hint_examples" require_relative "support/custom_news_elements_finder" diff --git a/spec/support/calculation_examples.rb b/spec/support/calculation_examples.rb deleted file mode 100644 index eef6c135f9..0000000000 --- a/spec/support/calculation_examples.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -module Alchemy - shared_examples_for "has image calculations" do - describe "#can_be_cropped_to?" do - context "picture is 300x400 and shall be cropped to 200x100" do - it "should return true" do - allow(picture).to receive(:image_file_width) { 400 } - allow(picture).to receive(:image_file_height) { 300 } - - expect(picture.can_be_cropped_to?("200x100")).to be(true) - end - end - - context "picture is 300x400 and shall be cropped to 600x500" do - it "should return false" do - allow(picture).to receive(:image_file_width) { 400 } - allow(picture).to receive(:image_file_height) { 300 } - - expect(picture.can_be_cropped_to?("600x500")).to be(false) - end - end - - context "picture is 300x400 and shall be cropped to 600x500 with upsample set to true" do - it "should return true" do - allow(picture).to receive(:image_file_width) { 400 } - allow(picture).to receive(:image_file_height) { 300 } - - expect(picture.can_be_cropped_to?("600x500", true)).to be(true) - end - end - end - end -end From dd2dd79402738f1a4a1e4517f8c2eec026bcb060 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:47 +0200 Subject: [PATCH 16/27] Use deletaged image_file methods for validations --- app/models/alchemy/picture.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index b4ee4b3769..fe22566566 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -291,7 +291,7 @@ def image_file_height end def image_file_extension - image_file&.filename&.extension + image_file&.filename&.extension&.downcase end alias_method :suffix, :image_file_extension @@ -313,15 +313,17 @@ def image_file_type_allowed allowed_filetypes = Config .get(:uploader) .dig("allowed_filetypes", "alchemy/pictures") || [] - unless MiniMime.lookup_by_content_type(image_file.content_type)&.extension&.in? allowed_filetypes + unless image_file_extension&.in?(allowed_filetypes) errors.add(:image_file, Alchemy.t("not a valid image")) end end def image_file_not_too_big - maximum = Config.get(:uploader)["file_size_limit"].megabytes - if image_file.byte_size > maximum - errors.add(:image_file, :too_big) + maximum = Config.get(:uploader)["file_size_limit"]&.megabytes + return true unless maximum + + if image_file_size > maximum + errors.add(:file, :too_big) end end end From f401da3eb376aeb8fec07a9209e403ee000bb198 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:49 +0200 Subject: [PATCH 17/27] Use ActiveStorage for Attachments as well --- .../alchemy/admin/attachments_controller.rb | 8 -- .../alchemy/attachments_controller.rb | 41 ++++--- app/models/alchemy/attachment.rb | 71 ++++++++--- .../admin/attachments/_files_list.html.erb | 2 +- config/routes.rb | 1 - .../factories/attachment_factory.rb | 18 ++- .../admin/attachments_controller_spec.rb | 33 +++-- .../alchemy/attachments_controller_spec.rb | 2 +- "spec/fixtures/my FileN\303\244m\303\274.png" | Bin 0 -> 73 bytes spec/models/alchemy/attachment_spec.rb | 116 +++++++++++------- .../alchemy/attachment_serializer_spec.rb | 14 ++- .../alchemy/ingredients/file_view_spec.rb | 4 +- 12 files changed, 191 insertions(+), 119 deletions(-) create mode 100644 "spec/fixtures/my FileN\303\244m\303\274.png" diff --git a/app/controllers/alchemy/admin/attachments_controller.rb b/app/controllers/alchemy/admin/attachments_controller.rb index 9d7f16ea00..0e8a451aa6 100644 --- a/app/controllers/alchemy/admin/attachments_controller.rb +++ b/app/controllers/alchemy/admin/attachments_controller.rb @@ -64,14 +64,6 @@ def destroy flash[:notice] = Alchemy.t("File deleted successfully", name: name) end - def download - @attachment = Attachment.find(params[:id]) - send_file @attachment.file.path, { - filename: @attachment.file_name, - type: @attachment.file_mime_type - } - end - private def search_filter_params diff --git a/app/controllers/alchemy/attachments_controller.rb b/app/controllers/alchemy/attachments_controller.rb index afd94dd5b8..0d46c31bdb 100644 --- a/app/controllers/alchemy/attachments_controller.rb +++ b/app/controllers/alchemy/attachments_controller.rb @@ -2,31 +2,22 @@ module Alchemy class AttachmentsController < BaseController + include ActiveStorage::Streaming + before_action :load_attachment - authorize_resource class: Alchemy::Attachment + + self.etag_with_template_digest = false # sends file inline. i.e. for viewing pdfs/movies in browser def show - response.headers["Content-Length"] = @attachment.file.size.to_s - send_file( - @attachment.file.path, - { - filename: @attachment.file_name, - type: @attachment.file_mime_type, - disposition: "inline" - } - ) + authorize! :show, @attachment + send_blob disposition: :inline end # sends file as attachment. aka download def download - response.headers["Content-Length"] = @attachment.file.size.to_s - send_file( - @attachment.file.path, { - filename: @attachment.file_name, - type: @attachment.file_mime_type - } - ) + authorize! :download, @attachment + send_blob disposition: :attachment end private @@ -34,5 +25,21 @@ def download def load_attachment @attachment = Attachment.find(params[:id]) end + + def send_blob(disposition: :inline) + @blob = @attachment.file.blob + + if request.headers["Range"].present? + send_blob_byte_range_data @blob, request.headers["Range"], disposition: disposition + else + http_cache_forever public: true do + response.headers["Accept-Ranges"] = "bytes" + send_blob_stream @blob, disposition: disposition + # Rails ActionController::Live removes the Content-Length header, + # but browsers need that to be able to show a progress bar during download. + response.headers["Content-Length"] = @blob.byte_size.to_s + end + end + end end end diff --git a/app/models/alchemy/attachment.rb b/app/models/alchemy/attachment.rb index d0d76c9edd..a8853439f4 100644 --- a/app/models/alchemy/attachment.rb +++ b/app/models/alchemy/attachment.rb @@ -24,9 +24,8 @@ class Attachment < BaseRecord include Alchemy::Taggable include Alchemy::TouchElements - dragonfly_accessor :file, app: :alchemy_attachments do - after_assign { |f| write_attribute(:file_mime_type, f.mime_type) } - end + # Use ActiveStorage file attachments + has_one_attached :file, service: :alchemy_cms stampable stamper_class_name: Alchemy.user_class.name @@ -38,7 +37,11 @@ class Attachment < BaseRecord has_many :elements, through: :file_ingredients has_many :pages, through: :elements - scope :by_file_type, ->(file_type) { where(file_mime_type: file_type) } + scope :by_file_type, + ->(file_type) { + with_attached_file.joins(:file_blob).where(active_storage_blobs: {content_type: file_type}) + } + scope :recent, -> { where("#{table_name}.created_at > ?", Time.current - 24.hours).order(:created_at) } scope :without_tag, -> { left_outer_joins(:taggings).where(gutentag_taggings: {id: nil}) } @@ -62,7 +65,7 @@ def alchemy_resource_filters [ { name: :by_file_type, - values: distinct.pluck(:file_mime_type).map { |type| [Alchemy.t(type, scope: "mime_types"), type] }.sort_by(&:first) + values: file_types }, { name: :misc, @@ -85,25 +88,32 @@ def searchable_alchemy_resource_attributes def allowed_filetypes Config.get(:uploader).fetch("allowed_filetypes", {}).fetch("alchemy/attachments", []) end + + private + + def file_types + ActiveStorage::Blob.joins(:attachments).merge( + ActiveStorage::Attachment.where(record_type: name) + ).distinct.pluck(:content_type) + end end validates_presence_of :file - validates_size_of :file, maximum: Config.get(:uploader)["file_size_limit"].megabytes - validates_property :ext, - of: :file, - in: allowed_filetypes, - case_sensitive: false, - message: Alchemy.t("not a valid file"), - unless: -> { self.class.allowed_filetypes.include?("*") } - before_save :set_name, if: :file_name_changed? + validate :file_not_too_big, if: -> { file.present? } + + validate :file_type_allowed, + unless: -> { self.class.allowed_filetypes.include?("*") }, + if: -> { file.present? } + + before_save :set_name, if: -> { file.changed? } scope :with_file_type, ->(file_type) { where(file_mime_type: file_type) } # Instance methods def url(options = {}) - if file + if file.present? self.class.url_class.new(self).call(options) end end @@ -118,9 +128,23 @@ def restricted? pages.any? && pages.not_restricted.blank? end + # File name + def file_name + file&.filename&.to_s + end + + # File size + def file_size + file&.byte_size + end + + def file_mime_type + super || file&.content_type + end + # File format suffix def extension - file_name.split(".").last + file&.filename&.extension end alias_method :suffix, :extension @@ -156,8 +180,23 @@ def icon_css_class private + def file_type_allowed + unless extension&.in?(self.class.allowed_filetypes) + errors.add(:file, Alchemy.t("not a valid file")) + end + end + + def file_not_too_big + maximum = Config.get(:uploader)["file_size_limit"]&.megabytes + return true unless maximum + + if file_size > maximum + errors.add(:file, :too_big) + end + end + def set_name - self.name = convert_to_humanized_name(file_name, file.ext) + self.name = convert_to_humanized_name(file_name, extension) end end end diff --git a/app/views/alchemy/admin/attachments/_files_list.html.erb b/app/views/alchemy/admin/attachments/_files_list.html.erb index 8bdc149220..53f8ad4cfd 100644 --- a/app/views/alchemy/admin/attachments/_files_list.html.erb +++ b/app/views/alchemy/admin/attachments/_files_list.html.erb @@ -49,7 +49,7 @@ <% table.with_action(:download) do |attachment| %> "> <%= link_to render_icon(:download), - alchemy.download_admin_attachment_path(attachment), + alchemy.download_attachment_path(attachment), target: "_blank", class: "icon_button" %> diff --git a/config/routes.rb b/config/routes.rb index 95096ba9b1..910e5cd79f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,7 +68,6 @@ resources :attachments, except: [:new] do member do - get :download put :assign end end diff --git a/lib/alchemy/test_support/factories/attachment_factory.rb b/lib/alchemy/test_support/factories/attachment_factory.rb index 98fdfc5f0c..94f5949195 100644 --- a/lib/alchemy/test_support/factories/attachment_factory.rb +++ b/lib/alchemy/test_support/factories/attachment_factory.rb @@ -2,9 +2,23 @@ FactoryBot.define do factory :alchemy_attachment, class: "Alchemy::Attachment" do - file do - File.new(Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png")) + transient do + file do + Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png") + end end + + after(:build) do |picture, acc| + if acc.file + picture.file.attach( + io: File.open(acc.file), + filename: File.basename(acc.file), + content_type: MiniMime.lookup_by_extension(File.extname(acc.file).remove("."))&.content_type || "application/octet-stream", + identify: false + ) + end + end + name { "image" } file_name { "image.png" } end diff --git a/spec/controllers/alchemy/admin/attachments_controller_spec.rb b/spec/controllers/alchemy/admin/attachments_controller_spec.rb index 9817941999..54ecc1dd13 100644 --- a/spec/controllers/alchemy/admin/attachments_controller_spec.rb +++ b/spec/controllers/alchemy/admin/attachments_controller_spec.rb @@ -6,13 +6,10 @@ module Alchemy describe Admin::AttachmentsController do routes { Alchemy::Engine.routes } - let(:attachment) { build_stubbed(:alchemy_attachment) } + let(:attachment) { create(:alchemy_attachment, file: file) } let(:file) do - fixture_file_upload( - File.expand_path("../../../fixtures/500x500.png", __dir__), - "image/png" - ) + File.expand_path("../../../fixtures/500x500.png", __dir__) end before do @@ -54,7 +51,7 @@ module Alchemy let!(:jpg) do create :alchemy_attachment, - file: File.new(File.expand_path("../../../fixtures/image3.jpeg", __dir__)) + file: File.expand_path("../../../fixtures/image3.jpeg", __dir__) end it "loads only attachments with matching content type" do @@ -80,6 +77,13 @@ module Alchemy subject { post :create, params: params } context "with passing validations" do + let(:file) do + fixture_file_upload( + File.expand_path("../../../fixtures/500x500.png", __dir__), + "image/png" + ) + end + let(:params) { {attachment: {file: file}} } it "renders json response with success message" do @@ -137,7 +141,7 @@ module Alchemy end it "replaces the file" do - expect { subject }.to change { attachment.reload.file_uid } + expect { subject }.to change { attachment.reload.file_blob } end end end @@ -170,7 +174,9 @@ module Alchemy end context "with failing validations" do - include_context "with invalid file" + before do + expect_any_instance_of(Alchemy::Attachment).to receive(:errors).at_least(:once) { ["invalid file"] } + end it "renders edit form" do is_expected.to render_template(:edit) @@ -213,17 +219,6 @@ module Alchemy end end - describe "#download" do - before do - expect(Attachment).to receive(:find).and_return(attachment) - end - - it "sends the file as download" do - get :download, params: {id: attachment.id} - expect(response.headers["Content-Disposition"]).to match(/attachment/) - end - end - describe "#assign" do let(:attachment) { create(:alchemy_attachment) } diff --git a/spec/controllers/alchemy/attachments_controller_spec.rb b/spec/controllers/alchemy/attachments_controller_spec.rb index 86b8b6ef2e..a70dfe9b97 100644 --- a/spec/controllers/alchemy/attachments_controller_spec.rb +++ b/spec/controllers/alchemy/attachments_controller_spec.rb @@ -6,7 +6,7 @@ module Alchemy describe AttachmentsController do routes { Alchemy::Engine.routes } - let(:attachment) { build_stubbed(:alchemy_attachment) } + let(:attachment) { create(:alchemy_attachment) } it "should raise ActiveRecord::RecordNotFound for requesting not existing attachments" do expect { get :download, params: {id: 0} }.to raise_error(ActiveRecord::RecordNotFound) diff --git "a/spec/fixtures/my FileN\303\244m\303\274.png" "b/spec/fixtures/my FileN\303\244m\303\274.png" new file mode 100644 index 0000000000000000000000000000000000000000..5fe8c6623bd8d7297145bc27ef1a65277ff897b8 GIT binary patch literal 73 zcmeAS@N?(olHy`uVBq!ia0vp^j3CUx1|;Q0k8}bl0Z$jlkcwN3tPH>YGfZh}s$-31 V68E@ql^ZC>;OXk;vd$@?2>|I-5On|m literal 0 HcmV?d00001 diff --git a/spec/models/alchemy/attachment_spec.rb b/spec/models/alchemy/attachment_spec.rb index fac3dd485e..1c93a4f631 100644 --- a/spec/models/alchemy/attachment_spec.rb +++ b/spec/models/alchemy/attachment_spec.rb @@ -4,14 +4,11 @@ module Alchemy describe Attachment do - let(:file) { File.new(File.expand_path("../../fixtures/image with spaces.png", __dir__)) } - let(:attachment) { Attachment.new(file: file) } + let(:file) { File.expand_path("../../fixtures/image with spaces.png", __dir__) } + let(:attachment) { build(:alchemy_attachment, file: file) } - describe "after assign" do - it "stores the file mime type into database" do - attachment.update(file: file) - expect(attachment.file_mime_type).not_to be_blank - end + it "has file mime type accessor" do + expect(attachment.file_mime_type).to eq("image/png") end describe "after save" do @@ -53,7 +50,7 @@ module Alchemy subject { attachment.url } context "without file" do - let(:attachment) { described_class.new } + let(:attachment) { build(:alchemy_attachment, file: nil) } it { is_expected.to be_nil } end @@ -104,45 +101,68 @@ module Alchemy end describe "urlname sanitizing" do - it "escapes unsafe url characters" do - attachment.file_name = "f#%&cking cute kitten pic.png" - expect(attachment.slug).to eq("f%23%25%26cking+cute+kitten+pic") + subject { attachment.slug } + + before do + expect(attachment).to receive(:file_name) { file_name } end - it "removes format suffix from end of file name" do - attachment.file_name = "pic.png.png" - expect(attachment.slug).to eq("pic+png") + context "unsafe url characters" do + let(:file_name) { "f#%&cking cute kitten pic.png" } + + it "get escaped" do + is_expected.to eq("f%23%25%26cking+cute+kitten+pic") + end end - it "converts dots into escaped spaces" do - attachment.file_name = "cute.kitten.pic.png" - expect(attachment.slug).to eq("cute+kitten+pic") + context "format suffix from end of file name" do + let(:file_name) { "pic.png.png" } + + it "gets removed" do + is_expected.to eq("pic+png") + end + end + + context "dots" do + let(:file_name) { "cute.kitten.pic.png" } + + it "get converted into escaped spaces" do + is_expected.to eq("cute+kitten+pic") + end end - it "escapes umlauts in the name" do - attachment.file_name = "süßes katzenbild.png" - expect(attachment.slug).to eq("s%C3%BC%C3%9Fes+katzenbild") + context "umlauts in the name" do + let(:file_name) { "süßes katzenbild.png" } + + it "get escaped" do + is_expected.to eq("s%C3%BC%C3%9Fes+katzenbild") + end end end describe "validations" do context "having a png, but only pdf allowed" do before do - allow(Config).to receive(:get) do - {"allowed_filetypes" => {"alchemy/attachments" => ["pdf"]}} - end + stub_alchemy_config(:uploader, { + "allowed_filetypes" => { + "alchemy/attachments" => ["pdf"] + } + }) end it "should not be valid" do expect(attachment).not_to be_valid + expect(attachment.errors[:file]).to eq(["not a valid file"]) end end context "having a png and everything allowed" do before do - allow(Config).to receive(:get) do - {"allowed_filetypes" => {"alchemy/attachments" => ["*"]}} - end + stub_alchemy_config(:uploader, { + "allowed_filetypes" => { + "alchemy/attachments" => ["*"] + } + }) end it "should be valid" do @@ -151,8 +171,9 @@ module Alchemy end context "having a filename with special characters" do + let(:file) { File.expand_path("../../fixtures/my FileNämü.png", __dir__) } + before do - attachment.file_name = "my FileNämü.pdf" attachment.save end @@ -162,98 +183,99 @@ module Alchemy end end - context "PNG image" do - subject { stub_model(Attachment, file_name: "kitten.png") } + describe "#extension" do + subject { attachment.extension } - describe "#extension" do - subject { super().extension } - it { is_expected.to eq("png") } - end + it { is_expected.to eq("png") } end describe "#icon_css_class" do subject { attachment.icon_css_class } + before do + expect(attachment).to receive(:file_mime_type) { mime_type } + end + context "mp3 file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "audio/mpeg") } + let(:mime_type) { "audio/mpeg" } it { is_expected.to eq("file-music") } end context "video file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "video/mpeg") } + let(:mime_type) { "video/mpeg" } it { is_expected.to eq("file-video") } end context "png file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "image/png") } + let(:mime_type) { "image/png" } it { is_expected.to eq("file-image") } end context "vcard file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/vcard") } + let(:mime_type) { "application/vcard" } it { is_expected.to eq("profile") } end context "zip file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/zip") } + let(:mime_type) { "application/zip" } it { is_expected.to eq("file-zip") } end context "photoshop file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "image/x-psd") } + let(:mime_type) { "image/x-psd" } it { is_expected.to eq("file-image") } end context "text file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "text/plain") } + let(:mime_type) { "text/plain" } it { is_expected.to eq("file-text") } end context "rtf file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/rtf") } + let(:mime_type) { "application/rtf" } it { is_expected.to eq("file-text") } end context "pdf file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/pdf") } + let(:mime_type) { "application/pdf" } it { is_expected.to eq("file-pdf-2") } end context "word file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/msword") } + let(:mime_type) { "application/msword" } it { is_expected.to eq("file-word-2") } end context "excel file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/vnd.ms-excel") } + let(:mime_type) { "application/vnd.ms-excel" } it { is_expected.to eq("file-excel-2") } end context "xlsx file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") } + let(:mime_type) { "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" } it { is_expected.to eq("file-excel-2") } end context "csv file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "text/csv") } + let(:mime_type) { "text/csv" } it { is_expected.to eq("file-excel-2") } end context "unknown file" do - let(:attachment) { stub_model(Attachment, file_mime_type: "") } + let(:mime_type) { "" } it { is_expected.to eq("file-3") } end diff --git a/spec/serializers/alchemy/attachment_serializer_spec.rb b/spec/serializers/alchemy/attachment_serializer_spec.rb index d3f2573eea..1c8a50e020 100644 --- a/spec/serializers/alchemy/attachment_serializer_spec.rb +++ b/spec/serializers/alchemy/attachment_serializer_spec.rb @@ -5,17 +5,21 @@ RSpec.describe Alchemy::AttachmentSerializer do subject { described_class.new(attachment).to_json } - let(:attachment) { build_stubbed(:alchemy_attachment) } + let(:file) do + Alchemy::Engine.root.join("lib", "alchemy", "test_support", "fixtures", "image.png") + end + + let(:attachment) { create(:alchemy_attachment, file: file) } it "includes all attributes" do json = JSON.parse(subject) expect(json).to eq( "id" => attachment.id, - "name" => attachment.name, - "file_name" => attachment.file_name, - "file_mime_type" => attachment.file_mime_type, + "name" => "image", + "file_name" => "image.png", + "file_mime_type" => "image/png", "file_size" => attachment.file_size, - "icon_css_class" => attachment.icon_css_class, + "icon_css_class" => "file-image", "tag_list" => attachment.tag_list, "created_at" => attachment.created_at.as_json, "updated_at" => attachment.updated_at.as_json, diff --git a/spec/views/alchemy/ingredients/file_view_spec.rb b/spec/views/alchemy/ingredients/file_view_spec.rb index fe85f6ab16..376b3730a5 100644 --- a/spec/views/alchemy/ingredients/file_view_spec.rb +++ b/spec/views/alchemy/ingredients/file_view_spec.rb @@ -4,11 +4,11 @@ describe "alchemy/ingredients/_file_view" do let(:file) do - File.new(File.expand_path("../../../fixtures/image with spaces.png", __dir__)) + File.expand_path("../../../fixtures/image with spaces.png", __dir__) end let(:attachment) do - build_stubbed(:alchemy_attachment, file: file, name: "an image", file_name: "image with spaces.png") + create(:alchemy_attachment, file: file, name: "an image") end let(:ingredient) { Alchemy::Ingredients::File.new(attachment: attachment) } From 61b3230e453bd444decdd6c8e5e1b5e9ffd465dc Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:51 +0200 Subject: [PATCH 18/27] Use file mime type for picture by format select Active storage has the mime type stored not the extension. --- app/models/alchemy/picture.rb | 11 +++++------ config/locales/alchemy.en.yml | 8 ++------ spec/models/alchemy/picture_spec.rb | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index fe22566566..f04c31fff9 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -96,7 +96,10 @@ def self.preprocessor_class=(klass) where("#{table_name}.id NOT IN (SELECT related_object_id FROM alchemy_ingredients WHERE related_object_type = 'Alchemy::Picture')") } scope :without_tag, -> { left_outer_joins(:taggings).where(gutentag_taggings: {id: nil}) } - scope :by_file_format, ->(format) { where(image_file_format: format) } + scope :by_file_format, + ->(file_format) { + with_attached_image_file.joins(:image_file_blob).where(active_storage_blobs: {content_type: file_format}) + } # Class methods @@ -116,14 +119,10 @@ def url_class=(klass) end def alchemy_resource_filters - @_file_formats ||= file_formats.map do |format| - MiniMime.lookup_by_content_type(format)&.extension - end - [ { name: :by_file_format, - values: @_file_formats + values: file_formats }, { name: :misc, diff --git a/config/locales/alchemy.en.yml b/config/locales/alchemy.en.yml index de5f81dcd6..1e7973355c 100644 --- a/config/locales/alchemy.en.yml +++ b/config/locales/alchemy.en.yml @@ -89,6 +89,7 @@ en: image/tiff: TIFF Image image/x-psd: Photoshop File image/svg+xml: SVG Image + image/webp: WebP Image text/plain: Plain Text Document text/x-vcard: vCard video/mp4: MPEG-4 Video @@ -115,12 +116,7 @@ en: by_file_format: name: File Type values: - gif: GIF Image - jpeg: JPG Image - png: PNG Image - svg: SVG Image - tiff: TIFF Image - webp: WebP Image + <<: *mime_types misc: name: Miscellaneous values: diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 4ba5bfb8fc..e1ef8ed7b5 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -70,7 +70,7 @@ module Alchemy expect(Alchemy::Picture.alchemy_resource_filters).to eq([ { name: :by_file_format, - values: ["png"] + values: ["image/png"] }, { name: :misc, From c1ab0b18062fa6b69a10ff1e8f8d01f6ea570661 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Apr 2022 17:56:52 +0200 Subject: [PATCH 19/27] Remove Dragonfly --- app/models/alchemy/picture.rb | 4 +-- config/alchemy/config.yml | 4 +-- config/initializers/dragonfly.rb | 25 ------------- .../dragonfly/processors/auto_orient.rb | 18 ---------- .../dragonfly/processors/crop_resize.rb | 35 ------------------- lib/alchemy/engine.rb | 1 - lib/alchemy_cms.rb | 1 - .../alchemy/install/install_generator.rb | 8 ----- .../alchemy/install/templates/dragonfly.rb.tt | 35 ------------------- .../dragonfly/processors/auto_orient_spec.rb | 17 --------- .../dragonfly/processors/crop_resize_spec.rb | 23 ------------ spec/support/dragonfly_test_app.rb | 8 ----- 12 files changed, 3 insertions(+), 176 deletions(-) delete mode 100644 config/initializers/dragonfly.rb delete mode 100644 lib/alchemy/dragonfly/processors/auto_orient.rb delete mode 100644 lib/alchemy/dragonfly/processors/crop_resize.rb delete mode 100644 lib/generators/alchemy/install/templates/dragonfly.rb.tt delete mode 100644 spec/libraries/dragonfly/processors/auto_orient_spec.rb delete mode 100644 spec/libraries/dragonfly/processors/crop_resize_spec.rb delete mode 100644 spec/support/dragonfly_test_app.rb diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index f04c31fff9..9391f0d5e1 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -59,8 +59,8 @@ class Picture < BaseRecord # # === CAUTION # - # This HAS to be placed for Dragonfly's class methods, - # to ensure this runs before Dragonfly's before_destroy callback. + # This HAS to be placed for ActiveStorage class methods, + # to ensure this runs before ActiveStorage before_destroy callback. # before_destroy unless: :deletable? do raise PictureInUseError, Alchemy.t(:cannot_delete_picture_notice) % {name: name} diff --git a/config/alchemy/config.yml b/config/alchemy/config.yml index ac64cef978..95111da6b8 100644 --- a/config/alchemy/config.yml +++ b/config/alchemy/config.yml @@ -62,7 +62,7 @@ items_per_page: 15 # === Picture rendering settings # -# Alchemy uses Dragonfly to render images. Settings for image rendering are specific to elements and are defined in elements.yml +# Alchemy uses ActiveStorage to render images. Settings for image rendering are specific to elements and are defined in elements.yml # # Example: # - name: some_element @@ -74,8 +74,6 @@ items_per_page: 15 # crop: true # turns on image cropping # size: '500x500' # image will be cropped to this size # -# See http://markevans.github.com/dragonfly for further info. -# # ==== Global Options: # # output_image_quality [Integer] # If image gets rendered as JPG or WebP this is the quality setting for it. (Default 85) diff --git a/config/initializers/dragonfly.rb b/config/initializers/dragonfly.rb deleted file mode 100644 index 7b6af50433..0000000000 --- a/config/initializers/dragonfly.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -require "dragonfly_svg" -require "alchemy/dragonfly/processors/crop_resize" -require "alchemy/dragonfly/processors/auto_orient" -require "alchemy/dragonfly/processors/thumbnail" - -# Logger -Dragonfly.logger = Rails.logger - -# Add model functionality -if defined?(ActiveRecord::Base) - ActiveRecord::Base.extend Dragonfly::Model - ActiveRecord::Base.extend Dragonfly::Model::Validations -end - -# Dragonfly 1.4.0 only allows `quality` as argument to `encode` -Dragonfly::ImageMagick::Processors::Encode::WHITELISTED_ARGS << "flatten" -Dragonfly::ImageMagick::Processors::Encode::WHITELISTED_ARGS << "background" - -Rails.application.config.after_initialize do - Dragonfly.app(:alchemy_pictures).add_processor(:crop_resize, Alchemy::Dragonfly::Processors::CropResize.new) - Dragonfly.app(:alchemy_pictures).add_processor(:auto_orient, Alchemy::Dragonfly::Processors::AutoOrient.new) - Dragonfly.app(:alchemy_pictures).add_processor(:thumbnail, Alchemy::Dragonfly::Processors::Thumbnail.new) -end diff --git a/lib/alchemy/dragonfly/processors/auto_orient.rb b/lib/alchemy/dragonfly/processors/auto_orient.rb deleted file mode 100644 index 732c4bc099..0000000000 --- a/lib/alchemy/dragonfly/processors/auto_orient.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -require "dragonfly/image_magick/commands" - -module Alchemy - module Dragonfly - module Processors - class AutoOrient - def call(content) - ::Dragonfly::ImageMagick::Commands.convert( - content, - "-auto-orient" - ) - end - end - end - end -end diff --git a/lib/alchemy/dragonfly/processors/crop_resize.rb b/lib/alchemy/dragonfly/processors/crop_resize.rb deleted file mode 100644 index be664ddf9d..0000000000 --- a/lib/alchemy/dragonfly/processors/crop_resize.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require "dragonfly/image_magick/commands" - -module Alchemy - module Dragonfly - module Processors - class CropResize - include ::Dragonfly::ParamValidators - - IS_CROP_ARGUMENT = ->(args_string) { - args_string.match?(::Dragonfly::ImageMagick::Processors::Thumb::CROP_GEOMETRY) - } - - IS_RESIZE_ARGUMENT = ->(args_string) { - args_string.match?(::Dragonfly::ImageMagick::Processors::Thumb::RESIZE_GEOMETRY) - } - - def call(content, crop_argument, resize_argument) - validate!(crop_argument, &IS_CROP_ARGUMENT) - validate!(resize_argument, &IS_RESIZE_ARGUMENT) - ::Dragonfly::ImageMagick::Commands.convert( - content, - "-crop #{crop_argument} -resize #{resize_argument}" - ) - end - - def update_url(attrs, _args = "", opts = {}) - format = opts["format"] - attrs.ext = format if format - end - end - end - end -end diff --git a/lib/alchemy/engine.rb b/lib/alchemy/engine.rb index 3d4ebddddb..114da91157 100644 --- a/lib/alchemy/engine.rb +++ b/lib/alchemy/engine.rb @@ -109,7 +109,6 @@ class Engine < Rails::Engine unless Mime::Type.lookup_by_extension(:webp) Mime::Type.register("image/webp", :webp) end - # Dragonfly uses Rack to read the mime type and guess what unless Rack::Mime::MIME_TYPES[".webp"] Rack::Mime::MIME_TYPES[".webp"] = "image/webp" end diff --git a/lib/alchemy_cms.rb b/lib/alchemy_cms.rb index 41aba2db6f..3c0fdd84da 100644 --- a/lib/alchemy_cms.rb +++ b/lib/alchemy_cms.rb @@ -10,7 +10,6 @@ require "active_storage/engine" require "awesome_nested_set" require "cancan" -require "dragonfly" require "gutentag" require "importmap-rails" require "kaminari" diff --git a/lib/generators/alchemy/install/install_generator.rb b/lib/generators/alchemy/install/install_generator.rb index 90f0e2e435..4af2237ade 100644 --- a/lib/generators/alchemy/install/install_generator.rb +++ b/lib/generators/alchemy/install/install_generator.rb @@ -80,14 +80,6 @@ def copy_demo_views copy_file "alchemy.en.yml", app_config_path.join("locales", "alchemy.en.yml") end - def copy_dragonfly_config - template( - "#{__dir__}/templates/dragonfly.rb.tt", - app_config_path.join("initializers", "dragonfly.rb"), - skip: options[:auto_accept] - ) - end - def install_gutentag_migrations rake "gutentag:install:migrations" end diff --git a/lib/generators/alchemy/install/templates/dragonfly.rb.tt b/lib/generators/alchemy/install/templates/dragonfly.rb.tt deleted file mode 100644 index ee1757af09..0000000000 --- a/lib/generators/alchemy/install/templates/dragonfly.rb.tt +++ /dev/null @@ -1,35 +0,0 @@ -# AlchemyCMS Dragonfly configuration. -# -# Consider using some kind of caching solution for image processing. -# For small projects, we have good experience with Rack::Cache. -# -# Larger installations should consider using a CDN, start reading -# http://markevans.github.io/dragonfly/cache/ -# -# A complete reference can be found at -# http://markevans.github.io/dragonfly/configuration -# -# Pictures -# -Dragonfly.app(:alchemy_pictures).configure do - dragonfly_url nil - plugin :imagemagick - plugin :svg - secret "<%= SecureRandom.hex(32) %>" - url_format "/pictures/:job/:basename.:ext" - - datastore :file, - root_path: Rails.root.join("uploads/pictures").to_s, - server_root: Rails.root.join("public"), - store_meta: false -end - -# Mount as middleware -Rails.application.middleware.use Dragonfly::Middleware, :alchemy_pictures - -# Attachments -Dragonfly.app(:alchemy_attachments).configure do - datastore :file, - root_path: Rails.root.join("uploads/attachments").to_s, - store_meta: false -end diff --git a/spec/libraries/dragonfly/processors/auto_orient_spec.rb b/spec/libraries/dragonfly/processors/auto_orient_spec.rb deleted file mode 100644 index f4441c78ce..0000000000 --- a/spec/libraries/dragonfly/processors/auto_orient_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" -require_relative "../../../support/dragonfly_test_app" - -RSpec.describe Alchemy::Dragonfly::Processors::AutoOrient do - let(:app) { dragonfly_test_app } - let(:file) { Pathname.new(File.expand_path("../../../fixtures/80x60.png", __dir__)) } - let(:image) { Dragonfly::Content.new(app, file) } - let(:processor) { described_class.new } - - it "works" do - expect { - processor.call(image) - }.to_not raise_error - end -end diff --git a/spec/libraries/dragonfly/processors/crop_resize_spec.rb b/spec/libraries/dragonfly/processors/crop_resize_spec.rb deleted file mode 100644 index ff61fd9a5e..0000000000 --- a/spec/libraries/dragonfly/processors/crop_resize_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" -require_relative "../../../support/dragonfly_test_app" - -RSpec.describe Alchemy::Dragonfly::Processors::CropResize do - let(:app) { dragonfly_test_app } - let(:file) { Pathname.new(File.expand_path("../../../fixtures/80x60.png", __dir__)) } - let(:image) { Dragonfly::Content.new(app, file) } - let(:processor) { described_class.new } - - it "validates bad crop and resize arguments" do - expect { - processor.call(image, "h4ck", "m3") - }.to raise_error(Dragonfly::ParamValidators::InvalidParameter) - end - - it "works with correct crop and resize arguments" do - expect { - processor.call(image, "4x4+0+0", "20x20>") - }.to_not raise_error - end -end diff --git a/spec/support/dragonfly_test_app.rb b/spec/support/dragonfly_test_app.rb deleted file mode 100644 index ddf38f2048..0000000000 --- a/spec/support/dragonfly_test_app.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -def dragonfly_test_app(name = nil) - app = Dragonfly::App.instance(name) - app.datastore = Dragonfly::MemoryDataStore.new - app.secret = "test secret" - app -end From 67a9faa59b4267b3179fc45cc61dcfa08592bbdb Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 2 Aug 2023 19:28:14 +0200 Subject: [PATCH 20/27] Add support for animated gifs ruby-vips and mini_magick need to be told to ignore the page in order to resize animated gifs. --- .../alchemy/dragonfly_to_image_processing.rb | 32 ++++ spec/models/alchemy/picture/url_spec.rb | 3 +- .../dragonfly_to_image_processing_spec.rb | 143 ++++++++++++++++-- 3 files changed, 164 insertions(+), 14 deletions(-) diff --git a/app/services/alchemy/dragonfly_to_image_processing.rb b/app/services/alchemy/dragonfly_to_image_processing.rb index 750b69000d..e3b3d3a2dd 100644 --- a/app/services/alchemy/dragonfly_to_image_processing.rb +++ b/app/services/alchemy/dragonfly_to_image_processing.rb @@ -11,6 +11,7 @@ class << self def call(options = {}) opts = crop_options(options).presence || resize_options(options) opts.merge!(format_options(options)) + opts.merge!(flatten_options(options)) opts.merge!(quality_options(options)) opts end @@ -56,6 +57,33 @@ def format_options(options) {format: format} end + def flatten_options(options) + case options[:flatten] + when true + {loader: flattened_loader_options} + when false, nil + {loader: not_flattened_loader_options} + end + end + + def flattened_loader_options + case variant_processor + when :vips + {n: 1} + when :mini_magick + {page: 0} + end + end + + def not_flattened_loader_options + case variant_processor + when :vips + {n: -1} + when :mini_magick + {page: nil} + end + end + def image_magick_string(options) if options[:crop] == true "#{options[:size]}#" @@ -95,6 +123,10 @@ def default_output_format Alchemy::Config.get(:image_output_format) end + + def variant_processor + Rails.application.config.active_storage.variant_processor + end end end end diff --git a/spec/models/alchemy/picture/url_spec.rb b/spec/models/alchemy/picture/url_spec.rb index e06504af7f..30b9f5868e 100644 --- a/spec/models/alchemy/picture/url_spec.rb +++ b/spec/models/alchemy/picture/url_spec.rb @@ -27,7 +27,8 @@ expect(picture.image_file).to receive(:variant).with( { resize_to_limit: [10, 10, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) subject diff --git a/spec/services/alchemy/dragonfly_to_image_processing_spec.rb b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb index c6e8f763de..15110dcce7 100644 --- a/spec/services/alchemy/dragonfly_to_image_processing_spec.rb +++ b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb @@ -20,7 +20,8 @@ { crop: [0, 0, 2000, 1000], resize_to_limit: [200, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -41,7 +42,8 @@ { crop: [0, 0, 2000, 1000], resize_to_limit: [200, 100, {}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -56,7 +58,8 @@ is_expected.to eq( { resize_to_limit: [100, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -69,7 +72,8 @@ is_expected.to eq( { resize_to_fit: [100, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -82,7 +86,8 @@ is_expected.to eq( { resize_to_fill: [100, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -95,7 +100,8 @@ is_expected.to eq( { resize_to_limit: [100, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -107,7 +113,8 @@ is_expected.to eq( { resize_to_fill: [100, 100, {sharpen: false}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -121,7 +128,8 @@ is_expected.to eq( { resize_to_limit: [100, 100, {}], - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -131,14 +139,20 @@ let(:options) { {} } it "just contains default quality option" do - is_expected.to eq({saver: {quality: 85}}) + is_expected.to eq({ + saver: {quality: 85}, + loader: {n: -1} + }) end context "if quality is given" do let(:options) { {quality: 15} } it "contains given quality option" do - is_expected.to eq({saver: {quality: 15}}) + is_expected.to eq({ + saver: {quality: 15}, + loader: {n: -1} + }) end end end @@ -150,7 +164,8 @@ is_expected.to eq( { format: "webp", - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -167,7 +182,8 @@ it "does not contain the format option" do is_expected.to eq( { - saver: {quality: 85} + saver: {quality: 85}, + loader: {n: -1} } ) end @@ -182,11 +198,112 @@ is_expected.to eq( { saver: {quality: 85}, - format: "webp" + format: "webp", + loader: {n: -1} } ) end end end end + + describe "flatten option" do + shared_context "vips variant processor" do + before do + expect(Rails.application.config.active_storage).to receive(:variant_processor) do + :vips + end + end + end + + shared_context "mini_magick variant processor" do + before do + expect(Rails.application.config.active_storage).to receive(:variant_processor) do + :mini_magick + end + end + end + + context "with flatten not set" do + let(:options) { {format: "gif"} } + + context "with vips variant processor" do + include_context "vips variant processor" + + it "does not flatten image" do + is_expected.to include({ + loader: {n: -1} + }) + end + end + + context "with mini_magick variant processor" do + include_context "mini_magick variant processor" + + it "does not flatten image" do + is_expected.to include({ + loader: {page: nil} + }) + end + end + end + + context "with flatten set to false" do + let(:options) do + { + format: "gif", + flatten: false + } + end + + context "with vips variant processor" do + include_context "vips variant processor" + + it "does not flatten image" do + is_expected.to include({ + loader: {n: -1} + }) + end + end + + context "with mini_magick variant processor" do + include_context "mini_magick variant processor" + + it "does not flatten image" do + is_expected.to include({ + loader: {page: nil} + }) + end + end + end + + context "flatten set to true" do + let(:options) do + { + format: "gif", + flatten: true + } + end + + context "with vips variant processor" do + include_context "vips variant processor" + + it "flattens image" do + is_expected.to include({ + loader: {n: 1} + }) + end + end + + context "with mini_magick variant processor" do + include_context "mini_magick variant processor" + + it "flattens image" do + is_expected.to include({ + loader: {page: 0} + }) + end + end + end + end end From e1c9cc0fc3e5c69ff38e4944b587647ad926a189 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Fri, 4 Aug 2023 14:44:52 +0200 Subject: [PATCH 21/27] Always return image format Before we where not telling active storage that we want a certain format explicitely. We need that for displaying animated PNGs properly. --- app/models/alchemy/picture/url.rb | 16 ++++-- .../alchemy/dragonfly_to_image_processing.rb | 14 ----- spec/models/alchemy/picture/url_spec.rb | 1 + .../dragonfly_to_image_processing_spec.rb | 53 +------------------ 4 files changed, 14 insertions(+), 70 deletions(-) diff --git a/app/models/alchemy/picture/url.rb b/app/models/alchemy/picture/url.rb index 1d1edf888d..7a97355385 100644 --- a/app/models/alchemy/picture/url.rb +++ b/app/models/alchemy/picture/url.rb @@ -18,6 +18,7 @@ def initialize(picture) # def call(options = {}) variant_options = DragonflyToImageProcessing.call(options) + variant_options[:format] = options[:format] || default_output_format variant = image_file&.variant(variant_options) return unless variant @@ -25,8 +26,8 @@ def call(options = {}) variant, { filename: filename(options), - only_path: true, - format: nil + format: variant_options[:format], + only_path: true } ) end @@ -34,13 +35,20 @@ def call(options = {}) private def filename(options = {}) - format = options[:format] || picture.image_file_extension if picture.name.presence - "#{picture.name.to_param}.#{format}" + picture.name.to_param else picture.image_file_name end end + + def default_output_format + if Alchemy::Config.get(:image_output_format) == "original" + picture.image_file_extension + else + Alchemy::Config.get(:image_output_format) + end + end end end end diff --git a/app/services/alchemy/dragonfly_to_image_processing.rb b/app/services/alchemy/dragonfly_to_image_processing.rb index e3b3d3a2dd..bcea2999cd 100644 --- a/app/services/alchemy/dragonfly_to_image_processing.rb +++ b/app/services/alchemy/dragonfly_to_image_processing.rb @@ -10,7 +10,6 @@ module DragonflyToImageProcessing class << self def call(options = {}) opts = crop_options(options).presence || resize_options(options) - opts.merge!(format_options(options)) opts.merge!(flatten_options(options)) opts.merge!(quality_options(options)) opts @@ -50,13 +49,6 @@ def quality_options(options) {saver: {quality: quality}} end - def format_options(options) - format = options[:format] || default_output_format - return {} if format.nil? - - {format: format} - end - def flatten_options(options) case options[:flatten] when true @@ -118,12 +110,6 @@ def sharpen_value(options) options.key?(:sharpen) ? options[:sharpen] : Alchemy::Config.get(:sharpen_images) end - def default_output_format - return nil if Alchemy::Config.get(:image_output_format) == "original" - - Alchemy::Config.get(:image_output_format) - end - def variant_processor Rails.application.config.active_storage.variant_processor end diff --git a/spec/models/alchemy/picture/url_spec.rb b/spec/models/alchemy/picture/url_spec.rb index 30b9f5868e..b1804569de 100644 --- a/spec/models/alchemy/picture/url_spec.rb +++ b/spec/models/alchemy/picture/url_spec.rb @@ -28,6 +28,7 @@ { resize_to_limit: [10, 10, {sharpen: false}], saver: {quality: 85}, + format: "png", loader: {n: -1} } ) diff --git a/spec/services/alchemy/dragonfly_to_image_processing_spec.rb b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb index 15110dcce7..795472205e 100644 --- a/spec/services/alchemy/dragonfly_to_image_processing_spec.rb +++ b/spec/services/alchemy/dragonfly_to_image_processing_spec.rb @@ -156,55 +156,6 @@ end end end - - context "Format option is given" do - let(:options) { {format: "webp"} } - - it "contains the format option" do - is_expected.to eq( - { - format: "webp", - saver: {quality: 85}, - loader: {n: -1} - } - ) - end - end - - context "Format option is not given" do - let(:options) { {} } - - context "and the image output format is configured to be original" do - before do - stub_alchemy_config(:image_output_format, "original") - end - - it "does not contain the format option" do - is_expected.to eq( - { - saver: {quality: 85}, - loader: {n: -1} - } - ) - end - end - - context "and the image output format is configured to webp" do - before do - stub_alchemy_config(:image_output_format, "webp") - end - - it "does not contain the format option" do - is_expected.to eq( - { - saver: {quality: 85}, - format: "webp", - loader: {n: -1} - } - ) - end - end - end end describe "flatten option" do @@ -225,7 +176,7 @@ end context "with flatten not set" do - let(:options) { {format: "gif"} } + let(:options) { {} } context "with vips variant processor" do include_context "vips variant processor" @@ -251,7 +202,6 @@ context "with flatten set to false" do let(:options) do { - format: "gif", flatten: false } end @@ -280,7 +230,6 @@ context "flatten set to true" do let(:options) do { - format: "gif", flatten: true } end From b70ab1c2622da2a600a53457b275179ffefa691d Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 23 Jan 2024 18:02:01 +0100 Subject: [PATCH 22/27] Use ActiveStorage redirect path Proxy downloads the file. --- app/models/alchemy/picture/url.rb | 2 +- spec/models/alchemy/picture/url_spec.rb | 2 +- spec/models/alchemy/picture_spec.rb | 2 +- spec/requests/alchemy/admin/pictures_controller_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/alchemy/picture/url.rb b/app/models/alchemy/picture/url.rb index 7a97355385..511ee47578 100644 --- a/app/models/alchemy/picture/url.rb +++ b/app/models/alchemy/picture/url.rb @@ -22,7 +22,7 @@ def call(options = {}) variant = image_file&.variant(variant_options) return unless variant - Rails.application.routes.url_helpers.rails_storage_proxy_url( + Rails.application.routes.url_helpers.rails_blob_path( variant, { filename: filename(options), diff --git a/spec/models/alchemy/picture/url_spec.rb b/spec/models/alchemy/picture/url_spec.rb index b1804569de..442ca42145 100644 --- a/spec/models/alchemy/picture/url_spec.rb +++ b/spec/models/alchemy/picture/url_spec.rb @@ -11,7 +11,7 @@ let(:options) { {} } it "returns the proxy url to the image" do - is_expected.to match(/\/rails\/active_storage\/representations\/proxy\/.+\/image\.png/) + is_expected.to match(/\/rails\/active_storage\/representations\/redirect\/.+\/image\.png/) end it "adds image name and format to url" do diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index e1ef8ed7b5..8c0a017ea7 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -215,7 +215,7 @@ module Alchemy end it "returns the url to the thumbnail" do - is_expected.to match(/\/rails\/active_storage\/representations\/proxy\/.+\/square\.png/) + is_expected.to match(/\/rails\/active_storage\/representations\/redirect\/.+\/square\.png/) end end end diff --git a/spec/requests/alchemy/admin/pictures_controller_spec.rb b/spec/requests/alchemy/admin/pictures_controller_spec.rb index 7849ada9c2..2122728743 100644 --- a/spec/requests/alchemy/admin/pictures_controller_spec.rb +++ b/spec/requests/alchemy/admin/pictures_controller_spec.rb @@ -23,7 +23,7 @@ get alchemy.url_admin_picture_path(picture) json = JSON.parse(response.body) expect(json).to match({ - "url" => /\/rails\/active_storage\/representations\/proxy\/.+\/image\.png/, + "url" => /\/rails\/active_storage\/representations\/redirect\/.+\/image\.png/, "alt" => picture.name, "title" => Alchemy.t(:image_name, name: picture.name) }) From 21777e87f314ed7d6e6a4963a1d7983f52aeac4f Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 23 Jan 2024 18:05:30 +0100 Subject: [PATCH 23/27] Fix picture and attachment search --- app/models/alchemy/attachment.rb | 11 ++++++++- app/models/alchemy/picture.rb | 11 ++++++++- lib/alchemy/engine.rb | 6 +++++ .../admin/attachments_controller_spec.rb | 4 ++-- .../alchemy/admin/pictures_controller_spec.rb | 4 ++-- spec/models/alchemy/attachment_spec.rb | 24 +++++++++++++++++++ spec/models/alchemy/picture_spec.rb | 24 +++++++++++++++++++ 7 files changed, 78 insertions(+), 6 deletions(-) diff --git a/app/models/alchemy/attachment.rb b/app/models/alchemy/attachment.rb index a8853439f4..5623cbf32c 100644 --- a/app/models/alchemy/attachment.rb +++ b/app/models/alchemy/attachment.rb @@ -81,8 +81,17 @@ def last_upload where(id: last_id) end + # Used by Alchemy::Resource#search_field_name to build the search query def searchable_alchemy_resource_attributes - %w[name file_name] + %w[name file_blob_filename] + end + + def ransackable_attributes(_auth_object = nil) + %w[name] + end + + def ransackable_associations(_auth_object = nil) + %w[file_blob] end def allowed_filetypes diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 9391f0d5e1..a059aad99d 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -131,8 +131,17 @@ def alchemy_resource_filters ] end + # Used by Alchemy::Resource#search_field_name to build the search query def searchable_alchemy_resource_attributes - %w[name image_file_name] + %w[name image_file_blob_filename] + end + + def ransackable_attributes(_auth_object = nil) + %w[name] + end + + def ransackable_associations(_auth_object = nil) + %w[image_file_blob] end def last_upload diff --git a/lib/alchemy/engine.rb b/lib/alchemy/engine.rb index 114da91157..b81ecc8aeb 100644 --- a/lib/alchemy/engine.rb +++ b/lib/alchemy/engine.rb @@ -87,6 +87,12 @@ class Engine < Rails::Engine down_arrow: '' } end + + ActiveSupport.on_load(:active_storage_blob) do + ActiveStorage::Blob.define_singleton_method(:ransackable_attributes) do |_auth_object| + %w[filename] + end + end end config.after_initialize do diff --git a/spec/controllers/alchemy/admin/attachments_controller_spec.rb b/spec/controllers/alchemy/admin/attachments_controller_spec.rb index 54ecc1dd13..0a7a739f40 100644 --- a/spec/controllers/alchemy/admin/attachments_controller_spec.rb +++ b/spec/controllers/alchemy/admin/attachments_controller_spec.rb @@ -154,7 +154,7 @@ module Alchemy context "with search params" do let(:search_filter_params) do { - q: {name_or_file_name_cont: "kitten"}, + q: {name_or_file_blob_filename_cont: "kitten"}, tagged_with: "cute", filter: {by_file_type: "pdf"}, page: 2 @@ -204,7 +204,7 @@ module Alchemy context "with search params" do let(:search_filter_params) do { - q: {name_or_file_name_cont: "kitten"}, + q: {name_or_file_blob_filename_cont: "kitten"}, tagged_with: "cute", filter: {by_file_type: "pdf"}, page: 2 diff --git a/spec/controllers/alchemy/admin/pictures_controller_spec.rb b/spec/controllers/alchemy/admin/pictures_controller_spec.rb index ea546feabc..264b8db69d 100644 --- a/spec/controllers/alchemy/admin/pictures_controller_spec.rb +++ b/spec/controllers/alchemy/admin/pictures_controller_spec.rb @@ -7,7 +7,7 @@ { filter: {misc: "last_upload"}, page: 2, - q: {name_or_image_file_name_cont: "kitten"}, + q: {name_or_image_file_blob_filename_cont: "kitten"}, size: "small", tagged_with: "cat" } @@ -34,7 +34,7 @@ module Alchemy let!(:picture_2) { create(:alchemy_picture, name: "nice beach") } it "assigns @pictures with filtered pictures" do - get :index, params: {q: {name_or_image_file_name_cont: "kitten"}} + get :index, params: {q: {name_or_image_file_blob_filename_cont: "kitten"}} expect(assigns(:pictures)).to include(picture_1) expect(assigns(:pictures)).to_not include(picture_2) end diff --git a/spec/models/alchemy/attachment_spec.rb b/spec/models/alchemy/attachment_spec.rb index 1c93a4f631..4ba1199690 100644 --- a/spec/models/alchemy/attachment_spec.rb +++ b/spec/models/alchemy/attachment_spec.rb @@ -11,6 +11,30 @@ module Alchemy expect(attachment.file_mime_type).to eq("image/png") end + describe ".searchable_alchemy_resource_attributes" do + subject { described_class.searchable_alchemy_resource_attributes } + + it "returns an array of attributes for the search field query" do + is_expected.to eq(%w[name file_blob_filename]) + end + end + + describe ".ransackable_attributes" do + subject { described_class.ransackable_attributes } + + it "returns an array of ransackable attributes" do + is_expected.to eq(%w[name]) + end + end + + describe ".ransackable_associations" do + subject { described_class.ransackable_associations } + + it "returns an array of ransackable associations" do + is_expected.to eq(%w[file_blob]) + end + end + describe "after save" do subject(:save) { attachment.save! } diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index 8c0a017ea7..ff1fd98b31 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -62,6 +62,30 @@ module Alchemy end end + describe ".searchable_alchemy_resource_attributes" do + subject { described_class.searchable_alchemy_resource_attributes } + + it "returns an array of attributes for the search field query" do + is_expected.to eq(%w[name image_file_blob_filename]) + end + end + + describe ".ransackable_attributes" do + subject { described_class.ransackable_attributes } + + it "returns an array of ransackable attributes" do + is_expected.to eq(%w[name]) + end + end + + describe ".ransackable_associations" do + subject { described_class.ransackable_associations } + + it "returns an array of ransackable associations" do + is_expected.to eq(%w[image_file_blob]) + end + end + describe ".alchemy_resource_filters" do context "with image file formats" do let!(:picture) { create(:alchemy_picture, image_file_format: "png") } From a07ff57c9122a5cad63da63891a99691248ab40a Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 29 Jan 2024 11:26:37 +0100 Subject: [PATCH 24/27] Preprocess thumbnails after upload Only works in Rails 7.1 --- app/models/alchemy/picture.rb | 6 ++++- app/models/alchemy/picture/preprocessor.rb | 28 ++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index a059aad99d..66b9fef71e 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -81,7 +81,11 @@ def self.preprocessor_class=(klass) end # Use ActiveStorage image processing - has_one_attached :image_file, service: :alchemy_cms + has_one_attached :image_file, service: :alchemy_cms do |attachable| + # Only works in Rails 7.1 + preprocessor_class.new(attachable).call + Preprocessor.generate_thumbs!(attachable) + end validates_presence_of :image_file validate :image_file_type_allowed, :image_file_not_too_big, diff --git a/app/models/alchemy/picture/preprocessor.rb b/app/models/alchemy/picture/preprocessor.rb index 86d2869602..8b63f88393 100644 --- a/app/models/alchemy/picture/preprocessor.rb +++ b/app/models/alchemy/picture/preprocessor.rb @@ -3,8 +3,8 @@ module Alchemy class Picture < BaseRecord class Preprocessor - def initialize(image_file) - @image_file = image_file + def initialize(attachable) + @attachable = attachable end # Preprocess images after upload @@ -15,14 +15,28 @@ def initialize(image_file) # def call max_image_size = Alchemy::Config.get(:preprocess_image_resize) - image_file.thumb!(max_image_size) if max_image_size.present? - # Auto orient the image so EXIF orientation data is taken into account - image_file.auto_orient! + if max_image_size.present? + self.class.process_thumb(attachable, size: max_image_size) + end end - private + attr_reader :attachable - attr_reader :image_file + class << self + def generate_thumbs!(attachable) + Alchemy::Picture::THUMBNAIL_SIZES.values.each do |size| + process_thumb(attachable, size: size, flatten: true) + end + end + + private + + def process_thumb(attachable, options = {}) + attachable.variant :thumb, + **Alchemy::DragonflyToImageProcessing.call(options), + preprocessed: true + end + end end end end From fb61906ac22d143610f6ba3be8363e3ecc7e7d6f Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 11 Jun 2024 22:23:00 +0200 Subject: [PATCH 25/27] Use image_file_extension as original format ActiveStorage expects an extension and not an mime type. --- app/models/alchemy/picture.rb | 2 +- app/models/concerns/alchemy/picture_thumbnails.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 66b9fef71e..269101ea02 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -192,7 +192,7 @@ def thumbnail_url(size: "160x120") url( flatten: true, - format: image_file_format || "jpg", + format: image_file_extension || "jpg", size: size ) end diff --git a/app/models/concerns/alchemy/picture_thumbnails.rb b/app/models/concerns/alchemy/picture_thumbnails.rb index 8edb17738d..992b257989 100644 --- a/app/models/concerns/alchemy/picture_thumbnails.rb +++ b/app/models/concerns/alchemy/picture_thumbnails.rb @@ -84,7 +84,7 @@ def thumbnail_url_options crop_from: crop && crop_from.presence || default_crop_from&.join("x"), crop_size: crop && crop_size.presence || default_crop_size&.join("x"), flatten: true, - format: picture&.image_file_format || "jpg" + format: picture&.image_file_extension || "jpg" } end From 853d5ad97197430d3f3f059b2f1196061db2605a Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 11 Jun 2024 22:43:05 +0200 Subject: [PATCH 26/27] Allow webp as image format for ActiveStorage It is only configured by default in unreleased Rails 7.2 --- lib/alchemy/engine.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/alchemy/engine.rb b/lib/alchemy/engine.rb index b81ecc8aeb..b27ca382dd 100644 --- a/lib/alchemy/engine.rb +++ b/lib/alchemy/engine.rb @@ -95,6 +95,11 @@ class Engine < Rails::Engine end end + initializer "alchemy.active_storage" do |app| + app.config.active_storage.web_image_content_types += %w[image/webp] + app.config.active_storage.content_types_allowed_inline += %w[image/webp] + end + config.after_initialize do if Alchemy.user_class ActiveSupport.on_load(:active_record) do From 38b49729639d83a5061e4215321b68408051ed8f Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 3 Dec 2024 22:03:59 +0100 Subject: [PATCH 27/27] Add upgrader for active storage --- app/models/alchemy/attachment.rb | 13 +++ app/models/alchemy/picture.rb | 19 +++ ...11080918_rename_alchemy_attachment_file.rb | 13 +++ ...80918_rename_alchemy_picture_image_file.rb | 16 +++ .../factories/attachment_factory.rb | 1 - lib/alchemy/upgrader/eight_zero.rb | 108 ++++++++++++++++++ .../tasks/active_storage_migration.rb | 100 ++++++++++++++++ .../alchemy/install/install_generator.rb | 13 +++ lib/tasks/alchemy/upgrade.rake | 32 +++++- spec/dummy/config/initializers/dragonfly.rb | 2 + spec/dummy/config/storage.yml | 4 + ..._rename_alchemy_attachment_file.alchemy.rb | 14 +++ ...name_alchemy_picture_image_file.alchemy.rb | 17 +++ spec/dummy/db/schema.rb | 24 ++-- spec/models/alchemy/picture_spec.rb | 4 +- spec/views/admin/pictures/show_spec.rb | 6 +- .../alchemy/admin/ingredients/edit_spec.rb | 6 +- .../alchemy/ingredients/audio_view_spec.rb | 2 +- .../alchemy/ingredients/video_view_spec.rb | 2 +- 19 files changed, 368 insertions(+), 28 deletions(-) create mode 100644 db/migrate/20240611080918_rename_alchemy_attachment_file.rb create mode 100644 db/migrate/20240611080918_rename_alchemy_picture_image_file.rb create mode 100644 lib/alchemy/upgrader/eight_zero.rb create mode 100644 lib/alchemy/upgrader/tasks/active_storage_migration.rb create mode 100644 spec/dummy/db/migrate/20240611152553_rename_alchemy_attachment_file.alchemy.rb create mode 100644 spec/dummy/db/migrate/20240611152554_rename_alchemy_picture_image_file.alchemy.rb diff --git a/app/models/alchemy/attachment.rb b/app/models/alchemy/attachment.rb index 5623cbf32c..196f0be7ef 100644 --- a/app/models/alchemy/attachment.rb +++ b/app/models/alchemy/attachment.rb @@ -24,6 +24,19 @@ class Attachment < BaseRecord include Alchemy::Taggable include Alchemy::TouchElements + attr_readonly( + :legacy_image_file_name, + :legacy_image_file_size, + :legacy_image_file_uid + ) + + deprecate( + :legacy_image_file_name, + :legacy_image_file_size, + :legacy_image_file_uid, + deprecator: Alchemy::Deprecation + ) + # Use ActiveStorage file attachments has_one_attached :file, service: :alchemy_cms diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb index 269101ea02..6d7ade35c6 100644 --- a/app/models/alchemy/picture.rb +++ b/app/models/alchemy/picture.rb @@ -80,6 +80,25 @@ def self.preprocessor_class=(klass) @_preprocessor_class = klass end + attr_readonly( + :legacy_image_file_format, + :legacy_image_file_height, + :legacy_image_file_name, + :legacy_image_file_size, + :legacy_image_file_uid, + :legacy_image_file_width + ) + + deprecate( + :legacy_image_file_format, + :legacy_image_file_height, + :legacy_image_file_name, + :legacy_image_file_size, + :legacy_image_file_uid, + :legacy_image_file_width, + deprecator: Alchemy::Deprecation + ) + # Use ActiveStorage image processing has_one_attached :image_file, service: :alchemy_cms do |attachable| # Only works in Rails 7.1 diff --git a/db/migrate/20240611080918_rename_alchemy_attachment_file.rb b/db/migrate/20240611080918_rename_alchemy_attachment_file.rb new file mode 100644 index 0000000000..caf952940c --- /dev/null +++ b/db/migrate/20240611080918_rename_alchemy_attachment_file.rb @@ -0,0 +1,13 @@ +class RenameAlchemyAttachmentFile < ActiveRecord::Migration[7.0] + COLUMNS = %i[ + file_name + file_size + file_uid + ] + + def change + COLUMNS.each do |column| + rename_column :alchemy_attachments, column, :"legacy_#{column}" + end + end +end diff --git a/db/migrate/20240611080918_rename_alchemy_picture_image_file.rb b/db/migrate/20240611080918_rename_alchemy_picture_image_file.rb new file mode 100644 index 0000000000..00b21693ee --- /dev/null +++ b/db/migrate/20240611080918_rename_alchemy_picture_image_file.rb @@ -0,0 +1,16 @@ +class RenameAlchemyPictureImageFile < ActiveRecord::Migration[7.0] + COLUMNS = %i[ + image_file_format + image_file_height + image_file_name + image_file_size + image_file_uid + image_file_width + ] + + def change + COLUMNS.each do |column| + rename_column :alchemy_pictures, column, :"legacy_#{column}" + end + end +end diff --git a/lib/alchemy/test_support/factories/attachment_factory.rb b/lib/alchemy/test_support/factories/attachment_factory.rb index 94f5949195..d988e7b9fb 100644 --- a/lib/alchemy/test_support/factories/attachment_factory.rb +++ b/lib/alchemy/test_support/factories/attachment_factory.rb @@ -20,6 +20,5 @@ end name { "image" } - file_name { "image.png" } end end diff --git a/lib/alchemy/upgrader/eight_zero.rb b/lib/alchemy/upgrader/eight_zero.rb new file mode 100644 index 0000000000..44681547c8 --- /dev/null +++ b/lib/alchemy/upgrader/eight_zero.rb @@ -0,0 +1,108 @@ +require "alchemy/shell" +require "alchemy/upgrader/tasks/active_storage_migration" +require "benchmark" +require "dragonfly" +require "dragonfly_svg" +require "fileutils" +require "thor" + +module Alchemy + class Upgrader::EightZero < Upgrader + include Thor::Base + include Thor::Actions + + class << self + def install_active_storage + Rake::Task["active_storage:install"].invoke + Rake::Task["db:migrate"].invoke + + text = <<-YAML.strip_heredoc + + alchemy_cms: + service: Disk + root: <%= Rails.root.join("storage") %> + YAML + + storage_yml = Rails.application.root.join("config/storage.yml") + if File.exist?(storage_yml) + task.insert_into_file(storage_yml, text) + else + task.create_file(storage_yml, text) + end + end + + def prepare_dragonfly_config + task.prepend_to_file "config/initializers/dragonfly.rb", <<~RUBY + require "dragonfly" + require "dragonfly_svg" + RUBY + end + + def migrate_pictures_to_active_storage + Dragonfly.logger = Rails.logger + + Alchemy::Picture.class_eval do + extend Dragonfly::Model + dragonfly_accessor :legacy_image_file, app: :alchemy_pictures + end + + pictures_without_as_attachment = Alchemy::Picture.where.missing(:image_file_attachment) + count = pictures_without_as_attachment.count + if count > 0 + log "Migrating #{count} Dragonfly image file(s) to ActiveStorage." + realtime = Benchmark.realtime do + pictures_without_as_attachment.find_each do |picture| + Alchemy::Upgrader::Tasks::ActiveStorageMigration.migrate_picture(picture) + print "." + end + end + puts "\nDone in #{realtime.round(2)}s!" + else + log "No Dragonfly image files for migration found.", :skip + end + end + + def migrate_attachments_to_active_storage + Dragonfly.logger = Rails.logger + + Alchemy::Attachment.class_eval do + extend Dragonfly::Model + dragonfly_accessor :legacy_file, app: :alchemy_attachments + end + + attachments_without_as_attachment = Alchemy::Attachment.where.missing(:file_attachment) + count = attachments_without_as_attachment.count + if count > 0 + log "Migrating #{count} Dragonfly attachment file(s) to ActiveStorage." + realtime = Benchmark.realtime do + attachments_without_as_attachment.find_each do |attachment| + Alchemy::Upgrader::Tasks::ActiveStorageMigration.migrate_attachment(attachment) + print "." + end + end + puts "\nDone in #{realtime.round(2)}s!" + else + log "No Dragonfly attachment files for migration found.", :skip + end + end + + def remove_dragonfly_todo + todo <<-TXT.strip_heredoc + Please check if all pictures and attachments are migrated to ActiveStorage. + + If so, you can remove the Dragonfly gem, its configuration and storage by running: + + rm config/initializers/dragonfly.rb + rm -rf uploads + + TXT + end + + private + + def task + @_task || new + end + end + end +end diff --git a/lib/alchemy/upgrader/tasks/active_storage_migration.rb b/lib/alchemy/upgrader/tasks/active_storage_migration.rb new file mode 100644 index 0000000000..a8c529224b --- /dev/null +++ b/lib/alchemy/upgrader/tasks/active_storage_migration.rb @@ -0,0 +1,100 @@ +require "active_storage/service" +require "active_storage/service/disk_service" + +module Alchemy + class Upgrader + module Tasks + class ActiveStorageMigration + DEFAULT_CONTENT_TYPE = "application/octet-stream" + DISK_SERVICE = ActiveStorage::Service::DiskService + SERVICE_NAME = :alchemy_cms + + METADATA = { + identified: true, # Skip identifying file type + analyzed: true, # Skip analyze job + composed: true # Skip checksum check + } + + class << self + def migrate_picture(picture) + Alchemy::Deprecation.silence do + uid = picture.legacy_image_file_uid + key = key_for_uid(uid) + content_type = Mime::Type.lookup_by_extension(picture.legacy_image_file_format) || DEFAULT_CONTENT_TYPE + Alchemy::Picture.transaction do + blob = ActiveStorage::Blob.create!( + key: key, + filename: picture.legacy_image_file_name, + byte_size: picture.legacy_image_file_size, + content_type: content_type, + # Prevents (down)loading the original file + metadata: METADATA.merge( + width: picture.legacy_image_file_width, + height: picture.legacy_image_file_height + ), + service_name: SERVICE_NAME + ) + picture.create_image_file_attachment!( + name: :image_file, + record: picture, + blob: blob + ) + end + move_file(Rails.root.join("uploads/pictures", uid), key) + end + end + + def migrate_attachment(attachment) + Alchemy::Deprecation.silence do + uid = attachment.legacy_file_uid + key = key_for_uid(uid) + Alchemy::Attachment.transaction do + blob = ActiveStorage::Blob.create!( + key: key, + filename: attachment.legacy_file_name, + byte_size: attachment.legacy_file_size, + content_type: attachment.file_mime_type.presence || DEFAULT_CONTENT_TYPE, + metadata: METADATA, + service_name: SERVICE_NAME + ) + attachment.create_file_attachment!( + record: attachment, + name: :file, + blob: blob + ) + end + move_file(Rails.root.join("uploads/attachments", uid), key) + end + end + + private + + # ActiveStorage::Service::DiskService stores files in a folder structure + # based on the first two characters of the file uid. + def key_for_uid(uid) + case service + when DISK_SERVICE + uid.split("/").last + else + uid + end + end + + def move_file(uid, key) + case service + when DISK_SERVICE + if File.exist?(uid) + service.send(:make_path_for, key) + FileUtils.cp uid, service.send(:path_for, key) + end + end + end + + def service + ActiveStorage::Blob.services.fetch(SERVICE_NAME) + end + end + end + end + end +end diff --git a/lib/generators/alchemy/install/install_generator.rb b/lib/generators/alchemy/install/install_generator.rb index 4af2237ade..4bfc118a00 100644 --- a/lib/generators/alchemy/install/install_generator.rb +++ b/lib/generators/alchemy/install/install_generator.rb @@ -71,6 +71,19 @@ def install_assets end end + def install_active_storage + rake "active_storage:install:migrations" + end + + def set_active_storage_service + insert_into_file app_config_path.join("storage.yml"), <<-YAML.strip_heredoc + + alchemy_cms: + service: Disk + root: <%= Rails.root.join("storage") %> + YAML + end + def copy_demo_views return if options[:skip_demo_files] diff --git a/lib/tasks/alchemy/upgrade.rake b/lib/tasks/alchemy/upgrade.rake index de7fa571fc..fe80d63f24 100644 --- a/lib/tasks/alchemy/upgrade.rake +++ b/lib/tasks/alchemy/upgrade.rake @@ -6,7 +6,8 @@ require "alchemy/version" namespace :alchemy do desc "Upgrades your app to AlchemyCMS v#{Alchemy::VERSION}." task upgrade: [ - "alchemy:upgrade:prepare" + "alchemy:upgrade:prepare", + "alchemy:upgrade:8.0:run" ] do Alchemy::Upgrader.display_todos end @@ -28,5 +29,34 @@ namespace :alchemy do task config: [:environment] do Alchemy::Upgrader.copy_new_config_file end + + namespace "8.0" do + task "run" => [ + "alchemy:upgrade:8.0:install_active_storage", + "alchemy:upgrade:8.0:prepare_dragonfly_config", + "alchemy:upgrade:8.0:migrate_pictures_to_active_storage", + "alchemy:upgrade:8.0:migrate_attachments_to_active_storage" + ] + + desc "Install active_storage" + task :install_active_storage do + Alchemy::Upgrader::EightZero.install_active_storage + end + + desc "Prepare Dragonfly config" + task :prepare_dragonfly_config do + Alchemy::Upgrader::EightZero.prepare_dragonfly_config + end + + desc "Migrate pictures to active_storage" + task :migrate_pictures_to_active_storage do + Alchemy::Upgrader::EightZero.migrate_pictures_to_active_storage + end + + desc "Migrate attachments to active_storage" + task :migrate_attachments_to_active_storage do + Alchemy::Upgrader::EightZero.migrate_attachments_to_active_storage + end + end end end diff --git a/spec/dummy/config/initializers/dragonfly.rb b/spec/dummy/config/initializers/dragonfly.rb index ea3b9b9854..74f11c4a19 100644 --- a/spec/dummy/config/initializers/dragonfly.rb +++ b/spec/dummy/config/initializers/dragonfly.rb @@ -1,3 +1,5 @@ +require "dragonfly" +require "dragonfly_svg" # frozen_string_literal: true # AlchemyCMS Dragonfly configuration. diff --git a/spec/dummy/config/storage.yml b/spec/dummy/config/storage.yml index d32f76e8fb..f1a392497c 100644 --- a/spec/dummy/config/storage.yml +++ b/spec/dummy/config/storage.yml @@ -32,3 +32,7 @@ local: # service: Mirror # primary: local # mirrors: [ amazon, google, microsoft ] + +alchemy_cms: + service: Disk + root: <%= Rails.root.join("storage") %> diff --git a/spec/dummy/db/migrate/20240611152553_rename_alchemy_attachment_file.alchemy.rb b/spec/dummy/db/migrate/20240611152553_rename_alchemy_attachment_file.alchemy.rb new file mode 100644 index 0000000000..3c56d08687 --- /dev/null +++ b/spec/dummy/db/migrate/20240611152553_rename_alchemy_attachment_file.alchemy.rb @@ -0,0 +1,14 @@ +# This migration comes from alchemy (originally 20240611080918) +class RenameAlchemyAttachmentFile < ActiveRecord::Migration[7.0] + COLUMNS = %i[ + file_name + file_size + file_uid + ] + + def change + COLUMNS.each do |column| + rename_column :alchemy_attachments, column, :"legacy_#{column}" + end + end +end diff --git a/spec/dummy/db/migrate/20240611152554_rename_alchemy_picture_image_file.alchemy.rb b/spec/dummy/db/migrate/20240611152554_rename_alchemy_picture_image_file.alchemy.rb new file mode 100644 index 0000000000..3a623642ae --- /dev/null +++ b/spec/dummy/db/migrate/20240611152554_rename_alchemy_picture_image_file.alchemy.rb @@ -0,0 +1,17 @@ +# This migration comes from alchemy (originally 20240611080918) +class RenameAlchemyPictureImageFile < ActiveRecord::Migration[7.0] + COLUMNS = %i[ + image_file_format + image_file_height + image_file_name + image_file_size + image_file_uid + image_file_width + ] + + def change + COLUMNS.each do |column| + rename_column :alchemy_pictures, column, :"legacy_#{column}" + end + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 2728ed3600..dc0f2a2af8 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_04_11_155901) do +ActiveRecord::Schema[7.2].define(version: 2024_06_11_152554) do create_table "active_storage_attachments", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -41,16 +41,16 @@ create_table "alchemy_attachments", force: :cascade do |t| t.string "name" - t.string "file_name" + t.string "legacy_file_name" t.string "file_mime_type" - t.integer "file_size" + t.integer "legacy_file_size" t.integer "creator_id" t.integer "updater_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.string "file_uid" + t.string "legacy_file_uid" t.index ["creator_id"], name: "index_alchemy_attachments_on_creator_id" - t.index ["file_uid"], name: "index_alchemy_attachments_on_file_uid" + t.index ["legacy_file_uid"], name: "index_alchemy_attachments_on_legacy_file_uid" t.index ["updater_id"], name: "index_alchemy_attachments_on_updater_id" end @@ -234,19 +234,19 @@ create_table "alchemy_pictures", force: :cascade do |t| t.string "name" - t.string "image_file_name" - t.integer "image_file_width" - t.integer "image_file_height" + t.string "legacy_image_file_name" + t.integer "legacy_image_file_width" + t.integer "legacy_image_file_height" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "creator_id" t.integer "updater_id" t.string "upload_hash" - t.string "image_file_uid" - t.integer "image_file_size" - t.string "image_file_format" + t.string "legacy_image_file_uid" + t.integer "legacy_image_file_size" + t.string "legacy_image_file_format" t.index ["creator_id"], name: "index_alchemy_pictures_on_creator_id" - t.index ["image_file_name"], name: "index_alchemy_pictures_on_image_file_name" + t.index ["legacy_image_file_name"], name: "index_alchemy_pictures_on_legacy_image_file_name" t.index ["name"], name: "index_alchemy_pictures_on_name" t.index ["updater_id"], name: "index_alchemy_pictures_on_updater_id" end diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb index ff1fd98b31..c3d68dd6a6 100644 --- a/spec/models/alchemy/picture_spec.rb +++ b/spec/models/alchemy/picture_spec.rb @@ -88,7 +88,7 @@ module Alchemy describe ".alchemy_resource_filters" do context "with image file formats" do - let!(:picture) { create(:alchemy_picture, image_file_format: "png") } + let!(:picture) { create(:alchemy_picture, image_file: image_file) } it "returns a list of filters with image file formats" do expect(Alchemy::Picture.alchemy_resource_filters).to eq([ @@ -403,7 +403,7 @@ module Alchemy describe "#convertible?" do let(:picture) do - Picture.new(image_file_format: "image/png") + build(:alchemy_picture, image_file: image_file) end subject { picture.convertible? } diff --git a/spec/views/admin/pictures/show_spec.rb b/spec/views/admin/pictures/show_spec.rb index 9b5af7b2d0..1ff3dd103b 100644 --- a/spec/views/admin/pictures/show_spec.rb +++ b/spec/views/admin/pictures/show_spec.rb @@ -11,11 +11,7 @@ end let(:picture) do - create(:alchemy_picture, { - image_file: image, - name: "animated", - image_file_name: "animated.gif" - }) + create(:alchemy_picture, image_file: image) end let(:language) { create(:alchemy_language) } diff --git a/spec/views/alchemy/admin/ingredients/edit_spec.rb b/spec/views/alchemy/admin/ingredients/edit_spec.rb index 57bf0e0ecb..2adbd6b43b 100644 --- a/spec/views/alchemy/admin/ingredients/edit_spec.rb +++ b/spec/views/alchemy/admin/ingredients/edit_spec.rb @@ -17,11 +17,7 @@ end let(:picture) do - create(:alchemy_picture, { - image_file: image, - name: "img", - image_file_name: "img.png" - }) + create(:alchemy_picture, image_file: image) end let(:ingredient) { Alchemy::Ingredients::Picture.new(id: 1, picture: picture) } diff --git a/spec/views/alchemy/ingredients/audio_view_spec.rb b/spec/views/alchemy/ingredients/audio_view_spec.rb index 7cf1c51448..5d37c29269 100644 --- a/spec/views/alchemy/ingredients/audio_view_spec.rb +++ b/spec/views/alchemy/ingredients/audio_view_spec.rb @@ -8,7 +8,7 @@ end let(:attachment) do - build_stubbed(:alchemy_attachment, file: file, name: "a podcast", file_name: "image with spaces.png") + build_stubbed(:alchemy_attachment, file: file) end let(:ingredient) do diff --git a/spec/views/alchemy/ingredients/video_view_spec.rb b/spec/views/alchemy/ingredients/video_view_spec.rb index 7441ce8b5e..66dcb2e338 100644 --- a/spec/views/alchemy/ingredients/video_view_spec.rb +++ b/spec/views/alchemy/ingredients/video_view_spec.rb @@ -8,7 +8,7 @@ end let(:attachment) do - build_stubbed(:alchemy_attachment, file: file, name: "a movie", file_name: "image with spaces.png") + build_stubbed(:alchemy_attachment, file: file) end let(:ingredient) do