diff options
-rw-r--r-- | app/helpers/icons_helper.rb | 2 | ||||
-rw-r--r-- | app/helpers/tree_helper.rb | 20 | ||||
-rw-r--r-- | app/views/projects/tree/_blob_item.html.haml | 12 | ||||
-rw-r--r-- | app/views/projects/tree/_spinner.html.haml | 3 | ||||
-rw-r--r-- | app/views/projects/tree/_submodule_item.html.haml | 6 | ||||
-rw-r--r-- | app/views/projects/tree/_tree_item.html.haml | 9 | ||||
-rw-r--r-- | app/views/projects/tree/_tree_row.html.haml | 33 | ||||
-rw-r--r-- | changelogs/unreleased/fast_project_blob_path.yml | 5 | ||||
-rw-r--r-- | spec/helpers/tree_helper_spec.rb | 45 | ||||
-rw-r--r-- | spec/views/projects/tree/_tree_row.html.haml_spec.rb (renamed from spec/views/projects/tree/_blob_item.html.haml_spec.rb) | 9 |
10 files changed, 98 insertions, 46 deletions
diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index 910c9e9446f..b0f63de2fb8 100644 --- a/app/helpers/icons_helper.rb +++ b/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 diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 6d2da5699fb..78a11616d4c 100644 --- a/app/helpers/tree_helper.rb +++ b/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 diff --git a/app/views/projects/tree/_blob_item.html.haml b/app/views/projects/tree/_blob_item.html.haml deleted file mode 100644 index f79f3af36d4..00000000000 --- a/app/views/projects/tree/_blob_item.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -- 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' diff --git a/app/views/projects/tree/_spinner.html.haml b/app/views/projects/tree/_spinner.html.haml deleted file mode 100644 index b47ad0f41e4..00000000000 --- a/app/views/projects/tree/_spinner.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -%span.log_loading.hide - %i.fa.fa-spinner.fa-spin - Loading commit data... diff --git a/app/views/projects/tree/_submodule_item.html.haml b/app/views/projects/tree/_submodule_item.html.haml deleted file mode 100644 index e563c8c4036..00000000000 --- a/app/views/projects/tree/_submodule_item.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -%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 diff --git a/app/views/projects/tree/_tree_item.html.haml b/app/views/projects/tree/_tree_item.html.haml deleted file mode 100644 index ce0cd95b468..00000000000 --- a/app/views/projects/tree/_tree_item.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -%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' diff --git a/app/views/projects/tree/_tree_row.html.haml b/app/views/projects/tree/_tree_row.html.haml index 0a5c6f048f7..8a27ea66523 100644 --- a/app/views/projects/tree/_tree_row.html.haml +++ b/app/views/projects/tree/_tree_row.html.haml @@ -1,6 +1,27 @@ -- 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... diff --git a/changelogs/unreleased/fast_project_blob_path.yml b/changelogs/unreleased/fast_project_blob_path.yml new file mode 100644 index 00000000000..b56c9d9cf59 --- /dev/null +++ b/changelogs/unreleased/fast_project_blob_path.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of tree rendering in repositories with lots of items +merge_request: +author: +type: performance diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index ffdf6561a53..ab4566e261b 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/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' } diff --git a/spec/views/projects/tree/_blob_item.html.haml_spec.rb b/spec/views/projects/tree/_tree_row.html.haml_spec.rb index 6a477c712ff..3353b7665e2 100644 --- a/spec/views/projects/tree/_blob_item.html.haml_spec.rb +++ b/spec/views/projects/tree/_tree_row.html.haml_spec.rb @@ -1,6 +1,6 @@ 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 |