From 48ff40a047103bf09d4ac53fdbc984d73bc464cb Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 29 Jul 2016 21:04:04 -0700
Subject: [PATCH] Improve diff performance by eliminating redundant checks for
text blobs
On a merge request with over 1000 changed files, there were redundant
calls to blob_text_viewable?, which incurred about 7% of the time.
Improves #14775
---
CHANGELOG | 1 +
app/helpers/blob_helper.rb | 2 +-
app/views/projects/blob/_actions.html.haml | 3 ++-
app/views/projects/diffs/_file.html.haml | 9 +++++----
spec/helpers/blob_helper_spec.rb | 18 ++++++++++++++++++
5 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 31a7eae90b93d..a4bb72a92210f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.11.0 (unreleased)
- Fix the title of the toggle dropdown button. !5515 (herminiotorres)
+ - Improve diff performance by eliminating redundant checks for text blobs
- Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell)
- Fix CI status icon link underline (ClemMakesApps)
- Cache the commit author in RequestStore to avoid extra lookups in PostReceive
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index abe115d8c68be..48c2782821905 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -13,7 +13,7 @@ def edit_blob_link(project = @project, ref = @ref, path = @path, options = {})
blob = project.repository.blob_at(ref, path) rescue nil
- return unless blob && blob_text_viewable?(blob)
+ return unless blob
from_mr = options[:from_merge_request_id]
link_opts = {}
diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml
index cdac50f7a8d3a..ff893ea74e187 100644
--- a/app/views/projects/blob/_actions.html.haml
+++ b/app/views/projects/blob/_actions.html.haml
@@ -16,6 +16,7 @@
- if current_user
.btn-group{ role: "group" }
- = edit_blob_link
+ - if blob_text_viewable?(@blob)
+ = edit_blob_link
= replace_blob_link
= delete_blob_link
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index c306909fb1ae6..1854c64cbd775 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -9,10 +9,11 @@
= icon('comment')
\
- - if editable_diff?(diff_file)
- = edit_blob_link(@merge_request.source_project,
- @merge_request.source_branch, diff_file.new_path,
- from_merge_request_id: @merge_request.id)
+ - if editable_diff?(diff_file)
+ = edit_blob_link(@merge_request.source_project,
+ @merge_request.source_branch, diff_file.new_path,
+ from_merge_request_id: @merge_request.id,
+ skip_visible_check: true)
= view_file_btn(diff_commit.id, diff_file, project)
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index bd0108f99380c..b2d6d59b1ee0c 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe BlobHelper do
+ include TreeHelper
+
let(:blob_name) { 'test.lisp' }
let(:no_context_content) { ":type \"assem\"))" }
let(:blob_content) { "(make-pathname :defaults name\n#{no_context_content}" }
@@ -65,4 +67,20 @@ def test(input):
expect(sanitize_svg(blob).data).to eq(expected)
end
end
+
+ describe "#edit_blob_link" do
+ let(:project) { create(:project) }
+
+ before do
+ allow(self).to receive(:current_user).and_return(double)
+ end
+
+ it 'verifies blob is text' do
+ expect(self).not_to receive(:blob_text_viewable?)
+
+ button = edit_blob_link(project, 'refs/heads/master', 'README.md')
+
+ expect(button).to start_with('<button')
+ end
+ end
end
--
GitLab