Implement fix for n+1 issue on `flatten_tree` helper
This commit is contained in:
parent
21935d8538
commit
c2e99b40f7
2
Gemfile
2
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
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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))
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
Loading…
Reference in New Issue