Merge pull request #3597 from amacarthur/fork-pull-request

updated fork feature to use gitlab-shell for v5 of gitlab

Merge pull request #3597 from amacarthur/fork-pull-request
updated fork feature to use gitlab-shell for v5 of gitlab
Dmitriy Zaporozhets
2 parents 36efe0f5 ec638048
Showing 19 changed files with 293 additions and 6 deletions Show diff stats
app/contexts/projects/fork_context.rb
app/controllers/projects_controller.rb
app/models/ability.rb
app/models/forked_project_link.rb
app/models/project.rb
app/observers/project_observer.rb
app/views/projects/_clone_panel.html.haml
config/routes.rb
db/migrate/20130319214458_create_forked_project_links.rb
db/schema.rb
features/project/fork_project.feature
features/steps/project/project_fork.rb
lib/gitlab/backend/shell.rb
spec/contexts/fork_context_spec.rb
spec/factories/forked_project_links.rb
spec/lib/gitlab/backend/shell_spec.rb
spec/models/forked_project_link_spec.rb
spec/models/project_spec.rb
spec/routing/project_routing_spec.rb
@@ -0,0 +1,45 @@
+module Projects
+  class ForkContext < BaseContext
+    include Gitlab::ShellAdapter
+
+    def initialize(project, user)
+      @from_project, @current_user = project, user
+    end
+
+    def execute
+      project = Project.new
+      project.initialize_dup(@from_project)
+      project.name = @from_project.name
+      project.path = @from_project.path
+      project.namespace = current_user.namespace
+      project.creator = 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
+      else
+        project.errors.add(:base, "Invalid fork destination")
+      end
+      project
+
+    end
+  end
+end
@@ -78,4 +78,19 @@ class ProjectsController &lt; ProjectResourceController
       format.html { redirect_to root_path }
     end
   end
+
+  def fork
+    @project = ::Projects::ForkContext.new(project, current_user).execute
+
+    respond_to do |format|
+      format.html do
+        if @project.saved? && @project.forked?
+          redirect_to(@project, notice: 'Project was successfully forked.')
+        else
+          render action: "new"
+        end
+      end
+      format.js
+    end
+  end
 end
@@ -67,7 +67,9 @@ class Ability
     def project_report_rules
       project_guest_rules + [
         :download_code,
-        :write_snippet
+        :write_snippet,
+        :fork_project
+
       ]
     end
@@ -0,0 +1,8 @@
+class ForkedProjectLink < ActiveRecord::Base
+  attr_accessible :forked_from_project_id, :forked_to_project_id
+
+  # Relations
+  belongs_to :forked_to_project, class_name: Project
+  belongs_to :forked_from_project, class_name: Project
+
+end
@@ -45,6 +45,8 @@ class Project &lt; ActiveRecord::Base
   has_one :last_event, class_name: 'Event', order: 'events.created_at DESC', foreign_key: 'project_id'
   has_one :gitlab_ci_service, dependent: :destroy
+  has_one :forked_project_link, dependent: :destroy, foreign_key: "forked_to_project_id"
+  has_one :forked_from_project, through: :forked_project_link
   has_many :events,             dependent: :destroy
   has_many :merge_requests,     dependent: :destroy
@@ -402,4 +404,9 @@ class Project &lt; ActiveRecord::Base
   def protected_branch? branch_name
     protected_branches_names.include?(branch_name)
   end
+
+  def forked?
+    !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
+  end
+
 end
 class ProjectObserver < BaseObserver
   def after_create(project)
-    GitlabShellWorker.perform_async(
-      :add_repository,
-      project.path_with_namespace
-    )
+    unless project.forked?
+      GitlabShellWorker.perform_async(
+          :add_repository,
+          project.path_with_namespace
+      )
-    log_info("#{project.owner.name} created a new project \"#{project.name_with_namespace}\"")
+      log_info("#{project.owner.name} created a new project \"#{project.name_with_namespace}\"")
+    end
   end
   def after_update(project)
@@ -5,6 +5,9 @@
     .span3.pull-right
       .pull-right
         - unless @project.empty_repo?
+          - if can? current_user, :fork_project, @project
+            = link_to fork_project_path(@project), title: "Fork", class: "btn small grouped", method: "POST" do
+              Fork
           - if can? current_user, :download_code, @project
             = link_to archive_project_repository_path(@project), class: "btn grouped" do
               %i.icon-download-alt
@@ -167,6 +167,7 @@ Gitlab::Application.routes.draw do
   resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do
     member do
       put :transfer
+      post :fork
     end
     resources :blob,    only: [:show], constraints: {id: /.+/}
@@ -0,0 +1,11 @@
+class CreateForkedProjectLinks < ActiveRecord::Migration
+  def change
+    create_table :forked_project_links do |t|
+      t.integer :forked_to_project_id, null: false
+      t.integer :forked_from_project_id, null: false
+
+      t.timestamps
+    end
+    add_index :forked_project_links, :forked_to_project_id, unique: true
+  end
+end
@@ -32,6 +32,15 @@ ActiveRecord::Schema.define(:version =&gt; 20130410175022) do
   add_index "events", ["target_id"], :name => "index_events_on_target_id"
   add_index "events", ["target_type"], :name => "index_events_on_target_type"
+  create_table "forked_project_links", :force => true do |t|
+    t.integer  "forked_to_project_id",   :null => false
+    t.integer  "forked_from_project_id", :null => false
+    t.datetime "created_at",             :null => false
+    t.datetime "updated_at",             :null => false
+  end
+
+  add_index "forked_project_links", ["forked_to_project_id"], :name => "index_forked_project_links_on_forked_to_project_id", :unique => true
+
   create_table "issues", :force => true do |t|
     t.string   "title"
     t.integer  "assignee_id"
@@ -0,0 +1,14 @@
+Feature: Fork Project
+  Background:
+    Given I sign in as a user
+    And I am a member of project "Shop"
+    When I visit project "Shop" page
+
+  Scenario: User fork a project
+    Given I click link "Fork"
+    Then I should see the forked project page
+
+  Scenario: User already has forked the project
+    Given I already have a project named "Shop" in my namespace
+    And I click link "Fork"
+    Then I should see a "Name has already been taken" warning
@@ -0,0 +1,30 @@
+class ForkProject < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedProject
+
+  step 'I click link "Fork"' do
+    Gitlab::Shell.any_instance.stub(:fork_repository).and_return(true)
+    click_link "Fork"
+  end
+
+  step 'I am a member of project "Shop"' do
+    @project = Project.find_by_name "Shop"
+    @project ||= create(:project_with_code, name: "Shop")
+    @project.team << [@user, :reporter]
+  end
+
+  step 'I should see the forked project page' do
+    page.should have_content "Project was successfully forked."
+    current_path.should include current_user.namespace.path
+  end
+
+  step 'I already have a project named "Shop" in my namespace' do
+    @my_project = create(:project_with_code, name: "Shop", namespace: current_user.namespace)
+  end
+
+  step 'I should see a "Name has already been taken" warning' do
+    page.should have_content "Name has already been taken"
+  end
+
+end
 \ No newline at end of file
@@ -36,6 +36,18 @@ module Gitlab
       system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-projects mv-project #{path}.git #{new_path}.git")
     end
+    # Fork repository to new namespace
+    #
+    # path - project path with namespace
+    # fork_namespace - namespace for forked project
+    #
+    # Ex.
+    #  fork_repository("gitlab/gitlab-ci", "randx")
+    #
+    def fork_repository(path, fork_namespace)
+      system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-projects fork-project #{path}.git #{fork_namespace}")
+    end
+
     # Remove repository from file system
     #
     # name - project path with namespace
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Projects::ForkContext do
+  describe :fork_by_user do
+    before do
+      @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
+
+      it "successfully creates project in the user namespace" do
+        @to_project = fork_project(@from_project, @to_user)
+
+        @to_project.owner.should == @to_user
+        @to_project.namespace.should == @to_user.namespace
+      end
+    end
+
+    context 'fork project failure' do
+
+      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
+
+    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
+
+  def fork_project(from_project, user, fork_success = true)
+    context = Projects::ForkContext.new(from_project, user)
+    shell = mock("gitlab_shell")
+    shell.stub(fork_repository: fork_success)
+    context.stub(gitlab_shell: shell)
+    context.execute
+  end
+
+end
@@ -0,0 +1,8 @@
+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :forked_project_link do
+    association :forked_to_project, factory: :project
+    association :forked_from_project, factory: :project
+  end
+end
@@ -12,6 +12,7 @@ describe Gitlab::Shell do
   it { should respond_to :remove_key }
   it { should respond_to :add_repository }
   it { should respond_to :remove_repository }
+  it { should respond_to :fork_repository }
   it { gitlab_shell.url_to_repo('diaspora').should == Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git" }
 end
@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe ForkedProjectLink, "add link on fork" do
+  let(:project_from) {create(:project)}
+  let(:namespace) {create(:namespace)}
+  let(:user) {create(:user, namespace: namespace)}
+
+  before do
+    @project_to = fork_project(project_from, user)
+  end
+
+  it "project_to should know it is forked" do
+    @project_to.forked?.should be_true
+  end
+
+  it "project should know who it is forked from" do
+    @project_to.forked_from_project.should == project_from
+  end
+end
+
+describe :forked_from_project do
+  let(:forked_project_link) {build(:forked_project_link)}
+  let(:project_from) {create(:project)}
+  let(:project_to) {create(:project, forked_project_link: forked_project_link)}
+
+
+  before :each do
+    forked_project_link.forked_from_project = project_from
+    forked_project_link.forked_to_project = project_to
+    forked_project_link.save!
+  end
+
+
+  it "project_to should know it is forked" do
+    project_to.forked?.should be_true
+  end
+
+  it "project_from should not be forked" do
+    project_from.forked?.should be_false
+  end
+
+  it "project_to.destroy should destroy fork_link" do
+    forked_project_link.should_receive(:destroy)
+    project_to.destroy
+  end
+
+end
+
+def fork_project(from_project, user)
+  context = Projects::ForkContext.new(from_project, user)
+  shell = mock("gitlab_shell")
+  shell.stub(fork_repository: true)
+  context.stub(gitlab_shell: shell)
+  context.execute
+end
+
@@ -40,6 +40,7 @@ describe Project do
     it { should have_many(:deploy_keys).dependent(:destroy) }
     it { should have_many(:hooks).dependent(:destroy) }
     it { should have_many(:protected_branches).dependent(:destroy) }
+    it { should have_one(:forked_project_link).dependent(:destroy) }
   end
   describe "Mass assignment" do
@@ -55,6 +55,7 @@ end
 #      projects POST   /projects(.:format)     projects#create
 #   new_project GET    /projects/new(.:format) projects#new
+#  fork_project POST   /:id/fork(.:format)     projects#fork
 #  wall_project GET    /:id/wall(.:format)     projects#wall
 # files_project GET    /:id/files(.:format)    projects#files
 #  edit_project GET    /:id/edit(.:format)     projects#edit
@@ -70,6 +71,10 @@ describe ProjectsController, &quot;routing&quot; do
     get("/projects/new").should route_to('projects#new')
   end
+  it "to #fork" do
+    post("/gitlabhq/fork").should route_to('projects#fork', id: 'gitlabhq')
+  end
+
   it "to #wall" do
     get("/gitlabhq/wall").should route_to('walls#show', project_id: 'gitlabhq')
   end
@@ -0,0 +1,45 @@		@@ -0,0 +1,45 @@
	1	+module Projects
	2	+ class ForkContext < BaseContext
	3	+ include Gitlab::ShellAdapter
	4	+
	5	+ def initialize(project, user)
	6	+ @from_project, @current_user = project, user
	7	+ end
	8	+
	9	+ def execute
	10	+ project = Project.new
	11	+ project.initialize_dup(@from_project)
	12	+ project.name = @from_project.name
	13	+ project.path = @from_project.path
	14	+ project.namespace = current_user.namespace
	15	+ project.creator = current_user
	16	+
	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
	37	+ end
	38	+ else
	39	+ project.errors.add(:base, "Invalid fork destination")
	40	+ end
	41	+ project
	42	+
	43	+ end
	44	+ end
	45	+end
	@@ -78,4 +78,19 @@ class ProjectsController < ProjectResourceController		@@ -78,4 +78,19 @@ class ProjectsController < ProjectResourceController
78	format.html { redirect_to root_path }	78	format.html { redirect_to root_path }
79	end	79	end
80	end	80	end
		81	+
		82	+ def fork
		83	+ @project = ::Projects::ForkContext.new(project, current_user).execute
		84	+
		85	+ respond_to do \|format\|
		86	+ format.html do
		87	+ if @project.saved? && @project.forked?
		88	+ redirect_to(@project, notice: 'Project was successfully forked.')
		89	+ else
		90	+ render action: "new"
		91	+ end
		92	+ end
		93	+ format.js
		94	+ end
		95	+ end
81	end	96	end
@@ -0,0 +1,8 @@		@@ -0,0 +1,8 @@
	1	+class ForkedProjectLink < ActiveRecord::Base
	2	+ attr_accessible :forked_from_project_id, :forked_to_project_id
	3	+
	4	+ # Relations
	5	+ belongs_to :forked_to_project, class_name: Project
	6	+ belongs_to :forked_from_project, class_name: Project
	7	+
	8	+end
	@@ -167,6 +167,7 @@ Gitlab::Application.routes.draw do		@@ -167,6 +167,7 @@ Gitlab::Application.routes.draw do
167	resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do	167	resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do
168	member do	168	member do
169	put :transfer	169	put :transfer
		170	+ post :fork
170	end	171	end
171		172
172	resources :blob, only: [:show], constraints: {id: /.+/}	173	resources :blob, only: [:show], constraints: {id: /.+/}
@@ -0,0 +1,11 @@		@@ -0,0 +1,11 @@
	1	+class CreateForkedProjectLinks < ActiveRecord::Migration
	2	+ def change
	3	+ create_table :forked_project_links do \|t\|
	4	+ t.integer :forked_to_project_id, null: false
	5	+ t.integer :forked_from_project_id, null: false
	6	+
	7	+ t.timestamps
	8	+ end
	9	+ add_index :forked_project_links, :forked_to_project_id, unique: true
	10	+ end
	11	+end
	@@ -32,6 +32,15 @@ ActiveRecord::Schema.define(:version => 20130410175022) do		@@ -32,6 +32,15 @@ ActiveRecord::Schema.define(:version => 20130410175022) do
32	add_index "events", ["target_id"], :name => "index_events_on_target_id"	32	add_index "events", ["target_id"], :name => "index_events_on_target_id"
33	add_index "events", ["target_type"], :name => "index_events_on_target_type"	33	add_index "events", ["target_type"], :name => "index_events_on_target_type"
34		34
		35	+ create_table "forked_project_links", :force => true do \|t\|
		36	+ t.integer "forked_to_project_id", :null => false
		37	+ t.integer "forked_from_project_id", :null => false
		38	+ t.datetime "created_at", :null => false
		39	+ t.datetime "updated_at", :null => false
		40	+ end
		41	+
		42	+ add_index "forked_project_links", ["forked_to_project_id"], :name => "index_forked_project_links_on_forked_to_project_id", :unique => true
		43	+
35	create_table "issues", :force => true do \|t\|	44	create_table "issues", :force => true do \|t\|
36	t.string "title"	45	t.string "title"
37	t.integer "assignee_id"	46	t.integer "assignee_id"
@@ -0,0 +1,14 @@		@@ -0,0 +1,14 @@
	1	+Feature: Fork Project
	2	+ Background:
	3	+ Given I sign in as a user
	4	+ And I am a member of project "Shop"
	5	+ When I visit project "Shop" page
	6	+
	7	+ Scenario: User fork a project
	8	+ Given I click link "Fork"
	9	+ Then I should see the forked project page
	10	+
	11	+ Scenario: User already has forked the project
	12	+ Given I already have a project named "Shop" in my namespace
	13	+ And I click link "Fork"
	14	+ Then I should see a "Name has already been taken" warning
	@@ -0,0 +1,30 @@		@@ -0,0 +1,30 @@
		1	+class ForkProject < Spinach::FeatureSteps
		2	+ include SharedAuthentication
		3	+ include SharedPaths
		4	+ include SharedProject
		5	+
		6	+ step 'I click link "Fork"' do
		7	+ Gitlab::Shell.any_instance.stub(:fork_repository).and_return(true)
		8	+ click_link "Fork"
		9	+ end
		10	+
		11	+ step 'I am a member of project "Shop"' do
		12	+ @project = Project.find_by_name "Shop"
		13	+ @project \|\|= create(:project_with_code, name: "Shop")
		14	+ @project.team << [@user, :reporter]
		15	+ end
		16	+
		17	+ step 'I should see the forked project page' do
		18	+ page.should have_content "Project was successfully forked."
		19	+ current_path.should include current_user.namespace.path
		20	+ end
		21	+
		22	+ step 'I already have a project named "Shop" in my namespace' do
		23	+ @my_project = create(:project_with_code, name: "Shop", namespace: current_user.namespace)
		24	+ end
		25	+
		26	+ step 'I should see a "Name has already been taken" warning' do
		27	+ page.should have_content "Name has already been taken"
		28	+ end
		29	+
		30	+end
0	\ No newline at end of file	31	\ No newline at end of file
	@@ -36,6 +36,18 @@ module Gitlab		@@ -36,6 +36,18 @@ module Gitlab
36	system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-projects mv-project #{path}.git #{new_path}.git")	36	system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-projects mv-project #{path}.git #{new_path}.git")
37	end	37	end
38		38
		39	+ # Fork repository to new namespace
		40	+ #
		41	+ # path - project path with namespace
		42	+ # fork_namespace - namespace for forked project
		43	+ #
		44	+ # Ex.
		45	+ # fork_repository("gitlab/gitlab-ci", "randx")
		46	+ #
		47	+ def fork_repository(path, fork_namespace)
		48	+ system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-projects fork-project #{path}.git #{fork_namespace}")
		49	+ end
		50	+
39	# Remove repository from file system	51	# Remove repository from file system
40	#	52	#
41	# name - project path with namespace	53	# name - project path with namespace
@@ -0,0 +1,57 @@		@@ -0,0 +1,57 @@
	1	+require 'spec_helper'
	2	+
	3	+describe Projects::ForkContext do
	4	+ describe :fork_by_user do
	5	+ before do
	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)
	11	+ end
	12	+
	13	+ context 'fork project' do
	14	+
	15	+ it "successfully creates project in the user namespace" do
	16	+ @to_project = fork_project(@from_project, @to_user)
	17	+
	18	+ @to_project.owner.should == @to_user
	19	+ @to_project.namespace.should == @to_user.namespace
	20	+ end
	21	+ end
	22	+
	23	+ context 'fork project failure' do
	24	+
	25	+ it "fails due to transaction failure" do
	26	+ # make the mock gitlab-shell fail
	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.")
	31	+ end
	32	+
	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
	45	+
	46	+ end
	47	+ end
	48	+
	49	+ def fork_project(from_project, user, fork_success = true)
	50	+ context = Projects::ForkContext.new(from_project, user)
	51	+ shell = mock("gitlab_shell")
	52	+ shell.stub(fork_repository: fork_success)
	53	+ context.stub(gitlab_shell: shell)
	54	+ context.execute
	55	+ end
	56	+
	57	+end