added better error handling. Also refactored some of the code and fixed a few issues in project_tree_saver

10 files changed, 61 insertions, 32 deletions

diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb
index fd569e1c9ae..87a259ed15a 100644
--- a/app/services/projects/import_export/export_service.rb
+++ b/app/services/projects/import_export/export_service.rb
@@ -4,8 +4,8 @@ module Projects
 
       def execute(options = {})
         @shared = Gitlab::ImportExport::Shared.new(relative_path: File.join(project.path_with_namespace, 'work'))
-        # TODO handle errors
         save_all if [save_project_tree, bundle_repo, bundle_wiki_repo].all?
+        notify_worker if @shared.errors.any?
       end
 
       private
@@ -23,7 +23,11 @@ module Projects
       end
 
       def save_all
-        Gitlab::ImportExport::Saver.save(storage_path: @shared.export_path)
+        Gitlab::ImportExport::Saver.save(shared: @shared)
+      end
+
+      def notify_worker
+        raise Gitlab::ImportExport::Error @shared.errors.join(', ')
       end
     end
   end
diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb
index 655373c03af..6835411ba70 100644
--- a/lib/gitlab/import_export.rb
+++ b/lib/gitlab/import_export.rb
@@ -6,10 +6,6 @@ module Gitlab
       File.join(storage_path, relative_path)
     end
 
-    def project_tree
-      Gitlab::ImportExport::ImportExportReader.new.project_tree
-    end
-
     def storage_path
       File.join(Settings.shared['path'], 'tmp/project_exports')
     end
diff --git a/lib/gitlab/import_export/error.rb b/lib/gitlab/import_export/error.rb
new file mode 100644
index 00000000000..b47a646c0b9
--- /dev/null
+++ b/lib/gitlab/import_export/error.rb
@@ -0,0 +1,5 @@
+module Gitlab
+  module ImportExport
+    class Error < StandardError; end
+  end
+end
\ No newline at end of file
diff --git a/lib/gitlab/import_export/import_export_reader.rb b/lib/gitlab/import_export/import_export_reader.rb
index 37093ff58ed..b7204d5a87c 100644
--- a/lib/gitlab/import_export/import_export_reader.rb
+++ b/lib/gitlab/import_export/import_export_reader.rb
@@ -2,16 +2,18 @@ module Gitlab
   module ImportExport
     class ImportExportReader
 
-      def initialize(config: 'lib/gitlab/import_export/import_export.yml')
-        config_hash = YAML.load_file(config).with_indifferent_access
+      def initialize(config: 'lib/gitlab/import_export/import_export.yml', shared:)
+        @shared = shared
+        config_hash = YAML.load_file(config).deep_symbolize_keys
         @tree = config_hash[:project_tree]
         @attributes_parser = Gitlab::ImportExport::AttributesFinder.new(included_attributes: config_hash[:included_attributes],
                                                                         excluded_attributes: config_hash[:excluded_attributes])
-        @json_config_hash = {}
       end
 
       def project_tree
         @attributes_parser.find_included(:project).merge(include: build_hash(@tree))
+      rescue => e
+        @shared.error(e.message)
       end
 
       private
@@ -27,6 +29,8 @@ module Gitlab
       end
 
       def build_json_config_hash(model_object_hash)
+        @json_config_hash = {}
+
         model_object_hash.values.flatten.each do |model_object|
           current_key = model_object_hash.keys.first
 
@@ -61,7 +65,7 @@ module Gitlab
       end
 
       def hash_or_merge(value, hash)
-        value.is_a?(Hash) ? value.merge(hash) : hash
+        value.is_a?(Hash) ? value.merge(hash) : { value => hash }
       end
     end
   end
diff --git a/lib/gitlab/import_export/project_tree_saver.rb b/lib/gitlab/import_export/project_tree_saver.rb
index 29d715c16d3..2287524c8a5 100644
--- a/lib/gitlab/import_export/project_tree_saver.rb
+++ b/lib/gitlab/import_export/project_tree_saver.rb
@@ -5,16 +5,16 @@ module Gitlab
 
       def initialize(project:, shared:)
         @project = project
-        @export_path = shared.export_path
-        @full_path = File.join(@export_path, project_filename)
+        @shared = shared
+        @full_path = File.join(@shared.export_path, project_filename)
       end
 
       def save
-        FileUtils.mkdir_p(@export_path)
+        FileUtils.mkdir_p(@shared.export_path)
         File.write(full_path, project_json_tree)
         true
-      rescue
-        # TODO: handle error
+      rescue => e
+        @shared.error(e.message)
         false
       end
 
@@ -26,7 +26,7 @@ module Gitlab
       end
 
       def project_json_tree
-        @project.to_json(Gitlab::ImportExport.project_tree)
+        @project.to_json(Gitlab::ImportExport::ImportExportReader.new(shared: @shared).project_tree)
       end
     end
   end
diff --git a/lib/gitlab/import_export/repo_bundler.rb b/lib/gitlab/import_export/repo_bundler.rb
index cd871687d76..bcf976fb624 100644
--- a/lib/gitlab/import_export/repo_bundler.rb
+++ b/lib/gitlab/import_export/repo_bundler.rb
@@ -7,21 +7,22 @@ module Gitlab
 
       def initialize(project: , shared: )
         @project = project
-        @export_path = shared.export_path
+        @shared = shared
       end
 
       def bundle
         return false if @project.empty_repo?
-        @full_path = File.join(@export_path, project_filename)
+        @full_path = File.join(@shared.export_path, project_filename)
         bundle_to_disk
       end
 
       private
 
       def bundle_to_disk
-        FileUtils.mkdir_p(@export_path)
+        FileUtils.mkdir_p(@shared.export_path)
         git_bundle(repo_path: path_to_repo, bundle_path: @full_path)
-      rescue
+      rescue => e
+        @shared.error(e.message)
         false
       end
 
diff --git a/lib/gitlab/import_export/saver.rb b/lib/gitlab/import_export/saver.rb
index 634e58e6039..024a0e1e785 100644
--- a/lib/gitlab/import_export/saver.rb
+++ b/lib/gitlab/import_export/saver.rb
@@ -7,32 +7,35 @@ module Gitlab
         new(*args).save
       end
 
-      def initialize(storage_path:)
-        @storage_path = storage_path
+      def initialize(shared:)
+        @shared = shared
       end
 
       def save
         if compress_and_save
-          remove_storage_path
+          remove_@shared.storage_path
           Rails.logger.info("Saved project export #{archive_file}")
           archive_file
         else
           false
         end
+      rescue => e
+        @shared.error(e.message)
+        false
       end
 
       private
 
       def compress_and_save
-        tar_czf(archive: archive_file, dir: @storage_path)
+        tar_czf(archive: archive_file, dir: @shared.storage_path)
       end
 
-      def remove_storage_path
-        FileUtils.rm_rf(@storage_path)
+      def remove_shared.storage_path
+        FileUtils.rm_rf(@shared.storage_path)
       end
 
       def archive_file
-        @archive_file ||= File.join(@storage_path, '..', "#{Time.now.strftime('%Y-%m-%d_%H-%M-%3N')}_project_export.tar.gz")
+        @archive_file ||= File.join(@shared.storage_path, '..', "#{Time.now.strftime('%Y-%m-%d_%H-%M-%3N')}_project_export.tar.gz")
       end
     end
   end
diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb
index a4ec33a6cf7..4246f73af90 100644
--- a/lib/gitlab/import_export/shared.rb
+++ b/lib/gitlab/import_export/shared.rb
@@ -1,13 +1,28 @@
 module Gitlab
   module ImportExport
     class Shared
+
+      attr_reader :errors
+
       def initialize(opts)
         @opts = opts
+        @errors = []
       end
 
       def export_path
         @export_path ||= Gitlab::ImportExport.export_path(relative_path: @opts[:relative_path])
       end
+
+      def error(message)
+        error_out(message, caller[0].dup)
+        @errors << message
+      end
+
+      private
+
+      def error_out(message, caller)
+        Rails.logger.error("Import/Export error raised on #{caller}: #{message}")
+      end
     end
   end
 end
diff --git a/lib/gitlab/import_export/wiki_repo_bundler.rb b/lib/gitlab/import_export/wiki_repo_bundler.rb
index 17f1530d5a0..a0000176bb5 100644
--- a/lib/gitlab/import_export/wiki_repo_bundler.rb
+++ b/lib/gitlab/import_export/wiki_repo_bundler.rb
@@ -4,14 +4,15 @@ module Gitlab
       def bundle
         @wiki = ProjectWiki.new(@project)
         return true if !wiki? # it's okay to have no Wiki
-        @full_path = File.join(@export_path, project_filename)
+        @full_path = File.join(@shared.export_path, project_filename)
         bundle_to_disk
       end
 
       def bundle_to_disk
-        FileUtils.mkdir_p(@export_path)
+        FileUtils.mkdir_p(@shared.export_path)
         git_bundle(repo_path: path_to_repo, bundle_path: @full_path)
-      rescue
+      rescue => e
+        @shared.error(e.message)
         false
       end
 
diff --git a/spec/lib/gitlab/import_export/import_export_reader_spec.rb b/spec/lib/gitlab/import_export/import_export_reader_spec.rb
index f826c5ec8e6..2fc9a39c68b 100644
--- a/spec/lib/gitlab/import_export/import_export_reader_spec.rb
+++ b/spec/lib/gitlab/import_export/import_export_reader_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::ImportExport::ImportExportReader do
-
+  let(:shared) { Gitlab::ImportExport::Shared.new(relative_path:'') }
   let(:test_config) { 'spec/support/import_export/import_export.yml' }
   let(:project_tree_hash) do
     {
@@ -17,6 +17,6 @@ describe Gitlab::ImportExport::ImportExportReader do
   end
 
   it 'should generate hash from project tree config' do
-    expect(described_class.new(config: test_config).project_tree) =~ (project_tree_hash)
+    expect(described_class.new(config: test_config, shared: shared).project_tree) =~ (project_tree_hash)
   end
 end