diff --git a/app/jobs/image_processor_job.rb b/app/jobs/image_processor_job.rb index 9efc16ec1..230db3344 100644 --- a/app/jobs/image_processor_job.rb +++ b/app/jobs/image_processor_job.rb @@ -22,6 +22,7 @@ class ImageProcessorJob < ApplicationJob return if ActiveStorage::Attachment.find_by(blob_id: blob.id)&.record_type == "ActiveStorage::VariantRecord" auto_rotate(blob) if ["image/jpeg", "image/jpg"].include?(blob.content_type) + uninterlace(blob) if blob.content_type == "image/png" create_representations(blob) if blob.representation_required? add_watermark(blob) if blob.watermark_pending? end @@ -40,6 +41,16 @@ class ImageProcessorJob < ApplicationJob end end + def uninterlace(blob) + blob.open do |file| + processed = UninterlaceService.new.process(file) + return if processed.blank? + + blob.upload(processed) + blob.save! + end + end + def create_representations(blob) blob.attachments.each do |attachment| next unless attachment&.representable? diff --git a/app/services/uninterlace_service.rb b/app/services/uninterlace_service.rb new file mode 100644 index 000000000..d4cef3aaf --- /dev/null +++ b/app/services/uninterlace_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class UninterlaceService + def process(file) + uninterlace_png(file) + end + + private + + def uninterlace_png(uploaded_file) + if interlaced?(uploaded_file.to_path) + chunky_img = ChunkyPNG::Image.from_io(uploaded_file.to_io) + chunky_img.save(uploaded_file.to_path, interlace: false) + uploaded_file.reopen(uploaded_file.to_path, 'rb') + end + uploaded_file + end + + def interlaced?(png_path) + png = MiniMagick::Image.open(png_path) + png.data["interlace"] != "None" + end +end diff --git a/spec/jobs/image_processor_job_spec.rb b/spec/jobs/image_processor_job_spec.rb index cc6d4fd9a..78c5702a0 100644 --- a/spec/jobs/image_processor_job_spec.rb +++ b/spec/jobs/image_processor_job_spec.rb @@ -13,6 +13,7 @@ describe ImageProcessorJob, type: :job do let(:antivirus_pending) { false } let(:watermark_service) { instance_double("WatermarkService") } let(:auto_rotate_service) { instance_double("AutoRotateService") } + let(:uninterlace_service) { instance_double("UninterlaceService") } before do virus_scanner_mock = instance_double("ActiveStorage::VirusScanner", pending?: antivirus_pending) @@ -23,9 +24,6 @@ describe ImageProcessorJob, type: :job do allow(WatermarkService).to receive(:new).and_return(watermark_service) allow(watermark_service).to receive(:process).and_return(true) - - allow(AutoRotateService).to receive(:new).and_return(auto_rotate_service) - allow(auto_rotate_service).to receive(:process).and_return(true) end context "when the blob is not scanned yet" do @@ -37,11 +35,33 @@ describe ImageProcessorJob, type: :job do end describe 'autorotate' do + let(:blob_info) do + { + filename: file.original_filename, + byte_size: file.size, + checksum: Digest::SHA256.file(file.path), + content_type: file.content_type, + # we don't want to run virus scanner on this file + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + } + end + + let(:blob) do + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_info) + blob.upload(file) + blob + end + + before do + allow(AutoRotateService).to receive(:new).and_return(auto_rotate_service) + allow(auto_rotate_service).to receive(:process).and_return(true) + end + context "when image is not a jpg" do - let(:rotated_file) { Tempfile.new("rotated.png") } + let(:file) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') } before do - allow(rotated_file).to receive(:size).and_return(100) + allow(file).to receive(:size).and_return(100) end it "it does not process autorotate" do @@ -52,6 +72,7 @@ describe ImageProcessorJob, type: :job do context "when image is a jpg " do let(:rotated_file) { Tempfile.new("rotated.jpg") } + let(:file) { fixture_file_upload('spec/fixtures/files/image-rotated.jpg', 'image/jpeg') } before do allow(rotated_file).to receive(:size).and_return(100) @@ -118,34 +139,67 @@ describe ImageProcessorJob, type: :job do describe 'watermark' do context "when watermark is already done" do before do - allow(blob).to receive(:watermark_done?).and_return(true) + allow(blob_jpg).to receive(:watermark_done?).and_return(true) end it "does not process the watermark" do expect(watermark_service).not_to receive(:process) - described_class.perform_now(blob) + described_class.perform_now(blob_jpg) end end context "when the blob is ready to be watermarked" do - let(:watermarked_file) { Tempfile.new("watermarked.png") } + let(:watermarked_file) { Tempfile.new("watermarked.jpg") } before do allow(watermarked_file).to receive(:size).and_return(100) - allow(blob).to receive(:watermark_pending?).and_return(true) + allow(blob_jpg).to receive(:watermark_pending?).and_return(true) end it "processes the blob with watermark" do expect(watermark_service).to receive(:process).and_return(watermarked_file) expect { - described_class.perform_now(blob) + described_class.perform_now(blob_jpg) }.to change { - blob.reload.checksum + blob_jpg.reload.checksum } - expect(blob.byte_size).to eq(100) - expect(blob.watermarked_at).to be_present + expect(blob_jpg.byte_size).to eq(100) + expect(blob_jpg.watermarked_at).to be_present + end + end + end + + describe 'uninterlace' do + let(:blob_info) do + { + filename: file.original_filename, + byte_size: file.size, + checksum: Digest::SHA256.file(file.path), + content_type: file.content_type, + # we don't want to run virus scanner on this file + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + } + end + + let(:blob) do + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_info) + blob.upload(file) + blob + end + + before do + allow(UninterlaceService).to receive(:new).and_return(uninterlace_service) + end + + context "when file is interlaced" do + let(:file) { fixture_file_upload('spec/fixtures/files/interlaced-black.png', 'image/png') } + let(:uninterlaced_file) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') } + + it "it process uninterlace" do + expect(uninterlace_service).to receive(:process).and_return(uninterlaced_file) + described_class.perform_now(blob) end end end