Merge pull request #98 from af83/4633-import-handle-incorrect-zip

4633 import handle incorrect zip Messages added

2 files changed, 39 insertions, 10 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/app/workers/workbench_import_worker.rb b/app/workers/workbench_import_worker.rb
index 994493944..300fad9e2 100644
--- a/app/workers/workbench_import_worker.rb
+++ b/app/workers/workbench_import_worker.rb
@@ -14,11 +14,13 @@ class WorkbenchImportWorker
     zip_service = ZipService.new(downloaded)
     upload zip_service
     @workbench_import.update(ended_at: Time.now)
+  rescue Zip::Error
+    handle_corrupt_zip_file
   end
 
   def download
     logger.info  "HTTP GET #{import_url}"
-    @zipfile_data = HTTPService.get_resource(
+    HTTPService.get_resource(
       host: import_host,
       path: import_path,
       params: {token: @workbench_import.token_download}).body
@@ -32,6 +34,10 @@ class WorkbenchImportWorker
       params: params(eg_file, eg_name))
   end
 
+  def handle_corrupt_zip_file
+    @workbench_import.messages.create(criticity: :error, message_key: 'corrupt_zip_file', message_attributes: {import_name: @workbench_import.name})
+  end
+
   def upload zip_service
     entry_group_streams = zip_service.subdirs
     @workbench_import.update total_steps: entry_group_streams.size
@@ -42,11 +48,24 @@ class WorkbenchImportWorker
     raise
   end
 
-  def upload_entry_group entry_pair, element_count
-    @workbench_import.update( current_step: element_count.succ )
-    # status = retry_service.execute(&upload_entry_group_proc(entry_pair))
-    eg_name = entry_pair.name
-    eg_stream = entry_pair.stream
+  def update_object_state entry, count
+    @workbench_import.update( current_step: count )
+    unless entry.spurious.empty?
+      @workbench_import.messages.create(
+        criticity: :warning,
+        message_key: 'inconsistent_zip_file',
+        message_attributes: {
+          'import_name' => @workbench_import.name,
+          'spurious_dirs' => entry.spurious.join(', ')
+        }) 
+    end
+  end
+
+  def upload_entry_group entry, element_count
+    update_object_state entry, element_count.succ
+    # status = retry_service.execute(&upload_entry_group_proc(entry))
+    eg_name = entry.name
+    eg_stream = entry.stream
 
     FileUtils.mkdir_p(Rails.root.join('tmp', 'imports'))