Merge branch 'fast_project_blob_path' into 'master'

Improve performance of tree rendering in repositories with a lot of items

See merge request gitlab-org/gitlab-ce!16511

app/helpers/icons_helper.rb

@@ -109,6 +109,8 @@ module IconsHelper
   def file_type_icon_class(type, mode, name)
     if type == 'folder'
       icon_class = 'folder'
+    elsif type == 'archive'
+      icon_class = 'archive'
     elsif mode == '120000'
       icon_class = 'share'
     else

app/helpers/tree_helper.rb

@@ -31,11 +31,21 @@ module TreeHelper
   # mode - File unix mode
   # name - File name
   def tree_icon(type, mode, name)
-    icon("#{file_type_icon_class(type, mode, name)} fw")
+    icon([file_type_icon_class(type, mode, name), 'fw'])
   end
 
-  def tree_hex_class(content)
-    "file_#{hexdigest(content.name)}"
+  # Using Rails `*_path` methods can be slow, especially when generating
+  # many paths, as with a repository tree that has thousands of items.
+  def fast_project_blob_path(project, blob_path)
+    Addressable::URI.escape(
+      File.join(relative_url_root, project.path_with_namespace, 'blob', blob_path)
+    )
+  end
+
+  def fast_project_tree_path(project, tree_path)
+    Addressable::URI.escape(
+      File.join(relative_url_root, project.path_with_namespace, 'tree', tree_path)
+    )
   end
 
   # Simple shortcut to File.join
@@ -142,4 +152,8 @@ module TreeHelper
   def selected_branch
     @branch_name || tree_edit_branch
   end
+
+  def relative_url_root
+    Gitlab.config.gitlab.relative_url_root.presence || '/'
+  end
 end

app/views/projects/tree/_blob_item.html.haml

-- is_lfs_blob = @lfs_blob_ids.include?(blob_item.id)
-%tr{ class: "tree-item #{tree_hex_class(blob_item)}" }
-  %td.tree-item-file-name
-    = tree_icon(type, blob_item.mode, blob_item.name)
-    - file_name = blob_item.name
-    = link_to project_blob_path(@project, tree_join(@id || @commit.id, blob_item.name)), class: 'str-truncated', title: file_name do
-      %span= file_name
-    - if is_lfs_blob
-      %span.badge.label-lfs.prepend-left-5 LFS
-  %td.d-none.d-sm-table-cell.tree-commit
-  %td.tree-time-ago.cgray.text-right
-    = render 'projects/tree/spinner'

app/views/projects/tree/_spinner.html.haml

-%span.log_loading.hide
-  %i.fa.fa-spinner.fa-spin
-  Loading commit data...

app/views/projects/tree/_submodule_item.html.haml

-%tr.tree-item
-  %td.tree-item-file-name
-    %i.fa.fa-archive.fa-fw
-    = submodule_link(submodule_item, @ref)
-  %td
-  %td.d-none.d-sm-table-cell

app/views/projects/tree/_tree_item.html.haml

-%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(@path, tree_item)
-    = link_to project_tree_path(@project, tree_join(@id || @commit.id, path)), class: 'str-truncated', title: path do
-      %span= path
-  %td.d-none.d-sm-table-cell.tree-commit
-  %td.tree-time-ago.text-right
-    = render 'projects/tree/spinner'

app/views/projects/tree/_tree_row.html.haml

-- if tree_row.type == :tree
-  = render partial: 'projects/tree/tree_item', object: tree_row, as: 'tree_item', locals: { type: 'folder' }
-- elsif tree_row.type == :blob
-  = render partial: 'projects/tree/blob_item', object: tree_row, as: 'blob_item', locals: { type: 'file' }
-- elsif tree_row.type == :commit
-  = render partial: 'projects/tree/submodule_item', object: tree_row, as: 'submodule_item'
+- tree_row_name = tree_row.name
+- tree_row_type = tree_row.type
+
+%tr{ class: "tree-item file_#{hexdigest(tree_row_name)}" }
+  %td.tree-item-file-name
+    - if tree_row_type == :tree
+      = tree_icon('folder', tree_row.mode, tree_row.name)
+      - path = flatten_tree(@path, tree_row)
+      %a.str-truncated{ href: fast_project_tree_path(@project, tree_join(@id || @commit.id, path)), title: path }
+        %span= path
+
+    - elsif tree_row_type == :blob
+      = tree_icon('file', tree_row.mode, tree_row_name)
+      %a.str-truncated{ href: fast_project_blob_path(@project, tree_join(@id || @commit.id, tree_row_name)), title: tree_row_name }
+        %span= tree_row_name
+      - if @lfs_blob_ids.include?(tree_row.id)
+        %span.badge.label-lfs.prepend-left-5 LFS
+
+    - elsif tree_row_type == :commit
+      = tree_icon('archive', tree_row.mode, tree_row.name)
+      = submodule_link(tree_row, @ref)
+
+  %td.d-none.d-sm-table-cell.tree-commit
+  %td.tree-time-ago.text-right
+    %span.log_loading.hide
+      %i.fa.fa-spinner.fa-spin
+      Loading commit data...

changelogs/unreleased/fast_project_blob_path.yml

+---
+title: Improve performance of tree rendering in repositories with lots of items
+merge_request:
+author:
+type: performance

spec/helpers/tree_helper_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe TreeHelper do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
-  let(:sha) { 'ce369011c189f62c815f5971d096b26759bab0d1' }
+  let(:sha) { 'c1c67abbaf91f624347bb3ae96eabe3a1b742478' }
 
   describe '.render_tree' do
     before do
@@ -32,6 +32,49 @@ describe TreeHelper do
     end
   end
 
+  describe '.fast_project_blob_path' do
+    it 'generates the same path as project_blob_path' do
+      blob_path = repository.tree(sha, 'with space').entries.first.path
+      fast_path = fast_project_blob_path(project, blob_path)
+      std_path  = project_blob_path(project, blob_path)
+
+      expect(fast_path).to eq(std_path)
+    end
+
+    it 'generates the same path with encoded file names' do
+      tree = repository.tree(sha, 'encoding')
+      blob_path = tree.entries.find { |entry| entry.path == 'encoding/テスト.txt' }.path
+      fast_path = fast_project_blob_path(project, blob_path)
+      std_path  = project_blob_path(project, blob_path)
+
+      expect(fast_path).to eq(std_path)
+    end
+
+    it 'respects a configured relative URL' do
+      allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
+      blob_path = repository.tree(sha, '').entries.first.path
+      fast_path = fast_project_blob_path(project, blob_path)
+
+      expect(fast_path).to start_with('/gitlab/root')
+    end
+  end
+
+  describe '.fast_project_tree_path' do
+    let(:tree_path) { repository.tree(sha, 'with space').path }
+    let(:fast_path) { fast_project_tree_path(project, tree_path) }
+    let(:std_path) { project_tree_path(project, tree_path) }
+
+    it 'generates the same path as project_tree_path' do
+      expect(fast_path).to eq(std_path)
+    end
+
+    it 'respects a configured relative URL' do
+      allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
+
+      expect(fast_path).to start_with('/gitlab/root')
+    end
+  end
+
   describe 'flatten_tree' do
     let(:tree) { repository.tree(sha, 'files') }
     let(:root_path) { 'files' }

spec/views/projects/tree/_blob_item.html.haml_spec.rb → spec/views/projects/tree/_tree_row.html.haml_spec.rb

 require 'spec_helper'
 
-describe 'projects/tree/_blob_item' do
+describe 'projects/tree/_tree_row' do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:blob_item) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID, 'files/ruby').first }
@@ -31,10 +31,7 @@ describe 'projects/tree/_blob_item' do
     end
   end
 
-  def render_partial(blob_item)
-    render partial: 'projects/tree/blob_item', locals: {
-      blob_item: blob_item,
-      type: 'blob'
-    }
+  def render_partial(items)
+    render partial: 'projects/tree/tree_row', collection: [items].flatten
   end
 end