Introduce MergeRequest#write_ref and Repository#write_ref
so that we don't have to fetch it for non-forks
This commit is contained in:
parent
64e13d1958
commit
c772464b68
6 changed files with 39 additions and 24 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue