diff --git a/app/jobs/image_processor_job.rb b/app/jobs/image_processor_job.rb index 6fa027e93..1c37040fe 100644 --- a/app/jobs/image_processor_job.rb +++ b/app/jobs/image_processor_job.rb @@ -13,7 +13,7 @@ class ImageProcessorJob < ApplicationJob def perform(blob) return if blob.nil? raise FileNotScannedYetError if blob.virus_scanner.pending? - return if ActiveStorage::Attachment.find_by(blob_id: blob.id).record_type == "ActiveStorage::VariantRecord" + 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) create_variants(blob) if blob.variant_required? diff --git a/app/models/concerns/attachment_image_processor_concern.rb b/app/models/concerns/attachment_image_processor_concern.rb index d1c44609c..459e54d76 100644 --- a/app/models/concerns/attachment_image_processor_concern.rb +++ b/app/models/concerns/attachment_image_processor_concern.rb @@ -16,6 +16,8 @@ module AttachmentImageProcessorConcern def process_image return if blob.nil? + return if blob.attachments.any? { _1.record_type == "Export" } + ImageProcessorJob.perform_later(blob) end end diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index f4f065948..27a02ae7f 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -1429,13 +1429,13 @@ describe Instructeurs::DossiersController, type: :controller do describe '#pieces_jointes' do let(:procedure) { create(:procedure, :published, types_de_champ_public: [{ type: :piece_justificative }], instructeurs:) } let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure: procedure) } + let(:path) { 'spec/fixtures/files/logo_test_procedure.png' } before do dossier.champs.first.piece_justificative_file.attach( - io: StringIO.new("image file"), - filename: "image.jpeg", - content_type: "image/jpeg", - # we don't want to run virus scanner on this file + io: File.open(path), + filename: "logo_test_procedure.png", + content_type: "image/png", metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } ) get :pieces_jointes, params: { @@ -1446,7 +1446,7 @@ describe Instructeurs::DossiersController, type: :controller do it do expect(response.body).to include('Télécharger le fichier toto.txt') - expect(response.body).to include('Télécharger le fichier image.jpeg') + expect(response.body).to include('Télécharger le fichier logo_test_procedure.png') expect(response.body).to include('Visualiser') end end diff --git a/spec/fixtures/files/image-no-exif.jpg b/spec/fixtures/files/image-no-exif.jpg new file mode 100644 index 000000000..4ec032c2b Binary files /dev/null and b/spec/fixtures/files/image-no-exif.jpg differ diff --git a/spec/fixtures/files/image-no-rotation.jpg b/spec/fixtures/files/image-no-rotation.jpg new file mode 100644 index 000000000..07b5159a9 Binary files /dev/null and b/spec/fixtures/files/image-no-rotation.jpg differ diff --git a/spec/jobs/image_processor_job_spec.rb b/spec/jobs/image_processor_job_spec.rb index e337a825f..9c520babd 100644 --- a/spec/jobs/image_processor_job_spec.rb +++ b/spec/jobs/image_processor_job_spec.rb @@ -3,26 +3,27 @@ describe ImageProcessorJob, type: :job do ActiveStorage::Blob.create_and_upload!(io: StringIO.new("toto"), filename: "toto.png") end + let(:blob_jpg) do + ActiveStorage::Blob.create_and_upload!(io: StringIO.new("toto"), filename: "toto.jpg") + end + + let(:attachment) { ActiveStorage::Attachment.new(name: "test", blob: blob) } let(:antivirus_pending) { false } let(:watermark_service) { instance_double("WatermarkService") } + let(:auto_rotate_service) { instance_double("AutoRotateService") } before do virus_scanner_mock = instance_double("ActiveStorage::VirusScanner", pending?: antivirus_pending) + allow(blob).to receive(:attachments).and_return([attachment]) + allow(blob).to receive(:virus_scanner).and_return(virus_scanner_mock) + allow(blob_jpg).to receive(:virus_scanner).and_return(virus_scanner_mock) allow(WatermarkService).to receive(:new).and_return(watermark_service) allow(watermark_service).to receive(:process).and_return(true) - end - context "when watermark is already done" do - before do - allow(blob).to receive(:watermark_done?).and_return(true) - end - - it "does not process the blob" do - expect(watermark_service).not_to receive(:process) - described_class.perform_now(blob) - end + 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 @@ -33,24 +34,102 @@ describe ImageProcessorJob, type: :job do end end - context "when the blob is ready to be watermarked" do - let(:watermarked_file) { Tempfile.new("watermarked.png") } + describe 'autorotate' do + context "when image is not a jpg" do + let(:rotated_file) { Tempfile.new("rotated.png") } - before do - allow(watermarked_file).to receive(:size).and_return(100) + before do + allow(rotated_file).to receive(:size).and_return(100) + end + + it "it does not process autorotate" do + expect(auto_rotate_service).not_to receive(:process) + described_class.perform_now(blob) + end end - it "processes the blob with watermark" do - expect(watermark_service).to receive(:process).and_return(watermarked_file) + context "when image is a jpg " do + let(:rotated_file) { Tempfile.new("rotated.jpg") } - expect { - described_class.perform_now(blob) - }.to change { - blob.reload.checksum + before do + allow(rotated_file).to receive(:size).and_return(100) + end + + it "it processes autorotate" do + expect(auto_rotate_service).to receive(:process).and_return(rotated_file) + described_class.perform_now(blob_jpg) + end + end + end + + describe 'create representation' do + let(:file) { fixture_file_upload('spec/fixtures/files/logo_test_procedure.png', 'image/png') } + 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 - expect(blob.byte_size).to eq(100) - expect(blob.watermarked_at).to be_present + let(:blob) do + blob = ActiveStorage::Blob.create_before_direct_upload!(**blob_info) + blob.upload(file) + blob + end + + context "when representation is not required" do + it "it does not create blob representation" do + expect { described_class.perform_now(blob) }.not_to change { ActiveStorage::VariantRecord.count } + end + end + + context "when representation is required" do + before do + allow(blob).to receive(:variant_required?).and_return(true) + end + + it "it creates blob representation" do + expect { described_class.perform_now(blob) }.to change { ActiveStorage::VariantRecord.count }.by(2) + end + end + end + + describe 'watermark' do + context "when watermark is already done" do + before do + allow(blob).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) + end + end + + context "when the blob is ready to be watermarked" do + let(:watermarked_file) { Tempfile.new("watermarked.png") } + + before do + allow(watermarked_file).to receive(:size).and_return(100) + allow(blob).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) + }.to change { + blob.reload.checksum + } + + expect(blob.byte_size).to eq(100) + expect(blob.watermarked_at).to be_present + end end end end diff --git a/spec/services/auto_rotate_service_spec.rb b/spec/services/auto_rotate_service_spec.rb index 9895be7d6..f8a007f76 100644 --- a/spec/services/auto_rotate_service_spec.rb +++ b/spec/services/auto_rotate_service_spec.rb @@ -1,14 +1,32 @@ RSpec.describe AutoRotateService do let(:image) { file_fixture("image-rotated.jpg") } + let(:image_no_exif) { file_fixture("image-no-exif.jpg") } + let(:image_no_rotation) { file_fixture("image-no-rotation.jpg") } let(:auto_rotate_service) { AutoRotateService.new } describe '#process' do it 'returns a tempfile if auto_rotate succeeds' do Tempfile.create do |output| - auto_rotate_service.process(image, output) + result = auto_rotate_service.process(image, output) expect(MiniMagick::Image.new(image.to_path)["%[orientation]"]).to eq('LeftBottom') expect(MiniMagick::Image.new(output.to_path)["%[orientation]"]).to eq('TopLeft') - expect(output.size).to be_between(image.size / 1.2, image.size) + expect(result.size).to be_between(image.size / 1.2, image.size) + end + end + + it 'returns nil if image does not need to be return' do + Tempfile.create do |output| + result = auto_rotate_service.process(image_no_rotation, output) + expect(MiniMagick::Image.new(image_no_rotation.to_path)["%[orientation]"]).to eq('TopLeft') + expect(result).to eq nil + end + end + + it 'returns nil if no exif info on image' do + Tempfile.create do |output| + result = auto_rotate_service.process(image_no_exif, output) + expect(MiniMagick::Image.new(image_no_exif.to_path)["%[orientation]"]).to eq('Undefined') + expect(result).to eq nil end end end