diff options
| -rw-r--r-- | app/services/zip_service.rb | 18 | ||||
| -rw-r--r-- | spec/fixtures/OFFRE_WITH_EXTRA.zip | bin | 0 -> 5586 bytes | |||
| -rw-r--r-- | spec/services/zip_service/regression_4273_spec.rb | 59 | ||||
| -rw-r--r-- | spec/services/zip_service_spec.rb | 68 |
4 files changed, 82 insertions, 63 deletions
diff --git a/app/services/zip_service.rb b/app/services/zip_service.rb index cab301b01..7a4bdad1b 100644 --- a/app/services/zip_service.rb +++ b/app/services/zip_service.rb @@ -1,10 +1,9 @@ class ZipService - # TODO: Remove me before merge https://github.com/rubyzip/rubyzip - class Subdir < Struct.new(:name, :stream) + class Subdir < Struct.new(:name, :stream, :spurious) end - attr_reader :current_key, :current_output, :yielder + attr_reader :current_key, :current_output, :current_spurious, :yielder def initialize data @zip_data = StringIO.new(data) @@ -36,6 +35,7 @@ class ZipService end def add_to_current_output entry + return if is_spurious! entry.name current_output.put_next_entry entry.name write_to_current_output entry.get_input_stream end @@ -51,7 +51,8 @@ class ZipService @yielder << Subdir.new( current_key, # Second part of the solution, yield the closed stream - current_output.close_buffer) + current_output.close_buffer, + current_spurious) end end @@ -59,10 +60,19 @@ class ZipService @current_key = entry_key # First piece of the solution, use internal way to create a Zip::OutputStream @current_output = Zip::OutputStream.new(StringIO.new(''), true, nil) + @current_spurious = [] end def entry_key entry # last dir name File.dirname.split("/").last entry.name.split('/', -1)[-2] end + + def is_spurious! entry_name + segments = entry_name.split('/', 3) + return false if segments.size < 3 + + current_spurious << segments.second + return true + end end diff --git a/spec/fixtures/OFFRE_WITH_EXTRA.zip b/spec/fixtures/OFFRE_WITH_EXTRA.zip Binary files differnew file mode 100644 index 000000000..97ea3f513 --- /dev/null +++ b/spec/fixtures/OFFRE_WITH_EXTRA.zip diff --git a/spec/services/zip_service/regression_4273_spec.rb b/spec/services/zip_service/regression_4273_spec.rb deleted file mode 100644 index 4fe0f6539..000000000 --- a/spec/services/zip_service/regression_4273_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -RSpec.describe ZipService do - describe 'Regression Issue # 4273 https://projects.af83.io/issues/4273' do - let( :zip_service ){ described_class } - let( :unzipper ){ zip_service.new(zip_data) } - let( :zip_data ){ File.read zip_file } - - context 'real test data' do - let( :subdir_names ){ %w<OFFRE_TRANSDEV_20170301122517 OFFRE_TRANSDEV_20170301122519> } - let( :expected_chksums ){ - checksum_trees( subdir_names.map{ |sn| subdir_file(sn, prefix: 'source_') } ) - } - - let( :zip_file ){ fixtures_path 'OFFRE_TRANSDEV_2017030112251.zip' } - # - # Remove potential test artefacts - before do - subdir_names.each do | subdir_name | - File.unlink( subdir_file subdir_name, suffix: '.zip' ) rescue nil - Dir.unlink( subdir_file subdir_name ) rescue nil - end - end - - it "yields the correct content" do - subdir_contents = {} - # Write ZipService Streams to files and inflate them to file system - unzipper.subdirs.each do | subdir | - File.open(subdir_file( subdir.name, suffix: '.zip' ), 'wb'){ |f| f.write subdir.stream.string } - unzip_subdir subdir - end - # Represent the inflated file_system as a checksum tree - actual_checksums = - checksum_trees( subdir_names.map{ |sn| subdir_file(sn, prefix: 'target/') } ) - expect( actual_checksums ).to eq( expected_chksums ) - end - - end - - end - - def checksum_trees *dirs - dirs.flatten.inject({},&method(:checksum_tree)) - end - def checksum_tree repr, dir - Dir.glob("#{dir}/**/*").each do |file| - if !File.directory?(file) - repr.merge!( File.basename(file) => %x{cksum #{file}}.split.first ){ |_, ov, nv| Array(ov) << nv } - end - end - repr - end - - def subdir_file( subdir, prefix: 'target_', suffix: '' ) - fixtures_path("#{prefix}#{subdir}#{suffix}") - end - - def unzip_subdir subdir - %x{unzip -oqq #{subdir_file subdir.name, suffix: '.zip'} -d #{fixture_path}/target} - end -end diff --git a/spec/services/zip_service_spec.rb b/spec/services/zip_service_spec.rb new file mode 100644 index 000000000..98cb9026d --- /dev/null +++ b/spec/services/zip_service_spec.rb @@ -0,0 +1,68 @@ +RSpec.describe ZipService do + + let( :zip_service ){ described_class } + let( :unzipper ){ zip_service.new(zip_data) } + let( :zip_data ){ File.read zip_file } + + + context 'correct test data' do + before do + subdir_names.each do | subdir_name | + File.unlink( subdir_file subdir_name, suffix: '.zip' ) rescue nil + Dir.unlink( subdir_file subdir_name ) rescue nil + end + end + let( :subdir_names ){ %w<OFFRE_TRANSDEV_20170301122517 OFFRE_TRANSDEV_20170301122519> } + let( :expected_chksums ){ + checksum_trees( subdir_names.map{ |sn| subdir_file(sn, prefix: 'source_') } ) + } + + let( :zip_file ){ fixtures_path 'OFFRE_TRANSDEV_2017030112251.zip' } + # + # Remove potential test artefacts + + it 'yields the correct content' do + # Write ZipService Streams to files and inflate them to file system + unzipper.subdirs.each do | subdir | + expect( subdir.spurious ).to be_empty + File.open(subdir_file( subdir.name, suffix: '.zip' ), 'wb'){ |f| f.write subdir.stream.string } + unzip_subdir subdir + end + # Represent the inflated file_system as a checksum tree + actual_checksums = + checksum_trees( subdir_names.map{ |sn| subdir_file(sn, prefix: 'target/') } ) + expect( actual_checksums ).to eq( expected_chksums ) + end + + end + + context 'test data with spurious directories' do + let( :zip_file ){ fixtures_path 'OFFRE_WITH_EXTRA.zip' } + + it 'returns the extra dir in the spurious field of the entry' do + expect( unzipper.subdirs.first.spurious ).to eq(%w{EXTRA}) + end + end + + + def checksum_trees *dirs + dirs.flatten.inject({},&method(:checksum_tree)) + end + def checksum_tree repr, dir + Dir.glob("#{dir}/**/*").each do |file| + if !File.directory?(file) + repr.merge!( File.basename(file) => %x{cksum #{file}}.split.first ){ |_, ov, nv| Array(ov) << nv } + end + end + repr + end + + def subdir_file( subdir, prefix: 'target_', suffix: '' ) + fixtures_path("#{prefix}#{subdir}#{suffix}") + end + + def unzip_subdir subdir + %x{unzip -oqq #{subdir_file subdir.name, suffix: '.zip'} -d #{fixture_path}/target} + end +end + |