diff --git a/Gemfile b/Gemfile index 10c92db..9a95876 100644 --- a/Gemfile +++ b/Gemfile @@ -26,8 +26,8 @@ gem "bootsnap", require: false # Add HTTP asset caching/compression and X-Sendfile acceleration to Puma [https://github.com/basecamp/thruster/] gem "thruster", require: false -# Use Active Storage variants [https://guides.rubyonrails.org/active_storage_overview.html#transforming-images] -# gem "image_processing", "~> 1.2" +# Used to convert images into AVIF encoding. +gem "image_processing", "~> 1.2" group :development, :test do # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem diff --git a/Gemfile.lock b/Gemfile.lock index d7397da..3cc1272 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -170,6 +170,14 @@ GEM multipart-post (~> 2.0) faraday-net_http (3.4.4) net-http (~> 0.5) + ffi (1.17.4-aarch64-linux-gnu) + ffi (1.17.4-aarch64-linux-musl) + ffi (1.17.4-arm-linux-gnu) + ffi (1.17.4-arm-linux-musl) + ffi (1.17.4-arm64-darwin) + ffi (1.17.4-x86_64-darwin) + ffi (1.17.4-x86_64-linux-gnu) + ffi (1.17.4-x86_64-linux-musl) fiddle (1.1.8) fugit (1.13.0) et-orbi (~> 1.4) @@ -187,6 +195,9 @@ GEM high_voltage (5.0.0) i18n (1.15.2) concurrent-ruby (~> 1.0) + image_processing (1.14.0) + mini_magick (>= 4.9.5, < 6) + ruby-vips (>= 2.0.17, < 3) io-console (0.9.2) irb (1.18.0) pp (>= 0.6.0) @@ -226,6 +237,8 @@ GEM marcel (1.2.1) matrix (0.4.3) method_source (1.1.0) + mini_magick (5.4.0) + logger mini_mime (1.1.5) minitest (6.0.6) drb (~> 2.0) @@ -424,6 +437,9 @@ GEM rubocop-performance (>= 1.24) rubocop-rails (>= 2.30) ruby-progressbar (1.13.0) + ruby-vips (2.3.0) + ffi (~> 1.12) + logger rubyzip (3.6.0) securerandom (0.4.1) selenium-webdriver (4.49.0) @@ -535,6 +551,7 @@ DEPENDENCIES faraday-follow_redirects hashid-rails high_voltage + image_processing (~> 1.2) jb kaminari lockbox diff --git a/app/components/uploads/index.rb b/app/components/uploads/index.rb index 820d9ea..77e5c7b 100644 --- a/app/components/uploads/index.rb +++ b/app/components/uploads/index.rb @@ -36,9 +36,24 @@ def header_section end end - label(for: "dropzone-file-input", class: "btn btn-primary", style: "cursor: pointer;") do - render Primer::Beta::Octicon.new(icon: :upload, mr: 1) - plain "Upload Files" + div(style: "display: flex; flex-direction: column; align-items: flex-end; gap: 6px;") do + label(for: "dropzone-file-input", class: "btn btn-primary", style: "cursor: pointer;") do + render Primer::Beta::Octicon.new(icon: :upload, mr: 1) + plain "Upload Files" + end + + label(for: "auto-convert-avif", style: "display: flex; align-items: center; gap: 6px; font-size: 13px; color: var(--fgColor-muted, #656d76); cursor: pointer;") do + input( + type: "checkbox", + name: "convert_to_avif", + value: "1", + id: "auto-convert-avif", + form: "dropzone-upload-form", + checked: true, + style: "cursor: pointer;" + ) + plain "Auto-convert images to AVIF" + end end end end @@ -125,7 +140,7 @@ def pagination_section end def dropzone_form - form_with url: uploads_path, method: :post, multipart: true, data: { dropzone_form: true } do + form_with url: uploads_path, method: :post, multipart: true, id: "dropzone-upload-form", data: { dropzone_form: true } do input(type: "file", name: "files[]", id: "dropzone-file-input", multiple: true, data: { dropzone_input: true }, style: "display: none;") end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 157ac29..aca6f41 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -26,7 +26,11 @@ def create return end - service = BatchUploadService.new(user: current_user, provenance: :web) + service = BatchUploadService.new( + user: current_user, + provenance: :web, + convert_to_avif: params[:convert_to_avif] == "1" + ) result = service.process_files(uploaded_files) flash_message = build_flash_message(result) diff --git a/app/services/batch_upload_service.rb b/app/services/batch_upload_service.rb index 2bcc339..7a114fb 100644 --- a/app/services/batch_upload_service.rb +++ b/app/services/batch_upload_service.rb @@ -6,9 +6,10 @@ class BatchUploadService Result = Data.define(:uploads, :failed) FailedUpload = Data.define(:filename, :reason) - def initialize(user:, provenance:) + def initialize(user:, provenance:, convert_to_avif: false) @user = user @provenance = provenance + @convert_to_avif = convert_to_avif @quota_service = QuotaService.new(user) @policy = @quota_service.current_policy end @@ -126,16 +127,27 @@ def create_upload(file) file.content_type || "application/octet-stream" content_type = Upload.normalize_content_type(content_type) + converted_file = nil + + if @convert_to_avif && content_type.start_with?("image/") && content_type != "image/avif" + converted_file = convert_to_avif(file) + converted_file = nil if converted_file.size > @policy.max_file_size + end + + filename = converted_file ? avif_filename(file.original_filename) : file.original_filename + io = converted_file || file.tempfile + content_type = "image/avif" if converted_file upload_id = SecureRandom.uuid_v7 - sanitized_filename = ActiveStorage::Filename.new(file.original_filename).sanitized + sanitized_filename = ActiveStorage::Filename.new(filename).sanitized storage_key = "#{upload_id}/#{sanitized_filename}" blob = ActiveStorage::Blob.create_and_upload!( - io: file.tempfile, - filename: file.original_filename, + io: io, + filename: filename, content_type: content_type, - key: storage_key + key: storage_key, + identify: false ) @user.uploads.create!( @@ -143,6 +155,21 @@ def create_upload(file) blob: blob, provenance: @provenance ) + ensure + converted_file&.close! + end + + def convert_to_avif(file) + ImageProcessing::Vips + .source(file.tempfile.path) + .convert(:avif) + .saver(Q: 80) + .call + end + + def avif_filename(filename) + extension = File.extname(filename) + extension.present? ? "#{filename.delete_suffix(extension)}.avif" : "#{filename}.avif" end def human_size(bytes) diff --git a/test/controllers/uploads_batch_test.rb b/test/controllers/uploads_batch_test.rb index 54b9255..e3910d6 100644 --- a/test/controllers/uploads_batch_test.rb +++ b/test/controllers/uploads_batch_test.rb @@ -21,6 +21,13 @@ def create_upload_for(user, filename: "existing.png") # --- batch upload --- + test "shows the AVIF conversion checkbox enabled by default" do + get uploads_url + + assert_response :success + assert_select 'input[name="convert_to_avif"][checked]' + end + test "uploads multiple files in one request" do files = [ fixture_file_upload("test.png", "image/png"), fixture_file_upload("test.png", "image/png") ] @@ -32,6 +39,18 @@ def create_upload_for(user, filename: "existing.png") assert_match(/Uploaded 2 files/, flash[:notice]) end + test "passes the AVIF conversion choice for an uploaded image" do + post uploads_url, params: { + files: [ fixture_file_upload("test.png", "image/png") ], + convert_to_avif: "1" + } + + assert_redirected_to uploads_path + upload = Upload.order(:created_at).last + assert_equal "test.avif", upload.filename.to_s + assert_equal "image/avif", upload.content_type + end + test "still accepts a single legacy file param" do assert_difference("Upload.count", 1) do post uploads_url, params: { file: fixture_file_upload("test.png", "image/png") } diff --git a/test/services/batch_upload_service_test.rb b/test/services/batch_upload_service_test.rb index d191770..2678d1d 100644 --- a/test/services/batch_upload_service_test.rb +++ b/test/services/batch_upload_service_test.rb @@ -24,8 +24,12 @@ def uploaded_file(name, size) ) end - def service - BatchUploadService.new(user: @user, provenance: :web) + def service(convert_to_avif: false) + BatchUploadService.new( + user: @user, + provenance: :web, + convert_to_avif: convert_to_avif + ) end test "uploads every file when the whole batch fits in quota" do @@ -40,6 +44,26 @@ def service assert_empty result.failed end + test "converts supported images to AVIF when requested" do + image = Rack::Test::UploadedFile.new(file_fixture("test.png").to_path, "image/png", true) + result = service(convert_to_avif: true).process_files([ image ]) + upload = result.uploads.sole + + assert_empty result.failed + assert_equal "test.avif", upload.filename.to_s + assert_equal "image/avif", upload.content_type + end + + test "leaves images unchanged when AVIF conversion is disabled" do + image = Rack::Test::UploadedFile.new(file_fixture("test.png").to_path, "image/png", true) + result = service.process_files([ image ]) + upload = result.uploads.sole + + assert_empty result.failed + assert_equal "test.png", upload.filename.to_s + assert_equal "image/png", upload.content_type + end + test "rejects a file larger than the per-file limit without uploading it" do files = [ uploaded_file("ok.txt", 1.megabyte),