From cbe77dd2f6896e573a3bfb7bcc75f0a88534150d Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Sep 2022 11:20:30 +0200 Subject: [PATCH] bug(instructeurs/dossiers#telecharger_pjs): zipline does not play well with not available active storage attachments --- app/controllers/experts/avis_controller.rb | 3 +- .../instructeurs/dossiers_controller.rb | 3 +- app/lib/active_storage/downloadable_file.rb | 21 ++++++++ .../instructeurs/dossiers_controller_spec.rb | 11 ++++ .../active_storage/downloadable_file_spec.rb | 52 +++++++++++++++++++ 5 files changed, 88 insertions(+), 2 deletions(-) diff --git a/app/controllers/experts/avis_controller.rb b/app/controllers/experts/avis_controller.rb index 108cc377f..ac33d5c2d 100644 --- a/app/controllers/experts/avis_controller.rb +++ b/app/controllers/experts/avis_controller.rb @@ -132,8 +132,9 @@ module Experts def telecharger_pjs files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: @dossier.id), true) + cleaned_files = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(files) - zipline(files, "dossier-#{@dossier.id}.zip") + zipline(cleaned_files, "dossier-#{@dossier.id}.zip") end private diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index d6f4cd03f..5af55767e 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -228,8 +228,9 @@ module Instructeurs def telecharger_pjs files = ActiveStorage::DownloadableFile.create_list_from_dossiers(Dossier.where(id: dossier.id)) + cleaned_files = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(files) - zipline(files, "dossier-#{dossier.id}.zip") + zipline(cleaned_files, "dossier-#{dossier.id}.zip") end def destroy diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index ac1cb2a8d..21fd434cf 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -4,6 +4,27 @@ class ActiveStorage::DownloadableFile PiecesJustificativesService.liste_documents(dossiers, for_expert) end + def self.cleanup_list_from_dossier(files) + if Rails.application.config.active_storage.service != :openstack + return files + end + + files.filter do |file, _filename| + if file.is_a?(PiecesJustificativesService::FakeAttachment) + true + else + service = file.blob.service + client = service.client + begin + client.head_object(service.container, file.blob.key) + true + rescue Fog::OpenStack::Storage::NotFound + false + end + end + end + end + private def self.bill_and_path(bill) diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index c21add410..6ca62a095 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -769,6 +769,17 @@ describe Instructeurs::DossiersController, type: :controller do dossier_id: dossier.id } end + + it 'includes an attachment' do + expect(subject.headers['Content-Disposition']).to start_with('attachment; ') + end + + it 'the attachment.zip is extractable' do + content = ZipTricks::FileReader.read_zip_structure(io: StringIO.new(subject.body)) + file_names = content.map(&:filename) + expect(file_names.size).to eq(1) + expect(file_names.first).to start_with("dossier-#{dossier.id}/export-") + end end describe "#destroy" do diff --git a/spec/lib/active_storage/downloadable_file_spec.rb b/spec/lib/active_storage/downloadable_file_spec.rb index f2e35f767..93fcccc98 100644 --- a/spec/lib/active_storage/downloadable_file_spec.rb +++ b/spec/lib/active_storage/downloadable_file_spec.rb @@ -9,4 +9,56 @@ describe ActiveStorage::DownloadableFile do it { expect(list.first[0].name).to eq "pdf_export_for_instructeur" } end end + + describe '.cleanup_list_from_dossier' do + context 'active_storage.service test' do + before { Rails.application.config.active_storage.service = :test } + it 'returns the list' do + list = [:a, :b, :c] + result = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(list) + expect(list).to eq(result) + end + end + + context 'active_storage.service local' do + before { Rails.application.config.active_storage.service = :local } + it 'returns the list' do + list = [:a, :b, :c] + result = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(list) + expect(list).to eq(result) + end + end + + context 'active_storage.service openstack' do + let(:object_storage_container) { 'object_storage' } + let(:available_blob_key) { 'available' } + let(:unavailable_blob_key) { 'broken' } + let(:active_storage_client) { double } + let(:active_storage_service) { double(client: active_storage_client, container: object_storage_container) } + + before do + require 'fog/openstack' + Rails.application.config.active_storage.service = :openstack + end + + it 'returns the list' do + available_blob = double(key: available_blob_key) + unavailable_blob = double(key: unavailable_blob_key) + [available_blob, unavailable_blob].map do |attachment| + allow(attachment).to receive(:service).and_return(active_storage_service) + end + expect(active_storage_client).to receive(:head_object).with(object_storage_container, available_blob_key).and_return(true) + expect(active_storage_client).to receive(:head_object).with(object_storage_container, unavailable_blob_key).and_raise(Fog::OpenStack::Storage::NotFound.new('Object storage 99.99% availability leave space to 0.01% failure')) + + list = [ + [instance_double(ActiveStorage::Attachment, blob: available_blob), 'filename.pdf'], + [instance_double(ActiveStorage::Attachment, blob: unavailable_blob), 'filename.pdf'] + ] + + result = ActiveStorage::DownloadableFile.cleanup_list_from_dossier(list) + expect(result.size).to eq(1) + expect(result.first.first.blob.key).to eq(available_blob_key) + end + end + end end