Merge branch 'pull-mirror-overhaul-ce-backport' into 'master'

backports changed import logic from pull mirroring feature into CE

See merge request !11850

7 files changed, 71 insertions, 28 deletions

diff --git a/spec/factories/forked_project_links.rb b/spec/factories/forked_project_links.rb
index b16c1272e68..66b0f248959 100644
--- a/spec/factories/forked_project_links.rb
+++ b/spec/factories/forked_project_links.rb
@@ -7,5 +7,9 @@ FactoryGirl.define do
       link.forked_from_project.reload
       link.forked_to_project.reload
     end
+
+    trait :forked_to_empty_project do
+      association :forked_to_project, factory: :empty_project
+    end
   end
 end
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 19a85e5a38f..e17e50db143 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -26,6 +26,22 @@ FactoryGirl.define do
       visibility_level Gitlab::VisibilityLevel::PRIVATE
     end
 
+    trait :import_scheduled do
+      import_status :scheduled
+    end
+
+    trait :import_started do
+      import_status :started
+    end
+
+    trait :import_finished do
+      import_status :finished
+    end
+
+    trait :import_failed do
+      import_status :failed
+    end
+
     trait :archived do
       archived true
     end
diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb
index 4860a2a7498..44013df3ea0 100644
--- a/spec/features/merge_requests/diffs_spec.rb
+++ b/spec/features/merge_requests/diffs_spec.rb
@@ -68,9 +68,14 @@ feature 'Diffs URL', js: true, feature: true do
     let(:merge_request) { create(:merge_request_with_diffs, source_project: forked_project, target_project: project, author: author_user) }
     let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") }
 
+    before do
+      forked_project.repository.after_import
+    end
+
     context 'as author' do
       it 'shows direct edit link' do
         login_as(author_user)
+
         visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
 
         # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
@@ -81,6 +86,7 @@ feature 'Diffs URL', js: true, feature: true do
     context 'as user who needs to fork' do
       it 'shows fork/cancel confirmation' do
         login_as(user)
+
         visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
 
         # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 86ab2550bfb..3ed52d42f86 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -50,7 +50,7 @@ describe Project, models: true do
     it { is_expected.to have_one(:external_wiki_service).dependent(:destroy) }
     it { is_expected.to have_one(:project_feature).dependent(:destroy) }
     it { is_expected.to have_one(:statistics).class_name('ProjectStatistics').dependent(:delete) }
-    it { is_expected.to have_one(:import_data).class_name('ProjectImportData').dependent(:destroy) }
+    it { is_expected.to have_one(:import_data).class_name('ProjectImportData').dependent(:delete) }
     it { is_expected.to have_one(:last_event).class_name('Event') }
     it { is_expected.to have_one(:forked_from_project).through(:forked_project_link) }
     it { is_expected.to have_many(:commit_statuses) }
@@ -1446,16 +1446,13 @@ describe Project, models: true do
   end
 
   describe 'Project import job' do
-    let(:project) { create(:empty_project) }
-    let(:mirror) { false }
+    let(:project) { create(:empty_project, import_url: generate(:url)) }
 
     before do
       allow_any_instance_of(Gitlab::Shell).to receive(:import_repository)
         .with(project.repository_storage_path, project.path_with_namespace, project.import_url)
         .and_return(true)
 
-      allow(project).to receive(:repository_exists?).and_return(true)
-
       expect_any_instance_of(Repository).to receive(:after_import)
         .and_call_original
     end
@@ -1463,8 +1460,7 @@ describe Project, models: true do
     it 'imports a project' do
       expect_any_instance_of(RepositoryImportWorker).to receive(:perform).and_call_original
 
-      project.import_start
-      project.add_import_job
+      project.import_schedule
 
       expect(project.reload.import_status).to eq('finished')
     end
@@ -1551,7 +1547,7 @@ describe Project, models: true do
 
   describe '#add_import_job' do
     context 'forked' do
-      let(:forked_project_link) { create(:forked_project_link) }
+      let(:forked_project_link) { create(:forked_project_link, :forked_to_empty_project) }
       let(:forked_from_project) { forked_project_link.forked_from_project }
       let(:project) { forked_project_link.forked_to_project }
 
@@ -1565,9 +1561,9 @@ describe Project, models: true do
     end
 
     context 'not forked' do
-      let(:project) { create(:empty_project) }
-
       it 'schedules a RepositoryImportWorker job' do
+        project = create(:empty_project, import_url: generate(:url))
+
         expect(RepositoryImportWorker).to receive(:perform_async).with(project.id)
 
         project.add_import_job
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index 033e6ecd18c..3c566c04d6b 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -161,15 +161,13 @@ describe Projects::CreateService, '#execute', services: true do
   end
 
   context 'when a bad service template is created' do
-    before do
-      create(:service, type: 'DroneCiService', project: nil, template: true, active: true)
-    end
-
     it 'reports an error in the imported project' do
       opts[:import_url] = 'http://www.gitlab.com/gitlab-org/gitlab-ce'
+      create(:service, type: 'DroneCiService', project: nil, template: true, active: true)
+
       project = create_project(user, opts)
 
-      expect(project.errors.full_messages_for(:base).first).to match /Unable to save project. Error: Unable to save DroneCiService/
+      expect(project.errors.full_messages_for(:base).first).to match(/Unable to save project. Error: Unable to save DroneCiService/)
       expect(project.services.count).to eq 0
     end
   end
diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb
index 5e1cb74c7fc..6ea5569b438 100644
--- a/spec/workers/repository_fork_worker_spec.rb
+++ b/spec/workers/repository_fork_worker_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe RepositoryForkWorker do
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository, :import_scheduled) }
   let(:fork_project) { create(:project, :repository, forked_from_project: project) }
   let(:shell) { Gitlab::Shell.new }
 
@@ -46,15 +46,27 @@ describe RepositoryForkWorker do
     end
 
     it "handles bad fork" do
+      source_path = project.full_path
+      target_path = fork_project.namespace.full_path
+      error_message = "Unable to fork project #{project.id} for repository #{source_path} -> #{target_path}"
+
       expect(shell).to receive(:fork_repository).and_return(false)
 
-      expect(subject.logger).to receive(:error)
+      expect do
+        subject.perform(project.id, '/test/path', source_path, target_path)
+      end.to raise_error(RepositoryForkWorker::ForkError, error_message)
+    end
 
-      subject.perform(
-        project.id,
-        '/test/path',
-        project.full_path,
-        fork_project.namespace.full_path)
+    it 'handles unexpected error' do
+      source_path = project.full_path
+      target_path = fork_project.namespace.full_path
+
+      allow_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_raise(RuntimeError)
+
+      expect do
+        subject.perform(project.id, '/test/path', source_path, target_path)
+      end.to raise_error(RepositoryForkWorker::ForkError)
+      expect(project.reload.import_status).to eq('failed')
     end
   end
 end
diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb
index 5a2c0671dac..9c277c501f1 100644
--- a/spec/workers/repository_import_worker_spec.rb
+++ b/spec/workers/repository_import_worker_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe RepositoryImportWorker do
-  let(:project) { create(:empty_project) }
+  let(:project) { create(:empty_project, :import_scheduled) }
 
   subject { described_class.new }
 
@@ -21,15 +21,26 @@ describe RepositoryImportWorker do
     context 'when the import has failed' do
       it 'hide the credentials that were used in the import URL' do
         error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found }
-        expect_any_instance_of(Projects::ImportService).to receive(:execute).
-          and_return({ status: :error, message: error })
-        allow(subject).to receive(:jid).and_return('123')
 
-        subject.perform(project.id)
+        expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
+        allow(subject).to receive(:jid).and_return('123')
 
-        expect(project.reload.import_error).to include("https://*****:*****@test.com/root/repoC.git/")
+        expect do
+          subject.perform(project.id)
+        end.to raise_error(RepositoryImportWorker::ImportError, error)
         expect(project.reload.import_jid).not_to be_nil
       end
     end
+
+    context 'with unexpected error' do
+      it 'marks import as failed' do
+        allow_any_instance_of(Projects::ImportService).to receive(:execute).and_raise(RuntimeError)
+
+        expect do
+          subject.perform(project.id)
+        end.to raise_error(RepositoryImportWorker::ImportError)
+        expect(project.reload.import_status).to eq('failed')
+      end
+    end
   end
 end