Extend API for exporting a project with direct upload URL

8 files changed, 383 insertions, 8 deletions

diff --git a/spec/fixtures/api/schemas/public_api/v4/project/export_status.json b/spec/fixtures/api/schemas/public_api/v4/project/export_status.json
index d24a6f93f4b..81c8815caf6 100644
--- a/spec/fixtures/api/schemas/public_api/v4/project/export_status.json
+++ b/spec/fixtures/api/schemas/public_api/v4/project/export_status.json
@@ -1,7 +1,9 @@
 {
   "type": "object",
   "allOf": [
-    { "$ref": "identity.json" },
+    {
+      "$ref": "identity.json"
+    },
     {
       "required": [
         "export_status"
@@ -9,7 +11,12 @@
       "properties": {
         "export_status": {
           "type": "string",
-          "enum": ["none", "started", "finished"]
+          "enum": [
+            "none",
+            "started",
+            "finished",
+            "after_export_action"
+          ]
         }
       }
     }
diff --git a/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb
new file mode 100644
index 00000000000..ed54d87de4a
--- /dev/null
+++ b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb
@@ -0,0 +1,104 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::AfterExportStrategies::BaseAfterExportStrategy do
+  let!(:service) { described_class.new }
+  let!(:project) { create(:project, :with_export) }
+  let(:shared) { project.import_export_shared }
+  let!(:user) { create(:user) }
+
+  describe '#execute' do
+    before do
+      allow(service).to receive(:strategy_execute)
+    end
+
+    it 'returns if project exported file is not found' do
+      allow(project).to receive(:export_project_path).and_return(nil)
+
+      expect(service).not_to receive(:strategy_execute)
+
+      service.execute(user, project)
+    end
+
+    it 'creates a lock file in the export dir' do
+      allow(service).to receive(:delete_after_export_lock)
+
+      service.execute(user, project)
+
+      expect(lock_path_exist?).to be_truthy
+    end
+
+    context 'when the method succeeds' do
+      it 'removes the lock file' do
+        service.execute(user, project)
+
+        expect(lock_path_exist?).to be_falsey
+      end
+    end
+
+    context 'when the method fails' do
+      before do
+        allow(service).to receive(:strategy_execute).and_call_original
+      end
+
+      context 'when validation fails' do
+        before do
+          allow(service).to receive(:invalid?).and_return(true)
+        end
+
+        it 'does not create the lock file' do
+          expect(service).not_to receive(:create_or_update_after_export_lock)
+
+          service.execute(user, project)
+        end
+
+        it 'does not execute main logic' do
+          expect(service).not_to receive(:strategy_execute)
+
+          service.execute(user, project)
+        end
+
+        it 'logs validation errors in shared context' do
+          expect(service).to receive(:log_validation_errors)
+
+          service.execute(user, project)
+        end
+      end
+
+      context 'when an exception is raised' do
+        it 'removes the lock' do
+          expect { service.execute(user, project) }.to raise_error(NotImplementedError)
+
+          expect(lock_path_exist?).to be_falsey
+        end
+      end
+    end
+  end
+
+  describe '#log_validation_errors' do
+    it 'add the message to the shared context' do
+      errors = %w(test_message test_message2)
+
+      allow(service).to receive(:invalid?).and_return(true)
+      allow(service.errors).to receive(:full_messages).and_return(errors)
+
+      expect(shared).to receive(:add_error_message).twice.and_call_original
+
+      service.execute(user, project)
+
+      expect(shared.errors).to eq errors
+    end
+  end
+
+  describe '#to_json' do
+    it 'adds the current strategy class to the serialized attributes' do
+      params = { param1: 1 }
+      result = params.merge(klass: described_class.to_s).to_json
+
+      expect(described_class.new(params).to_json).to eq result
+    end
+  end
+
+  def lock_path_exist?
+    File.exist?(described_class.lock_file_path(project))
+  end
+end
diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
new file mode 100644
index 00000000000..5fe57d9987b
--- /dev/null
+++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do
+  let(:example_url) { 'http://www.example.com' }
+  let(:strategy) { subject.new(url: example_url, http_method: 'post') }
+  let!(:project) { create(:project, :with_export) }
+  let!(:user) { build(:user) }
+
+  subject { described_class }
+
+  describe 'validations' do
+    it 'only POST and PUT method allowed' do
+      %w(POST post PUT put).each do |method|
+        expect(subject.new(url: example_url, http_method: method)).to be_valid
+      end
+
+      expect(subject.new(url: example_url, http_method: 'whatever')).not_to be_valid
+    end
+
+    it 'onyl allow urls as upload urls' do
+      expect(subject.new(url: example_url)).to be_valid
+      expect(subject.new(url: 'whatever')).not_to be_valid
+    end
+  end
+
+  describe '#execute' do
+    it 'removes the exported project file after the upload' do
+      allow(strategy).to receive(:send_file)
+      allow(strategy).to receive(:handle_response_error)
+
+      expect(project).to receive(:remove_exported_project_file)
+
+      strategy.execute(user, project)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/after_export_strategy_builder_spec.rb b/spec/lib/gitlab/import_export/after_export_strategy_builder_spec.rb
new file mode 100644
index 00000000000..bf727285a9f
--- /dev/null
+++ b/spec/lib/gitlab/import_export/after_export_strategy_builder_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::AfterExportStrategyBuilder do
+  let!(:strategies_namespace) { 'Gitlab::ImportExport::AfterExportStrategies' }
+
+  describe '.build!' do
+    context 'when klass param is' do
+      it 'null it returns the default strategy' do
+        expect(described_class.build!(nil).class).to eq described_class.default_strategy
+      end
+
+      it 'not a valid class it raises StrategyNotFoundError exception' do
+        expect { described_class.build!('Whatever') }.to raise_error(described_class::StrategyNotFoundError)
+      end
+
+      it 'not a descendant of AfterExportStrategy' do
+        expect { described_class.build!('User') }.to raise_error(described_class::StrategyNotFoundError)
+      end
+    end
+
+    it 'initializes strategy with attributes param' do
+      params = { param1: 1, param2: 2, param3: 3 }
+
+      strategy = described_class.build!("#{strategies_namespace}::DownloadNotificationStrategy", params)
+
+      params.each { |k, v| expect(strategy.public_send(k)).to eq v }
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 05014222623..96adf64bcec 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2560,7 +2560,7 @@ describe Project do
     end
   end
 
-  describe '#remove_exports' do
+  describe '#remove_export' do
     let(:legacy_project) { create(:project, :legacy_storage, :with_export) }
     let(:project) { create(:project, :with_export) }
 
@@ -2608,6 +2608,23 @@ describe Project do
     end
   end
 
+  describe '#remove_exported_project_file' do
+    let(:project) { create(:project, :with_export) }
+
+    it 'removes the exported project file' do
+      exported_file = project.export_project_path
+
+      expect(File.exist?(exported_file)).to be_truthy
+
+      allow(FileUtils).to receive(:rm_f).and_call_original
+      expect(FileUtils).to receive(:rm_f).with(exported_file).and_call_original
+
+      project.remove_exported_project_file
+
+      expect(File.exist?(exported_file)).to be_falsy
+    end
+  end
+
   describe '#forks_count' do
     it 'returns the number of forks' do
       project = build(:project)
diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb
index 12583109b59..3834d27d0a9 100644
--- a/spec/requests/api/project_export_spec.rb
+++ b/spec/requests/api/project_export_spec.rb
@@ -5,6 +5,7 @@ describe API::ProjectExport do
   set(:project_none) { create(:project) }
   set(:project_started) { create(:project) }
   set(:project_finished) { create(:project) }
+  set(:project_after_export) { create(:project) }
   set(:user) { create(:user) }
   set(:admin) { create(:admin) }
 
@@ -12,11 +13,13 @@ describe API::ProjectExport do
   let(:path_none) { "/projects/#{project_none.id}/export" }
   let(:path_started) { "/projects/#{project_started.id}/export" }
   let(:path_finished) { "/projects/#{project_finished.id}/export" }
+  let(:path_after_export) { "/projects/#{project_after_export.id}/export" }
 
   let(:download_path) { "/projects/#{project.id}/export/download" }
   let(:download_path_none) { "/projects/#{project_none.id}/export/download" }
   let(:download_path_started) { "/projects/#{project_started.id}/export/download" }
   let(:download_path_finished) { "/projects/#{project_finished.id}/export/download" }
+  let(:download_path_export_action) { "/projects/#{project_after_export.id}/export/download" }
 
   let(:export_path) { "#{Dir.tmpdir}/project_export_spec" }
 
@@ -29,6 +32,11 @@ describe API::ProjectExport do
     # simulate exported
     FileUtils.mkdir_p project_finished.export_path
     FileUtils.touch File.join(project_finished.export_path, '_export.tar.gz')
+
+    # simulate in after export action
+    FileUtils.mkdir_p project_after_export.export_path
+    FileUtils.touch File.join(project_after_export.export_path, '_export.tar.gz')
+    FileUtils.touch Gitlab::ImportExport::AfterExportStrategies::BaseAfterExportStrategy.lock_file_path(project_after_export)
   end
 
   after do
@@ -73,6 +81,14 @@ describe API::ProjectExport do
         expect(json_response['export_status']).to eq('started')
       end
 
+      it 'is after_export' do
+        get api(path_after_export, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/project/export_status')
+        expect(json_response['export_status']).to eq('after_export_action')
+      end
+
       it 'is finished' do
         get api(path_finished, user)
 
@@ -99,6 +115,7 @@ describe API::ProjectExport do
           project_none.add_master(user)
           project_started.add_master(user)
           project_finished.add_master(user)
+          project_after_export.add_master(user)
         end
 
         it_behaves_like 'get project export status ok'
@@ -163,6 +180,36 @@ describe API::ProjectExport do
       end
     end
 
+    shared_examples_for 'get project export upload after action' do
+      context 'and is uploading' do
+        it 'downloads' do
+          get api(download_path_export_action, user)
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'when upload complete' do
+        before do
+          FileUtils.rm_rf(project_after_export.export_path)
+        end
+
+        it_behaves_like '404 response' do
+          let(:request) { get api(download_path_export_action, user) }
+        end
+      end
+    end
+
+    shared_examples_for 'get project download by strategy' do
+      context 'when upload strategy set' do
+        it_behaves_like 'get project export upload after action'
+      end
+
+      context 'when download strategy set' do
+        it_behaves_like 'get project export download'
+      end
+    end
+
     it_behaves_like 'when project export is disabled' do
       let(:request) { get api(download_path, admin) }
     end
@@ -171,7 +218,7 @@ describe API::ProjectExport do
       context 'when user is an admin' do
         let(:user) { admin }
 
-        it_behaves_like 'get project export download'
+        it_behaves_like 'get project download by strategy'
       end
 
       context 'when user is a master' do
@@ -180,9 +227,10 @@ describe API::ProjectExport do
           project_none.add_master(user)
           project_started.add_master(user)
           project_finished.add_master(user)
+          project_after_export.add_master(user)
         end
 
-        it_behaves_like 'get project export download'
+        it_behaves_like 'get project download by strategy'
       end
 
       context 'when user is a developer' do
@@ -229,10 +277,30 @@ describe API::ProjectExport do
     end
 
     shared_examples_for 'post project export start' do
-      it 'starts' do
-        post api(path, user)
+      context 'with upload strategy' do
+        context 'when params invalid' do
+          it_behaves_like '400 response' do
+            let(:request) { post(api(path, user), 'upload[url]' => 'whatever') }
+          end
+        end
+
+        it 'starts' do
+          allow_any_instance_of(Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy).to receive(:send_file)
+
+          post(api(path, user), 'upload[url]' => 'http://gitlab.com')
 
-        expect(response).to have_gitlab_http_status(202)
+          expect(response).to have_gitlab_http_status(202)
+        end
+      end
+
+      context 'with download strategy' do
+        it 'starts' do
+          expect_any_instance_of(Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy).not_to receive(:send_file)
+
+          post api(path, user)
+
+          expect(response).to have_gitlab_http_status(202)
+        end
       end
     end
 
@@ -253,6 +321,7 @@ describe API::ProjectExport do
           project_none.add_master(user)
           project_started.add_master(user)
           project_finished.add_master(user)
+          project_after_export.add_master(user)
         end
 
         it_behaves_like 'post project export start'
diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb
new file mode 100644
index 00000000000..51491c7d529
--- /dev/null
+++ b/spec/services/projects/import_export/export_service_spec.rb
@@ -0,0 +1,85 @@
+require 'spec_helper'
+
+describe Projects::ImportExport::ExportService do
+  describe '#execute' do
+    let!(:user) { create(:user) }
+    let(:project) { create(:project) }
+    let(:shared) { project.import_export_shared }
+    let(:service) { described_class.new(project, user) }
+    let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new }
+
+    context 'when all saver services succeed' do
+      before do
+        allow(service).to receive(:save_services).and_return(true)
+      end
+
+      it 'saves the project in the file system' do
+        expect(Gitlab::ImportExport::Saver).to receive(:save).with(project: project, shared: shared)
+
+        service.execute
+      end
+
+      it 'calls the after export strategy' do
+        expect(after_export_strategy).to receive(:execute)
+
+        service.execute(after_export_strategy)
+      end
+
+      context 'when after export strategy fails' do
+        before do
+          allow(after_export_strategy).to receive(:execute).and_return(false)
+        end
+
+        after do
+          service.execute(after_export_strategy)
+        end
+
+        it 'removes the remaining exported data' do
+          allow(shared).to receive(:export_path).and_return('whatever')
+          allow(FileUtils).to receive(:rm_rf)
+
+          expect(FileUtils).to receive(:rm_rf).with(shared.export_path)
+        end
+
+        it 'notifies the user' do
+          expect_any_instance_of(NotificationService).to receive(:project_not_exported)
+        end
+
+        it 'notifies logger' do
+          allow(Rails.logger).to receive(:error)
+
+          expect(Rails.logger).to receive(:error)
+        end
+      end
+    end
+
+    context 'when saver services fail' do
+      before do
+        allow(service).to receive(:save_services).and_return(false)
+      end
+
+      after do
+        expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
+      end
+
+      it 'removes the remaining exported data' do
+        allow(shared).to receive(:export_path).and_return('whatever')
+        allow(FileUtils).to receive(:rm_rf)
+
+        expect(FileUtils).to receive(:rm_rf).with(shared.export_path)
+      end
+
+      it 'notifies the user' do
+        expect_any_instance_of(NotificationService).to receive(:project_not_exported)
+      end
+
+      it 'notifies logger' do
+        expect(Rails.logger).to receive(:error)
+      end
+
+      it 'the after export strategy is not called' do
+        expect(service).not_to receive(:execute_after_export_action)
+      end
+    end
+  end
+end
diff --git a/spec/workers/project_export_worker_spec.rb b/spec/workers/project_export_worker_spec.rb
new file mode 100644
index 00000000000..8899969c178
--- /dev/null
+++ b/spec/workers/project_export_worker_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe ProjectExportWorker do
+  let!(:user) { create(:user) }
+  let!(:project) { create(:project) }
+
+  subject { described_class.new }
+
+  describe '#perform' do
+    context 'when it succeeds' do
+      it 'calls the ExportService' do
+        expect_any_instance_of(::Projects::ImportExport::ExportService).to receive(:execute)
+
+        subject.perform(user.id, project.id, { 'klass' => 'Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy' })
+      end
+    end
+
+    context 'when it fails' do
+      it 'raises an exception when params are invalid' do
+        expect_any_instance_of(::Projects::ImportExport::ExportService).not_to receive(:execute)
+
+        expect { subject.perform(1234, project.id, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(user.id, 1234, {}) }.to raise_exception(ActiveRecord::RecordNotFound)
+        expect { subject.perform(user.id, project.id, { 'klass' => 'Whatever' }) }.to raise_exception(Gitlab::ImportExport::AfterExportStrategyBuilder::StrategyNotFoundError)
+      end
+    end
+  end
+end