Support both ref and ref-name in protected_for?

2 files changed, 87 insertions, 8 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index f5dc58cd67f..61840c972ee 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -36,6 +36,7 @@ class Project < ActiveRecord::Base
   extend Gitlab::ConfigHelper
 
   BoardLimitExceeded = Class.new(StandardError)
+  AmbiguousRef = Class.new(StandardError)
 
   STATISTICS_ATTRIBUTE = 'repositories_count'.freeze
   NUMBER_OF_PERMITTED_BOARDS = 1
@@ -1160,6 +1161,21 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def resolve_ref(ref)
+    tag_exists = repository.tag_exists?(ref)
+    branch_exists = repository.branch_exists?(ref)
+
+    if tag_exists && branch_exists
+      raise AmbiguousRef
+    elsif tag_exists
+      Gitlab::Git::TAG_REF_PREFIX + ref
+    elsif branch_exists
+      Gitlab::Git::BRANCH_REF_PREFIX + ref
+    else
+      ref
+    end
+  end
+
   def root_ref?(branch)
     repository.root_ref == branch
   end
@@ -1737,10 +1753,13 @@ class Project < ActiveRecord::Base
   end
 
   def protected_for?(ref)
-    if repository.branch_exists?(ref)
-      ProtectedBranch.protected?(self, ref)
-    elsif repository.tag_exists?(ref)
-      ProtectedTag.protected?(self, ref)
+    full_ref = resolve_ref(ref)
+    ref_name = Gitlab::Git.ref_name(full_ref)
+
+    if Gitlab::Git.branch_ref?(full_ref)
+      ProtectedBranch.protected?(self, ref_name)
+    elsif Gitlab::Git.tag_ref?(full_ref)
+      ProtectedTag.protected?(self, ref_name)
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 1c85411dc3b..a519435322c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2520,6 +2520,10 @@ describe Project do
     end
 
     context 'when the ref is not protected' do
+      before do
+        allow(project).to receive(:protected_for?).with('ref').and_return(false)
+      end
+
       it 'contains only the CI variables' do
         is_expected.to contain_exactly(ci_variable)
       end
@@ -2563,7 +2567,13 @@ describe Project do
 
     subject { project.protected_for?('ref') }
 
+    before do
+      allow(project).to receive(:resolve_ref).and_return(ref)
+    end
+
     context 'when the ref is not protected' do
+      let(:ref) { 'refs/heads/ref' }
+
       before do
         stub_application_setting(
           default_branch_protection: Gitlab::Access::PROTECTION_NONE)
@@ -2575,9 +2585,9 @@ describe Project do
     end
 
     context 'when the ref is a protected branch' do
+      let(:ref) { 'refs/heads/ref' }
+
       before do
-        allow(project).to receive(:repository).and_call_original
-        allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true)
         create(:protected_branch, name: 'ref', project: project)
       end
 
@@ -2587,9 +2597,9 @@ describe Project do
     end
 
     context 'when the ref is a protected tag' do
+      let(:ref) { 'refs/tags/ref' }
+
       before do
-        allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false)
-        allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true)
         create(:protected_tag, name: 'ref', project: project)
       end
 
@@ -2778,6 +2788,56 @@ describe Project do
     end
   end
 
+  describe '#resolve_ref' do
+    let(:project) { create(:project) }
+
+    subject { project.resolve_ref(ref) }
+
+    context 'when ref is full ref' do
+      let(:ref) { 'refs/heads/master' }
+
+      it 'returns the unchanged ref' do
+        is_expected.to eq(ref)
+      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 }
+
+        it 'raises an error' do
+          expect { subject }.to raise_error(described_class::AmbiguousRef)
+        end
+      end
+
+      context 'when ref is tag name' do
+        let(:tag_exists) { true }
+        let(:branch_exists) { false }
+
+        it 'returns full tag ref path' do
+          is_expected.to eq('refs/tags/ref')
+        end
+      end
+
+      context 'when ref is branch name' do
+        let(:tag_exists) { false }
+        let(:branch_exists) { true }
+
+        it 'returns full branch ref path' do
+          is_expected.to eq('refs/heads/ref')
+        end
+      end
+    end
+  end
+
   describe '#http_url_to_repo' do
     let(:project) { create(:project) }