diff options
author | Matija Čupić <matteeyah@gmail.com> | 2018-11-15 22:54:21 +0100 |
---|---|---|
committer | Matija Čupić <matteeyah@gmail.com> | 2018-12-08 19:28:34 +0100 |
commit | 855e7c32b9f3541fec085726d338802c8ca9b9f4 (patch) | |
tree | 83794f5482e6b36e1f779788eb2d2c0fc24e51c0 | |
parent | ce14c20a82af697b927666123443a25f19dab2ae (diff) | |
download | gitlab-ce-855e7c32b9f3541fec085726d338802c8ca9b9f4.tar.gz |
Use Gitlab::Git::Ref in Project#resolve_ref
Reworks Project#resolve_ref to return Gitlab::Git::Branch,
Gitlab::Git::Tag or raise an AmbiguousRef error.
-rw-r--r-- | app/models/project.rb | 11 | ||||
-rw-r--r-- | app/models/repository.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/git/branch.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/git/ref.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/git/tag.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/git/branch_spec.rb | 12 | ||||
-rw-r--r-- | spec/lib/gitlab/git/tag_spec.rb | 13 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 55 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 28 |
9 files changed, 113 insertions, 28 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 61840c972ee..6893f76dda9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1168,11 +1168,11 @@ class Project < ActiveRecord::Base if tag_exists && branch_exists raise AmbiguousRef elsif tag_exists - Gitlab::Git::TAG_REF_PREFIX + ref + repository.find_tag(ref) elsif branch_exists - Gitlab::Git::BRANCH_REF_PREFIX + ref + repository.find_branch(ref) else - ref + repository.find_ref(ref) end end @@ -1753,8 +1753,9 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - full_ref = resolve_ref(ref) - ref_name = Gitlab::Git.ref_name(full_ref) + resolved_ref = resolve_ref(ref) + full_ref = resolved_ref.full_ref + ref_name = resolved_ref.name if Gitlab::Git.branch_ref?(full_ref) ProtectedBranch.protected?(self, ref_name) diff --git a/app/models/repository.rb b/app/models/repository.rb index 35dd120856d..10635eb0cf4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -182,6 +182,16 @@ class Repository tags.find { |tag| tag.name == name } end + def find_ref(ref) + if Gitlab::Git.tag_ref?(ref) + find_tag(Gitlab::Git.ref_name(ref)) + elsif Gitlab::Git.branch_ref?(ref) + find_branch(Gitlab::Git.ref_name(ref)) + else + nil + end + end + def add_branch(user, branch_name, ref) branch = raw_repository.add_branch(branch_name, user: user, target: ref) diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index 9447cfa0fb6..d25eebd5f47 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -28,6 +28,10 @@ module Gitlab def state active? ? :active : :stale end + + def full_ref + Gitlab::Git::BRANCH_REF_PREFIX + name + end end end end diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index eec91194949..b9c8b7c08e9 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -39,6 +39,10 @@ module Gitlab nil end end + + def full_ref + raise NotImplementedError + end end end end diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb index 23d989ff258..ec89bc4f7e6 100644 --- a/lib/gitlab/git/tag.rb +++ b/lib/gitlab/git/tag.rb @@ -62,6 +62,10 @@ module Gitlab encode! @message end + def full_ref + Gitlab::Git::TAG_REF_PREFIX + name + end + private def message_from_gitaly_tag diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 0df282d0ae3..1b26b5ef7fd 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -124,6 +124,18 @@ describe Gitlab::Git::Branch, :seed_helper do it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) } + describe '#full_ref' do + subject do + described_class.new(repository, 'master', + repository.commit.sha, + repository.commit).full_ref + end + + it 'returns the full ref' do + is_expected.to eq('refs/heads/master') + end + end + def create_commit params[:message].delete!("\r") Rugged::Commit.create(rugged, params.merge(committer: committer.merge(time: Time.now))) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index b51e3879f49..f5d0b6af6f0 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -69,4 +69,17 @@ describe Gitlab::Git::Tag, :seed_helper do end end end + + describe '#full_ref' do + subject do + described_class.new(repository, { name: 'master', + target: repository.commit.sha, + target_commit: repository.commit }) + .full_ref + end + + it 'returns the full ref' do + is_expected.to eq('refs/tags/master') + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a519435322c..e2e8a76ab72 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2563,7 +2563,7 @@ describe Project do end describe '#protected_for?' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } subject { project.protected_for?('ref') } @@ -2572,7 +2572,7 @@ describe Project do end context 'when the ref is not protected' do - let(:ref) { 'refs/heads/ref' } + let(:ref) { project.repository.find_branch('master') } before do stub_application_setting( @@ -2585,10 +2585,10 @@ describe Project do end context 'when the ref is a protected branch' do - let(:ref) { 'refs/heads/ref' } + let(:ref) { project.repository.find_branch('master') } before do - create(:protected_branch, name: 'ref', project: project) + create(:protected_branch, name: 'master', project: project) end it 'returns true' do @@ -2597,10 +2597,10 @@ describe Project do end context 'when the ref is a protected tag' do - let(:ref) { 'refs/tags/ref' } + let(:ref) { project.repository.find_tag('v1.0.0') } before do - create(:protected_tag, name: 'ref', project: project) + create(:protected_tag, name: 'v1.0.0', project: project) end it 'returns true' do @@ -2789,29 +2789,36 @@ describe Project do end describe '#resolve_ref' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } subject { project.resolve_ref(ref) } context 'when ref is full ref' do - let(:ref) { 'refs/heads/master' } + context 'when ref exists' do + let(:ref) { 'refs/heads/master' } + + it 'returns the ref' do + is_expected.to be_a(Gitlab::Git::Ref) + end + end + + context 'when ref does not exist' do + let(:ref) { 'refs/tags/doesnotexist' } - it 'returns the unchanged ref' do - is_expected.to eq(ref) + it 'returns nil' do + is_expected.to eq(nil) + end end end context 'when ref is a tag or branch name' do let(:ref) { 'ref' } - before do - allow(project.repository).to receive(:tag_exists?).and_return(tag_exists) - allow(project.repository).to receive(:branch_exists?).and_return(branch_exists) - end - context 'when ref is ambiguous' do - let(:tag_exists) { true } - let(:branch_exists) { true } + before do + project.repository.add_tag(project.creator, ref, 'master') + project.repository.add_branch(project.creator, ref, 'master') + end it 'raises an error' do expect { subject }.to raise_error(described_class::AmbiguousRef) @@ -2819,20 +2826,22 @@ describe Project do end context 'when ref is tag name' do - let(:tag_exists) { true } - let(:branch_exists) { false } + before do + project.repository.add_tag(project.creator, ref, 'master') + end it 'returns full tag ref path' do - is_expected.to eq('refs/tags/ref') + is_expected.to be_a(Gitlab::Git::Tag) end end context 'when ref is branch name' do - let(:tag_exists) { false } - let(:branch_exists) { true } + before do + project.repository.add_branch(project.creator, ref, 'master') + end it 'returns full branch ref path' do - is_expected.to eq('refs/heads/ref') + is_expected.to be_a(Gitlab::Git::Branch) end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f09b4b67061..df0ab7bc0f4 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1699,6 +1699,34 @@ describe Repository do end end + describe '#find_ref' do + subject { repository.find_ref(ref) } + + context 'when ref is a tag' do + let(:ref) { 'refs/tags/v1.0.0' } + + it 'returns the tag' do + is_expected.to be_a(Gitlab::Git::Tag) + end + end + + context 'when ref is a branch' do + let(:ref) { 'refs/heads/master' } + + it 'returns the branch' do + is_expected.to be_a(Gitlab::Git::Branch) + end + end + + context 'when ref is invalid' do + let(:ref) { 'refs/notvalid/ref' } + + it 'returns nil' do + is_expected.to eq(nil) + end + end + end + describe '#avatar' do it 'returns nil if repo does not exist' do allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) |