summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlejandro Rodríguez <alejorro70@gmail.com>2017-09-06 17:47:25 -0300
committerAlejandro Rodríguez <alejorro70@gmail.com>2017-09-06 17:47:25 -0300
commitc2e99b40f71ca7036cc07596aae164e92378263b (patch)
tree85e912ba19b4091364206b63e55d1facf2e423d9
parent21935d85382989e38dd4cc12de55966e0c9b6eba (diff)
downloadgitlab-ce-c2e99b40f71ca7036cc07596aae164e92378263b.tar.gz
Implement fix for n+1 issue on `flatten_tree` helpergitaly-tree-entries-fix
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/helpers/tree_helper.rb4
-rw-r--r--app/views/projects/tree/_tree_item.html.haml2
-rw-r--r--lib/gitlab/git/tree.rb11
-rw-r--r--lib/gitlab/gitaly_client/commit_service.rb6
-rw-r--r--spec/helpers/tree_helper_spec.rb26
-rw-r--r--spec/lib/gitlab/git/tree_spec.rb3
8 files changed, 38 insertions, 20 deletions
diff --git a/Gemfile b/Gemfile
index 0341f2609ad..0221d41fad7 100644
--- a/Gemfile
+++ b/Gemfile
@@ -397,7 +397,7 @@ group :ed25519 do
end
# Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.32.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.33.0', require: 'gitaly'
gem 'toml-rb', '~> 0.3.15', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 320d42b8974..c29d304fd48 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -275,7 +275,7 @@ GEM
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
gherkin-ruby (0.3.2)
- gitaly-proto (0.32.0)
+ gitaly-proto (0.33.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
github-linguist (4.7.6)
@@ -1021,7 +1021,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.2.0)
- gitaly-proto (~> 0.32.0)
+ gitaly-proto (~> 0.33.0)
github-linguist (~> 4.7.0)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-markup (~> 1.5.1)
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index e0d3e9b88f3..95dbdc8ec46 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -99,7 +99,9 @@ module TreeHelper
end
# returns the relative path of the first subdir that doesn't have only one directory descendant
- def flatten_tree(tree)
+ def flatten_tree(root_path, tree)
+ return tree.flat_path.sub(/\A#{root_path}\//, '') if tree.flat_path.present?
+
subtree = Gitlab::Git::Tree.where(@repository, @commit.id, tree.path)
if subtree.count == 1 && subtree.first.dir?
return tree_join(tree.name, flatten_tree(subtree.first))
diff --git a/app/views/projects/tree/_tree_item.html.haml b/app/views/projects/tree/_tree_item.html.haml
index 0c9c8750f2c..56197382a70 100644
--- a/app/views/projects/tree/_tree_item.html.haml
+++ b/app/views/projects/tree/_tree_item.html.haml
@@ -1,7 +1,7 @@
%tr{ class: "tree-item #{tree_hex_class(tree_item)}" }
%td.tree-item-file-name
= tree_icon(type, tree_item.mode, tree_item.name)
- - path = flatten_tree(tree_item)
+ - path = flatten_tree(@path, tree_item)
= link_to project_tree_path(@project, tree_join(@id || @commit.id, path)), title: path do
%span.str-truncated= path
%td.hidden-xs.tree-commit
diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb
index b54962a4456..5cf336af3c6 100644
--- a/lib/gitlab/git/tree.rb
+++ b/lib/gitlab/git/tree.rb
@@ -5,7 +5,7 @@ module Gitlab
class Tree
include Gitlab::EncodingHelper
- attr_accessor :id, :root_id, :name, :path, :type,
+ attr_accessor :id, :root_id, :name, :path, :flat_path, :type,
:mode, :commit_id, :submodule_url
class << self
@@ -19,8 +19,7 @@ module Gitlab
Gitlab::GitalyClient.migrate(:tree_entries) do |is_enabled|
if is_enabled
- client = Gitlab::GitalyClient::CommitService.new(repository)
- client.tree_entries(repository, sha, path)
+ repository.gitaly_commit_client.tree_entries(repository, sha, path)
else
tree_entries_from_rugged(repository, sha, path)
end
@@ -88,7 +87,7 @@ module Gitlab
end
def initialize(options)
- %w(id root_id name path type mode commit_id).each do |key|
+ %w(id root_id name path flat_path type mode commit_id).each do |key|
self.send("#{key}=", options[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend
end
end
@@ -101,6 +100,10 @@ module Gitlab
encode! @path
end
+ def flat_path
+ encode! @flat_path
+ end
+
def dir?
type == :tree
end
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 21a32a7e0db..0825a3a7694 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -88,14 +88,14 @@ module Gitlab
response.flat_map do |message|
message.entries.map do |gitaly_tree_entry|
- entry_path = gitaly_tree_entry.path.dup
Gitlab::Git::Tree.new(
id: gitaly_tree_entry.oid,
root_id: gitaly_tree_entry.root_oid,
type: gitaly_tree_entry.type.downcase,
mode: gitaly_tree_entry.mode.to_s(8),
- name: File.basename(entry_path),
- path: entry_path,
+ name: File.basename(gitaly_tree_entry.path),
+ path: GitalyClient.encode(gitaly_tree_entry.path),
+ flat_path: GitalyClient.encode(gitaly_tree_entry.flat_path),
commit_id: gitaly_tree_entry.commit_oid
)
end
diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb
index 9523d0f4aa6..d7b66e6f078 100644
--- a/spec/helpers/tree_helper_spec.rb
+++ b/spec/helpers/tree_helper_spec.rb
@@ -3,25 +3,35 @@ require 'spec_helper'
describe TreeHelper do
describe 'flatten_tree' do
let(:project) { create(:project, :repository) }
+ let(:repository) { project.repository }
+ let(:sha) { 'ce369011c189f62c815f5971d096b26759bab0d1' }
+ let(:tree) { repository.tree(sha, 'files') }
+ let(:root_path) { 'files' }
+ let(:tree_item) { tree.entries.find { |entry| entry.path == path } }
- before do
- @repository = project.repository
- @commit = project.commit("e56497bb")
- end
+ subject { flatten_tree(root_path, tree_item) }
context "on a directory containing more than one file/directory" do
- let(:tree_item) { double(name: "files", path: "files") }
+ let(:path) { 'files/html' }
it "returns the directory name" do
- expect(flatten_tree(tree_item)).to match('files')
+ expect(subject).to match('html')
end
end
context "on a directory containing only one directory" do
- let(:tree_item) { double(name: "foo", path: "foo") }
+ let(:path) { 'files/flat' }
it "returns the flattened path" do
- expect(flatten_tree(tree_item)).to match('foo/bar')
+ expect(subject).to match('flat/path/correct')
+ end
+
+ context "with a nested root path" do
+ let(:root_path) { 'files/flat' }
+
+ it "returns the flattened path with the root path suffix removed" do
+ expect(subject).to match('path/correct')
+ end
end
end
end
diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb
index c07a2d91768..86f7bcb8e38 100644
--- a/spec/lib/gitlab/git/tree_spec.rb
+++ b/spec/lib/gitlab/git/tree_spec.rb
@@ -20,6 +20,7 @@ describe Gitlab::Git::Tree, seed_helper: true do
it { expect(dir.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(dir.name).to eq('encoding') }
it { expect(dir.path).to eq('encoding') }
+ it { expect(dir.flat_path).to eq('encoding') }
it { expect(dir.mode).to eq('40000') }
context :subdir do
@@ -30,6 +31,7 @@ describe Gitlab::Git::Tree, seed_helper: true do
it { expect(subdir.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(subdir.name).to eq('html') }
it { expect(subdir.path).to eq('files/html') }
+ it { expect(subdir.flat_path).to eq('files/html') }
end
context :subdir_file do
@@ -40,6 +42,7 @@ describe Gitlab::Git::Tree, seed_helper: true do
it { expect(subdir_file.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(subdir_file.name).to eq('popen.rb') }
it { expect(subdir_file.path).to eq('files/ruby/popen.rb') }
+ it { expect(subdir_file.flat_path).to eq('files/ruby/popen.rb') }
end
end