chore: move uninterlace to image processor job

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?

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

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_jpg.byte_size).to eq(100)
-        expect(blob.watermarked_at).to be_present
+        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