diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 6060b6e55bc..771a86530cd 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(SafeDiffs::Commit.new(@commit, diff_options: diff_options)) + render_diff_for_path(@commit.diff_file_collection(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 = SafeDiffs::Commit.new(commit, diff_options: opts) + @diffs = commit.diff_file_collection(opts) @notes_count = commit.notes.count end diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 2eda950a1bd..252ddfa429a 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(SafeDiffs::Compare.new(@compare, project: @project, diff_options: diff_options)) + render_diff_for_path(Compare.decorate(@compare, @project).diff_file_collection(diff_options: diff_options)) end def create @@ -51,7 +51,7 @@ class Projects::CompareController < Projects::ApplicationController 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) + @diffs = Compare.decorate(@compare, @project).diff_file_collection(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 78a6a3c5715..39e7d0f6182 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -85,7 +85,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController respond_to do |format| format.html { define_discussion_vars } - format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } + format.json do + @diffs = @merge_request.diff_file_collection(diff_options) if @merge_request_diff.collected? + + render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } + end end end @@ -104,7 +108,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController define_commit_vars - render_diff_for_path(SafeDiffs::MergeRequest.new(merge_request, diff_options: diff_options)) + render_diff_for_path(@merge_request.diff_file_collection(diff_options)) end def commits @@ -153,10 +157,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit 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) + @diffs = Compare.decorate(@merge_request.compare, @project).diff_file_collection( + diff_options: diff_options, + diff_refs: @merge_request.diff_refs + ) end @diff_notes_disabled = true diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 6497282af57..2abe24b78bf 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -23,7 +23,7 @@ module DiffHelper end def diff_options - options = SafeDiffs.default_options.merge( + options = Gitlab::Diff::FileCollection.default_options.merge( ignore_whitespace_change: hide_whitespace?, no_collapse: expand_all_diffs? ) diff --git a/app/models/commit.rb b/app/models/commit.rb index c52b4a051c2..d22ecb222e5 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -317,6 +317,10 @@ class Commit nil end + def diff_file_collection(diff_options) + Gitlab::Diff::FileCollection::Commit.new(self, diff_options: diff_options) + end + private def find_author_by_any_email diff --git a/app/models/compare.rb b/app/models/compare.rb new file mode 100644 index 00000000000..6672d1bf059 --- /dev/null +++ b/app/models/compare.rb @@ -0,0 +1,23 @@ +class Compare + delegate :commits, :same, :head, :base, to: :@compare + + def self.decorate(compare, project) + if compare.is_a?(Compare) + compare + else + self.new(compare, project) + end + end + + def initialize(compare, project) + @compare = compare + @project = project + end + + def diff_file_collection(diff_options:, diff_refs: nil) + Gitlab::Diff::FileCollection::Compare.new(@compare, + project: @project, + diff_options: diff_options, + diff_refs: diff_refs) + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 774851cc90f..abc8bacbe59 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -168,6 +168,10 @@ class MergeRequest < ActiveRecord::Base merge_request_diff ? merge_request_diff.diffs(*args) : compare.diffs(*args) end + def diff_file_collection(diff_options) + Gitlab::Diff::FileCollection::MergeRequest.new(self, diff_options: diff_options) + end + def diff_size merge_request_diff.size end diff --git a/app/models/safe_diffs.rb b/app/models/safe_diffs.rb deleted file mode 100644 index 8ca9ec4cc39..00000000000 --- a/app/models/safe_diffs.rb +++ /dev/null @@ -1,5 +0,0 @@ -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 deleted file mode 100644 index dfc4708e293..00000000000 --- a/app/models/safe_diffs/base.rb +++ /dev/null @@ -1,55 +0,0 @@ -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 deleted file mode 100644 index 338878f32e0..00000000000 --- a/app/models/safe_diffs/commit.rb +++ /dev/null @@ -1,10 +0,0 @@ -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 deleted file mode 100644 index 6b64b81137d..00000000000 --- a/app/models/safe_diffs/compare.rb +++ /dev/null @@ -1,10 +0,0 @@ -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 deleted file mode 100644 index 111b9a54f91..00000000000 --- a/app/models/safe_diffs/merge_request.rb +++ /dev/null @@ -1,52 +0,0 @@ -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 index 0a1905f7137..982540ba7f5 100644 --- a/app/services/merge_requests/merge_request_diff_cache_service.rb +++ b/app/services/merge_requests/merge_request_diff_cache_service.rb @@ -2,7 +2,7 @@ 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 + merge_request.diff_file_collection(Gitlab::Diff::FileCollection.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 2d1a98caeaa..c161ecc3463 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.diff_lines.each do |line| + - diff_file.highlighted_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/_ci_menu.html.haml b/app/views/projects/commit/_ci_menu.html.haml index ea33aa472a6..935433306ea 100644 --- a/app/views/projects/commit/_ci_menu.html.haml +++ b/app/views/projects/commit/_ci_menu.html.haml @@ -2,7 +2,7 @@ = nav_link(path: 'commit#show') do = link_to namespace_project_commit_path(@project.namespace, @project, @commit.id) do Changes - %span.badge= @diffs.count + %span.badge= @diffs.size = nav_link(path: 'commit#builds') do = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id) do Builds diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml index a483927671e..5970b9abf2b 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.diff_lines.each do |line| + - diff_file.highlighted_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/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index ed2765356db..5b842dd9280 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,7 +1,6 @@ - if @merge_request_diff.collected? - - 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 + = 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/app/workers/irker_worker.rb b/app/workers/irker_worker.rb index 605ec4f04e5..a3c34e02baa 100644 --- a/app/workers/irker_worker.rb +++ b/app/workers/irker_worker.rb @@ -142,7 +142,7 @@ class IrkerWorker def files_count(commit) files = "#{commit.diffs.real_size} file" - files += 's' if commit.diffs.count > 1 + files += 's' if commit.diffs.size > 1 files end diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 77b3798d78f..e47df508ca2 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -63,7 +63,7 @@ module Gitlab diff_refs.try(:head_sha) end - attr_writer :diff_lines, :highlighted_diff_lines + attr_writer :highlighted_diff_lines # Array of Gitlab::Diff::Line objects def diff_lines diff --git a/lib/gitlab/diff/file_collection.rb b/lib/gitlab/diff/file_collection.rb new file mode 100644 index 00000000000..ce6717c7205 --- /dev/null +++ b/lib/gitlab/diff/file_collection.rb @@ -0,0 +1,9 @@ +module Gitlab + module Diff + module FileCollection + def self.default_options + ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false) + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb new file mode 100644 index 00000000000..20562773c14 --- /dev/null +++ b/lib/gitlab/diff/file_collection/base.rb @@ -0,0 +1,30 @@ +module Gitlab + module Diff + module FileCollection + + class Base + attr_reader :project, :diff_options, :diff_view, :diff_refs + + delegate :count, :size, :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 + @diffs.decorate! { |diff| decorate_diff!(diff) } + end + + private + + def decorate_diff!(diff) + return diff if diff.is_a?(Gitlab::Diff::File) + Gitlab::Diff::File.new(diff, diff_refs: @diff_refs, repository: @project.repository) + end + end + end + end +end diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb new file mode 100644 index 00000000000..2a46109ad99 --- /dev/null +++ b/lib/gitlab/diff/file_collection/commit.rb @@ -0,0 +1,14 @@ +module Gitlab + module Diff + module FileCollection + 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 + end +end diff --git a/lib/gitlab/diff/file_collection/compare.rb b/lib/gitlab/diff/file_collection/compare.rb new file mode 100644 index 00000000000..1bcda145f15 --- /dev/null +++ b/lib/gitlab/diff/file_collection/compare.rb @@ -0,0 +1,14 @@ +module Gitlab + module Diff + module FileCollection + 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 + end +end diff --git a/lib/gitlab/diff/file_collection/merge_request.rb b/lib/gitlab/diff/file_collection/merge_request.rb new file mode 100644 index 00000000000..7c40622d594 --- /dev/null +++ b/lib/gitlab/diff/file_collection/merge_request.rb @@ -0,0 +1,88 @@ +module Gitlab + module Diff + module FileCollection + 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 + + def diff_files + super.tap { |_| store_highlight_cache } + end + + private + + # Extracted method to highlight in the same iteration to the diff_collection. Iteration in the DiffCollections + # seems particularly slow on big diffs (event when already populated). + def decorate_diff!(diff) + highlight! super + end + + def highlight!(diff_file) + if cacheable? + cache_highlight!(diff_file) + else + highlight_diff_file!(diff_file) + end + end + + def highlight_diff_file!(diff_file) + diff_file.highlighted_diff_lines = Gitlab::Diff::Highlight.new(diff_file, repository: diff_file.repository).highlight + diff_file + end + + def highlight_diff_file_from_cache!(diff_file, cache_diff_lines) + diff_file.highlighted_diff_lines = cache_diff_lines.map do |line| + Gitlab::Diff::Line.init_from_hash(line) + end + end + + # + # 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 a Hash where the key correspond to the file_path and the values are Arrays of + # hashes that represent serialized diff lines. + # + def cache_highlight!(diff_file) + file_path = diff_file.file_path + + if highlight_cache[file_path] + highlight_diff_file_from_cache!(diff_file, highlight_cache[file_path]) + else + highlight_diff_file!(diff_file) + highlight_cache[file_path] = diff_file.highlighted_diff_lines.map(&:to_hash) + end + + diff_file + end + + def highlight_cache + return @highlight_cache if defined?(@highlight_cache) + + @highlight_cache = Rails.cache.read(cache_key) || {} + @highlight_cache_was_empty = highlight_cache.empty? + @highlight_cache + end + + def store_highlight_cache + Rails.cache.write(cache_key, highlight_cache) if @highlight_cache_was_empty + end + + def cacheable? + @merge_request.merge_request_diff.present? + end + + def cache_key + [@merge_request.merge_request_diff, 'highlighted-diff-files', diff_options] + end + end + end + end +end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 48946ba355b..71213813e17 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -40,16 +40,18 @@ module Gitlab def diffs return unless compare - - @diffs ||= SafeDiffs::Compare.new(compare, diff_options: { max_files: 30 }, project: project, diff_refs: diff_refs).diff_files + + @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }, diff_refs: diff_refs).diff_files end def diffs_count - diffs.count if diffs + diffs.size if diffs end def compare - @opts[:compare] + if @opts[:compare] + Compare.decorate(@opts[:compare], project) + end end def diff_refs diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 30121facd7d..940019b708b 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -267,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, safe_diffs| - expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) - meth.call(safe_diffs) + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(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 6272a5f111d..ed4cc36de58 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -32,10 +32,11 @@ describe Projects::CompareController do w: 1) expect(response).to be_success - expect(assigns(:diffs).diff_files.first).not_to be_nil + diff_file = assigns(:diffs).diff_files.first + expect(diff_file).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).diff_files.first.diff.diff.split("\n") + diff_splits = diff_file.diff.diff.split("\n") expect(diff_splits.length).to be <= 2 end @@ -87,9 +88,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, safe_diffs| - expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) - meth.call(safe_diffs) + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(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 9da43c9cee7..1f6bc84dfe8 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, safe_diffs| - expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) - meth.call(safe_diffs) + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(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, safe_diffs| - expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) - meth.call(safe_diffs) + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(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, safe_diffs| - expect(safe_diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) - meth.call(safe_diffs) + expect(controller).to receive(:render_diff_for_path).and_wrap_original do |meth, diffs| + expect(diffs.diff_files.map(&:new_path)).to contain_exactly(existing_path) + meth.call(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' }) diff --git a/spec/lib/gitlab/email/message/repository_push_spec.rb b/spec/lib/gitlab/email/message/repository_push_spec.rb index c19f33e2224..c1d07329983 100644 --- a/spec/lib/gitlab/email/message/repository_push_spec.rb +++ b/spec/lib/gitlab/email/message/repository_push_spec.rb @@ -62,12 +62,12 @@ describe Gitlab::Email::Message::RepositoryPush do describe '#diffs_count' do subject { message.diffs_count } - it { is_expected.to eq compare.diffs.count } + it { is_expected.to eq compare.diffs.size } end describe '#compare' do subject { message.compare } - it { is_expected.to be_an_instance_of Gitlab::Git::Compare } + it { is_expected.to be_an_instance_of Compare } end describe '#compare_timeout' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 21d22c776e9..fa1f7edae8e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -660,6 +660,12 @@ describe MergeRequest, models: true do subject.reload_diff end + it "executs diff cache service" do + expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject) + + subject.reload_diff + end + it "updates diff note positions" do old_diff_refs = subject.diff_refs diff --git a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb new file mode 100644 index 00000000000..c6cceed31ad --- /dev/null +++ b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe MergeRequests::MergeRequestDiffCacheService do + + let(:subject) { MergeRequests::MergeRequestDiffCacheService.new } + + describe '#execute' do + it 'retrieve the diff files to cache the highlighted result' do + merge_request = create(:merge_request) + cache_key = [merge_request.merge_request_diff, 'highlighted-diff-files', Gitlab::Diff::FileCollection.default_options] + + expect(Rails.cache).to receive(:read).with(cache_key).and_return({}) + expect(Rails.cache).to receive(:write).with(cache_key, anything) + + subject.execute(merge_request) + end + end +end