diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-07-22 02:27:04 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-07-22 02:27:04 +0300 |
commit | e599afac95d6cdaadb73a6b102d4458f8e280212 (patch) | |
tree | 9eb4cac860b85cd5a7d3f5c90e360959d62ba67f | |
parent | c53bd5a778e00d05abc713887e436b75f385c666 (diff) | |
parent | 065a65adfe3508581664358779821b802476ee0d (diff) |
Merge branch '17073-tagscontroller-index-is-terrible-response-time-goes-up-to-5-seconds' into 'master'
Improve performance of tags controller
See merge request !5375
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | Gemfile | 2 | ||||
-rw-r--r-- | Gemfile.lock | 4 | ||||
-rw-r--r-- | app/models/repository.rb | 25 | ||||
-rw-r--r-- | app/services/delete_branch_service.rb | 2 | ||||
-rw-r--r-- | app/services/delete_tag_service.rb | 2 | ||||
-rw-r--r-- | app/services/git_tag_push_service.rb | 4 | ||||
-rw-r--r-- | app/views/projects/branches/_commit.html.haml | 2 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 26 |
9 files changed, 29 insertions, 39 deletions
diff --git a/CHANGELOG b/CHANGELOG index 938e3a496a6..bc09a51cc1e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -82,6 +82,7 @@ v 8.10.0 (unreleased) - API: Todos. !3188 (Robert Schilling) - API: Expose shared groups for projects and shared projects for groups. !5050 (Robert Schilling) - API: Expose `developers_can_push` and `developers_can_merge` for branches. !5208 (Robert Schilling) + - Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects - Add "Enabled Git access protocols" to Application Settings - Diffs will create button/diff form on demand no on server side - Reduce size of HTML used by diff comment forms @@ -52,7 +52,7 @@ gem 'browser', '~> 2.2' # Extracting information from a git repository # Provide access to Gitlab::Git library -gem 'gitlab_git', '~> 10.3.2' +gem 'gitlab_git', '~> 10.4.1' # LDAP Auth # GitLab fork with several improvements to original library. For full list of changes diff --git a/Gemfile.lock b/Gemfile.lock index e2b3d55ee0c..195516d1bf1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -274,7 +274,7 @@ GEM diff-lcs (~> 1.1) mime-types (>= 1.16, < 3) posix-spawn (~> 0.3) - gitlab_git (10.3.2) + gitlab_git (10.4.1) activesupport (~> 4.0) charlock_holmes (~> 0.7.3) github-linguist (~> 4.7.0) @@ -861,7 +861,7 @@ DEPENDENCIES github-linguist (~> 4.7.0) github-markup (~> 1.4) gitlab-flowdock-git-hook (~> 1.0.1) - gitlab_git (~> 10.3.2) + gitlab_git (~> 10.4.1) gitlab_meta (= 7.0) gitlab_omniauth-ldap (~> 1.2.1) gollum-lib (~> 4.2) diff --git a/app/models/repository.rb b/app/models/repository.rb index a6580e85498..46a04eb80cd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -70,7 +70,12 @@ class Repository def commit(ref = 'HEAD') return nil unless exists? - commit = Gitlab::Git::Commit.find(raw_repository, ref) + commit = + if ref.is_a?(Gitlab::Git::Commit) + ref + else + Gitlab::Git::Commit.find(raw_repository, ref) + end commit = ::Commit.new(commit, @project) if commit commit rescue Rugged::OdbError @@ -247,10 +252,10 @@ class Repository # Rugged seems to throw a `ReferenceError` when given branch_names rather # than SHA-1 hashes number_commits_behind = raw_repository. - count_commits_between(branch.target, root_ref_hash) + count_commits_between(branch.target.sha, root_ref_hash) number_commits_ahead = raw_repository. - count_commits_between(root_ref_hash, branch.target) + count_commits_between(root_ref_hash, branch.target.sha) { behind: number_commits_behind, ahead: number_commits_ahead } end @@ -674,9 +679,7 @@ class Repository end def local_branches - @local_branches ||= rugged.branches.each(:local).map do |branch| - Gitlab::Git::Branch.new(branch.name, branch.target) - end + @local_branches ||= raw_repository.local_branches end alias_method :branches, :local_branches @@ -817,7 +820,7 @@ class Repository end def revert(user, commit, base_branch, revert_tree_id = nil) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha revert_tree_id ||= check_revert_content(commit, base_branch) return false unless revert_tree_id @@ -834,7 +837,7 @@ class Repository end def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) return false unless cherry_pick_tree_id @@ -855,7 +858,7 @@ class Repository end def check_revert_content(commit, base_branch) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha args = [commit.id, source_sha] args << { mainline: 1 } if commit.merge_commit? @@ -869,7 +872,7 @@ class Repository end def check_cherry_pick_content(commit, base_branch) - source_sha = find_branch(base_branch).target + source_sha = find_branch(base_branch).target.sha args = [commit.id, source_sha] args << 1 if commit.merge_commit? @@ -1034,7 +1037,7 @@ class Repository end def tags_sorted_by_committed_date - tags.sort_by { |tag| commit(tag.target).committed_date } + tags.sort_by { |tag| tag.target.committed_date } end def keep_around_ref_name(sha) diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 332c55581a1..87f066edb6f 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -40,6 +40,6 @@ class DeleteBranchService < BaseService def build_push_data(branch) Gitlab::PushDataBuilder - .build(project, current_user, branch.target, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) + .build(project, current_user, branch.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) end end diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index 1e41fbe34b6..32e0eed6b63 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -34,6 +34,6 @@ class DeleteTagService < BaseService def build_push_data(tag) Gitlab::PushDataBuilder - .build(project, current_user, tag.target, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) + .build(project, current_user, tag.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) end end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 58573078048..969530c4fdc 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -26,8 +26,8 @@ class GitTagPushService < BaseService unless Gitlab::Git.blank_ref?(params[:newrev]) tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) - - if tag && tag.target == params[:newrev] + + if tag && tag.object_sha == params[:newrev] commit = project.commit(tag.target) commits = [commit].compact message = tag.message diff --git a/app/views/projects/branches/_commit.html.haml b/app/views/projects/branches/_commit.html.haml index 9fe65cbb104..d54c76ff9c8 100644 --- a/app/views/projects/branches/_commit.html.haml +++ b/app/views/projects/branches/_commit.html.haml @@ -1,5 +1,5 @@ .branch-commit - = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-id monospace" + = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-id monospace" · %span.str-truncated = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message" diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 38b0c345b48..3e133143bbc 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -50,8 +50,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now) double_last = double(committed_date: Time.now - 1.second) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_first) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_last) + allow(tag_a).to receive(:target).and_return(double_first) + allow(tag_b).to receive(:target).and_return(double_last) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } @@ -64,8 +65,9 @@ describe Repository, models: true do double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) - allow(repository).to receive(:commit).with(tag_a.target).and_return(double_last) - allow(repository).to receive(:commit).with(tag_b.target).and_return(double_first) + allow(tag_a).to receive(:target).and_return(double_last) + allow(tag_b).to receive(:target).and_return(double_first) + allow(repository).to receive(:tags).and_return([tag_a, tag_b]) end it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } @@ -1161,17 +1163,6 @@ describe Repository, models: true do end end - describe '#local_branches' do - it 'returns the local branches' do - masterrev = repository.find_branch('master').target - create_remote_branch('joe', 'remote_branch', masterrev) - repository.add_branch(user, 'local_branch', masterrev) - - expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) - expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) - end - end - describe "#keep_around" do it "stores a reference to the specified commit sha so it isn't garbage collected" do repository.keep_around(sample_commit.id) @@ -1179,9 +1170,4 @@ describe Repository, models: true do expect(repository.kept_around?(sample_commit.id)).to be_truthy end end - - def create_remote_branch(remote_name, branch_name, target) - rugged = repository.rugged - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target) - end end |