diff --git a/app/controllers/concerns/redirects_for_missing_path_on_tree.rb b/app/controllers/concerns/redirects_for_missing_path_on_tree.rb index 92574dfade9f2dad783c33a095d13b8e1c95d26e..97c23a2cf3c80cff66b9bae03289ce5b0057e381 100644 --- a/app/controllers/concerns/redirects_for_missing_path_on_tree.rb +++ b/app/controllers/concerns/redirects_for_missing_path_on_tree.rb @@ -1,8 +1,8 @@ # 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 diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 28393e1f365e61bd88a53d8f2c34394bf87d288b..a60cc5301e2a4e4deb86062cccd2bbcd50ffa24b 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/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 diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb index c8f698d6193559c77cc1051983fd637c32f9fc08..d2a820d93b00bd2e24ca0d125c76d13a949bc764 100644 --- a/app/controllers/projects/tree_controller.rb +++ b/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 diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 71a34a40bd0724c9a59c1efae49eb3b467a60799..bc4831d7772b6b61e6fb785d6fa1ee2215027c88 100644 --- a/app/controllers/projects_controller.rb +++ b/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 diff --git a/config/feature_flags/development/redirect_with_ref_type.yml b/config/feature_flags/development/redirect_with_ref_type.yml new file mode 100644 index 0000000000000000000000000000000000000000..74a4d31eb2f78662e50162325609854c0792fab3 --- /dev/null +++ b/config/feature_flags/development/redirect_with_ref_type.yml @@ -0,0 +1,8 @@ +--- +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 diff --git a/lib/extracts_ref.rb b/lib/extracts_ref.rb index 49ec564eb8d64a830bc5aa99ef05fa0808580f5a..2a48b66bb5c6e4ab1017cdd05c7f8c62dfdd8763 100644 --- a/lib/extracts_ref.rb +++ b/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 diff --git a/lib/extracts_ref/requested_ref.rb b/lib/extracts_ref/requested_ref.rb new file mode 100644 index 0000000000000000000000000000000000000000..f20018b5ef494e29a38cd17b458c40a9d8ffc218 --- /dev/null +++ b/lib/extracts_ref/requested_ref.rb @@ -0,0 +1,61 @@ +# 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 diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index b07cb7a228dbacc0e5f0a5f65e0c051ab7e3f139..49c1935c4a3c2e285dfc5e5dd3390b4b1b0841d1 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/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) diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index 61998d516e8e71b97f2c6a3ceae01a8a95c7f0a6..ffec670e97d097ed02a0d273aaf49adaeabfc7e8 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/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 diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 6adddccfda7aedb6ad981158d287f85849e1a0fe..46913cfa6496ca132e0692f1fccf091a19ba55f6 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/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 diff --git a/spec/lib/extracts_ref/requested_ref_spec.rb b/spec/lib/extracts_ref/requested_ref_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..80d3575b360aef72c3130e3091267b358dc28634 --- /dev/null +++ b/spec/lib/extracts_ref/requested_ref_spec.rb @@ -0,0 +1,153 @@ +# 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