summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaco Guzman <pacoguzmanp@gmail.com>2016-07-20 18:25:36 +0200
committerPaco Guzman <pacoguzmanp@gmail.com>2016-08-03 07:00:19 +0200
commitcd7c2cb6ddd4d9c9f9bdae00c887c0022c121c17 (patch)
treefad9386721cc514f28a011ef734b8791fc1c13be
parent195b20e1b9ff08437c5a436dc14f04e7f1bee807 (diff)
downloadgitlab-ce-cd7c2cb6ddd4d9c9f9bdae00c887c0022c121c17.tar.gz
Cache highlighted diff lines for merge requests
Introducing the concept of SafeDiffs which relates diffs with UI highlighting.
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/concerns/diff_for_path.rb6
-rw-r--r--app/controllers/projects/commit_controller.rb4
-rw-r--r--app/controllers/projects/compare_controller.rb6
-rw-r--r--app/controllers/projects/merge_requests_controller.rb10
-rw-r--r--app/helpers/commits_helper.rb4
-rw-r--r--app/helpers/diff_helper.rb11
-rw-r--r--app/models/merge_request.rb2
-rw-r--r--app/models/safe_diffs.rb5
-rw-r--r--app/models/safe_diffs/base.rb55
-rw-r--r--app/models/safe_diffs/commit.rb10
-rw-r--r--app/models/safe_diffs/compare.rb10
-rw-r--r--app/models/safe_diffs/merge_request.rb52
-rw-r--r--app/services/merge_requests/merge_request_diff_cache_service.rb8
-rw-r--r--app/views/notify/repository_push_email.html.haml2
-rw-r--r--app/views/projects/commit/show.html.haml2
-rw-r--r--app/views/projects/compare/show.html.haml2
-rw-r--r--app/views/projects/diffs/_diffs.html.haml2
-rw-r--r--app/views/projects/diffs/_file.html.haml4
-rw-r--r--app/views/projects/diffs/_text_file.html.haml2
-rw-r--r--app/views/projects/merge_requests/_new_submit.html.haml2
-rw-r--r--app/views/projects/merge_requests/show/_diffs.html.haml5
-rw-r--r--lib/gitlab/diff/file.rb5
-rw-r--r--lib/gitlab/diff/highlight.rb7
-rw-r--r--lib/gitlab/diff/line.rb14
-rw-r--r--lib/gitlab/email/message/repository_push.rb2
-rw-r--r--spec/controllers/projects/commit_controller_spec.rb13
-rw-r--r--spec/controllers/projects/compare_controller_spec.rb14
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb18
29 files changed, 221 insertions, 57 deletions
diff --git a/CHANGELOG b/CHANGELOG
index db2617dcbd7..581b8b09af9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,7 @@ v 8.11.0 (unreleased)
- The Repository class is now instrumented
- Cache the commit author in RequestStore to avoid extra lookups in PostReceive
- Expand commit message width in repo view (ClemMakesApps)
+ - Cache highlighted diff lines for merge requests
- Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
- Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable
- Optimize maximum user access level lookup in loading of notes
diff --git a/app/controllers/concerns/diff_for_path.rb b/app/controllers/concerns/diff_for_path.rb
index 026d8b2e1e0..aeec3009f15 100644
--- a/app/controllers/concerns/diff_for_path.rb
+++ b/app/controllers/concerns/diff_for_path.rb
@@ -1,8 +1,8 @@
module DiffForPath
extend ActiveSupport::Concern
- def render_diff_for_path(diffs, diff_refs, project)
- diff_file = safe_diff_files(diffs, diff_refs: diff_refs, repository: project.repository).find do |diff|
+ def render_diff_for_path(diffs)
+ diff_file = diffs.diff_files.find do |diff|
diff.old_path == params[:old_path] && diff.new_path == params[:new_path]
end
@@ -14,7 +14,7 @@ module DiffForPath
locals = {
diff_file: diff_file,
diff_commit: diff_commit,
- diff_refs: diff_refs,
+ diff_refs: diffs.diff_refs,
blob: blob,
project: project
}
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 7ae034f9398..6060b6e55bc 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -28,7 +28,7 @@ class Projects::CommitController < Projects::ApplicationController
end
def diff_for_path
- render_diff_for_path(@diffs, @commit.diff_refs, @project)
+ render_diff_for_path(SafeDiffs::Commit.new(@commit, diff_options: diff_options))
end
def builds
@@ -110,7 +110,7 @@ class Projects::CommitController < Projects::ApplicationController
opts = diff_options
opts[:ignore_whitespace_change] = true if params[:format] == 'diff'
- @diffs = commit.diffs(opts)
+ @diffs = SafeDiffs::Commit.new(commit, diff_options: opts)
@notes_count = commit.notes.count
end
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index 8c004724f02..2eda950a1bd 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -21,7 +21,7 @@ class Projects::CompareController < Projects::ApplicationController
def diff_for_path
return render_404 unless @compare
- render_diff_for_path(@diffs, @diff_refs, @project)
+ render_diff_for_path(SafeDiffs::Compare.new(@compare, project: @project, diff_options: diff_options))
end
def create
@@ -46,12 +46,12 @@ class Projects::CompareController < Projects::ApplicationController
@commit = @project.commit(@head_ref)
@base_commit = @project.merge_base_commit(@start_ref, @head_ref)
- @diffs = @compare.diffs(diff_options)
- @diff_refs = Gitlab::Diff::DiffRefs.new(
+ diff_refs = Gitlab::Diff::DiffRefs.new(
base_sha: @base_commit.try(:sha),
start_sha: @start_commit.try(:sha),
head_sha: @commit.try(:sha)
)
+ @diffs = SafeDiffs::Compare.new(@compare, project: @project, diff_options: diff_options, diff_refs: diff_refs)
@diff_notes_disabled = true
@grouped_diff_discussions = {}
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 116e7904a4e..78a6a3c5715 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -103,9 +103,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
define_commit_vars
- diffs = @merge_request.diffs(diff_options)
- render_diff_for_path(diffs, @merge_request.diff_refs, @merge_request.project)
+ render_diff_for_path(SafeDiffs::MergeRequest.new(merge_request, diff_options: diff_options))
end
def commits
@@ -153,7 +152,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@commits = @merge_request.compare_commits.reverse
@commit = @merge_request.diff_head_commit
@base_commit = @merge_request.diff_base_commit
- @diffs = @merge_request.compare.diffs(diff_options) if @merge_request.compare
+ if @merge_request.compare
+ @diffs = SafeDiffs::Compare.new(@merge_request.compare,
+ project: @merge_request.project,
+ diff_refs: @merge_request.diff_refs,
+ diff_options: diff_options)
+ end
@diff_notes_disabled = true
@pipeline = @merge_request.pipeline
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index f497626e21a..7a02d0b10d9 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -206,10 +206,10 @@ module CommitsHelper
end
end
- def view_file_btn(commit_sha, diff, project)
+ def view_file_btn(commit_sha, diff_new_path, project)
link_to(
namespace_project_blob_path(project.namespace, project,
- tree_join(commit_sha, diff.new_path)),
+ tree_join(commit_sha, diff_new_path)),
class: 'btn view-file js-view-file btn-file-option'
) do
raw('View file @') + content_tag(:span, commit_sha[0..6],
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index f35e2f6ddcd..6497282af57 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -23,18 +23,17 @@ module DiffHelper
end
def diff_options
- options = { ignore_whitespace_change: hide_whitespace?, no_collapse: expand_all_diffs? }
+ options = SafeDiffs.default_options.merge(
+ ignore_whitespace_change: hide_whitespace?,
+ no_collapse: expand_all_diffs?
+ )
if action_name == 'diff_for_path'
options[:no_collapse] = true
options[:paths] = params.values_at(:old_path, :new_path)
end
- Commit.max_diff_options.merge(options)
- end
-
- def safe_diff_files(diffs, diff_refs: nil, repository: nil)
- diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
+ options
end
def unfold_bottom_class(bottom)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index a99c4ba52a4..774851cc90f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -313,6 +313,8 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff.reload_content
+ MergeRequests::MergeRequestDiffCacheService.new.execute(self)
+
new_diff_refs = self.diff_refs
update_diff_notes_positions(
diff --git a/app/models/safe_diffs.rb b/app/models/safe_diffs.rb
new file mode 100644
index 00000000000..8ca9ec4cc39
--- /dev/null
+++ b/app/models/safe_diffs.rb
@@ -0,0 +1,5 @@
+module SafeDiffs
+ def self.default_options
+ ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false)
+ end
+end
diff --git a/app/models/safe_diffs/base.rb b/app/models/safe_diffs/base.rb
new file mode 100644
index 00000000000..dfc4708e293
--- /dev/null
+++ b/app/models/safe_diffs/base.rb
@@ -0,0 +1,55 @@
+module SafeDiffs
+ class Base
+ attr_reader :project, :diff_options, :diff_view, :diff_refs
+
+ delegate :count, :real_size, to: :diff_files
+
+ def initialize(diffs, project:, diff_options:, diff_refs: nil)
+ @diffs = diffs
+ @project = project
+ @diff_options = diff_options
+ @diff_refs = diff_refs
+ end
+
+ def diff_files
+ @diff_files ||= begin
+ diffs = @diffs.decorate! do |diff|
+ Gitlab::Diff::File.new(diff, diff_refs: @diff_refs, repository: @project.repository)
+ end
+
+ highlight!(diffs)
+ diffs
+ end
+ end
+
+ private
+
+ def highlight!(diff_files)
+ if cacheable?
+ cache_highlight!(diff_files)
+ else
+ diff_files.each { |diff_file| highlight_diff_file!(diff_file) }
+ end
+ end
+
+ def cacheable?
+ false
+ end
+
+ def cache_highlight!
+ raise NotImplementedError
+ end
+
+ def highlight_diff_file_from_cache!(diff_file, cache_diff_lines)
+ diff_file.diff_lines = cache_diff_lines.map do |line|
+ Gitlab::Diff::Line.init_from_hash(line)
+ end
+ end
+
+ def highlight_diff_file!(diff_file)
+ diff_file.diff_lines = Gitlab::Diff::Highlight.new(diff_file, repository: diff_file.repository).highlight
+ diff_file.highlighted_diff_lines = diff_file.diff_lines # To be used on parallel diff
+ diff_file
+ end
+ end
+end
diff --git a/app/models/safe_diffs/commit.rb b/app/models/safe_diffs/commit.rb
new file mode 100644
index 00000000000..338878f32e0
--- /dev/null
+++ b/app/models/safe_diffs/commit.rb
@@ -0,0 +1,10 @@
+module SafeDiffs
+ class Commit < Base
+ def initialize(commit, diff_options:)
+ super(commit.diffs(diff_options),
+ project: commit.project,
+ diff_options: diff_options,
+ diff_refs: commit.diff_refs)
+ end
+ end
+end
diff --git a/app/models/safe_diffs/compare.rb b/app/models/safe_diffs/compare.rb
new file mode 100644
index 00000000000..6b64b81137d
--- /dev/null
+++ b/app/models/safe_diffs/compare.rb
@@ -0,0 +1,10 @@
+module SafeDiffs
+ class Compare < Base
+ def initialize(compare, project:, diff_options:, diff_refs: nil)
+ super(compare.diffs(diff_options),
+ project: project,
+ diff_options: diff_options,
+ diff_refs: diff_refs)
+ end
+ end
+end
diff --git a/app/models/safe_diffs/merge_request.rb b/app/models/safe_diffs/merge_request.rb
new file mode 100644
index 00000000000..111b9a54f91
--- /dev/null
+++ b/app/models/safe_diffs/merge_request.rb
@@ -0,0 +1,52 @@
+module SafeDiffs
+ class MergeRequest < Base
+ def initialize(merge_request, diff_options:)
+ @merge_request = merge_request
+
+ super(merge_request.diffs(diff_options),
+ project: merge_request.project,
+ diff_options: diff_options,
+ diff_refs: merge_request.diff_refs)
+ end
+
+ private
+
+ #
+ # If we find the highlighted diff files lines on the cache we replace existing diff_files lines (no highlighted)
+ # for the highlighted ones, so we just skip their execution.
+ # If the highlighted diff files lines are not cached we calculate and cache them.
+ #
+ # The content of the cache is and Hash where the key correspond to the file_path and the values are Arrays of
+ # hashes than represent serialized diff lines.
+ #
+ def cache_highlight!(diff_files)
+ highlighted_cache = Rails.cache.read(cache_key) || {}
+ highlighted_cache_was_empty = highlighted_cache.empty?
+
+ diff_files.each do |diff_file|
+ file_path = diff_file.file_path
+
+ if highlighted_cache[file_path]
+ highlight_diff_file_from_cache!(diff_file, highlighted_cache[file_path])
+ else
+ highlight_diff_file!(diff_file)
+ highlighted_cache[file_path] = diff_file.diff_lines.map(&:to_hash)
+ end
+ end
+
+ if highlighted_cache_was_empty
+ Rails.cache.write(cache_key, highlighted_cache)
+ end
+
+ diff_files
+ end
+
+ def cacheable?
+ @merge_request.merge_request_diff.present?
+ end
+
+ def cache_key
+ [@merge_request.merge_request_diff, 'highlighted-safe-diff-files', diff_options]
+ end
+ end
+end
diff --git a/app/services/merge_requests/merge_request_diff_cache_service.rb b/app/services/merge_requests/merge_request_diff_cache_service.rb
new file mode 100644
index 00000000000..0a1905f7137
--- /dev/null
+++ b/app/services/merge_requests/merge_request_diff_cache_service.rb
@@ -0,0 +1,8 @@
+module MergeRequests
+ class MergeRequestDiffCacheService
+ def execute(merge_request)
+ # Executing the iteration we cache all the highlighted diff information
+ SafeDiffs::MergeRequest.new(merge_request, diff_options: SafeDiffs.default_options).diff_files.to_a
+ end
+ end
+end
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index c161ecc3463..2d1a98caeaa 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -75,7 +75,7 @@
- blob = diff_file.blob
- if blob && blob.respond_to?(:text?) && blob_text_viewable?(blob)
%table.code.white
- - diff_file.highlighted_diff_lines.each do |line|
+ - diff_file.diff_lines.each do |line|
= render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
- else
No preview for this file type
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index d0da2606587..11b2020f99b 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -7,7 +7,7 @@
= render "ci_menu"
- else
%div.block-connector
-= render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @commit.diff_refs
+= render "projects/diffs/diffs", diff_files: @diffs.diff_files, project: @diffs.project, diff_refs: @diffs.diff_refs
= render "projects/notes/notes_with_form"
- if can_collaborate_with_project?
- %w(revert cherry-pick).each do |type|
diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml
index 28a50e7031a..eb8a1bd5289 100644
--- a/app/views/projects/compare/show.html.haml
+++ b/app/views/projects/compare/show.html.haml
@@ -8,7 +8,7 @@
- if @commits.present?
= render "projects/commits/commit_list"
- = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs
+ = render "projects/diffs/diffs", diff_files: @diffs.diff_files, project: @diffs.project, diff_refs: @diffs.diff_refs
- else
.light-well
.center
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index 4bf3ccace20..45895a9a3de 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -2,8 +2,6 @@
- if diff_view == 'parallel'
- fluid_layout true
-- diff_files = safe_diff_files(diffs, diff_refs: diff_refs, repository: project.repository)
-
.content-block.oneline-block.files-changed
.inline-parallel-buttons
- if !expand_all_diffs? && diff_files.any? { |diff_file| diff_file.collapsed? }
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index 1854c64cbd7..f914e13a1ec 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -15,6 +15,6 @@
from_merge_request_id: @merge_request.id,
skip_visible_check: true)
- = view_file_btn(diff_commit.id, diff_file, project)
+ = view_file_btn(diff_commit.id, diff_file.new_path, project)
- = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, diff_refs: diff_refs, blob: blob, project: project
+ = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, blob: blob, project: project
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index 5970b9abf2b..a483927671e 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -5,7 +5,7 @@
%table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' }
- last_line = 0
- - diff_file.highlighted_diff_lines.each do |line|
+ - diff_file.diff_lines.each do |line|
- last_line = line.new_pos
= render "projects/diffs/line", line: line, diff_file: diff_file
diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml
index a5e67b95727..cb2b623691c 100644
--- a/app/views/projects/merge_requests/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/_new_submit.html.haml
@@ -42,7 +42,7 @@
%h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits.
%p To preserve performance the line changes are not shown.
- else
- = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @merge_request.diff_refs, show_whitespace_toggle: false
+ = render "projects/diffs/diffs", diff_files: @diffs.diff_files, project: @diffs.project, diff_refs: @merge_request.diff_refs, show_whitespace_toggle: false
- if @pipeline
#builds.builds.tab-pane
= render "projects/merge_requests/show/builds"
diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml
index 1b0bae86ad4..ed2765356db 100644
--- a/app/views/projects/merge_requests/show/_diffs.html.haml
+++ b/app/views/projects/merge_requests/show/_diffs.html.haml
@@ -1,6 +1,7 @@
- if @merge_request_diff.collected?
- = render "projects/diffs/diffs", diffs: @merge_request.diffs(diff_options),
- project: @merge_request.project, diff_refs: @merge_request.diff_refs
+ - diffs = SafeDiffs::MergeRequest.new(@merge_request, diff_options: diff_options)
+ = render "projects/diffs/diffs", diff_files: diffs.diff_files,
+ diff_refs: diffs.diff_refs, project: diffs.project
- elsif @merge_request_diff.empty?
.nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
- else
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index b09ca1fb8b0..77b3798d78f 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -63,15 +63,18 @@ module Gitlab
diff_refs.try(:head_sha)
end
+ attr_writer :diff_lines, :highlighted_diff_lines
+
# Array of Gitlab::Diff::Line objects
def diff_lines
- @lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
+ @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
end
def highlighted_diff_lines
@highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
end
+ # Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted
def parallel_diff_lines
@parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize
end
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 649a265a02c..9ea976e18fa 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -40,8 +40,6 @@ module Gitlab
def highlight_line(diff_line)
return unless diff_file && diff_file.diff_refs
- line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' '
-
rich_line =
if diff_line.unchanged? || diff_line.added?
new_lines[diff_line.new_pos - 1]
@@ -51,7 +49,10 @@ module Gitlab
# Only update text if line is found. This will prevent
# issues with submodules given the line only exists in diff content.
- "#{line_prefix}#{rich_line}".html_safe if rich_line
+ if rich_line
+ line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' '
+ "#{line_prefix}#{rich_line}".html_safe
+ end
end
def inline_diffs
diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb
index c6189d660c2..cf097e0d0de 100644
--- a/lib/gitlab/diff/line.rb
+++ b/lib/gitlab/diff/line.rb
@@ -9,6 +9,20 @@ module Gitlab
@old_pos, @new_pos = old_pos, new_pos
end
+ def self.init_from_hash(hash)
+ new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos])
+ end
+
+ def serialize_keys
+ @serialize_keys ||= %i(text type index old_pos new_pos)
+ end
+
+ def to_hash
+ hash = {}
+ serialize_keys.each { |key| hash[key] = send(key) }
+ hash
+ end
+
def old_line
old_pos unless added? || meta?
end
diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb
index 97701b0cd42..48946ba355b 100644
--- a/lib/gitlab/email/message/repository_push.rb
+++ b/lib/gitlab/email/message/repository_push.rb
@@ -41,7 +41,7 @@ module Gitlab
def diffs
return unless compare
- @diffs ||= safe_diff_files(compare.diffs(max_files: 30), diff_refs: diff_refs, repository: project.repository)
+ @diffs ||= SafeDiffs::Compare.new(compare, diff_options: { max_files: 30 }, project: project, diff_refs: diff_refs).diff_files
end
def diffs_count
diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb
index df902da86f8..30121facd7d 100644
--- a/spec/controllers/projects/commit_controller_spec.rb
+++ b/spec/controllers/projects/commit_controller_spec.rb
@@ -83,7 +83,7 @@ describe Projects::CommitController do
let(:format) { :diff }
it "should really only be a git diff" do
- go(id: commit.id, format: format)
+ go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format)
expect(response.body).to start_with("diff --git")
end
@@ -92,8 +92,9 @@ describe Projects::CommitController do
go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1)
expect(response.body).to start_with("diff --git")
- # without whitespace option, there are more than 2 diff_splits
- diff_splits = assigns(:diffs).first.diff.split("\n")
+
+ # without whitespace option, there are more than 2 diff_splits for other formats
+ diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n")
expect(diff_splits.length).to be <= 2
end
end
@@ -266,9 +267,9 @@ describe Projects::CommitController do
end
it 'only renders the diffs for the path given' do
- expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project|
- expect(diffs.map(&:new_path)).to contain_exactly(existing_path)
- meth.call(diffs, diff_refs, project)
+ expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, safe_diffs|
+ expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path)
+ meth.call(safe_diffs)
end
diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path)
diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb
index 4058d5e2453..6272a5f111d 100644
--- a/spec/controllers/projects/compare_controller_spec.rb
+++ b/spec/controllers/projects/compare_controller_spec.rb
@@ -19,7 +19,7 @@ describe Projects::CompareController do
to: ref_to)
expect(response).to be_success
- expect(assigns(:diffs).first).not_to be_nil
+ expect(assigns(:diffs).diff_files.first).not_to be_nil
expect(assigns(:commits).length).to be >= 1
end
@@ -32,10 +32,10 @@ describe Projects::CompareController do
w: 1)
expect(response).to be_success
- expect(assigns(:diffs).first).not_to be_nil
+ expect(assigns(:diffs).diff_files.first).not_to be_nil
expect(assigns(:commits).length).to be >= 1
# without whitespace option, there are more than 2 diff_splits
- diff_splits = assigns(:diffs).first.diff.split("\n")
+ diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n")
expect(diff_splits.length).to be <= 2
end
@@ -48,7 +48,7 @@ describe Projects::CompareController do
to: ref_to)
expect(response).to be_success
- expect(assigns(:diffs).to_a).to eq([])
+ expect(assigns(:diffs).diff_files.to_a).to eq([])
expect(assigns(:commits)).to eq([])
end
@@ -87,9 +87,9 @@ describe Projects::CompareController do
end
it 'only renders the diffs for the path given' do
- expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project|
- expect(diffs.map(&:new_path)).to contain_exactly(existing_path)
- meth.call(diffs, diff_refs, project)
+ expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, safe_diffs|
+ expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path)
+ meth.call(safe_diffs)
end
diff_for_path(from: ref_from, to: ref_to, old_path: existing_path, new_path: existing_path)
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 210085e3b1a..9da43c9cee7 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -392,9 +392,9 @@ describe Projects::MergeRequestsController do
end
it 'only renders the diffs for the path given' do
- expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project|
- expect(diffs.map(&:new_path)).to contain_exactly(existing_path)
- meth.call(diffs, diff_refs, project)
+ expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, safe_diffs|
+ expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path)
+ meth.call(safe_diffs)
end
diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path)
@@ -455,9 +455,9 @@ describe Projects::MergeRequestsController do
end
it 'only renders the diffs for the path given' do
- expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project|
- expect(diffs.map(&:new_path)).to contain_exactly(existing_path)
- meth.call(diffs, diff_refs, project)
+ expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, safe_diffs|
+ expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path)
+ meth.call(safe_diffs)
end
diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_branch: 'feature', target_branch: 'master' })
@@ -477,9 +477,9 @@ describe Projects::MergeRequestsController do
end
it 'only renders the diffs for the path given' do
- expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs, diff_refs, project|
- expect(diffs.map(&:new_path)).to contain_exactly(existing_path)
- meth.call(diffs, diff_refs, project)
+ expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, safe_diffs|
+ expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path)
+ meth.call(safe_diffs)
end
diff_for_path(old_path: existing_path, new_path: existing_path, merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' })