diff --git a/app/contexts/projects/fork_context.rb b/app/contexts/projects/fork_context.rb index e206a1cdf8799b4f56d282cfb50c2aa0690cac81..f2a0684b01c32d20c56b8c5f3986b4239c57df83 100644 --- a/app/contexts/projects/fork_context.rb +++ b/app/contexts/projects/fork_context.rb @@ -10,28 +10,36 @@ module Projects project = Project.new project.initialize_dup(@from_project) project.name = @from_project.name - project.path = @from_project.path + project.path = @from_project.path project.namespace = current_user.namespace + project.creator = current_user - Project.transaction do - #First save the DB entries as they can be rolled back if the repo fork fails - project.creator = current_user - project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id) - if project.save - project.users_projects.create(project_access: UsersProject::MASTER, user: current_user) + # If the project cannot save, we do not want to trigger the project destroy + # as this can have the side effect of deleting a repo attached to an existing + # project with the same name and namespace + if project.valid? + begin + Project.transaction do + #First save the DB entries as they can be rolled back if the repo fork fails + project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id) + if project.save + project.users_projects.create(project_access: UsersProject::MASTER, user: current_user) + end + #Now fork the repo + unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path) + raise "forking failed in gitlab-shell" + end + project.ensure_satellite_exists + end + rescue => ex + project.errors.add(:base, "Fork transaction failed.") + project.destroy end - #Now fork the repo - unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path) - raise "forking failed in gitlab-shell" - end - project.ensure_satellite_exists - + else + project.errors.add(:base, "Invalid fork destination") end project - rescue => ex - project.errors.add(:base, "Can't fork project. Please try again later") - project.destroy - end + end end end diff --git a/spec/contexts/fork_context_spec.rb b/spec/contexts/fork_context_spec.rb index 285590bdd84bf552d39a0da86d94f51275062edf..ed51b0c3f8e7b8fabeb97cc254d1cb9aec7b5baf 100644 --- a/spec/contexts/fork_context_spec.rb +++ b/spec/contexts/fork_context_spec.rb @@ -3,29 +3,45 @@ require 'spec_helper' describe Projects::ForkContext do describe :fork_by_user do before do - @from_user = create :user - @from_project = create(:project, creator_id: @from_user.id) - @to_user = create :user + @from_namespace = create(:namespace) + @from_user = create(:user, namespace: @from_namespace ) + @from_project = create(:project, creator_id: @from_user.id, namespace: @from_namespace) + @to_namespace = create(:namespace) + @to_user = create(:user, namespace: @to_namespace) end context 'fork project' do - before do + + it "successfully creates project in the user namespace" do @to_project = fork_project(@from_project, @to_user) - end - it { @to_project.owner.should == @to_user } - it { @to_project.namespace.should == @to_user.namespace } + @to_project.owner.should == @to_user + @to_project.namespace.should == @to_user.namespace + end end context 'fork project failure' do - before do - #corrupt the project so the attempt to fork will fail - @from_project = create(:project, path: "empty") + + it "fails due to transaction failure" do + # make the mock gitlab-shell fail @to_project = fork_project(@from_project, @to_user, false) + + @to_project.errors.should_not be_empty + @to_project.errors[:base].should include("Fork transaction failed.") end - it {@to_project.errors.should_not be_empty} - it {@to_project.errors[:base].should include("Can't fork project. Please try again later") } + end + + context 'project already exists' do + + it "should fail due to validation, not transaction failure" do + @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) + @to_project = fork_project(@from_project, @to_user) + + @existing_project.persisted?.should be_true + @to_project.errors[:base].should include("Invalid fork destination") + @to_project.errors[:base].should_not include("Fork transaction failed.") + end end end