Introduce MergeRequest#write_ref and Repository#write_ref

so that we don't have to fetch it for non-forks

6 files changed, 39 insertions, 24 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index f90194041b1..b6c9b8f21c7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -792,11 +792,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def fetch_ref
-    target_project.repository.fetch_ref(
-      source_project.repository.path_to_repo,
-      "refs/heads/#{source_branch}",
-      ref_path
-    )
+    write_ref
     update_column(:ref_fetched, true)
   end
 
@@ -939,4 +935,18 @@ class MergeRequest < ActiveRecord::Base
 
     true
   end
+
+  private
+
+  def write_ref
+    if for_fork?
+      target_project.repository.fetch_ref(
+        source_project.repository.path_to_repo,
+        "refs/heads/#{source_branch}",
+        ref_path
+      )
+    else
+      source_project.repository.write_ref(ref_path, source_branch_sha)
+    end
+  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index 7010664e1c8..7cdd00bc17b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1048,9 +1048,7 @@ class Project < ActiveRecord::Base
   def change_head(branch)
     if repository.branch_exists?(branch)
       repository.before_change_head
-      repository.rugged.references.create('HEAD',
-                                          "refs/heads/#{branch}",
-                                          force: true)
+      repository.write_ref('HEAD', "refs/heads/#{branch}")
       repository.copy_gitattributes(branch)
       repository.after_change_head
       reload_default_branch
diff --git a/app/models/repository.rb b/app/models/repository.rb
index f8139ff595e..04403cdd035 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -224,7 +224,7 @@ class Repository
 
     # This will still fail if the file is corrupted (e.g. 0 bytes)
     begin
-      rugged.references.create(keep_around_ref_name(sha), sha, force: true)
+      write_ref(keep_around_ref_name(sha), sha)
     rescue Rugged::ReferenceError => ex
       Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
     rescue Rugged::OSError => ex
@@ -237,6 +237,10 @@ class Repository
     ref_exists?(keep_around_ref_name(sha))
   end
 
+  def write_ref(ref_path, sha)
+    rugged.references.create(ref_path, sha, force: true)
+  end
+
   def diverging_commit_counts(branch)
     root_ref_hash = raw_repository.rev_parse_target(root_ref).oid
     cache.fetch(:"diverging_commit_counts_#{branch.name}") do
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 04493981945..cbec716d6ea 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -72,10 +72,10 @@ FactoryGirl.define do
       target_project = merge_request.target_project
       source_project = merge_request.source_project
 
-      # Fake `fetch_ref` if we don't have repository
+      # Fake `write_ref` if we don't have repository
       # We have too many existing tests replying on this behaviour
       unless [target_project, source_project].all?(&:repository_exists?)
-        allow(target_project.repository).to receive(:fetch_ref)
+        allow(merge_request).to receive(:write_ref)
       end
     end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 858616117d5..19f576d216c 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1196,8 +1196,9 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
   def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
     source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
-    rugged = repository.rugged
-    rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha)
+    repository.write_ref(
+      "refs/remotes/#{remote_name}/#{branch_name}",
+      source_branch.dereferenced_target.sha)
   end
 
   # Build the options hash that's passed to Rugged::Commit#create
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index ef7516fc28f..68052ef1035 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -315,15 +315,17 @@ describe API::MergeRequests do
       let!(:fork_project) { create(:project, forked_from_project: project,  namespace: user2.namespace, creator_id: user2.id) }
       let!(:unrelated_project) { create(:project,  namespace: create(:user).namespace, creator_id: user2.id) }
 
-      before :each do |each|
-        fork_project.team << [user2, :reporter]
+      before do
+        fork_project.add_reporter(user2)
+
+        allow_any_instance_of(MergeRequest).to receive(:write_ref)
       end
 
       it "returns merge_request" do
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
           title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master",
           author: user2, target_project_id: project.id, description: 'Test description for Test merge_request'
-        expect(response).to have_http_status(201)
+        expect(response).to have_gitlab_http_status(201)
         expect(json_response['title']).to eq('Test merge_request')
         expect(json_response['description']).to eq('Test description for Test merge_request')
       end
@@ -334,7 +336,7 @@ describe API::MergeRequests do
         expect(fork_project.forked_from_project).to eq(project)
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
         title: 'Test merge_request', source_branch: "master", target_branch: "master", author: user2, target_project_id: project.id
-        expect(response).to have_http_status(201)
+        expect(response).to have_gitlab_http_status(201)
         expect(json_response['title']).to eq('Test merge_request')
       end
 
@@ -348,25 +350,25 @@ describe API::MergeRequests do
              author: user2,
              target_project_id: project.id
 
-        expect(response).to have_http_status(422)
+        expect(response).to have_gitlab_http_status(422)
       end
 
       it "returns 400 when source_branch is missing" do
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
         title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id
-        expect(response).to have_http_status(400)
+        expect(response).to have_gitlab_http_status(400)
       end
 
       it "returns 400 when target_branch is missing" do
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
         title: 'Test merge_request', target_branch: "master", author: user2, target_project_id: project.id
-        expect(response).to have_http_status(400)
+        expect(response).to have_gitlab_http_status(400)
       end
 
       it "returns 400 when title is missing" do
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
         target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: project.id
-        expect(response).to have_http_status(400)
+        expect(response).to have_gitlab_http_status(400)
       end
 
       context 'when target_branch is specified' do
@@ -377,7 +379,7 @@ describe API::MergeRequests do
                source_branch: 'markdown',
                author: user,
                target_project_id: fork_project.id
-          expect(response).to have_http_status(422)
+          expect(response).to have_gitlab_http_status(422)
         end
 
         it 'returns 422 if targeting a different fork' do
@@ -387,14 +389,14 @@ describe API::MergeRequests do
                source_branch: 'markdown',
                author: user2,
                target_project_id: unrelated_project.id
-          expect(response).to have_http_status(422)
+          expect(response).to have_gitlab_http_status(422)
         end
       end
 
       it "returns 201 when target_branch is specified and for the same project" do
         post v3_api("/projects/#{fork_project.id}/merge_requests", user2),
         title: 'Test merge_request', target_branch: 'master', source_branch: 'markdown', author: user2, target_project_id: fork_project.id
-        expect(response).to have_http_status(201)
+        expect(response).to have_gitlab_http_status(201)
       end
     end
   end