diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 11e97ca81..89d52a61d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,7 +81,8 @@ jobs: - name: Install build dependancies # - fonts pickable by ImageMagick # - rust for YJIT support - run: sudo apt-get update && sudo apt-get install -y gsfonts rustc redis-server + # - poppler-utils for pdf previews + run: sudo apt-get update && sudo apt-get install -y gsfonts rustc redis-server poppler-utils - name: Setup the app runtime and dependencies uses: ./.github/actions/ci-setup-rails diff --git a/app/helpers/gallery_helper.rb b/app/helpers/gallery_helper.rb new file mode 100644 index 000000000..98f7ffc1e --- /dev/null +++ b/app/helpers/gallery_helper.rb @@ -0,0 +1,27 @@ +module GalleryHelper + def displayable_pdf?(blob) + blob.previewable? && blob.content_type.in?(AUTHORIZED_PDF_TYPES) + end + + def displayable_image?(blob) + blob.variable? && blob.content_type.in?(AUTHORIZED_IMAGE_TYPES) + end + + def preview_url_for(attachment) + attachment.preview(resize_to_limit: [400, 400]).processed.url + rescue ActiveStorage::Error + 'pdf-placeholder.png' + end + + def variant_url_for(attachment) + attachment.variant(resize_to_limit: [400, 400]).processed.url + rescue ActiveStorage::Error + 'apercu-indisponible.png' + end + + def blob_url(attachment) + attachment.blob.content_type.in?(RARE_IMAGE_TYPES) ? attachment.variant(resize_to_limit: [2000, 2000]).processed.url : attachment.blob.url + rescue ActiveStorage::Error + attachment.blob.url + end +end diff --git a/app/jobs/image_processor_job.rb b/app/jobs/image_processor_job.rb index 8017d2b35..ee914a925 100644 --- a/app/jobs/image_processor_job.rb +++ b/app/jobs/image_processor_job.rb @@ -10,6 +10,10 @@ class ImageProcessorJob < ApplicationJob # (to avoid modifying the file while it is being scanned). retry_on FileNotScannedYetError, wait: :exponentially_longer, attempts: 10 + rescue_from ActiveStorage::PreviewError do + retry_or_discard + end + def perform(blob) return if blob.nil? raise FileNotScannedYetError if blob.virus_scanner.pending? @@ -38,6 +42,9 @@ class ImageProcessorJob < ApplicationJob blob.attachments.each do |attachment| next unless attachment&.representable? attachment.representation(resize_to_limit: [400, 400]).processed + if attachment.blob.content_type.in?(RARE_IMAGE_TYPES) + attachment.variant(resize_to_limit: [2000, 2000]).processed + end end end @@ -55,4 +62,14 @@ class ImageProcessorJob < ApplicationJob end end end + + def retry_or_discard + if executions < max_attempts + retry_job wait: 5.minutes + end + end + + def max_attempts + 3 + end end diff --git a/app/views/instructeurs/dossiers/pieces_jointes.html.haml b/app/views/instructeurs/dossiers/pieces_jointes.html.haml index 58a867ccd..d770433d4 100644 --- a/app/views/instructeurs/dossiers/pieces_jointes.html.haml +++ b/app/views/instructeurs/dossiers/pieces_jointes.html.haml @@ -8,20 +8,20 @@ - champ.piece_justificative_file.with_all_variant_records.each do |attachment| .gallery-item - blob = attachment.blob - - if blob.content_type.in?(AUTHORIZED_PDF_TYPES) + - if displayable_pdf?(blob) = link_to blob.url, id: blob.id, data: { iframe: true, src: blob.url }, class: 'gallery-link', type: blob.content_type, title: "#{champ.libelle} -- #{blob.filename}" do .thumbnail - = image_tag("pdf-placeholder.png") + = image_tag(preview_url_for(attachment), loading: :lazy) .fr-btn.fr-btn--tertiary.fr-btn--icon-left.fr-icon-eye{ role: :button } Visualiser .champ-libelle = champ.libelle.truncate(25) = render Attachment::ShowComponent.new(attachment: attachment, truncate: true) - - elsif blob.content_type.in?(AUTHORIZED_IMAGE_TYPES) - = link_to image_url(blob.url), title: "#{champ.libelle} -- #{blob.filename}", data: { src: blob.url }, class: 'gallery-link' do + - elsif displayable_image?(blob) + = link_to image_url(blob_url(attachment)), title: "#{champ.libelle} -- #{blob.filename}", data: { src: blob.url }, class: 'gallery-link' do .thumbnail - = image_tag(attachment.variant(resize_to_limit: [400, 400]).processed.url, loading: :lazy) + = image_tag(variant_url_for(attachment), loading: :lazy) .fr-btn.fr-btn--tertiary.fr-btn--icon-left.fr-icon-eye{ role: :button } Visualiser .champ-libelle diff --git a/app/views/shared/champs/piece_justificative/_show.html.haml b/app/views/shared/champs/piece_justificative/_show.html.haml index 734aeadd8..abb301872 100644 --- a/app/views/shared/champs/piece_justificative/_show.html.haml +++ b/app/views/shared/champs/piece_justificative/_show.html.haml @@ -8,17 +8,17 @@ - champ.piece_justificative_file.attachments.with_all_variant_records.each do |attachment| .gallery-item - blob = attachment.blob - - if blob.content_type.in?(AUTHORIZED_PDF_TYPES) + - if displayable_pdf?(blob) = link_to blob.url, id: blob.id, data: { iframe: true, src: blob.url }, class: 'gallery-link', type: blob.content_type, title: "#{champ.libelle} -- #{blob.filename}" do .thumbnail - = image_tag("pdf-placeholder.png") + = image_tag(preview_url_for(attachment), loading: :lazy) .fr-btn.fr-btn--tertiary.fr-btn--icon-left.fr-icon-eye{ role: :button } = 'Visualiser' - - elsif blob.content_type.in?(AUTHORIZED_IMAGE_TYPES) - = link_to image_url(blob.url), title: "#{champ.libelle} -- #{blob.filename}", data: { src: blob.url }, class: 'gallery-link' do + - elsif displayable_image?(blob) + = link_to image_url(blob_url(attachment)), title: "#{champ.libelle} -- #{blob.filename}", data: { src: blob.url }, class: 'gallery-link' do .thumbnail - = image_tag(attachment.variant(resize_to_limit: [400, 400]).processed.url, loading: :lazy) + = image_tag(variant_url_for(attachment), loading: :lazy) .fr-btn.fr-btn--tertiary.fr-btn--icon-left.fr-icon-eye{ role: :button } = 'Visualiser' - else diff --git a/config/initializers/authorized_content_types.rb b/config/initializers/authorized_content_types.rb index e5af1c74f..eaa16fbd9 100644 --- a/config/initializers/authorized_content_types.rb +++ b/config/initializers/authorized_content_types.rb @@ -15,6 +15,10 @@ AUTHORIZED_IMAGE_TYPES = [ 'image/vnd.dwg' # multimedia x 137 auto desk ] +RARE_IMAGE_TYPES = [ + 'image/tiff' # multimedia x 3985 +] + AUTHORIZED_CONTENT_TYPES = AUTHORIZED_IMAGE_TYPES + AUTHORIZED_PDF_TYPES + [ # multimedia 'video/mp4', # multimedia x 2075 diff --git a/spec/fixtures/files/pencil.tiff b/spec/fixtures/files/pencil.tiff new file mode 100644 index 000000000..67af5a81a Binary files /dev/null and b/spec/fixtures/files/pencil.tiff differ diff --git a/spec/helpers/gallery_helper_spec.rb b/spec/helpers/gallery_helper_spec.rb new file mode 100644 index 000000000..41469798c --- /dev/null +++ b/spec/helpers/gallery_helper_spec.rb @@ -0,0 +1,57 @@ +RSpec.describe GalleryHelper, type: :helper do + let(:procedure) { create(:procedure_with_dossiers) } + let(:type_de_champ_pj) { create(:type_de_champ_piece_justificative, stable_id: 3, libelle: 'Justificatif de domicile', procedure:) } + let(:champ_pj) { create(:champ_piece_justificative, type_de_champ: type_de_champ_pj) } + 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 + let(:attachment) { ActiveStorage::Attachment.create(name: "test", blob: blob, record: champ_pj) } + + describe ".variant_url_for" do + subject { variant_url_for(attachment) } + + context "when attachment can be represented with a variant" do + let(:file) { fixture_file_upload('spec/fixtures/files/logo_test_procedure.png', 'image/png') } + + it { expect { subject }.to change { ActiveStorage::VariantRecord.count }.by(1) } + it { is_expected.not_to eq("apercu-indisponible.png") } + end + + context "when attachment cannot be represented with a variant" do + let(:file) { fixture_file_upload('spec/fixtures/files/instructeurs-file.csv', 'text/csv') } + + it { expect { subject }.not_to change { ActiveStorage::VariantRecord.count } } + it { is_expected.to eq("apercu-indisponible.png") } + end + end + + describe ".preview_url_for" do + subject { preview_url_for(attachment) } + + context "when attachment can be represented with a preview" do + let(:file) { fixture_file_upload('spec/fixtures/files/RIB.pdf', 'application/pdf') } + + it { expect { subject }.to change { ActiveStorage::VariantRecord.count }.by(1) } + it { is_expected.not_to eq("pdf-placeholder.png") } + end + + context "when attachment cannot be represented with a preview" do + let(:file) { fixture_file_upload('spec/fixtures/files/instructeurs-file.csv', 'text/csv') } + + it { expect { subject }.not_to change { ActiveStorage::VariantRecord.count } } + it { is_expected.to eq("pdf-placeholder.png") } + end + end +end diff --git a/spec/jobs/image_processor_job_spec.rb b/spec/jobs/image_processor_job_spec.rb index e32999273..91d7de2c1 100644 --- a/spec/jobs/image_processor_job_spec.rb +++ b/spec/jobs/image_processor_job_spec.rb @@ -63,7 +63,6 @@ describe ImageProcessorJob, type: :job do 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, @@ -81,19 +80,35 @@ describe ImageProcessorJob, type: :job do 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 } + context "when type image is usual" do + let(:file) { fixture_file_upload('spec/fixtures/files/logo_test_procedure.png', 'image/png') } + + 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(:representation_required?).and_return(true) + end + + it "it creates blob representation" do + expect { described_class.perform_now(blob) }.to change { ActiveStorage::VariantRecord.count }.by(1) + end end end - context "when representation is required" do + context "when type image is rare" do + let(:file) { fixture_file_upload('spec/fixtures/files/pencil.tiff', 'image/tiff') } + before do allow(blob).to receive(:representation_required?).and_return(true) end - it "it creates blob representation" do - expect { described_class.perform_now(blob) }.to change { ActiveStorage::VariantRecord.count }.by(1) + it "creates a second variant" do + expect { described_class.perform_now(blob) }.to change { ActiveStorage::VariantRecord.count }.by(2) end end end