Merge branch 'dm-diff-cleanup' into 'master'

Clean up diff rendering

See merge request !11390

7 files changed, 76 insertions, 41 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 01cc8e8e1ca..cf4affe1758 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -252,7 +252,9 @@ module API
 
     class RepoDiff < Grape::Entity
       expose :old_path, :new_path, :a_mode, :b_mode, :diff
-      expose :new_file, :renamed_file, :deleted_file
+      expose :new_file?, as: :new_file
+      expose :renamed_file?, as: :renamed_file
+      expose :deleted_file?, as: :deleted_file
     end
 
     class Milestone < ProjectEntity
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index c6bf25b5874..2aef7fdaa35 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -1,16 +1,17 @@
 module Gitlab
   module Diff
     class File
-      attr_reader :diff, :repository, :diff_refs
+      attr_reader :diff, :repository, :diff_refs, :fallback_diff_refs
 
-      delegate :new_file, :deleted_file, :renamed_file,
-        :old_path, :new_path, :a_mode, :b_mode,
+      delegate :new_file?, :deleted_file?, :renamed_file?,
+        :old_path, :new_path, :a_mode, :b_mode, :mode_changed?,
         :submodule?, :too_large?, :collapsed?, to: :diff, prefix: false
 
-      def initialize(diff, repository:, diff_refs: nil)
+      def initialize(diff, repository:, diff_refs: nil, fallback_diff_refs: nil)
         @diff = diff
         @repository = repository
         @diff_refs = diff_refs
+        @fallback_diff_refs = fallback_diff_refs
       end
 
       def position(line)
@@ -49,24 +50,60 @@ module Gitlab
         line_code(line) if line
       end
 
+      def old_sha
+        diff_refs&.base_sha
+      end
+
+      def new_sha
+        diff_refs&.head_sha
+      end
+
+      def content_sha
+        return old_content_sha if deleted_file?
+        return @content_sha if defined?(@content_sha)
+
+        refs = diff_refs || fallback_diff_refs
+        @content_sha = refs&.head_sha
+      end
+
       def content_commit
-        return unless diff_refs
+        return @content_commit if defined?(@content_commit)
+
+        sha = content_sha
+        @content_commit = repository.commit(sha) if sha
+      end
+
+      def old_content_sha
+        return if new_file?
+        return @old_content_sha if defined?(@old_content_sha)
 
-        repository.commit(deleted_file ? old_ref : new_ref)
+        refs = diff_refs || fallback_diff_refs
+        @old_content_sha = refs&.base_sha
       end
 
       def old_content_commit
-        return unless diff_refs
+        return @old_content_commit if defined?(@old_content_commit)
 
-        repository.commit(old_ref)
+        sha = old_content_sha
+        @old_content_commit = repository.commit(sha) if sha
       end
 
-      def old_ref
-        diff_refs.try(:base_sha)
+      def blob
+        return @blob if defined?(@blob)
+
+        sha = content_sha
+        return @blob = nil unless sha
+
+        repository.blob_at(sha, file_path)
       end
 
-      def new_ref
-        diff_refs.try(:head_sha)
+      def old_blob
+        return @old_blob if defined?(@old_blob)
+
+        sha = old_content_sha
+        return @old_blob = nil unless sha
+
+        @old_blob = repository.blob_at(sha, old_path)
       end
 
       attr_writer :highlighted_diff_lines
@@ -85,10 +122,6 @@ module Gitlab
         @parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize
       end
 
-      def mode_changed?
-        a_mode && b_mode && a_mode != b_mode
-      end
-
       def raw_diff
         diff.diff.to_s
       end
@@ -117,20 +150,8 @@ module Gitlab
         diff_lines.count(&:removed?)
       end
 
-      def old_blob(commit = old_content_commit)
-        return unless commit
-
-        repository.blob_at(commit.id, old_path)
-      end
-
-      def blob(commit = content_commit)
-        return unless commit
-
-        repository.blob_at(commit.id, file_path)
-      end
-
       def file_identifier
-        "#{file_path}-#{new_file}-#{deleted_file}-#{renamed_file}"
+        "#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
       end
     end
   end
diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb
index 7c32adc6ce7..79836a2fbab 100644
--- a/lib/gitlab/diff/file_collection/base.rb
+++ b/lib/gitlab/diff/file_collection/base.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Diff
     module FileCollection
       class Base
-        attr_reader :project, :diff_options, :diff_view, :diff_refs
+        attr_reader :project, :diff_options, :diff_refs, :fallback_diff_refs
 
         delegate :count, :size, :real_size, to: :diff_files
 
@@ -10,14 +10,15 @@ module Gitlab
           ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false)
         end
 
-        def initialize(diffable, project:, diff_options: nil, diff_refs: nil)
+        def initialize(diffable, project:, diff_options: nil, diff_refs: nil, fallback_diff_refs: nil)
           diff_options = self.class.default_options.merge(diff_options || {})
 
-          @diffable     = diffable
-          @diffs        = diffable.raw_diffs(diff_options)
-          @project      = project
+          @diffable = diffable
+          @diffs = diffable.raw_diffs(diff_options)
+          @project = project
           @diff_options = diff_options
-          @diff_refs    = diff_refs
+          @diff_refs = diff_refs
+          @fallback_diff_refs = fallback_diff_refs
         end
 
         def diff_files
@@ -35,7 +36,7 @@ module Gitlab
         private
 
         def decorate_diff!(diff)
-          Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs)
+          Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs, fallback_diff_refs: fallback_diff_refs)
         end
       end
     end
diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb
index 0bd226ef050..9a58b500a2c 100644
--- a/lib/gitlab/diff/file_collection/merge_request_diff.rb
+++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb
@@ -8,7 +8,8 @@ module Gitlab
           super(merge_request_diff,
             project: merge_request_diff.project,
             diff_options: diff_options,
-            diff_refs: merge_request_diff.diff_refs)
+            diff_refs: merge_request_diff.diff_refs,
+            fallback_diff_refs: merge_request_diff.fallback_diff_refs)
         end
 
         def diff_files
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 7db896522a9..ed2f541977a 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -3,7 +3,7 @@ module Gitlab
     class Highlight
       attr_reader :diff_file, :diff_lines, :raw_lines, :repository
 
-      delegate :old_path, :new_path, :old_ref, :new_ref, to: :diff_file, prefix: :diff
+      delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
 
       def initialize(diff_lines, repository: nil)
         @repository = repository
@@ -61,12 +61,12 @@ module Gitlab
 
       def old_lines
         return unless diff_file
-        @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_ref, diff_old_path)
+        @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_sha, diff_old_path)
       end
 
       def new_lines
         return unless diff_file
-        @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_ref, diff_new_path)
+        @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_sha, diff_new_path)
       end
     end
   end
diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index 31d1b66b4f7..deade337354 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -11,6 +11,10 @@ module Gitlab
       # Stats properties
       attr_accessor :new_file, :renamed_file, :deleted_file
 
+      alias_method :new_file?, :new_file
+      alias_method :deleted_file?, :deleted_file
+      alias_method :renamed_file?, :renamed_file
+
       attr_accessor :too_large
 
       # The maximum size of a diff to display.
@@ -208,6 +212,10 @@ module Gitlab
         hash
       end
 
+      def mode_changed?
+        a_mode && b_mode && a_mode != b_mode
+      end
+
       def submodule?
         a_mode == '160000' || b_mode == '160000'
       end
diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb
index bcbad8ec829..bce3aef8bd3 100644
--- a/lib/gitlab/git/diff_collection.rb
+++ b/lib/gitlab/git/diff_collection.rb
@@ -64,6 +64,8 @@ module Gitlab
         collection
       end
 
+      alias_method :to_ary, :to_a
+
       private
 
       def populate!