Merge branch...

Merge branch '20526-impossible-to-browse-a-branch-if-a-tag-exists-with-the-same-name-be-2' into 'master'

Update redirects and commit lookup code for controllers

See merge request !122237



Merged-by: Jerry Seto <jseto@gitlab.com>
Approved-by: Patrick Cyiza <jpcyiza@gitlab.com>
Reviewed-by: Matthias Käppler <mkaeppler@gitlab.com>
Reviewed-by: Patrick Cyiza <jpcyiza@gitlab.com>

app/controllers/concerns/redirects_for_missing_path_on_tree.rb

 # frozen_string_literal: true
 
 module RedirectsForMissingPathOnTree
-  def redirect_to_tree_root_for_missing_path(project, ref, path)
-    redirect_to project_tree_path(project, ref), notice: missing_path_on_ref(path, ref)
+  def redirect_to_tree_root_for_missing_path(project, ref, path, ref_type: nil)
+    redirect_to project_tree_path(project, ref, ref_type: ref_type), notice: missing_path_on_ref(path, ref)
   end
 
   private

app/controllers/projects/blob_controller.rb

@@ -160,6 +160,8 @@ def blob
   end
 
   def check_for_ambiguous_ref
+    return if Feature.enabled?(:redirect_with_ref_type, @project)
+
     @ref_type = ref_type
 
     if @ref_type == ExtractsRef::BRANCH_REF_TYPE && ambiguous_ref?(@project, @ref)
@@ -169,7 +171,17 @@ def check_for_ambiguous_ref
   end
 
   def commit
-    @commit ||= @repository.commit(@ref)
+    if Feature.enabled?(:redirect_with_ref_type, @project)
+      response = ::ExtractsRef::RequestedRef.new(@repository, ref_type: ref_type, ref: @ref).find
+      @commit = response[:commit]
+      @ref_type = response[:ref_type]
+
+      if response[:ambiguous]
+        return redirect_to(project_blob_path(@project, File.join(@ref_type ? @ref : @commit.id, @path), ref_type: @ref_type))
+      end
+    else
+      @commit ||= @repository.commit(@ref)
+    end
 
     return render_404 unless @commit
   end

app/controllers/projects/tree_controller.rb

@@ -12,6 +12,7 @@ class Projects::TreeController < Projects::ApplicationController
 
   before_action :require_non_empty_project, except: [:new, :create]
   before_action :assign_ref_vars
+  before_action :find_requested_ref, only: [:show]
   before_action :assign_dir_vars, only: [:create_dir]
   before_action :authorize_read_code!
   before_action :authorize_edit_tree!, only: [:create_dir]
@@ -28,18 +29,20 @@ class Projects::TreeController < Projects::ApplicationController
   def show
     return render_404 unless @commit
 
-    @ref_type = ref_type
-    if @ref_type == BRANCH_REF_TYPE && ambiguous_ref?(@project, @ref)
-      branch = @project.repository.find_branch(@ref)
-      if branch
-        redirect_to project_tree_path(@project, branch.target)
-        return
+    unless Feature.enabled?(:redirect_with_ref_type, @project)
+      @ref_type = ref_type
+      if @ref_type == BRANCH_REF_TYPE && ambiguous_ref?(@project, @ref)
+        branch = @project.repository.find_branch(@ref)
+        if branch
+          redirect_to project_tree_path(@project, branch.target)
+          return
+        end
       end
     end
 
     if tree.entries.empty?
       if @repository.blob_at(@commit.id, @path)
-        redirect_to project_blob_path(@project, File.join(@ref, @path))
+        redirect_to project_blob_path(@project, File.join(@ref, @path), ref_type: @ref_type)
       elsif @path.present?
         redirect_to_tree_root_for_missing_path(@project, @ref, @path)
       end
@@ -59,6 +62,23 @@ def create_dir
 
   private
 
+  def find_requested_ref
+    return unless Feature.enabled?(:redirect_with_ref_type, @project)
+
+    @ref_type = ref_type
+    if @ref_type.present?
+      @tree = @repo.tree(@ref, @path, ref_type: @ref_type)
+    else
+      response = ExtractsPath::RequestedRef.new(@repository, ref_type: nil, ref: @ref).find
+      @ref_type = response[:ref_type]
+      @commit = response[:commit]
+
+      if response[:ambiguous]
+        redirect_to(project_tree_path(@project, File.join(@ref_type ? @ref : @commit.id, @path), ref_type: @ref_type))
+      end
+    end
+  end
+
   def redirect_renamed_default_branch?
     action_name == 'show'
   end

app/controllers/projects_controller.rb

@@ -173,7 +173,9 @@ def show
       flash.now[:alert] = _("Project '%{project_name}' queued for deletion.") % { project_name: @project.name }
     end
 
-    if ambiguous_ref?(@project, @ref)
+    if Feature.enabled?(:redirect_with_ref_type, @project)
+      @ref_type = 'heads'
+    elsif ambiguous_ref?(@project, @ref)
       branch = @project.repository.find_branch(@ref)
 
       # The files view would render a ref other than the default branch

config/feature_flags/development/redirect_with_ref_type.yml

+---
+name: redirect_with_ref_type
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122237
+rollout_issue_url:
+milestone: '16.2'
+type: development
+group: group::source code
+default_enabled: false

lib/extracts_ref.rb

@@ -100,7 +100,7 @@ def assign_ref_vars
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
   def tree
-    @tree ||= @repo.tree(@commit.id, @path) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+    @tree ||= @repo.tree(@commit.id, @path, ref_type: ref_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 
   def extract_ref_path

lib/extracts_ref/requested_ref.rb

+# frozen_string_literal: true
+
+module ExtractsRef
+  class RequestedRef
+    include Gitlab::Utils::StrongMemoize
+
+    SYMBOLIC_REF_PREFIX = %r{((refs/)?(heads|tags)/)+}
+    def initialize(repository, ref_type:, ref:)
+      @ref_type = ref_type
+      @ref = ref
+      @repository = repository
+    end
+
+    attr_reader :repository, :ref_type, :ref
+
+    def find
+      case ref_type
+      when 'tags'
+        { ref_type: ref_type, commit: tag }
+      when 'heads'
+        { ref_type: ref_type, commit: branch }
+      else
+        commit_without_ref_type
+      end
+    end
+
+    private
+
+    def commit_without_ref_type
+      if commit.nil?
+        { ref_type: nil, commit: nil }
+      elsif commit.id == ref
+        # ref is probably complete 40 character sha
+        { ref_type: nil, commit: commit }
+      elsif tag.present?
+        { ref_type: 'tags', commit: tag, ambiguous: branch.present? }
+      elsif branch.present?
+        { ref_type: 'heads', commit: branch }
+      else
+        { ref_type: nil, commit: commit, ambiguous: ref.match?(SYMBOLIC_REF_PREFIX) }
+      end
+    end
+
+    def commit
+      repository.commit(ref)
+    end
+    strong_memoize_attr :commit
+
+    def tag
+      raw_commit = repository.find_tag(ref)&.dereferenced_target
+      ::Commit.new(raw_commit, repository.container) if raw_commit
+    end
+    strong_memoize_attr :tag
+
+    def branch
+      raw_commit = repository.find_branch(ref)&.dereferenced_target
+      ::Commit.new(raw_commit, repository.container) if raw_commit
+    end
+    strong_memoize_attr :branch
+  end
+end

spec/controllers/projects/blob_controller_spec.rb

@@ -5,15 +5,21 @@
 RSpec.describe Projects::BlobController, feature_category: :source_code_management do
   include ProjectForksHelper
 
-  let(:project) { create(:project, :public, :repository, previous_default_branch: previous_default_branch) }
-  let(:previous_default_branch) { nil }
+  let_it_be(:project) { create(:project, :public, :repository) }
 
   describe "GET show" do
-    let(:params) { { namespace_id: project.namespace, project_id: project, id: id } }
+    let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } }
+    let(:ref_type) { nil }
     let(:request) do
       get(:show, params: params)
     end
 
+    let(:redirect_with_ref_type) { true }
+
+    before do
+      stub_feature_flags(redirect_with_ref_type: redirect_with_ref_type)
+    end
+
     render_views
 
     context 'with file path' do
@@ -24,25 +30,43 @@
         request
       end
 
+      after do
+        project.repository.rm_tag(project.creator, 'ambiguous_ref')
+        project.repository.rm_branch(project.creator, 'ambiguous_ref')
+      end
+
       context 'when the ref is ambiguous' do
         let(:ref) { 'ambiguous_ref' }
         let(:path) { 'README.md' }
         let(:id) { "#{ref}/#{path}" }
-        let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } }
 
-        context 'and explicitly requesting a branch' do
-          let(:ref_type) { 'heads' }
+        context 'and the redirect_with_ref_type flag is disabled' do
+          let(:redirect_with_ref_type) { false }
+
+          context 'and explicitly requesting a branch' do
+            let(:ref_type) { 'heads' }
+
+            it 'redirects to blob#show with sha for the branch' do
+              expect(response).to redirect_to(project_blob_path(project, "#{RepoHelpers.another_sample_commit.id}/#{path}"))
+            end
+          end
+
+          context 'and explicitly requesting a tag' do
+            let(:ref_type) { 'tags' }
 
-          it 'redirects to blob#show with sha for the branch' do
-            expect(response).to redirect_to(project_blob_path(project, "#{RepoHelpers.another_sample_commit.id}/#{path}"))
+            it 'responds with success' do
+              expect(response).to be_ok
+            end
           end
         end
 
-        context 'and explicitly requesting a tag' do
-          let(:ref_type) { 'tags' }
+        context 'and the redirect_with_ref_type flag is enabled' do
+          context 'when the ref_type is nil' do
+            let(:ref_type) { nil }
 
-          it 'responds with success' do
-            expect(response).to be_ok
+            it 'redirects to the tag' do
+              expect(response).to redirect_to(project_blob_path(project, id, ref_type: 'tags'))
+            end
           end
         end
       end
@@ -68,18 +92,20 @@
         it { is_expected.to respond_with(:not_found) }
       end
 
-      context "renamed default branch, valid file" do
-        let(:id) { 'old-default-branch/README.md' }
-        let(:previous_default_branch) { 'old-default-branch' }
+      context 'when default branch was renamed' do
+        let_it_be_with_reload(:project) { create(:project, :public, :repository, previous_default_branch: 'old-default-branch') }
 
-        it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/README.md") }
-      end
+        context "renamed default branch, valid file" do
+          let(:id) { 'old-default-branch/README.md' }
+
+          it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/README.md") }
+        end
 
-      context "renamed default branch, invalid file" do
-        let(:id) { 'old-default-branch/invalid-path.rb' }
-        let(:previous_default_branch) { 'old-default-branch' }
+        context "renamed default branch, invalid file" do
+          let(:id) { 'old-default-branch/invalid-path.rb' }
 
-        it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/invalid-path.rb") }
+          it { is_expected.to redirect_to("/#{project.full_path}/-/blob/#{project.default_branch}/invalid-path.rb") }
+        end
       end
 
       context "binary file" do
@@ -100,7 +126,7 @@
         let(:id) { 'master/README.md' }
 
         before do
-          get :show, params: { namespace_id: project.namespace, project_id: project, id: id }, format: :json
+          get :show, params: params, format: :json
         end
 
         it do
@@ -114,7 +140,7 @@
         let(:id) { 'master/README.md' }
 
         before do
-          get :show, params: { namespace_id: project.namespace, project_id: project, id: id, viewer: 'none' }, format: :json
+          get :show, params: { namespace_id: project.namespace, project_id: project, id: id, ref_type: 'heads', viewer: 'none' }, format: :json
         end
 
         it do
@@ -127,7 +153,7 @@
 
     context 'with tree path' do
       before do
-        get :show, params: { namespace_id: project.namespace, project_id: project, id: id }
+        get :show, params: params
 
         controller.instance_variable_set(:@blob, nil)
       end
@@ -414,6 +440,10 @@ def blob_after_edit_path
       let(:after_delete_path) { project_tree_path(project, 'master/files') }
 
       it 'redirects to the sub directory' do
+        expect_next_instance_of(Files::DeleteService) do |instance|
+          expect(instance).to receive(:execute).and_return({ status: :success })
+        end
+
         delete :destroy, params: default_params
 
         expect(response).to redirect_to(after_delete_path)

spec/controllers/projects/tree_controller_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Projects::TreeController, feature_category: :source_code_management do
-  let(:project) { create(:project, :repository, previous_default_branch: previous_default_branch) }
-  let(:previous_default_branch) { nil }
+  let_it_be(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
+  let(:redirect_with_ref_type) { true }
 
   before do
     sign_in(user)
@@ -17,10 +17,14 @@
   describe "GET show" do
     let(:params) do
       {
-        namespace_id: project.namespace.to_param, project_id: project, id: id
+        namespace_id: project.namespace.to_param, project_id: project, id: id, ref_type: ref_type
       }
     end
 
+    let(:request) { get :show, params: params }
+
+    let(:ref_type) { nil }
+
     # Make sure any errors accessing the tree in our views bubble up to this spec
     render_views
 
@@ -28,26 +32,79 @@
       expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
       project.repository.add_tag(project.creator, 'ambiguous_ref', RepoHelpers.sample_commit.id)
       project.repository.add_branch(project.creator, 'ambiguous_ref', RepoHelpers.another_sample_commit.id)
-      get :show, params: params
+
+      stub_feature_flags(redirect_with_ref_type: redirect_with_ref_type)
+    end
+
+    after do
+      project.repository.rm_tag(project.creator, 'ambiguous_ref')
+      project.repository.rm_branch(project.creator, 'ambiguous_ref')
     end
 
-    context 'when the ref is ambiguous' do
-      let(:id) { 'ambiguous_ref' }
-      let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } }
+    context 'when the redirect_with_ref_type flag is disabled' do
+      let(:redirect_with_ref_type) { false }
 
-      context 'and explicitly requesting a branch' do
-        let(:ref_type) { 'heads' }
+      context 'when there is a ref and tag with the same name' do
+        let(:id) { 'ambiguous_ref' }
+        let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } }
 
-        it 'redirects to blob#show with sha for the branch' do
-          expect(response).to redirect_to(project_tree_path(project, RepoHelpers.another_sample_commit.id))
+        context 'and explicitly requesting a branch' do
+          let(:ref_type) { 'heads' }
+
+          it 'redirects to blob#show with sha for the branch' do
+            request
+            expect(response).to redirect_to(project_tree_path(project, RepoHelpers.another_sample_commit.id))
+          end
+        end
+
+        context 'and explicitly requesting a tag' do
+          let(:ref_type) { 'tags' }
+
+          it 'responds with success' do
+            request
+            expect(response).to be_ok
+          end
         end
       end
+    end
 
-      context 'and explicitly requesting a tag' do
-        let(:ref_type) { 'tags' }
+    describe 'delegating to ExtractsRef::RequestedRef' do
+      context 'when there is a ref and tag with the same name' do
+        let(:id) { 'ambiguous_ref' }
+        let(:params) { { namespace_id: project.namespace, project_id: project, id: id, ref_type: ref_type } }
 
-        it 'responds with success' do
-          expect(response).to be_ok
+        let(:requested_ref_double) { ExtractsRef::RequestedRef.new(project.repository, ref_type: ref_type, ref: id) }
+
+        before do
+          allow(ExtractsRef::RequestedRef).to receive(:new).with(kind_of(Repository), ref_type: ref_type, ref: id).and_return(requested_ref_double)
+        end
+
+        context 'and not specifying a ref_type' do
+          it 'finds the tags and redirects' do
+            expect(requested_ref_double).to receive(:find).and_call_original
+            request
+            expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{id}/?ref_type=tags")
+          end
+        end
+
+        context 'and explicitly requesting a branch' do
+          let(:ref_type) { 'heads' }
+
+          it 'finds the branch' do
+            expect(requested_ref_double).not_to receive(:find)
+            request
+            expect(response).to be_ok
+          end
+        end
+
+        context 'and explicitly requesting a tag' do
+          let(:ref_type) { 'tags' }
+
+          it 'finds the tag' do
+            expect(requested_ref_double).not_to receive(:find)
+            request
+            expect(response).to be_ok
+          end
         end
       end
     end
@@ -55,19 +112,26 @@
     context "valid branch, no path" do
       let(:id) { 'master' }
 
-      it { is_expected.to respond_with(:success) }
+      it 'responds with success' do
+        request
+        expect(response).to be_ok
+      end
     end
 
     context "valid branch, valid path" do
       let(:id) { 'master/encoding/' }
 
-      it { is_expected.to respond_with(:success) }
+      it 'responds with success' do
+        request
+        expect(response).to be_ok
+      end
     end
 
     context "valid branch, invalid path" do
       let(:id) { 'master/invalid-path/' }
 
       it 'redirects' do
+        request
         expect(subject)
             .to redirect_to("/#{project.full_path}/-/tree/master")
       end
@@ -76,54 +140,91 @@
     context "invalid branch, valid path" do
       let(:id) { 'invalid-branch/encoding/' }
 
-      it { is_expected.to respond_with(:not_found) }
+      it 'responds with not_found' do
+        request
+        expect(subject).to respond_with(:not_found)
+      end
     end
 
-    context "renamed default branch, valid file" do
-      let(:id) { 'old-default-branch/encoding/' }
-      let(:previous_default_branch) { 'old-default-branch' }
+    context 'when default branch was renamed' do
+      let_it_be_with_reload(:project) { create(:project, :repository, previous_default_branch: 'old-default-branch') }
 
-      it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/encoding/") }
-    end
+      context "and the file is valid" do
+        let(:id) { 'old-default-branch/encoding/' }
+
+        it 'redirects' do
+          request
+          expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/encoding/")
+        end
+      end
 
-    context "renamed default branch, invalid file" do
-      let(:id) { 'old-default-branch/invalid-path/' }
-      let(:previous_default_branch) { 'old-default-branch' }
+      context "and the file is invalid" do
+        let(:id) { 'old-default-branch/invalid-path/' }
 
-      it { is_expected.to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/invalid-path/") }
+        it 'redirects' do
+          request
+          expect(subject).to redirect_to("/#{project.full_path}/-/tree/#{project.default_branch}/invalid-path/")
+        end
+      end
     end
 
     context "valid empty branch, invalid path" do
       let(:id) { 'empty-branch/invalid-path/' }
 
       it 'redirects' do
-        expect(subject)
-            .to redirect_to("/#{project.full_path}/-/tree/empty-branch")
+        request
+        expect(subject).to redirect_to("/#{project.full_path}/-/tree/empty-branch")
       end
     end
 
     context "valid empty branch" do
       let(:id) { 'empty-branch' }
 
-      it { is_expected.to respond_with(:success) }
+      it 'responds with success' do
+        request
+        expect(response).to be_ok
+      end
     end
 
     context "invalid SHA commit ID" do
       let(:id) { 'ff39438/.gitignore' }
 
-      it { is_expected.to respond_with(:not_found) }
+      it 'responds with not_found' do
+        request
+        expect(subject).to respond_with(:not_found)
+      end
     end
 
     context "valid SHA commit ID" do
       let(:id) { '6d39438' }
 
-      it { is_expected.to respond_with(:success) }
+      it 'responds with success' do
+        request
+        expect(response).to be_ok
+      end
+
+      context 'and there is a tag with the same name' do
+        before do
+          project.repository.add_tag(project.creator, id, RepoHelpers.sample_commit.id)
+        end
+
+        it 'responds with success' do
+          request
+
+          # This uses the tag
+          # TODO: Should we redirect in this case?
+          expect(response).to be_ok
+        end
+      end
     end
 
     context "valid SHA commit ID with path" do
       let(:id) { '6d39438/.gitignore' }
 
-      it { expect(response).to have_gitlab_http_status(:found) }
+      it 'responds with found' do
+        request
+        expect(response).to have_gitlab_http_status(:found)
+      end
     end
   end
 
@@ -149,7 +250,7 @@
 
     before do
       get :show, params: {
-        namespace_id: project.namespace.to_param, project_id: project, id: id
+        namespace_id: project.namespace.to_param, project_id: project, id: id, ref_type: 'heads'
       }
     end
 
@@ -157,7 +258,7 @@
       let(:id) { 'master/README.md' }
 
       it 'redirects' do
-        redirect_url = "/#{project.full_path}/-/blob/master/README.md"
+        redirect_url = "/#{project.full_path}/-/blob/master/README.md?ref_type=heads"
         expect(subject).to redirect_to(redirect_url)
       end
     end

spec/controllers/projects_controller_spec.rb

@@ -164,107 +164,113 @@ def get_activity(project)
           end
         end
 
-        context 'when there is a tag with the same name as the default branch' do
-          let_it_be(:tagged_project) { create(:project, :public, :custom_repo, files: ['somefile']) }
-          let(:tree_with_default_branch) do
-            branch = tagged_project.repository.find_branch(tagged_project.default_branch)
-            project_tree_path(tagged_project, branch.target)
-          end
-
+        context 'when redirect_with_ref_type is disabled' do
           before do
-            tagged_project.repository.create_file(
-              tagged_project.creator,
-              'file_for_tag',
-              'content for file',
-              message: "Automatically created file",
-              branch_name: 'branch-to-tag'
-            )
-
-            tagged_project.repository.add_tag(
-              tagged_project.creator,
-              tagged_project.default_branch, # tag name
-              'branch-to-tag' # target
-            )
-          end
-
-          it 'redirects to tree view for the default branch' do
-            get :show, params: { namespace_id: tagged_project.namespace, id: tagged_project }
-            expect(response).to redirect_to(tree_with_default_branch)
-          end
-        end
-
-        context 'when the default branch name is ambiguous' do
-          let_it_be(:project_with_default_branch) do
-            create(:project, :public, :custom_repo, files: ['somefile'])
+            stub_feature_flags(redirect_with_ref_type: false)
           end
 
-          shared_examples 'ambiguous ref redirects' do
-            let(:project) { project_with_default_branch }
-            let(:branch_ref) { "refs/heads/#{ref}" }
-            let(:repo) { project.repository }
+          context 'when there is a tag with the same name as the default branch' do
+            let_it_be(:tagged_project) { create(:project, :public, :custom_repo, files: ['somefile']) }
+            let(:tree_with_default_branch) do
+              branch = tagged_project.repository.find_branch(tagged_project.default_branch)
+              project_tree_path(tagged_project, branch.target)
+            end
 
             before do
-              repo.create_branch(branch_ref, 'master')
-              repo.change_head(ref)
+              tagged_project.repository.create_file(
+                tagged_project.creator,
+                'file_for_tag',
+                'content for file',
+                message: "Automatically created file",
+                branch_name: 'branch-to-tag'
+              )
+
+              tagged_project.repository.add_tag(
+                tagged_project.creator,
+                tagged_project.default_branch, # tag name
+                'branch-to-tag' # target
+              )
             end
 
-            after do
-              repo.change_head('master')
-              repo.delete_branch(branch_ref)
+            it 'redirects to tree view for the default branch' do
+              get :show, params: { namespace_id: tagged_project.namespace, id: tagged_project }
+              expect(response).to redirect_to(tree_with_default_branch)
             end
+          end
 
-            subject do
-              get(
-                :show,
-                params: {
-                  namespace_id: project.namespace,
-                  id: project
-                }
-              )
+          context 'when the default branch name is ambiguous' do
+            let_it_be(:project_with_default_branch) do
+              create(:project, :public, :custom_repo, files: ['somefile'])
             end
 
-            context 'when there is no conflicting ref' do
-              let(:other_ref) { 'non-existent-ref' }
+            shared_examples 'ambiguous ref redirects' do
+              let(:project) { project_with_default_branch }
+              let(:branch_ref) { "refs/heads/#{ref}" }
+              let(:repo) { project.repository }
 
-              it { is_expected.to have_gitlab_http_status(:ok) }
-            end
+              before do
+                repo.create_branch(branch_ref, 'master')
+                repo.change_head(ref)
+              end
+
+              after do
+                repo.change_head('master')
+                repo.delete_branch(branch_ref)
+              end
 
-            context 'and that other ref exists' do
-              let(:other_ref) { 'master' }
+              subject do
+                get(
+                  :show,
+                  params: {
+                    namespace_id: project.namespace,
+                    id: project
+                  }
+                )
+              end
+
+              context 'when there is no conflicting ref' do
+                let(:other_ref) { 'non-existent-ref' }
 
-              let(:project_default_root_tree_path) do
-                sha = repo.find_branch(project.default_branch).target
-                project_tree_path(project, sha)
+                it { is_expected.to have_gitlab_http_status(:ok) }
               end
 
-              it 'redirects to tree view for the default branch' do
-                is_expected.to redirect_to(project_default_root_tree_path)
+              context 'and that other ref exists' do
+                let(:other_ref) { 'master' }
+
+                let(:project_default_root_tree_path) do
+                  sha = repo.find_branch(project.default_branch).target
+                  project_tree_path(project, sha)
+                end
+
+                it 'redirects to tree view for the default branch' do
+                  is_expected.to redirect_to(project_default_root_tree_path)
+                end
               end
             end
-          end
 
-          context 'when ref starts with ref/heads/' do
-            let(:ref) { "refs/heads/#{other_ref}" }
+            context 'when ref starts with ref/heads/' do
+              let(:ref) { "refs/heads/#{other_ref}" }
 
-            include_examples 'ambiguous ref redirects'
-          end
+              include_examples 'ambiguous ref redirects'
+            end
 
-          context 'when ref starts with ref/tags/' do
-            let(:ref) { "refs/tags/#{other_ref}" }
+            context 'when ref starts with ref/tags/' do
+              let(:ref) { "refs/tags/#{other_ref}" }
 
-            include_examples 'ambiguous ref redirects'
-          end
+              include_examples 'ambiguous ref redirects'
+            end
 
-          context 'when ref starts with heads/' do
-            let(:ref) { "heads/#{other_ref}" }
+            context 'when ref starts with heads/' do
+              let(:ref) { "heads/#{other_ref}" }
 
-            include_examples 'ambiguous ref redirects'
-          end
+              include_examples 'ambiguous ref redirects'
+            end
 
-          context 'when ref starts with tags/' do
-            let(:ref) { "tags/#{other_ref}" }
+            context 'when ref starts with tags/' do
+              let(:ref) { "tags/#{other_ref}" }
 
-            include_examples 'ambiguous ref redirects'
+              include_examples 'ambiguous ref redirects'
+            end
           end
         end
       end

spec/lib/extracts_ref/requested_ref_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ExtractsRef::RequestedRef, feature_category: :source_code_management do
+  describe '#find' do
+    subject { described_class.new(project.repository, ref_type: ref_type, ref: ref).find }
+
+    let_it_be(:project) { create(:project, :repository) }
+    let(:ref_type) { nil }
+
+    # Create branches and tags consistently with the same shas to make comparison easier to follow
+    let(:tag_sha) { RepoHelpers.sample_commit.id }
+    let(:branch_sha) { RepoHelpers.another_sample_commit.id }
+
+    shared_context 'when a branch exists' do
+      before do
+        project.repository.create_branch(branch_name, branch_sha)
+      end
+
+      after do
+        project.repository.rm_branch(project.owner, branch_name)
+      end
+    end
+
+    shared_context 'when a tag exists' do
+      before do
+        project.repository.add_tag(project.owner, tag_name, tag_sha)
+      end
+
+      after do
+        project.repository.rm_tag(project.owner, tag_name)
+      end
+    end
+
+    shared_examples 'RequestedRef when ref_type is specified' do |branch_sha, tag_sha|
+      context 'when ref_type is heads' do
+        let(:ref_type) { 'heads' }
+
+        it 'returns the branch commit' do
+          expect(subject[:ref_type]).to eq('heads')
+          expect(subject[:commit].id).to eq(branch_sha)
+        end
+      end
+
+      context 'when ref_type is tags' do
+        let(:ref_type) { 'tags' }
+
+        it 'returns the tag commit' do
+          expect(subject[:ref_type]).to eq('tags')
+          expect(subject[:commit].id).to eq(tag_sha)
+        end
+      end
+    end
+
+    context 'when the ref is the sha for a commit' do
+      let(:ref) { branch_sha }
+
+      context 'and a tag and branch with that sha as a name' do
+        include_context 'when a branch exists' do
+          let(:branch_name) { ref }
+        end
+
+        include_context 'when a tag exists' do
+          let(:tag_name) { ref }
+        end
+
+        it_behaves_like 'RequestedRef when ref_type is specified',
+          RepoHelpers.another_sample_commit.id,
+          RepoHelpers.sample_commit.id
+
+        it 'returns the commit' do
+          expect(subject[:ref_type]).to be_nil
+          expect(subject[:commit].id).to eq(ref)
+        end
+      end
+    end
+
+    context 'when ref is for a tag' do
+      include_context 'when a tag exists' do
+        let(:tag_name) { SecureRandom.uuid }
+      end
+
+      let(:ref) { tag_name }
+
+      it 'returns the tag commit' do
+        expect(subject[:ref_type]).to eq('tags')
+        expect(subject[:commit].id).to eq(tag_sha)
+      end
+
+      context 'and there is a branch with the same name' do
+        include_context 'when a branch exists' do
+          let(:branch_name) { ref }
+        end
+
+        it_behaves_like 'RequestedRef when ref_type is specified',
+          RepoHelpers.another_sample_commit.id,
+          RepoHelpers.sample_commit.id
+
+        it 'returns the tag commit' do
+          expect(subject[:ref_type]).to eq('tags')
+          expect(subject[:commit].id).to eq(tag_sha)
+          expect(subject[:ambiguous]).to be_truthy
+        end
+      end
+    end
+
+    context 'when ref is only for a branch' do
+      let(:ref) { SecureRandom.uuid }
+
+      include_context 'when a branch exists' do
+        let(:branch_name) { ref }
+      end
+
+      it 'returns the branch commit' do
+        expect(subject[:ref_type]).to eq('heads')
+        expect(subject[:commit].id).to eq(branch_sha)
+      end
+    end
+
+    context 'when ref is an abbreviated commit sha' do
+      let(:ref) { branch_sha.first(8) }
+
+      it 'returns the commit' do
+        expect(subject[:ref_type]).to be_nil
+        expect(subject[:commit].id).to eq(branch_sha)
+      end
+    end
+
+    context 'when ref does not exist' do
+      let(:ref) { SecureRandom.uuid }
+
+      it 'returns the commit' do
+        expect(subject[:ref_type]).to be_nil
+        expect(subject[:commit]).to be_nil
+      end
+    end
+
+    context 'when ref is symbolic' do
+      let(:ref) { "heads/#{branch_name}" }
+
+      include_context 'when a branch exists' do
+        let(:branch_name) { SecureRandom.uuid }
+      end
+
+      it 'returns the commit' do
+        expect(subject[:ref_type]).to be_nil
+        expect(subject[:commit].id).to eq(branch_sha)
+        expect(subject[:ambiguous]).to be_truthy
+      end
+    end
+  end
+end