diff --git a/CHANGELOG b/CHANGELOG index 2ae83d5b3ce..ddfd384f8c8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 8.0.0 (unreleased) - Fix emoji URLs in Markdown when relative_url_root is used (Stan Hu) - Omit filename in Content-Disposition header in raw file download to avoid RFC 6266 encoding issues (Stan HU) - Fix broken Wiki Page History (Stan Hu) + - Import forked repositories asynchronously to prevent large repositories from timing out (Stan Hu) - Prevent anchors from being hidden by header (Stan Hu) - Fix bug where only the first 15 Bitbucket issues would be imported (Stan Hu) - Sort issues by creation date in Bitbucket importer (Stan Hu) diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb index 9e72597ea87..8a785076bb7 100644 --- a/app/controllers/projects/forks_controller.rb +++ b/app/controllers/projects/forks_controller.rb @@ -13,10 +13,14 @@ class Projects::ForksController < Projects::ApplicationController @forked_project = ::Projects::ForkService.new(project, current_user, namespace: namespace).execute if @forked_project.saved? && @forked_project.forked? - redirect_to( - namespace_project_path(@forked_project.namespace, @forked_project), - notice: 'Project was successfully forked.' - ) + if @forked_project.import_in_progress? + redirect_to namespace_project_import_path(@forked_project.namespace, @forked_project) + else + redirect_to( + namespace_project_path(@forked_project.namespace, @forked_project), + notice: 'Project was successfully forked.' + ) + end else render :error end diff --git a/app/models/project.rb b/app/models/project.rb index 072d7d73f41..49525eb9227 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -144,7 +144,7 @@ class Project < ActiveRecord::Base validates_uniqueness_of :path, scope: :namespace_id validates :import_url, format: { with: /\A#{URI.regexp(%w(ssh git http https))}\z/, message: 'should be a valid url' }, - if: :import? + if: :external_import? validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_limit, on: :create validate :avatar_type, @@ -275,7 +275,13 @@ class Project < ActiveRecord::Base end def add_import_job - RepositoryImportWorker.perform_in(2.seconds, id) + if forked? + unless RepositoryForkWorker.perform_async(id, forked_from_project.path_with_namespace, self.namespace.path) + import_fail + end + else + RepositoryImportWorker.perform_in(2.seconds, id) + end end def clear_import_data @@ -283,6 +289,10 @@ class Project < ActiveRecord::Base end def import? + external_import? || forked? + end + + def external_import? import_url.present? end @@ -702,14 +712,8 @@ class Project < ActiveRecord::Base end def create_repository - if forked? - if gitlab_shell.fork_repository(forked_from_project.path_with_namespace, self.namespace.path) - true - else - errors.add(:base, 'Failed to fork repository via gitlab-shell') - false - end - else + # Forked import is handled asynchronously + unless forked? if gitlab_shell.add_repository(path_with_namespace) true else diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 1bb2462565a..e54a13ed6c5 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -55,9 +55,7 @@ module Projects @project.save if @project.persisted? && !@project.import? - unless @project.create_repository - raise 'Failed to create repository' - end + raise 'Failed to create repository' unless @project.create_repository end end diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb new file mode 100644 index 00000000000..acd1c43f06b --- /dev/null +++ b/app/workers/repository_fork_worker.rb @@ -0,0 +1,34 @@ +class RepositoryForkWorker + include Sidekiq::Worker + include Gitlab::ShellAdapter + + sidekiq_options queue: :gitlab_shell + + def perform(project_id, source_path, target_path) + project = Project.find_by_id(project_id) + + unless project.present? + logger.error("Project #{project_id} no longer exists!") + return + end + + result = gitlab_shell.fork_repository(source_path, target_path) + + unless result + logger.error("Unable to fork project #{project_id} for repository #{source_path} -> #{target_path}") + project.import_fail + project.save + return + end + + if project.valid_repo? + ProjectCacheWorker.perform_async(project.id) + project.import_finish + else + project.import_fail + logger.error("Project #{id} had an invalid repository after fork") + end + + project.save + end +end diff --git a/features/steps/project/fork.rb b/features/steps/project/fork.rb index 0e433781d7a..370960845cc 100644 --- a/features/steps/project/fork.rb +++ b/features/steps/project/fork.rb @@ -15,7 +15,7 @@ class Spinach::Features::ProjectFork < Spinach::FeatureSteps end step 'I should see the forked project page' do - expect(page).to have_content "Project was successfully forked." + expect(page).to have_content "Forked from" end step 'I already have a project named "Shop" in my namespace' do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index ff4ed2dd484..25277f07482 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -96,6 +96,17 @@ describe Projects::CreateService do expect(project.saved?).to be(true) end end + + context 'repository creation' do + it 'should synchronously create the repository' do + expect_any_instance_of(Project).to receive(:create_repository) + + project = create_project(@user, @opts) + expect(project).to be_valid + expect(project.owner).to eq(@user) + expect(project.namespace).to eq(@user.namespace) + end + end end def create_project(user, opts) diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index c04e842c67e..7c4bb74b77f 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -28,8 +28,7 @@ describe Projects::ForkService do context 'fork project failure' do it "fails due to transaction failure" do @to_project = fork_project(@from_project, @to_user, false) - expect(@to_project.errors).not_to be_empty - expect(@to_project.errors[:base]).to include("Failed to fork repository via gitlab-shell") + expect(@to_project.import_failed?) end end @@ -100,7 +99,7 @@ describe Projects::ForkService do end def fork_project(from_project, user, fork_success = true, params = {}) - allow_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(fork_success) + allow(RepositoryForkWorker).to receive(:perform_async).and_return(fork_success) Projects::ForkService.new(from_project, user, params).execute end end diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb new file mode 100644 index 00000000000..aa031106968 --- /dev/null +++ b/spec/workers/repository_fork_worker_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe RepositoryForkWorker do + let(:project) { create(:project) } + let(:fork_project) { create(:project, forked_from_project: project) } + + subject { RepositoryForkWorker.new } + + describe "#perform" do + it "creates a new repository from a fork" do + expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).with( + project.path_with_namespace, + fork_project.namespace.path). + and_return(true) + expect(ProjectCacheWorker).to receive(:perform_async) + + subject.perform(project.id, + project.path_with_namespace, + fork_project.namespace.path) + end + + it "handles bad fork" do + expect_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_return(false) + subject.perform(project.id, + project.path_with_namespace, + fork_project.namespace.path) + end + end +end