summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaco Guzman <pacoguzmanp@gmail.com>2016-07-27 19:00:34 +0200
committerPaco Guzman <pacoguzmanp@gmail.com>2016-08-03 07:00:20 +0200
commitc86c1905b5574cac234315598d8d715fcaee3ea7 (patch)
tree31ba7ab51c04b07ea70d15db88f2370f9ca6359e
parent1d0c7b74920a94e488e6a2c090abb3e525438053 (diff)
downloadgitlab-ce-c86c1905b5574cac234315598d8d715fcaee3ea7.tar.gz
switch from diff_file_collection to diffs20034-safe-diffs
So we have raw_diffs too
-rw-r--r--app/controllers/projects/commit_controller.rb4
-rw-r--r--app/controllers/projects/compare_controller.rb4
-rw-r--r--app/controllers/projects/merge_requests_controller.rb8
-rw-r--r--app/helpers/diff_helper.rb5
-rw-r--r--app/models/commit.rb10
-rw-r--r--app/models/compare.rb36
-rw-r--r--app/models/legacy_diff_note.rb4
-rw-r--r--app/models/merge_request.rb8
-rw-r--r--app/models/repository.rb2
-rw-r--r--app/services/compare_service.rb1
-rw-r--r--app/services/merge_requests/merge_request_diff_cache_service.rb2
-rw-r--r--app/views/projects/diffs/_diffs.html.haml14
-rw-r--r--app/views/projects/merge_requests/show/_diffs.html.haml3
-rw-r--r--app/workers/emails_on_push_worker.rb9
-rw-r--r--app/workers/irker_worker.rb6
-rw-r--r--lib/api/commits.rb4
-rw-r--r--lib/api/entities.rb2
-rw-r--r--lib/gitlab/diff/file_collection.rb9
-rw-r--r--lib/gitlab/diff/file_collection/base.rb17
-rw-r--r--lib/gitlab/diff/file_collection/commit.rb5
-rw-r--r--lib/gitlab/diff/file_collection/compare.rb5
-rw-r--r--lib/gitlab/diff/file_collection/merge_request.rb23
-rw-r--r--lib/gitlab/email/message/repository_push.rb3
-rw-r--r--spec/helpers/diff_helper_spec.rb24
-rw-r--r--spec/lib/gitlab/diff/file_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/highlight_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/line_mapper_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/parallel_diff_spec.rb2
-rw-r--r--spec/lib/gitlab/diff/parser_spec.rb2
-rw-r--r--spec/models/compare_spec.rb77
-rw-r--r--spec/models/legacy_diff_note_spec.rb2
-rw-r--r--spec/models/merge_request_spec.rb29
-rw-r--r--spec/models/repository_spec.rb4
-rw-r--r--spec/requests/api/commits_spec.rb4
-rw-r--r--spec/services/merge_requests/merge_request_diff_cache_service_spec.rb4
35 files changed, 208 insertions, 130 deletions
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 771a86530cd..fdfe7c65b7b 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(@commit.diff_file_collection(diff_options))
+ render_diff_for_path(@commit.diffs(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.diff_file_collection(opts)
+ @diffs = commit.diffs(opts)
@notes_count = commit.notes.count
end
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index 7fca5e77f32..4a42a7d091b 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(@compare.diff_file_collection(diff_options: diff_options))
+ render_diff_for_path(@compare.diffs(diff_options: diff_options))
end
def create
@@ -45,7 +45,7 @@ class Projects::CompareController < Projects::ApplicationController
@commit = @compare.commit
@base_commit = @compare.base_commit
- @diffs = @compare.diff_file_collection(diff_options: diff_options)
+ @diffs = @compare.diffs(diff_options: diff_options)
@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 20afc6afcb2..2cf6a2dd1b3 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -86,7 +86,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
respond_to do |format|
format.html { define_discussion_vars }
format.json do
- @diffs = @merge_request.diff_file_collection(diff_options)
+ @diffs = @merge_request.diffs(diff_options)
render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") }
end
@@ -108,7 +108,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
define_commit_vars
- render_diff_for_path(@merge_request.diff_file_collection(diff_options))
+ render_diff_for_path(@merge_request.diffs(diff_options))
end
def commits
@@ -156,9 +156,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@commits = @merge_request.compare_commits.reverse
@commit = @merge_request.diff_head_commit
@base_commit = @merge_request.diff_base_commit
- if @merge_request.compare
- @diffs = @merge_request.diff_file_collection(diff_options)
- end
+ @diffs = @merge_request.diffs(diff_options) if @merge_request.compare
@diff_notes_disabled = true
@pipeline = @merge_request.pipeline
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 2abe24b78bf..cc7121b1163 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -23,10 +23,7 @@ module DiffHelper
end
def diff_options
- options = Gitlab::Diff::FileCollection.default_options.merge(
- ignore_whitespace_change: hide_whitespace?,
- no_collapse: expand_all_diffs?
- )
+ options = { ignore_whitespace_change: hide_whitespace?, no_collapse: expand_all_diffs? }
if action_name == 'diff_for_path'
options[:no_collapse] = true
diff --git a/app/models/commit.rb b/app/models/commit.rb
index a339d47f5f3..d58c2fb8106 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -104,7 +104,7 @@ class Commit
end
def diff_line_count
- @diff_line_count ||= Commit::diff_line_count(self.diffs)
+ @diff_line_count ||= Commit::diff_line_count(raw_diffs)
@diff_line_count
end
@@ -317,7 +317,11 @@ class Commit
nil
end
- def diff_file_collection(diff_options = nil)
+ def raw_diffs(*args)
+ raw.diffs(*args)
+ end
+
+ def diffs(diff_options = nil)
Gitlab::Diff::FileCollection::Commit.new(self, diff_options: diff_options)
end
@@ -330,7 +334,7 @@ class Commit
def repo_changes
changes = { added: [], modified: [], removed: [] }
- diffs.each do |diff|
+ raw_diffs.each do |diff|
if diff.deleted_file
changes[:removed] << diff.old_path
elsif diff.renamed_file || diff.new_file
diff --git a/app/models/compare.rb b/app/models/compare.rb
index 05c8fbbcc36..98c042f3809 100644
--- a/app/models/compare.rb
+++ b/app/models/compare.rb
@@ -1,6 +1,8 @@
class Compare
delegate :same, :head, :base, to: :@compare
+ attr_reader :project
+
def self.decorate(compare, project)
if compare.is_a?(Compare)
compare
@@ -15,49 +17,47 @@ class Compare
end
def commits
- @commits ||= Commit.decorate(@compare.commits, @project)
+ @commits ||= Commit.decorate(@compare.commits, project)
end
def start_commit
return @start_commit if defined?(@start_commit)
commit = @compare.base
- @start_commit = commit ? ::Commit.new(commit, @project) : nil
+ @start_commit = commit ? ::Commit.new(commit, project) : nil
end
- def commit
- return @commit if defined?(@commit)
+ def head_commit
+ return @head_commit if defined?(@head_commit)
commit = @compare.head
- @commit = commit ? ::Commit.new(commit, @project) : nil
- end
- alias_method :head_commit, :commit
-
- # Used only on emails_on_push_worker.rb
- def base_commit=(commit)
- @base_commit = commit
+ @head_commit = commit ? ::Commit.new(commit, project) : nil
end
+ alias_method :commit, :head_commit
def base_commit
return @base_commit if defined?(@base_commit)
- @base_commit = if start_commit && commit
- @project.merge_base_commit(start_commit.id, commit.id)
+ @base_commit = if start_commit && head_commit
+ project.merge_base_commit(start_commit.id, head_commit.id)
else
nil
end
end
- # keyword args until we get ride of diff_refs as argument
- def diff_file_collection(diff_options:, diff_refs: self.diff_refs)
- Gitlab::Diff::FileCollection::Compare.new(@compare,
- project: @project,
+ def raw_diffs(*args)
+ @compare.diffs(*args)
+ end
+
+ def diffs(diff_options:)
+ Gitlab::Diff::FileCollection::Compare.new(self,
+ project: project,
diff_options: diff_options,
diff_refs: diff_refs)
end
def diff_refs
- @diff_refs ||= Gitlab::Diff::DiffRefs.new(
+ Gitlab::Diff::DiffRefs.new(
base_sha: base_commit.try(:sha),
start_sha: start_commit.try(:sha),
head_sha: commit.try(:sha)
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 865712268a0..6ed66001513 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -85,7 +85,7 @@ class LegacyDiffNote < Note
return nil unless noteable
return @diff if defined?(@diff)
- @diff = noteable.diffs(Commit.max_diff_options).find do |d|
+ @diff = noteable.raw_diffs(Commit.max_diff_options).find do |d|
d.new_path && Digest::SHA1.hexdigest(d.new_path) == diff_file_hash
end
end
@@ -116,7 +116,7 @@ class LegacyDiffNote < Note
# Find the diff on noteable that matches our own
def find_noteable_diff
- diffs = noteable.diffs(Commit.max_diff_options)
+ diffs = noteable.raw_diffs(Commit.max_diff_options)
diffs.find { |d| d.new_path == self.diff.new_path }
end
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 62e5573dfdc..009262d6b48 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -164,13 +164,13 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff ? merge_request_diff.first_commit : compare_commits.first
end
- def diffs(*args)
- merge_request_diff ? merge_request_diff.diffs(*args) : compare.diffs(*args)
+ def raw_diffs(*args)
+ merge_request_diff ? merge_request_diff.diffs(*args) : compare.raw_diffs(*args)
end
- def diff_file_collection(diff_options = nil)
+ def diffs(diff_options = nil)
if self.compare
- self.compare.diff_file_collection(diff_options: diff_options, diff_refs: diff_refs)
+ self.compare.diffs(diff_options: diff_options)
else
Gitlab::Diff::FileCollection::MergeRequest.new(self, diff_options: diff_options)
end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index bac37483c47..3d95344a68f 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -372,7 +372,7 @@ class Repository
# We don't want to flush the cache if the commit didn't actually make any
# changes to any of the possible avatar files.
if revision && commit = self.commit(revision)
- return unless commit.diffs.
+ return unless commit.raw_diffs.
any? { |diff| AVATAR_FILES.include?(diff.new_path) }
end
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index bb3aff72b47..6d6075628af 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -26,7 +26,6 @@ class CompareService
source_sha
)
- # REVIEW be sure if it's target_project or source_project
Compare.new(raw_compare, target_project)
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 8151c24d1b0..2945a7fd4e4 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
- merge_request.diff_file_collection.diff_files.to_a
+ merge_request.diffs.diff_files.to_a
end
end
end
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index 35fdf7d5278..20dc280c3b2 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -9,11 +9,11 @@
= link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: nil)), class: 'btn btn-default'
- if show_whitespace_toggle
- if current_controller?(:commit)
- = commit_diff_whitespace_link(@project, @commit, class: 'hidden-xs')
+ = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs')
- elsif current_controller?(:merge_requests)
- = diff_merge_request_whitespace_link(@project, @merge_request, class: 'hidden-xs')
+ = diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'hidden-xs')
- elsif current_controller?(:compare)
- = diff_compare_whitespace_link(@project, params[:from], params[:to], class: 'hidden-xs')
+ = diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'hidden-xs')
.btn-group
= inline_diff_btn
= parallel_diff_btn
@@ -22,12 +22,12 @@
- if diff_files.overflow?
= render 'projects/diffs/warning', diff_files: diff_files
-.files{data: {can_create_note: (!@diff_notes_disabled && can?(current_user, :create_note, @project))}}
+.files{data: {can_create_note: (!@diff_notes_disabled && can?(current_user, :create_note, diffs.project))}}
- diff_files.each_with_index do |diff_file, index|
- diff_commit = commit_for_diff(diff_file)
- blob = diff_file.blob(diff_commit)
- next unless blob
- - blob.load_all_data!(@project.repository) unless blob.only_display_raw?
+ - blob.load_all_data!(diffs.project.repository) unless blob.only_display_raw?
- = render 'projects/diffs/file', i: index, project: @project,
- diff_file: diff_file, diff_commit: diff_commit, blob: blob, diff_refs: diffs.diff_refs
+ = render 'projects/diffs/file', i: index, project: diffs.project,
+ diff_file: diff_file, diff_commit: diff_commit, blob: blob
diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml
index c6d2567af35..013b05628fa 100644
--- a/app/views/projects/merge_requests/show/_diffs.html.haml
+++ b/app/views/projects/merge_requests/show/_diffs.html.haml
@@ -1,6 +1,5 @@
- if @merge_request_diff.collected?
- = render "projects/diffs/diffs", diffs: @diffs,
- diff_refs: @diffs.diff_refs, project: @diffs.project
+ = render "projects/diffs/diffs", diffs: @diffs
- 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/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index 0b63913cfd1..c6a5af2809a 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -33,18 +33,13 @@ class EmailsOnPushWorker
reverse_compare = false
if action == :push
- base_commit = project.merge_base_commit(before_sha, after_sha)
- compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha)
- compare = Compare.decorate(compare, project)
- compare.base_commit = base_commit
+ compare = CompareService.new.execute(project, before_sha, project, after_sha)
diff_refs = compare.diff_refs
return false if compare.same
if compare.commits.empty?
- compare = Gitlab::Git::Compare.new(project.repository.raw_repository, after_sha, before_sha)
- compare = Compare.decorate(compare, project)
- compare.base_commit = base_commit
+ compare = CompareService.new.execute(project, after_sha, project, before_sha)
diff_refs = compare.diff_refs
reverse_compare = true
diff --git a/app/workers/irker_worker.rb b/app/workers/irker_worker.rb
index a3c34e02baa..07cc7c1cbd7 100644
--- a/app/workers/irker_worker.rb
+++ b/app/workers/irker_worker.rb
@@ -141,8 +141,10 @@ class IrkerWorker
end
def files_count(commit)
- files = "#{commit.diffs.real_size} file"
- files += 's' if commit.diffs.size > 1
+ diffs = commit.raw_diffs
+
+ files = "#{diffs.real_size} file"
+ files += 's' if diffs.size > 1
files
end
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index 4a11c8e3620..b4eaf1813d4 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -54,7 +54,7 @@ module API
sha = params[:sha]
commit = user_project.commit(sha)
not_found! "Commit" unless commit
- commit.diffs.to_a
+ commit.raw_diffs.to_a
end
# Get a commit's comments
@@ -96,7 +96,7 @@ module API
}
if params[:path] && params[:line] && params[:line_type]
- commit.diffs(all_diffs: true).each do |diff|
+ commit.raw_diffs(all_diffs: true).each do |diff|
next unless diff.new_path == params[:path]
lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3e21b7a0b8a..e5b00dc45a5 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -224,7 +224,7 @@ module API
class MergeRequestChanges < MergeRequest
expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _|
- compare.diffs(all_diffs: true).to_a
+ compare.raw_diffs(all_diffs: true).to_a
end
end
diff --git a/lib/gitlab/diff/file_collection.rb b/lib/gitlab/diff/file_collection.rb
deleted file mode 100644
index ce6717c7205..00000000000
--- a/lib/gitlab/diff/file_collection.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-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
index a0c88265c45..2b9fc65b985 100644
--- a/lib/gitlab/diff/file_collection/base.rb
+++ b/lib/gitlab/diff/file_collection/base.rb
@@ -1,28 +1,33 @@
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: nil, diff_refs: nil)
- @diffs = diffs
+ def self.default_options
+ ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false)
+ end
+
+ def initialize(diffable, project:, diff_options: nil, diff_refs: nil)
+ diff_options = self.class.default_options.merge(diff_options || {})
+
+ @diffable = diffable
+ @diffs = diffable.raw_diffs(diff_options)
@project = project
@diff_options = diff_options
@diff_refs = diff_refs
end
def diff_files
- @diffs.decorate! { |diff| decorate_diff!(diff) }
+ @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)
+ Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs)
end
end
end
diff --git a/lib/gitlab/diff/file_collection/commit.rb b/lib/gitlab/diff/file_collection/commit.rb
index 19def300b74..4dc297ec036 100644
--- a/lib/gitlab/diff/file_collection/commit.rb
+++ b/lib/gitlab/diff/file_collection/commit.rb
@@ -3,10 +3,7 @@ module Gitlab
module FileCollection
class Commit < Base
def initialize(commit, diff_options:)
- # Not merge just set defaults
- diff_options = diff_options || Gitlab::Diff::FileCollection.default_options
-
- super(commit.diffs(diff_options),
+ super(commit,
project: commit.project,
diff_options: diff_options,
diff_refs: commit.diff_refs)
diff --git a/lib/gitlab/diff/file_collection/compare.rb b/lib/gitlab/diff/file_collection/compare.rb
index aba5a28b51f..20d8f891cc3 100644
--- a/lib/gitlab/diff/file_collection/compare.rb
+++ b/lib/gitlab/diff/file_collection/compare.rb
@@ -3,10 +3,7 @@ module Gitlab
module FileCollection
class Compare < Base
def initialize(compare, project:, diff_options:, diff_refs: nil)
- # Not merge just set defaults
- diff_options = diff_options || Gitlab::Diff::FileCollection.default_options
-
- super(compare.diffs(diff_options),
+ super(compare,
project: project,
diff_options: diff_options,
diff_refs: diff_refs)
diff --git a/lib/gitlab/diff/file_collection/merge_request.rb b/lib/gitlab/diff/file_collection/merge_request.rb
index 9fde0bba183..4f946908e2f 100644
--- a/lib/gitlab/diff/file_collection/merge_request.rb
+++ b/lib/gitlab/diff/file_collection/merge_request.rb
@@ -4,10 +4,8 @@ module Gitlab
class MergeRequest < Base
def initialize(merge_request, diff_options:)
@merge_request = merge_request
- # Not merge just set defaults
- diff_options = diff_options || Gitlab::Diff::FileCollection.default_options
- super(merge_request.diffs(diff_options),
+ super(merge_request,
project: merge_request.project,
diff_options: diff_options,
diff_refs: merge_request.diff_refs)
@@ -19,18 +17,11 @@ module Gitlab
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).
+ # Extracted method to highlight in the same iteration to the diff_collection.
def decorate_diff!(diff)
- highlight! super
- end
-
- def highlight!(diff_file)
- if cacheable?
- cache_highlight!(diff_file)
- else
- diff_file # Don't need to eager load highlighted diff lines
- end
+ diff_file = super
+ cache_highlight!(diff_file) if cacheable?
+ diff_file
end
def highlight_diff_file_from_cache!(diff_file, cache_diff_lines)
@@ -55,15 +46,13 @@ module Gitlab
else
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_was_empty = @highlight_cache.empty?
@highlight_cache
end
diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb
index 16491ede71b..62d29387d60 100644
--- a/lib/gitlab/email/message/repository_push.rb
+++ b/lib/gitlab/email/message/repository_push.rb
@@ -41,7 +41,8 @@ module Gitlab
def diffs
return unless compare
- @diffs ||= compare.diff_file_collection(diff_options: { max_files: 30 }).diff_files
+ # This diff is more moderated in number of files and lines
+ @diffs ||= compare.diffs(diff_options: { max_files: 30, max_lines: 5000, no_collapse: true }).diff_files
end
def diffs_count
diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb
index c2fd2c8a533..4949280d641 100644
--- a/spec/helpers/diff_helper_spec.rb
+++ b/spec/helpers/diff_helper_spec.rb
@@ -6,7 +6,7 @@ describe DiffHelper do
let(:project) { create(:project) }
let(:repository) { project.repository }
let(:commit) { project.commit(sample_commit.id) }
- let(:diffs) { commit.diffs }
+ let(:diffs) { commit.raw_diffs }
let(:diff) { diffs.first }
let(:diff_refs) { [commit.parent, commit] }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
@@ -32,16 +32,6 @@ describe DiffHelper do
end
describe 'diff_options' do
- it 'should return hard limit for a diff if force diff is true' do
- allow(controller).to receive(:params) { { force_show_diff: true } }
- expect(diff_options).to include(Commit.max_diff_options)
- end
-
- it 'should return hard limit for a diff if expand_all_diffs is true' do
- allow(controller).to receive(:params) { { expand_all_diffs: true } }
- expect(diff_options).to include(Commit.max_diff_options)
- end
-
it 'should return no collapse false' do
expect(diff_options).to include(no_collapse: false)
end
@@ -55,6 +45,18 @@ describe DiffHelper do
allow(controller).to receive(:action_name) { 'diff_for_path' }
expect(diff_options).to include(no_collapse: true)
end
+
+ it 'should return paths if action name diff_for_path and param old path' do
+ allow(controller).to receive(:params) { { old_path: 'lib/wadus.rb' } }
+ allow(controller).to receive(:action_name) { 'diff_for_path' }
+ expect(diff_options[:paths]).to include('lib/wadus.rb')
+ end
+
+ it 'should return paths if action name diff_for_path and param new path' do
+ allow(controller).to receive(:params) { { new_path: 'lib/wadus.rb' } }
+ allow(controller).to receive(:action_name) { 'diff_for_path' }
+ expect(diff_options[:paths]).to include('lib/wadus.rb')
+ end
end
describe 'unfold_bottom_class' do
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index e883a6eb9c2..0650cb291e5 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Diff::File, lib: true do
let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) }
- let(:diff) { commit.diffs.first }
+ let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
describe '#diff_lines' do
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index 88e4115c453..1c2ddeed692 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Diff::Highlight, lib: true do
let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) }
- let(:diff) { commit.diffs.first }
+ let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
describe '#highlight' do
diff --git a/spec/lib/gitlab/diff/line_mapper_spec.rb b/spec/lib/gitlab/diff/line_mapper_spec.rb
index 4e50e03bb7e..4b943fa382d 100644
--- a/spec/lib/gitlab/diff/line_mapper_spec.rb
+++ b/spec/lib/gitlab/diff/line_mapper_spec.rb
@@ -6,7 +6,7 @@ describe Gitlab::Diff::LineMapper, lib: true do
let(:project) { create(:project) }
let(:repository) { project.repository }
let(:commit) { project.commit(sample_commit.id) }
- let(:diffs) { commit.diffs }
+ let(:diffs) { commit.raw_diffs }
let(:diff) { diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
subject { described_class.new(diff_file) }
diff --git a/spec/lib/gitlab/diff/parallel_diff_spec.rb b/spec/lib/gitlab/diff/parallel_diff_spec.rb
index 2aa5ae44f54..af18d3c25a6 100644
--- a/spec/lib/gitlab/diff/parallel_diff_spec.rb
+++ b/spec/lib/gitlab/diff/parallel_diff_spec.rb
@@ -6,7 +6,7 @@ describe Gitlab::Diff::ParallelDiff, lib: true do
let(:project) { create(:project) }
let(:repository) { project.repository }
let(:commit) { project.commit(sample_commit.id) }
- let(:diffs) { commit.diffs }
+ let(:diffs) { commit.raw_diffs }
let(:diff) { diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
subject { described_class.new(diff_file) }
diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb
index c3359627652..b983d73f8be 100644
--- a/spec/lib/gitlab/diff/parser_spec.rb
+++ b/spec/lib/gitlab/diff/parser_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Diff::Parser, lib: true do
let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) }
- let(:diff) { commit.diffs.first }
+ let(:diff) { commit.raw_diffs.first }
let(:parser) { Gitlab::Diff::Parser.new }
describe '#parse' do
diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb
new file mode 100644
index 00000000000..49ab3c4b6e9
--- /dev/null
+++ b/spec/models/compare_spec.rb
@@ -0,0 +1,77 @@
+require 'spec_helper'
+
+describe Compare, models: true do
+ include RepoHelpers
+
+ let(:project) { create(:project, :public) }
+ let(:commit) { project.commit }
+
+ let(:start_commit) { sample_image_commit }
+ let(:head_commit) { sample_commit }
+
+ let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, start_commit.id, head_commit.id) }
+
+ subject { described_class.new(raw_compare, project) }
+
+ describe '#start_commit' do
+ it 'returns raw compare base commit' do
+ expect(subject.start_commit.id).to eq(start_commit.id)
+ end
+
+ it 'returns nil if compare base commit is nil' do
+ expect(raw_compare).to receive(:base).and_return(nil)
+
+ expect(subject.start_commit).to eq(nil)
+ end
+ end
+
+ describe '#commit' do
+ it 'returns raw compare head commit' do
+ expect(subject.commit.id).to eq(head_commit.id)
+ end
+
+ it 'returns nil if compare head commit is nil' do
+ expect(raw_compare).to receive(:head).and_return(nil)
+
+ expect(subject.commit).to eq(nil)
+ end
+ end
+
+ describe '#base_commit' do
+ let(:base_commit) { Commit.new(another_sample_commit, project) }
+
+ it 'returns project merge base commit' do
+ expect(project).to receive(:merge_base_commit).with(start_commit.id, head_commit.id).and_return(base_commit)
+
+ expect(subject.base_commit).to eq(base_commit)
+ end
+
+ it 'returns nil if there is no start_commit' do
+ expect(subject).to receive(:start_commit).and_return(nil)
+
+ expect(subject.base_commit).to eq(nil)
+ end
+
+ it 'returns nil if there is no head commit' do
+ expect(subject).to receive(:head_commit).and_return(nil)
+
+ expect(subject.base_commit).to eq(nil)
+ end
+ end
+
+ describe '#diff_refs' do
+ it 'uses base_commit sha as base_sha' do
+ expect(subject).to receive(:base_commit).at_least(:once).and_call_original
+
+ expect(subject.diff_refs.base_sha).to eq(subject.base_commit.id)
+ end
+
+ it 'uses start_commit sha as start_sha' do
+ expect(subject.diff_refs.start_sha).to eq(start_commit.id)
+ end
+
+ it 'uses commit sha as head sha' do
+ expect(subject.diff_refs.head_sha).to eq(head_commit.id)
+ end
+ end
+end
diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb
index d23fc06c3ad..c8ee656fe3b 100644
--- a/spec/models/legacy_diff_note_spec.rb
+++ b/spec/models/legacy_diff_note_spec.rb
@@ -58,7 +58,7 @@ describe LegacyDiffNote, models: true do
# Generate a real line_code value so we know it will match. We use a
# random line from a random diff just for funsies.
- diff = merge.diffs.to_a.sample
+ diff = merge.raw_diffs.to_a.sample
line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index fa1f7edae8e..152e0cce5ad 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -128,7 +128,7 @@ describe MergeRequest, models: true do
end
end
- describe '#diffs' do
+ describe '#raw_diffs' do
let(:merge_request) { build(:merge_request) }
let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } }
@@ -138,6 +138,31 @@ describe MergeRequest, models: true do
expect(merge_request.merge_request_diff).to receive(:diffs).with(options)
+ merge_request.raw_diffs(options)
+ end
+ end
+
+ context 'when there are no MR diffs' do
+ it 'delegates to the compare object' do
+ merge_request.compare = double(:compare)
+
+ expect(merge_request.compare).to receive(:raw_diffs).with(options)
+
+ merge_request.raw_diffs(options)
+ end
+ end
+ end
+
+ describe '#diffs' do
+ let(:merge_request) { build(:merge_request) }
+ let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } }
+
+ context 'when there are MR diffs' do
+ it 'delegates to the MR diffs' do
+ merge_request.merge_request_diff = MergeRequestDiff.new
+
+ expect(merge_request.merge_request_diff).to receive(:diffs).with(hash_including(options))
+
merge_request.diffs(options)
end
end
@@ -146,7 +171,7 @@ describe MergeRequest, models: true do
it 'delegates to the compare object' do
merge_request.compare = double(:compare)
- expect(merge_request.compare).to receive(:diffs).with(options)
+ expect(merge_request.compare).to receive(:diffs).with(diff_options: options)
merge_request.diffs(options)
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index cce15538b93..2a053b1804f 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1154,7 +1154,7 @@ describe Repository, models: true do
it 'does not flush the cache if the commit does not change any logos' do
diff = double(:diff, new_path: 'test.txt')
- expect(commit).to receive(:diffs).and_return([diff])
+ expect(commit).to receive(:raw_diffs).and_return([diff])
expect(cache).not_to receive(:expire)
repository.expire_avatar_cache(repository.root_ref, '123')
@@ -1163,7 +1163,7 @@ describe Repository, models: true do
it 'flushes the cache if the commit changes any of the logos' do
diff = double(:diff, new_path: Repository::AVATAR_FILES[0])
- expect(commit).to receive(:diffs).and_return([diff])
+ expect(commit).to receive(:raw_diffs).and_return([diff])
expect(cache).to receive(:expire).with(:avatar)
repository.expire_avatar_cache(repository.root_ref, '123')
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index e4ea8506598..51ee2167d47 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -173,10 +173,10 @@ describe API::API, api: true do
end
it 'should return the inline comment' do
- post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.diffs.first.new_path, line: 7, line_type: 'new'
+ post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 7, line_type: 'new'
expect(response).to have_http_status(201)
expect(json_response['note']).to eq('My comment')
- expect(json_response['path']).to eq(project.repository.commit.diffs.first.new_path)
+ expect(json_response['path']).to eq(project.repository.commit.raw_diffs.first.new_path)
expect(json_response['line']).to eq(7)
expect(json_response['line_type']).to eq('new')
end
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
index c6cceed31ad..8f71d71b0f0 100644
--- a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb
+++ b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb
@@ -5,9 +5,9 @@ describe MergeRequests::MergeRequestDiffCacheService do
let(:subject) { MergeRequests::MergeRequestDiffCacheService.new }
describe '#execute' do
- it 'retrieve the diff files to cache the highlighted result' do
+ it 'retrieves 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]
+ cache_key = [merge_request.merge_request_diff, 'highlighted-diff-files', Gitlab::Diff::FileCollection::MergeRequest.default_options]
expect(Rails.cache).to receive(:read).with(cache_key).and_return({})
expect(Rails.cache).to receive(:write).with(cache_key, anything)