DGNum/demarches-normaliennes
15
1
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity

Merge pull request #7065 from betagouv/US/fix-conflicting-archive-file-name

Browse source 
fix(ProcedureArchiveService.zip_root_folder): should take archive instance otherwise when we generate many archive for the same procedure, errors may occures
This commit is contained in:
mfo 2022-03-30 16:36:36 +02:00 committed by GitHub
commit 3b5b9a63c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
app/services/procedure_archive_service.rb
View file

@ -26,7 +26,7 @@ class ProcedureArchiveService
end end
attachments = create_list_of_attachments(dossiers) attachments = create_list_of_attachments(dossiers)
download_and_zip(attachments) do |zip_filepath| download_and_zip(archive, attachments) do |zip_filepath|
ArchiveUploader.new(procedure: @procedure, archive: archive, filepath: zip_filepath) ArchiveUploader.new(procedure: @procedure, archive: archive, filepath: zip_filepath)
.upload .upload
end end
@ -46,10 +46,10 @@ class ProcedureArchiveService
private private
def download_and_zip(attachments, &block) def download_and_zip(archive, attachments, &block)
Dir.mktmpdir(nil, ARCHIVE_CREATION_DIR) do |tmp_dir| Dir.mktmpdir(nil, ARCHIVE_CREATION_DIR) do |tmp_dir|
archive_dir = File.join(tmp_dir, zip_root_folder) archive_dir = File.join(tmp_dir, zip_root_folder(archive))
zip_path = File.join(ARCHIVE_CREATION_DIR, "#{zip_root_folder}.zip") zip_path = File.join(ARCHIVE_CREATION_DIR, "#{zip_root_folder(archive)}.zip")
begin begin
FileUtils.remove_entry_secure(archive_dir) if Dir.exist?(archive_dir) FileUtils.remove_entry_secure(archive_dir) if Dir.exist?(archive_dir)
@ -60,7 +60,7 @@ class ProcedureArchiveService
Dir.chdir(tmp_dir) do Dir.chdir(tmp_dir) do
File.delete(zip_path) if File.exist?(zip_path) File.delete(zip_path) if File.exist?(zip_path)
system 'zip', '-0', '-r', zip_path, zip_root_folder system 'zip', '-0', '-r', zip_path, zip_root_folder(archive)
end end
yield(zip_path) yield(zip_path)
ensure ensure
@ -70,8 +70,8 @@ class ProcedureArchiveService
end end
end end
def zip_root_folder def zip_root_folder(archive)
"procedure-#{@procedure.id}" "procedure-#{@procedure.id}-#{archive.id}"
end end
def create_list_of_attachments(dossiers) def create_list_of_attachments(dossiers)

65
spec/services/procedure_archive_service_spec.rb
View file

@ -55,11 +55,11 @@ describe ProcedureArchiveService do
files = ZipTricks::FileReader.read_zip_structure(io: f) files = ZipTricks::FileReader.read_zip_structure(io: f)
structure = [ structure = [
"procedure-#{procedure.id}/", "procedure-#{procedure.id}-#{archive.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/attestation-dossier--05-03-2021-00-00-#{dossier.attestation.pdf.id % 10000}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/attestation-dossier--05-03-2021-00-00-#{dossier.attestation.pdf.id % 10000}.pdf",
"procedure-#{procedure.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf" "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf"
] ]
expect(files.map(&:filename)).to match_array(structure) expect(files.map(&:filename)).to match_array(structure)
end end
@ -75,11 +75,11 @@ describe ProcedureArchiveService do
archive.file.open do |f| archive.file.open do |f|
files = ZipTricks::FileReader.read_zip_structure(io: f) files = ZipTricks::FileReader.read_zip_structure(io: f)
structure = [ structure = [
"procedure-#{procedure.id}/", "procedure-#{procedure.id}-#{archive.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf",
"procedure-#{procedure.id}/LISEZMOI.txt" "procedure-#{procedure.id}-#{archive.id}/LISEZMOI.txt"
] ]
expect(files.map(&:filename)).to match_array(structure) expect(files.map(&:filename)).to match_array(structure)
end end
@ -122,16 +122,16 @@ describe ProcedureArchiveService do
archive.file.open do |f| archive.file.open do |f|
zip_entries = ZipTricks::FileReader.read_zip_structure(io: f) zip_entries = ZipTricks::FileReader.read_zip_structure(io: f)
structure = [ structure = [
"procedure-#{procedure.id}/", "procedure-#{procedure.id}-#{archive.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/export-dossier-05-03-2020-00-00-1.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/export-dossier-05-03-2020-00-00-1.pdf",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf",
"procedure-#{procedure.id}/LISEZMOI.txt" "procedure-#{procedure.id}-#{archive.id}/LISEZMOI.txt"
] ]
expect(zip_entries.map(&:filename)).to match_array(structure) expect(zip_entries.map(&:filename)).to match_array(structure)
zip_entries.map do |entry| zip_entries.map do |entry|
next unless entry.filename == "procedure-#{procedure.id}/LISEZMOI.txt" next unless entry.filename == "procedure-#{procedure.id}-#{archive.id}/LISEZMOI.txt"
extracted_content = "" extracted_content = ""
extractor = entry.extractor_from(f) extractor = entry.extractor_from(f)
extracted_content << extractor.extract(1024 * 1024) until extractor.eof? extracted_content << extractor.extract(1024 * 1024) until extractor.eof?
@ -158,15 +158,15 @@ describe ProcedureArchiveService do
archive.file.open do |f| archive.file.open do |f|
files = ZipTricks::FileReader.read_zip_structure(io: f) files = ZipTricks::FileReader.read_zip_structure(io: f)
structure = [ structure = [
"procedure-#{procedure.id}/", "procedure-#{procedure.id}-#{archive.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/",
"procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/attestation-dossier--05-03-2020-00-00-#{dossier.attestation.pdf.id % 10000}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/pieces_justificatives/attestation-dossier--05-03-2020-00-00-#{dossier.attestation.pdf.id % 10000}.pdf",
"procedure-#{procedure.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2020-00-00-#{dossier.id}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2020-00-00-#{dossier.id}.pdf",
"procedure-#{procedure.id}/dossier-#{dossier_2020.id}/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier_2020.id}/",
"procedure-#{procedure.id}/dossier-#{dossier_2020.id}/export-#{dossier_2020.id}-05-03-2020-00-00-#{dossier_2020.id}.pdf", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier_2020.id}/export-#{dossier_2020.id}-05-03-2020-00-00-#{dossier_2020.id}.pdf",
"procedure-#{procedure.id}/dossier-#{dossier_2020.id}/pieces_justificatives/", "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier_2020.id}/pieces_justificatives/",
"procedure-#{procedure.id}/dossier-#{dossier_2020.id}/pieces_justificatives/attestation-dossier--05-03-2020-00-00-#{dossier_2020.attestation.pdf.id % 10000}.pdf" "procedure-#{procedure.id}-#{archive.id}/dossier-#{dossier_2020.id}/pieces_justificatives/attestation-dossier--05-03-2020-00-00-#{dossier_2020.attestation.pdf.id % 10000}.pdf"
] ]
expect(files.map(&:filename)).to match_array(structure) expect(files.map(&:filename)).to match_array(structure)
end end
@ -176,29 +176,30 @@ describe ProcedureArchiveService do
end end
describe '#download_and_zip' do describe '#download_and_zip' do
let(:archive) { build(:archive, id: '3') }
it 'create a tmpdir while block is running' do it 'create a tmpdir while block is running' do
previous_dir_list = Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR) previous_dir_list = Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR)
service.send(:download_and_zip, []) do |_zip_file| service.send(:download_and_zip, archive, []) do |_zip_file|
new_dir_list = Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR) new_dir_list = Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR)
expect(previous_dir_list).not_to eq(new_dir_list) expect(previous_dir_list).not_to eq(new_dir_list)
end end
end end
it 'cleans up its tmpdir after block execution' do it 'cleans up its tmpdir after block execution' do
expect { service.send(:download_and_zip, []) { |zip_file| } } expect { service.send(:download_and_zip, archive, []) { |zip_file| } }
.not_to change { Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR) } .not_to change { Dir.entries(ProcedureArchiveService::ARCHIVE_CREATION_DIR) }
end end
it 'creates a zip with zip utility' do it 'creates a zip with zip utility' do
expected_zip_path = File.join(ProcedureArchiveService::ARCHIVE_CREATION_DIR, "#{service.send(:zip_root_folder)}.zip") expected_zip_path = File.join(ProcedureArchiveService::ARCHIVE_CREATION_DIR, "#{service.send(:zip_root_folder, archive)}.zip")
expect(service).to receive(:system).with('zip', '-0', '-r', expected_zip_path, an_instance_of(String)) expect(service).to receive(:system).with('zip', '-0', '-r', expected_zip_path, an_instance_of(String))
service.send(:download_and_zip, []) { |zip_path| } service.send(:download_and_zip, archive, []) { |zip_path| }
end end
it 'cleans up its generated zip' do it 'cleans up its generated zip' do
expected_zip_path = File.join(ProcedureArchiveService::ARCHIVE_CREATION_DIR, "#{service.send(:zip_root_folder)}.zip") expected_zip_path = File.join(ProcedureArchiveService::ARCHIVE_CREATION_DIR, "#{service.send(:zip_root_folder, archive)}.zip")
service.send(:download_and_zip, []) do |_zip_path| service.send(:download_and_zip, archive, []) do |_zip_path|
expect(File.exist?(expected_zip_path)).to be_truthy expect(File.exist?(expected_zip_path)).to be_truthy
end end
expect(File.exist?(expected_zip_path)).to be_falsey expect(File.exist?(expected_zip_path)).to be_falsey