Download as
Browse Code »

Commit de8df1be1a75cfde02a5a1d3f52a888b770c54e0

Authored by Angus MacArthur
1 parent ae33fdf2
Exists in master and in 4 other branches 6-8-stable, 7-0-stable, 7-0-stable-spb, spb-stable

Fix bug for repeated fork requests 

When asking to fork a project and a project with the same name
already exists (likely from a previous fork), the recovery from
the fork failure would inadvertantly delete the repo of the
existing destination project.
Showing 2 changed files with 53 additions and 29 deletions   Show diff stats
app/contexts/projects/fork_context.rb
  Diff comments   View file @de8df1b
@@ -10,28 +10,36 @@ module Projects @@ -10,28 +10,36 @@ module Projects
10 project = Project.new 10 project = Project.new
11 project.initialize_dup(@from_project) 11 project.initialize_dup(@from_project)
12 project.name = @from_project.name 12 project.name = @from_project.name
13 - project.path = @from_project.path 13 + project.path = @from_project.path
14 project.namespace = current_user.namespace 14 project.namespace = current_user.namespace
  15 + project.creator = current_user
15 16
16 - Project.transaction do  
17 - #First save the DB entries as they can be rolled back if the repo fork fails  
18 - project.creator = current_user  
19 - project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id)  
20 - if project.save  
21 - project.users_projects.create(project_access: UsersProject::MASTER, user: current_user) 17 + # If the project cannot save, we do not want to trigger the project destroy
  18 + # as this can have the side effect of deleting a repo attached to an existing
  19 + # project with the same name and namespace
  20 + if project.valid?
  21 + begin
  22 + Project.transaction do
  23 + #First save the DB entries as they can be rolled back if the repo fork fails
  24 + project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id)
  25 + if project.save
  26 + project.users_projects.create(project_access: UsersProject::MASTER, user: current_user)
  27 + end
  28 + #Now fork the repo
  29 + unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path)
  30 + raise "forking failed in gitlab-shell"
  31 + end
  32 + project.ensure_satellite_exists
  33 + end
  34 + rescue => ex
  35 + project.errors.add(:base, "Fork transaction failed.")
  36 + project.destroy
22 end 37 end
23 - #Now fork the repo  
24 - unless gitlab_shell.fork_repository(@from_project.path_with_namespace, project.namespace.path)  
25 - raise "forking failed in gitlab-shell"  
26 - end  
27 - project.ensure_satellite_exists  
28 - 38 + else
  39 + project.errors.add(:base, "Invalid fork destination")
29 end 40 end
30 project 41 project
31 - rescue => ex  
32 - project.errors.add(:base, "Can't fork project. Please try again later")  
33 - project.destroy  
34 - end  
35 42
  43 + end
36 end 44 end
37 end 45 end
spec/contexts/fork_context_spec.rb
  Diff comments   View file @de8df1b
@@ -3,29 +3,45 @@ require 'spec_helper' @@ -3,29 +3,45 @@ require 'spec_helper'
3 describe Projects::ForkContext do 3 describe Projects::ForkContext do
4 describe :fork_by_user do 4 describe :fork_by_user do
5 before do 5 before do
6 - @from_user = create :user  
7 - @from_project = create(:project, creator_id: @from_user.id)  
8 - @to_user = create :user 6 + @from_namespace = create(:namespace)
  7 + @from_user = create(:user, namespace: @from_namespace )
  8 + @from_project = create(:project, creator_id: @from_user.id, namespace: @from_namespace)
  9 + @to_namespace = create(:namespace)
  10 + @to_user = create(:user, namespace: @to_namespace)
9 end 11 end
10 12
11 context 'fork project' do 13 context 'fork project' do
12 - before do 14 +
  15 + it "successfully creates project in the user namespace" do
13 @to_project = fork_project(@from_project, @to_user) 16 @to_project = fork_project(@from_project, @to_user)
14 - end  
15 17
16 - it { @to_project.owner.should == @to_user }  
17 - it { @to_project.namespace.should == @to_user.namespace } 18 + @to_project.owner.should == @to_user
  19 + @to_project.namespace.should == @to_user.namespace
  20 + end
18 end 21 end
19 22
20 context 'fork project failure' do 23 context 'fork project failure' do
21 - before do  
22 - #corrupt the project so the attempt to fork will fail  
23 - @from_project = create(:project, path: "empty") 24 +
  25 + it "fails due to transaction failure" do
  26 + # make the mock gitlab-shell fail
24 @to_project = fork_project(@from_project, @to_user, false) 27 @to_project = fork_project(@from_project, @to_user, false)
  28 +
  29 + @to_project.errors.should_not be_empty
  30 + @to_project.errors[:base].should include("Fork transaction failed.")
25 end 31 end
26 32
27 - it {@to_project.errors.should_not be_empty}  
28 - it {@to_project.errors[:base].should include("Can't fork project. Please try again later") } 33 + end
  34 +
  35 + context 'project already exists' do
  36 +
  37 + it "should fail due to validation, not transaction failure" do
  38 + @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace)
  39 + @to_project = fork_project(@from_project, @to_user)
  40 +
  41 + @existing_project.persisted?.should be_true
  42 + @to_project.errors[:base].should include("Invalid fork destination")
  43 + @to_project.errors[:base].should_not include("Fork transaction failed.")
  44 + end
29 45
30 end 46 end
31 end 47 end