Commit d9313795 authored by Annabel Dunstone Gray's avatar Annabel Dunstone Gray

Merge branch '37824-many-branches-lock-server' into 'master'

Project with many branches can lock server running "git branch --contains XXX"

Closes #37824

See merge request gitlab-org/gitlab-ce!14812
parents 673b6be1 fb41187b
......@@ -52,6 +52,37 @@
.label.label-gray {
background-color: $well-expand-item;
}
.branches {
display: inline;
}
.branch-link {
margin-bottom: 2px;
}
.limit-box {
cursor: pointer;
display: inline-flex;
align-items: center;
background-color: $red-100;
border-radius: $border-radius-default;
text-align: center;
&:hover {
background-color: $red-200;
}
.limit-icon {
margin: 0 8px;
}
.limit-message {
line-height: 16px;
margin-right: 8px;
font-size: 12px;
}
}
}
.light-well {
......
......@@ -16,6 +16,8 @@ class Projects::CommitController < Projects::ApplicationController
before_action :define_note_vars, only: [:show, :diff_for_path]
before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
BRANCH_SEARCH_LIMIT = 1000
def show
apply_diff_view_cookie!
......@@ -56,8 +58,14 @@ class Projects::CommitController < Projects::ApplicationController
end
def branches
@branches = @project.repository.branch_names_contains(commit.id)
@tags = @project.repository.tag_names_contains(commit.id)
# branch_names_contains/tag_names_contains can take a long time when there are thousands of
# branches/tags - each `git branch --contains xxx` request can consume a cpu core.
# so only do the query when there are a manageable number of branches/tags
@branches_limit_exceeded = @project.repository.branch_count > BRANCH_SEARCH_LIMIT
@branches = @branches_limit_exceeded ? [] : @project.repository.branch_names_contains(commit.id)
@tags_limit_exceeded = @project.repository.tag_count > BRANCH_SEARCH_LIMIT
@tags = @tags_limit_exceeded ? [] : @project.repository.tag_names_contains(commit.id)
render layout: false
end
......
......@@ -60,23 +60,33 @@ module CommitsHelper
branches.include?(project.default_branch) ? branches.delete(project.default_branch) : branches.pop
end
# Returns a link formatted as a commit branch link
def commit_branch_link(url, text)
link_to(url, class: 'label label-gray ref-name branch-link') do
icon('code-fork') + " #{text}"
end
end
# Returns the sorted alphabetically links to branches, separated by a comma
def commit_branches_links(project, branches)
branches.sort.map do |branch|
link_to(project_ref_path(project, branch), class: "label label-gray ref-name") do
icon('code-fork') + " #{branch}"
end
end.join(" ").html_safe
commit_branch_link(project_ref_path(project, branch), branch)
end.join(' ').html_safe
end
# Returns a link formatted as a commit tag link
def commit_tag_link(url, text)
link_to(url, class: 'label label-gray ref-name') do
icon('tag') + " #{text}"
end
end
# Returns the sorted links to tags, separated by a comma
def commit_tags_links(project, tags)
sorted = VersionSorter.rsort(tags)
sorted.map do |tag|
link_to(project_ref_path(project, tag), class: "label label-gray ref-name") do
icon('tag') + " #{tag}"
end
end.join(" ").html_safe
commit_tag_link(project_ref_path(project, tag), tag)
end.join(' ').html_safe
end
def link_to_browse_code(project, commit)
......
......@@ -61,7 +61,7 @@
%span.cgray= n_('parent', 'parents', @commit.parents.count)
- @commit.parents.each do |parent|
= link_to parent.short_id, project_commit_path(@project, parent), class: "commit-sha"
%span.commit-info.branches
.commit-info.branches
%i.fa.fa-spinner.fa-spin
- if @commit.last_pipeline
......
.has-tooltip{ class: "limit-box limit-box-#{objects} prepend-left-5", data: { title: "Project has too many #{label_for_message} to search"} }
.limit-icon
- if objects == :branch
= icon('code-fork')
- else
= icon('tag')
.limit-message
%span #{label_for_message.capitalize} unavailable
- if @branches.any? || @tags.any?
- if @branches_limit_exceeded
= render 'limit_exceeded_message', objects: :branch, label_for_message: "branches"
- elsif @branches.any?
- branch = commit_default_branch(@project, @branches)
= link_to(project_ref_path(@project, branch), class: "label label-gray ref-name") do
= icon('code-fork')
= branch
= commit_branch_link(project_ref_path(@project, branch), branch)
-# `commit_default_branch` deletes the default branch from `@branches`,
-# so only render this if we have more branches left
- if @branches.any? || @tags.any?
%span
= link_to "…", "#", class: "js-details-expand label label-gray"
%span.js-details-content.hide
= commit_branches_links(@project, @branches) if @branches.any?
= commit_tags_links(@project, @tags) if @tags.any?
- if @branches.any? || @tags.any? || @tags_limit_exceeded
%span
= link_to "…", "#", class: "js-details-expand label label-gray"
%span.js-details-content.hide
= commit_branches_links(@project, @branches)
- if @tags_limit_exceeded
= render 'limit_exceeded_message', objects: :tag, label_for_message: "tags"
- else
= commit_tags_links(@project, @tags)
---
title: While displaying a commit, do not show list of related branches if there are
thousands of branches
merge_request: 14812
author:
type: other
......@@ -134,8 +134,8 @@ describe Projects::CommitController do
end
end
describe "GET branches" do
it "contains branch and tags information" do
describe 'GET branches' do
it 'contains branch and tags information' do
commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
get(:branches,
......@@ -143,8 +143,26 @@ describe Projects::CommitController do
project_id: project,
id: commit.id)
expect(assigns(:branches)).to include("master", "feature_conflict")
expect(assigns(:tags)).to include("v1.1.0")
expect(assigns(:branches)).to include('master', 'feature_conflict')
expect(assigns(:branches_limit_exceeded)).to be_falsey
expect(assigns(:tags)).to include('v1.1.0')
expect(assigns(:tags_limit_exceeded)).to be_falsey
end
it 'returns :limit_exceeded when number of branches/tags reach a threshhold' do
commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
allow_any_instance_of(Repository).to receive(:branch_count).and_return(1001)
allow_any_instance_of(Repository).to receive(:tag_count).and_return(1001)
get(:branches,
namespace_id: project.namespace,
project_id: project,
id: commit.id)
expect(assigns(:branches)).to eq([])
expect(assigns(:branches_limit_exceeded)).to be_truthy
expect(assigns(:tags)).to eq([])
expect(assigns(:tags_limit_exceeded)).to be_truthy
end
end
......
require 'spec_helper'
describe 'projects/commit/branches.html.haml' do
let(:project) { create(:project, :repository) }
before do
assign(:project, project)
end
context 'when branches and tags are available' do
before do
assign(:branches, ['master', 'test-branch'])
assign(:branches_limit_exceeded, false)
assign(:tags, ['tag1'])
assign(:tags_limit_exceeded, false)
render
end
it 'shows default branch' do
expect(rendered).to have_link('master')
end
it 'shows js expand link' do
expect(rendered).to have_selector('.js-details-expand')
end
it 'shows branch and tag links' do
expect(rendered).to have_link('test-branch')
expect(rendered).to have_link('tag1')
end
end
context 'when branches are available but no tags' do
before do
assign(:branches, ['master', 'test-branch'])
assign(:branches_limit_exceeded, false)
assign(:tags, [])
assign(:tags_limit_exceeded, true)
render
end
it 'shows branches' do
expect(rendered).to have_link('master')
expect(rendered).to have_link('test-branch')
end
it 'shows js expand link' do
expect(rendered).to have_selector('.js-details-expand')
end
it 'shows limit exceeded message for tags' do
expect(rendered).to have_text('Tags unavailable')
end
end
context 'when tags are available but no branches (just default)' do
before do
assign(:branches, ['master'])
assign(:branches_limit_exceeded, true)
assign(:tags, %w(tag1 tag2))
assign(:tags_limit_exceeded, false)
render
end
it 'shows default branch' do
expect(rendered).to have_text('master')
end
it 'shows js expand link' do
expect(rendered).to have_selector('.js-details-expand')
end
it 'shows tags' do
expect(rendered).to have_link('tag1')
expect(rendered).to have_link('tag2')
end
it 'shows limit exceeded for branches' do
expect(rendered).to have_text('Branches unavailable')
end
end
context 'when branches and tags are not available' do
before do
assign(:branches, ['master'])
assign(:branches_limit_exceeded, true)
assign(:tags, [])
assign(:tags_limit_exceeded, true)
render
end
it 'shows default branch' do
expect(rendered).to have_text('master')
end
it 'shows js expand link' do
expect(rendered).to have_selector('.js-details-expand')
end
it 'shows too many to search' do
expect(rendered).to have_text('Branches unavailable')
expect(rendered).to have_text('Tags unavailable')
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment