diff options
| -rw-r--r-- | app/controllers/projects/branches_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/repository.rb | 21 | ||||
| -rw-r--r-- | app/views/projects/branches/_branch.html.haml | 5 | ||||
| -rw-r--r-- | app/views/projects/branches/index.html.haml | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/use-git-branch-merged.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/git/branch.rb | 8 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 11 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/branch_spec.rb | 32 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 26 | ||||
| -rw-r--r-- | spec/models/repository_spec.rb | 18 |
10 files changed, 121 insertions, 9 deletions
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 7f03ce07dec..f28df83d5a5 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -15,6 +15,8 @@ class Projects::BranchesController < Projects::ApplicationController respond_to do |format| format.html do @refs_pipelines = @project.pipelines.latest_successful_for_refs(@branches.map(&:name)) + @merged_branch_names = + repository.merged_branch_names(@branches.map(&:name)) # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37429 Gitlab::GitalyClient.allow_n_plus_1_calls do @max_commits = @branches.reduce(0) do |memo, branch| diff --git a/app/models/repository.rb b/app/models/repository.rb index 54388d1c8b4..44a1e9ce529 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -902,18 +902,27 @@ class Repository end end - def merged_to_root_ref?(branch_name) - branch_commit = commit(branch_name) - root_ref_commit = commit(root_ref) + def merged_to_root_ref?(branch_or_name, pre_loaded_merged_branches = nil) + branch = Gitlab::Git::Branch.find(self, branch_or_name) + + if branch + root_ref_sha = commit(root_ref).sha + same_head = branch.target == root_ref_sha + merged = + if pre_loaded_merged_branches + pre_loaded_merged_branches.include?(branch.name) + else + ancestor?(branch.target, root_ref_sha) + end - if branch_commit - same_head = branch_commit.id == root_ref_commit.id - !same_head && ancestor?(branch_commit.id, root_ref_commit.id) + !same_head && merged else nil end end + delegate :merged_branch_names, to: :raw_repository + def merge_base(first_commit_id, second_commit_id) first_commit_id = commit(first_commit_id).try(:id) || first_commit_id second_commit_id = commit(second_commit_id).try(:id) || second_commit_id diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 49101d1efa4..6e02ae6c9cc 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,3 +1,4 @@ +- merged = local_assigns.fetch(:merged, false) - commit = @repository.commit(branch.dereferenced_target) - bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 - diverging_commit_counts = @repository.diverging_commit_counts(branch) @@ -12,7 +13,7 @@ - if branch.name == @repository.root_ref %span.label.label-primary default - - elsif @repository.merged_to_root_ref? branch.name + - elsif merged %span.label.label-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } = s_('Branches|merged') @@ -47,7 +48,7 @@ target: "#modal-delete-branch", delete_path: project_branch_path(@project, branch.name), branch_name: branch.name, - is_merged: ("true" if @repository.merged_to_root_ref?(branch.name)) } } + is_merged: ("true" if merged) } } = icon("trash-o") - else %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 7d9645d79e6..aade310236e 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -38,7 +38,7 @@ - if @branches.any? %ul.content-list.all-branches - @branches.each do |branch| - = render "projects/branches/branch", branch: branch + = render "projects/branches/branch", branch: branch, merged: @repository.merged_to_root_ref?(branch, @merged_branch_names) = paginate @branches, theme: 'gitlab' - else .nothing-here-block diff --git a/changelogs/unreleased/use-git-branch-merged.yml b/changelogs/unreleased/use-git-branch-merged.yml new file mode 100644 index 00000000000..24ec226250c --- /dev/null +++ b/changelogs/unreleased/use-git-branch-merged.yml @@ -0,0 +1,5 @@ +--- +title: Improve branch listing page performance +merge_request: 14729 +author: +type: performance diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index c53882787f1..3487e099381 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -3,6 +3,14 @@ module Gitlab module Git class Branch < Ref + def self.find(repo, branch_name) + if branch_name.is_a?(Gitlab::Git::Branch) + branch_name + else + repo.find_branch(branch_name) + end + end + def initialize(repository, name, target, target_commit) super(repository, name, target, target_commit) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 408616d174b..fc8af38d4d9 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -511,6 +511,10 @@ module Gitlab gitaly_commit_client.ancestor?(from, to) end + def merged_branch_names(branch_names = []) + Set.new(git_merged_branch_names(branch_names)) + end + # Return an array of Diff objects that represent the diff # between +from+ and +to+. See Diff::filter_diff_options for the allowed # diff options. The +options+ hash can also include :break_rewrites to @@ -1190,6 +1194,13 @@ module Gitlab sort_branches(branches, sort_by) end + def git_merged_branch_names(branch_names = []) + lines = run_git(['branch', '--merged', root_ref] + branch_names) + .first.lines + + lines.map(&:strip) + end + def log_using_shell?(options) options[:path].present? || options[:disable_walk] || diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 318a7b7a332..708870060e7 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -7,6 +7,38 @@ describe Gitlab::Git::Branch, seed_helper: true do it { is_expected.to be_kind_of Array } + describe '.find' do + subject { described_class.find(repository, branch) } + + before do + allow(repository).to receive(:find_branch).with(branch) + .and_call_original + end + + context 'when finding branch via branch name' do + let(:branch) { 'master' } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject.name).to eq(branch) + + expect(repository).to have_received(:find_branch).with(branch) + end + end + + context 'when the branch is already a branch' do + let(:commit) { repository.commit('master') } + let(:branch) { described_class.new(repository, 'master', commit.sha, commit) } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject).to eq(branch) + + expect(repository).not_to have_received(:find_branch).with(branch) + end + end + end + describe '#size' do subject { super().size } it { is_expected.to eq(SeedRepo::Repo::BRANCHES.size) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index e3d320631bc..b161d25b96c 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1135,6 +1135,32 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#merged_branch_names' do + context 'when branch names are passed' do + it 'only returns the names we are asking' do + names = repository.merged_branch_names(%w[merge-test]) + + expect(names).to contain_exactly('merge-test') + end + + it 'does not return unmerged branch names' do + names = repository.merged_branch_names(%w[feature]) + + expect(names).to be_empty + end + end + + context 'when no branch names are specified' do + it 'returns all merged branch names' do + names = repository.merged_branch_names + + expect(names).to include('merge-test') + expect(names).to include('fix-mode') + expect(names).not_to include('feature') + end + end + end + describe "#ls_files" do let(:master_file_paths) { repository.ls_files("master") } let(:not_existed_branch) { repository.ls_files("not_existed_branch") } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 455d5e8a656..d7c07676911 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -299,6 +299,24 @@ describe Repository do it { is_expected.to be_falsey } end + + context 'when pre-loaded merged branches are provided' do + using RSpec::Parameterized::TableSyntax + + where(:branch, :pre_loaded, :expected) do + 'not-merged-branch' | ['branch-merged'] | false + 'branch-merged' | ['not-merged-branch'] | false + 'branch-merged' | ['branch-merged'] | true + 'not-merged-branch' | ['not-merged-branch'] | false + 'master' | ['master'] | false + end + + with_them do + subject { repository.merged_to_root_ref?(branch, pre_loaded) } + + it { is_expected.to eq(expected) } + end + end end describe '#can_be_merged?' do |