Merge pull request #10641 from demarches-simplifiees/move-image-treatment-to-jobs

Technique : déplace les traitements d‘images côté jobs

app/controllers/administrateurs/attestation_template_v2s_controller.rb

@@ -2,8 +2,6 @@
 
 module Administrateurs
   class AttestationTemplateV2sController < AdministrateurController
-    include UninterlacePngConcern
-
     before_action :retrieve_procedure
     before_action :ensure_feature_active
     before_action :retrieve_attestation_template
@@ -74,17 +72,6 @@ module Administrateurs
         @attestation_template.procedure = @procedure
       end
 
-      logo_file = attestation_params.delete(:logo)
-      signature_file = attestation_params.delete(:signature)
-
-      if logo_file
-        attestation_params[:logo] = uninterlace_png(logo_file)
-      end
-
-      if signature_file
-        attestation_params[:signature] = uninterlace_png(signature_file)
-      end
-
       @attestation_template.assign_attributes(attestation_params)
 
       if @attestation_template.invalid?

app/controllers/administrateurs/attestation_templates_controller.rb

@@ -2,8 +2,6 @@
 
 module Administrateurs
   class AttestationTemplatesController < AdministrateurController
-    include UninterlacePngConcern
-
     before_action :retrieve_procedure
     before_action :preload_revisions
 
@@ -63,16 +61,6 @@ module Administrateurs
       if @activated_attestation_params.nil?
         @activated_attestation_params = params.require(:attestation_template)
           .permit(:title, :body, :footer, :activated, :logo, :signature)
-
-        logo_file = params['attestation_template'].delete('logo')
-        signature_file = params['attestation_template'].delete('signature')
-
-        if logo_file.present?
-          @activated_attestation_params[:logo] = uninterlace_png(logo_file)
-        end
-        if signature_file.present?
-          @activated_attestation_params[:signature] = uninterlace_png(signature_file)
-        end
       end
 
       @activated_attestation_params

app/controllers/administrateurs/groupe_instructeurs_controller.rb

@@ -5,7 +5,6 @@ module Administrateurs
     include ActiveSupport::NumberHelper
     include EmailSanitizableConcern
     include Logic
-    include UninterlacePngConcern
     include GroupeInstructeursSignatureConcern
 
     before_action :ensure_not_super_admin!, only: [:add_instructeur]

app/controllers/concerns/groupe_instructeurs_signature_concern.rb

@@ -19,9 +19,7 @@ module GroupeInstructeursSignatureConcern
         flash[:alert] = "Aucun fichier joint pour le tampon de l'attestation"
         render :show
       else
-        signature = uninterlace_png(signature_file)
+        if @groupe_instructeur.signature.attach(signature_file)
-
-        if @groupe_instructeur.signature.attach(signature)
           handle_redirect :success
         else
           handle_redirect :alert

app/controllers/concerns/uninterlace_png_concern.rb

@@ -1,21 +0,0 @@
-# frozen_string_literal: true
-
-module UninterlacePngConcern
-  extend ActiveSupport::Concern
-
-  private
-
-  def uninterlace_png(uploaded_file)
-    if uploaded_file&.content_type == 'image/png' && interlaced?(uploaded_file.tempfile.to_path)
-      chunky_img = ChunkyPNG::Image.from_io(uploaded_file.to_io)
-      chunky_img.save(uploaded_file.tempfile.to_path, interlace: false)
-      uploaded_file.tempfile.reopen(uploaded_file.tempfile.to_path, 'rb')
-    end
-    uploaded_file
-  end
-
-  def interlaced?(png_path)
-    png = MiniMagick::Image.open(png_path)
-    png.data["interlace"] != "None"
-  end
-end

app/controllers/instructeurs/groupe_instructeurs_controller.rb

@@ -3,7 +3,6 @@
 module Instructeurs
   class GroupeInstructeursController < InstructeurController
     include EmailSanitizableConcern
-    include UninterlacePngConcern
     include GroupeInstructeursSignatureConcern
 
     before_action :ensure_allowed!

app/controllers/procedures_controller.rb

@@ -5,7 +5,12 @@ class ProceduresController < ApplicationController
 
   def logo
     if @procedure.logo.attached?
-      redirect_to url_for(@procedure.logo.variant(:email))
+      logo_variant = @procedure.logo.variant(resize_to_limit: [400, 400])
+      if logo_variant.key.present?
+        redirect_to url_for(logo_variant.processed)
+      else
+        redirect_to url_for(@procedure.logo)
+      end
     else
       redirect_to ActionController::Base.helpers.image_url(PROCEDURE_DEFAULT_LOGO_SRC)
     end

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?
@@ -47,6 +58,9 @@ class ImageProcessorJob < ApplicationJob
       if attachment.blob.content_type.in?(RARE_IMAGE_TYPES)
         attachment.variant(resize_to_limit: [2000, 2000]).processed
       end
+      if attachment.record.class == ActionText::RichText
+        attachment.variant(resize_to_limit: [1024, 768]).processed
+      end
     end
   end
 

app/models/attestation_template.rb

@@ -106,7 +106,8 @@ class AttestationTemplate < ApplicationRecord
 
   def logo_url
     if logo.attached?
-      Rails.application.routes.url_helpers.url_for(logo)
+      logo_variant = logo.variant(resize_to_limit: [400, 400])
+      logo_variant.key.present? ? logo_variant.processed.url : Rails.application.routes.url_helpers.url_for(logo)
     end
   end
 

app/models/concerns/blob_image_processor_concern.rb

@@ -10,7 +10,7 @@ module BlobImageProcessorConcern
   end
 
   def representation_required?
-    from_champ? || from_messagerie?
+    from_champ? || from_messagerie? || logo? || from_action_text?
   end
 
   private
@@ -23,6 +23,14 @@ module BlobImageProcessorConcern
     attachments.any? { _1.record.class == Commentaire }
   end
 
+  def logo?
+    attachments.any? { _1.name == 'logo' }
+  end
+
+  def from_action_text?
+    attachments.any? { _1.record.class == ActionText::RichText }
+  end
+
   def watermark_required?
     attachments.any? { _1.record.class == Champs::TitreIdentiteChamp }
   end

app/models/procedure.rb

@@ -146,9 +146,7 @@ class Procedure < ApplicationRecord
 
   belongs_to :defaut_groupe_instructeur, class_name: 'GroupeInstructeur', inverse_of: false, optional: true
 
-  has_one_attached :logo do |attachable|
+  has_one_attached :logo
-    attachable.variant :email, resize_to_limit: [450, 450]
-  end
   has_one_attached :notice
   has_one_attached :deliberation
 
@@ -679,7 +677,8 @@ class Procedure < ApplicationRecord
 
   def logo_url
     if logo.attached?
-      Rails.application.routes.url_helpers.url_for(logo)
+      logo_variant = logo.variant(resize_to_limit: [400, 400])
+      logo_variant.key.present? ? logo_variant.processed.url : Rails.application.routes.url_helpers.url_for(logo)
     else
       ActionController::Base.helpers.image_url(PROCEDURE_DEFAULT_LOGO_SRC)
     end

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

app/views/active_storage/blobs/_blob.html.erb

@@ -1,14 +1,19 @@
-<figure class="attachment attachment--<%= blob.representable? ? "preview" : "file" %> attachment--<%= blob.filename.extension %>">
+<% if blob.representable? %>
-  <% if blob.representable? %>
+  <% representation = blob.representation(resize_to_limit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %>
-    <%= image_tag blob.representation(resize_to_limit: local_assigns[:in_gallery] ? [ 800, 600 ] : [ 1024, 768 ]) %>
+  <% if representation.image&.attached? || representation.key.present? %>
-  <% end %>
+    <figure class="attachment attachment--preview attachment--<%= blob.filename.extension %>">
-
+      <%= image_tag representation %>
-  <figcaption class="attachment__caption">
-    <% if caption = blob.try(:caption) %>
-      <%= caption %>
     <% else %>
-      <span class="attachment__name"><%= blob.filename %></span>
+    <figure class="attachment attachment--file attachment--<%= blob.filename.extension %>">
-      <span class="attachment__size"><%= number_to_human_size blob.byte_size %></span>
+      <%= image_tag blob %>
     <% end %>
-  </figcaption>
+    </figure>
-</figure>
+      <figcaption class="attachment__caption">
+        <% if caption = blob.try(:caption) %>
+          <%= caption %>
+        <% else %>
+          <span class="attachment__name"><%= blob.filename %></span>
+          <span class="attachment__size"><%= number_to_human_size blob.byte_size %></span>
+        <% end %>
+      </figcaption>
+<% end %>

spec/controllers/administrateurs/attestation_templates_controller_spec.rb

@@ -8,8 +8,6 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do
   let(:logo2) { fixture_file_upload('spec/fixtures/files/white.png', 'image/png') }
   let(:signature) { fixture_file_upload('spec/fixtures/files/black.png', 'image/png') }
   let(:signature2) { fixture_file_upload('spec/fixtures/files/black.png', 'image/png') }
-  let(:interlaced_logo) { fixture_file_upload('spec/fixtures/files/interlaced-black.png', 'image/png') }
-  let(:uninterlaced_logo) { fixture_file_upload('spec/fixtures/files/uninterlaced-black.png', 'image/png') }
   let(:invalid_logo) { fixture_file_upload('spec/fixtures/files/invalid_file_format.json', 'application/json') }
 
   before do

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