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.

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)