switch from diff_file_collection to diffs20034-safe-diffs

So we have raw_diffs too

15 files changed, 55 insertions, 61 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