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

Implement diff viewers

Closes #30501 and #19931

See merge request !11777

28 files changed, 305 insertions, 35 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 2ae3a616933..06822747d11 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -124,6 +124,30 @@ module DiffHelper
     !diff_file.deleted_file? && @merge_request && @merge_request.source_project
   end
 
+  def diff_render_error_reason(viewer)
+    case viewer.render_error
+    when :too_large
+      "it is too large"
+    when :server_side_but_stored_externally
+      case viewer.diff_file.external_storage
+      when :lfs
+        'it is stored in LFS'
+      else
+        'it is stored externally'
+      end
+    end
+  end
+
+  def diff_render_error_options(viewer)
+    diff_file = viewer.diff_file
+    options = []
+
+    blob_url = namespace_project_blob_path(@project.namespace, @project, tree_join(diff_file.content_sha, diff_file.file_path))
+    options << link_to('view the blob', blob_url)
+
+    options
+  end
+
   private
 
   def diff_btn(title, name, selected)
diff --git a/app/models/blob_viewer/server_side.rb b/app/models/blob_viewer/server_side.rb
index e6bcacf7f70..fbc1b520c01 100644
--- a/app/models/blob_viewer/server_side.rb
+++ b/app/models/blob_viewer/server_side.rb
@@ -13,14 +13,12 @@ module BlobViewer
     end
 
     def render_error
-      if blob.stored_externally?
-        # Files that are not stored in the repository, like LFS files and
-        # build artifacts, can only be rendered using a client-side viewer,
-        # since we do not want to read large amounts of data into memory on the
-        # server side. Client-side viewers use JS and can fetch the file from
-        # `blob_raw_url` using AJAX.
-        return :server_side_but_stored_externally
-      end
+      # Files that are not stored in the repository, like LFS files and
+      # build artifacts, can only be rendered using a client-side viewer,
+      # since we do not want to read large amounts of data into memory on the
+      # server side. Client-side viewers use JS and can fetch the file from
+      # `blob_raw_url` using AJAX.
+      return :server_side_but_stored_externally if blob.stored_externally?
 
       super
     end
diff --git a/app/models/diff_viewer/added.rb b/app/models/diff_viewer/added.rb
new file mode 100644
index 00000000000..1909e6ef9d8
--- /dev/null
+++ b/app/models/diff_viewer/added.rb
@@ -0,0 +1,8 @@
+module DiffViewer
+  class Added < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'added'
+  end
+end
diff --git a/app/models/diff_viewer/base.rb b/app/models/diff_viewer/base.rb
new file mode 100644
index 00000000000..0cbe714288d
--- /dev/null
+++ b/app/models/diff_viewer/base.rb
@@ -0,0 +1,87 @@
+module DiffViewer
+  class Base
+    PARTIAL_PATH_PREFIX = 'projects/diffs/viewers'.freeze
+
+    class_attribute :partial_name, :type, :extensions, :file_types, :binary, :switcher_icon, :switcher_title
+
+    # These limits relate to the sum of the old and new blob sizes.
+    # Limits related to the actual size of the diff are enforced in Gitlab::Diff::File.
+    class_attribute :collapse_limit, :size_limit
+
+    delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class
+
+    attr_reader :diff_file
+
+    delegate :project, to: :diff_file
+
+    def initialize(diff_file)
+      @diff_file = diff_file
+      @initially_binary = diff_file.binary?
+    end
+
+    def self.partial_path
+      File.join(PARTIAL_PATH_PREFIX, partial_name)
+    end
+
+    def self.rich?
+      type == :rich
+    end
+
+    def self.simple?
+      type == :simple
+    end
+
+    def self.binary?
+      binary
+    end
+
+    def self.text?
+      !binary?
+    end
+
+    def self.can_render?(diff_file, verify_binary: true)
+      can_render_blob?(diff_file.old_blob, verify_binary: verify_binary) &&
+        can_render_blob?(diff_file.new_blob, verify_binary: verify_binary)
+    end
+
+    def self.can_render_blob?(blob, verify_binary: true)
+      return true if blob.nil?
+      return false if verify_binary && binary? != blob.binary?
+      return true if extensions&.include?(blob.extension)
+      return true if file_types&.include?(blob.file_type)
+
+      false
+    end
+
+    def collapsed?
+      return @collapsed if defined?(@collapsed)
+      return @collapsed = true if diff_file.collapsed?
+
+      @collapsed = !diff_file.expanded? && collapse_limit && diff_file.raw_size > collapse_limit
+    end
+
+    def too_large?
+      return @too_large if defined?(@too_large)
+      return @too_large = true if diff_file.too_large?
+
+      @too_large = size_limit && diff_file.raw_size > size_limit
+    end
+
+    def binary_detected_after_load?
+      !@initially_binary && diff_file.binary?
+    end
+
+    # This method is used on the server side to check whether we can attempt to
+    # render the diff_file at all. Human-readable error messages are found in the
+    # `BlobHelper#diff_render_error_reason` helper.
+    def render_error
+      if too_large?
+        :too_large
+      end
+    end
+
+    def prepare!
+      # To be overridden by subclasses
+    end
+  end
+end
diff --git a/app/models/diff_viewer/client_side.rb b/app/models/diff_viewer/client_side.rb
new file mode 100644
index 00000000000..cf41d07f8eb
--- /dev/null
+++ b/app/models/diff_viewer/client_side.rb
@@ -0,0 +1,10 @@
+module DiffViewer
+  module ClientSide
+    extend ActiveSupport::Concern
+
+    included do
+      self.collapse_limit = 1.megabyte
+      self.size_limit = 10.megabytes
+    end
+  end
+end
diff --git a/app/models/diff_viewer/deleted.rb b/app/models/diff_viewer/deleted.rb
new file mode 100644
index 00000000000..9c129bac694
--- /dev/null
+++ b/app/models/diff_viewer/deleted.rb
@@ -0,0 +1,8 @@
+module DiffViewer
+  class Deleted < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'deleted'
+  end
+end
diff --git a/app/models/diff_viewer/image.rb b/app/models/diff_viewer/image.rb
new file mode 100644
index 00000000000..759d9a36ebb
--- /dev/null
+++ b/app/models/diff_viewer/image.rb
@@ -0,0 +1,12 @@
+module DiffViewer
+  class Image < Base
+    include Rich
+    include ClientSide
+
+    self.partial_name = 'image'
+    self.extensions = UploaderHelper::IMAGE_EXT
+    self.binary = true
+    self.switcher_icon = 'picture-o'
+    self.switcher_title = 'image diff'
+  end
+end
diff --git a/app/models/diff_viewer/mode_changed.rb b/app/models/diff_viewer/mode_changed.rb
new file mode 100644
index 00000000000..d487d996f8d
--- /dev/null
+++ b/app/models/diff_viewer/mode_changed.rb
@@ -0,0 +1,8 @@
+module DiffViewer
+  class ModeChanged < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'mode_changed'
+  end
+end
diff --git a/app/models/diff_viewer/no_preview.rb b/app/models/diff_viewer/no_preview.rb
new file mode 100644
index 00000000000..5455fee4490
--- /dev/null
+++ b/app/models/diff_viewer/no_preview.rb
@@ -0,0 +1,9 @@
+module DiffViewer
+  class NoPreview < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'no_preview'
+    self.binary = true
+  end
+end
diff --git a/app/models/diff_viewer/not_diffable.rb b/app/models/diff_viewer/not_diffable.rb
new file mode 100644
index 00000000000..4f9638626ea
--- /dev/null
+++ b/app/models/diff_viewer/not_diffable.rb
@@ -0,0 +1,9 @@
+module DiffViewer
+  class NotDiffable < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'not_diffable'
+    self.binary = true
+  end
+end
diff --git a/app/models/diff_viewer/renamed.rb b/app/models/diff_viewer/renamed.rb
new file mode 100644
index 00000000000..f1fbfd8c6d5
--- /dev/null
+++ b/app/models/diff_viewer/renamed.rb
@@ -0,0 +1,8 @@
+module DiffViewer
+  class Renamed < Base
+    include Simple
+    include Static
+
+    self.partial_name = 'renamed'
+  end
+end
diff --git a/app/models/diff_viewer/rich.rb b/app/models/diff_viewer/rich.rb
new file mode 100644
index 00000000000..3b0ca6e4cff
--- /dev/null
+++ b/app/models/diff_viewer/rich.rb
@@ -0,0 +1,11 @@
+module DiffViewer
+  module Rich
+    extend ActiveSupport::Concern
+
+    included do
+      self.type = :rich
+      self.switcher_icon = 'file-text-o'
+      self.switcher_title = 'rendered diff'
+    end
+  end
+end
diff --git a/app/models/diff_viewer/server_side.rb b/app/models/diff_viewer/server_side.rb
new file mode 100644
index 00000000000..aed1a0791b1
--- /dev/null
+++ b/app/models/diff_viewer/server_side.rb
@@ -0,0 +1,26 @@
+module DiffViewer
+  module ServerSide
+    extend ActiveSupport::Concern
+
+    included do
+      self.collapse_limit = 1.megabyte
+      self.size_limit = 5.megabytes
+    end
+
+    def prepare!
+      diff_file.old_blob&.load_all_data!
+      diff_file.new_blob&.load_all_data!
+    end
+
+    def render_error
+      # Files that are not stored in the repository, like LFS files and
+      # build artifacts, can only be rendered using a client-side viewer,
+      # since we do not want to read large amounts of data into memory on the
+      # server side. Client-side viewers use JS and can fetch the file from
+      # `diff_file_blob_raw_path` and `diff_file_old_blob_raw_path` using AJAX.
+      return :server_side_but_stored_externally if diff_file.stored_externally?
+
+      super
+    end
+  end
+end
diff --git a/app/models/diff_viewer/simple.rb b/app/models/diff_viewer/simple.rb
new file mode 100644
index 00000000000..65750996ee4
--- /dev/null
+++ b/app/models/diff_viewer/simple.rb
@@ -0,0 +1,11 @@
+module DiffViewer
+  module Simple
+    extend ActiveSupport::Concern
+
+    included do
+      self.type = :simple
+      self.switcher_icon = 'code'
+      self.switcher_title = 'source diff'
+    end
+  end
+end
diff --git a/app/models/diff_viewer/static.rb b/app/models/diff_viewer/static.rb
new file mode 100644
index 00000000000..d761328b3f6
--- /dev/null
+++ b/app/models/diff_viewer/static.rb
@@ -0,0 +1,10 @@
+module DiffViewer
+  module Static
+    extend ActiveSupport::Concern
+
+    # We can always render a static viewer, even if the diff is too large.
+    def render_error
+      nil
+    end
+  end
+end
diff --git a/app/models/diff_viewer/text.rb b/app/models/diff_viewer/text.rb
new file mode 100644
index 00000000000..98f4b2aea2a
--- /dev/null
+++ b/app/models/diff_viewer/text.rb
@@ -0,0 +1,15 @@
+module DiffViewer
+  class Text < Base
+    include Simple
+    include ServerSide
+
+    self.partial_name = 'text'
+    self.binary = false
+
+    # Since the text diff viewer doesn't render the old and new blobs in full,
+    # we only need the limits related to the actual size of the diff which are
+    # already enforced in Gitlab::Diff::File.
+    self.collapse_limit = nil
+    self.size_limit = nil
+  end
+end
diff --git a/app/views/projects/diffs/_collapsed.html.haml b/app/views/projects/diffs/_collapsed.html.haml
new file mode 100644
index 00000000000..8772bd4705f
--- /dev/null
+++ b/app/views/projects/diffs/_collapsed.html.haml
@@ -0,0 +1,5 @@
+- diff_file = viewer.diff_file
+- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
+.nothing-here-block.diff-collapsed{ data: { diff_for_path: url } }
+  This diff is collapsed.
+  %a.click-to-expand Click to expand it.
diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml
index ec1c434a4b8..68f74f702ea 100644
--- a/app/views/projects/diffs/_content.html.haml
+++ b/app/views/projects/diffs/_content.html.haml
@@ -1,27 +1,2 @@
-- blob = diff_file.blob
-
 .diff-content
-  - if diff_file.too_large?
-    .nothing-here-block This diff could not be displayed because it is too large.
-  - elsif blob.truncated?
-    .nothing-here-block The file could not be displayed because it is too large.
-  - elsif blob.readable_text?
-    - if !diff_file.diffable?
-      .nothing-here-block This diff was suppressed by a .gitattributes entry.
-    - elsif diff_file.collapsed?
-      - url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
-      .nothing-here-block.diff-collapsed{ data: { diff_for_path: url } }
-        This diff is collapsed.
-        %a.click-to-expand
-          Click to expand it.
-    - elsif diff_file.diff_lines.length > 0
-      = render "projects/diffs/viewers/text", diff_file: diff_file
-    - else
-      - if diff_file.mode_changed?
-        .nothing-here-block File mode changed
-      - elsif diff_file.renamed_file?
-        .nothing-here-block File moved
-  - elsif blob.image?
-    = render "projects/diffs/viewers/image", diff_file: diff_file
-  - else
-    .nothing-here-block No preview for this file type
+  = render 'projects/diffs/viewer', viewer: diff_file.rich_viewer || diff_file.simple_viewer
diff --git a/app/views/projects/diffs/_render_error.html.haml b/app/views/projects/diffs/_render_error.html.haml
new file mode 100644
index 00000000000..47a9ac3ee6b
--- /dev/null
+++ b/app/views/projects/diffs/_render_error.html.haml
@@ -0,0 +1,6 @@
+.nothing-here-block
+  This #{viewer.switcher_title} could not be displayed because #{diff_render_error_reason(viewer)}.
+
+  You can
+  = diff_render_error_options(viewer).to_sentence(two_words_connector: ' or ', last_word_connector: ', or ').html_safe
+  instead.
diff --git a/app/views/projects/diffs/_viewer.html.haml b/app/views/projects/diffs/_viewer.html.haml
new file mode 100644
index 00000000000..5c4d1760871
--- /dev/null
+++ b/app/views/projects/diffs/_viewer.html.haml
@@ -0,0 +1,16 @@
+- hidden = local_assigns.fetch(:hidden, false)
+
+.diff-viewer{ data: { type: viewer.type }, class: ('hidden' if hidden) }
+  - if viewer.render_error
+    = render 'projects/diffs/render_error', viewer: viewer
+  - elsif viewer.collapsed?
+    = render 'projects/diffs/collapsed', viewer: viewer
+  - else
+    - viewer.prepare!
+
+    -# In the rare case where the first kilobyte of the file looks like text,
+    -# but the file turns out to actually be binary after loading all data,
+    -# we fall back on the binary No Preview viewer.
+    - viewer = DiffViewer::NoPreview.new(viewer.diff_file) if viewer.binary_detected_after_load?
+
+    = render viewer.partial_path, viewer: viewer
diff --git a/app/views/projects/diffs/viewers/_added.html.haml b/app/views/projects/diffs/viewers/_added.html.haml
new file mode 100644
index 00000000000..8004fe16688
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_added.html.haml
@@ -0,0 +1,2 @@
+.nothing-here-block
+  File added
diff --git a/app/views/projects/diffs/viewers/_deleted.html.haml b/app/views/projects/diffs/viewers/_deleted.html.haml
new file mode 100644
index 00000000000..0ac7b4ca8f6
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_deleted.html.haml
@@ -0,0 +1,2 @@
+.nothing-here-block
+  File deleted
diff --git a/app/views/projects/diffs/viewers/_image.html.haml b/app/views/projects/diffs/viewers/_image.html.haml
index ea75373581e..19d08181223 100644
--- a/app/views/projects/diffs/viewers/_image.html.haml
+++ b/app/views/projects/diffs/viewers/_image.html.haml
@@ -1,3 +1,4 @@
+- diff_file = viewer.diff_file
 - blob = diff_file.blob
 - old_blob = diff_file.old_blob
 - blob_raw_path = diff_file_blob_raw_path(diff_file)
diff --git a/app/views/projects/diffs/viewers/_mode_changed.html.haml b/app/views/projects/diffs/viewers/_mode_changed.html.haml
new file mode 100644
index 00000000000..69bc96bbdad
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_mode_changed.html.haml
@@ -0,0 +1,3 @@
+- diff_file = viewer.diff_file
+.nothing-here-block
+  File mode changed from #{diff_file.a_mode} to #{diff_file.b_mode}
diff --git a/app/views/projects/diffs/viewers/_no_preview.html.haml b/app/views/projects/diffs/viewers/_no_preview.html.haml
new file mode 100644
index 00000000000..befe070af2b
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_no_preview.html.haml
@@ -0,0 +1,2 @@
+.nothing-here-block
+  No preview for this file type
diff --git a/app/views/projects/diffs/viewers/_not_diffable.html.haml b/app/views/projects/diffs/viewers/_not_diffable.html.haml
new file mode 100644
index 00000000000..b2c677ec59c
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_not_diffable.html.haml
@@ -0,0 +1,2 @@
+.nothing-here-block
+  This diff was suppressed by a .gitattributes entry.
diff --git a/app/views/projects/diffs/viewers/_renamed.html.haml b/app/views/projects/diffs/viewers/_renamed.html.haml
new file mode 100644
index 00000000000..ef05ee38d8d
--- /dev/null
+++ b/app/views/projects/diffs/viewers/_renamed.html.haml
@@ -0,0 +1,2 @@
+.nothing-here-block
+  File moved
diff --git a/app/views/projects/diffs/viewers/_text.html.haml b/app/views/projects/diffs/viewers/_text.html.haml
index 120d3540223..509e68598c9 100644
--- a/app/views/projects/diffs/viewers/_text.html.haml
+++ b/app/views/projects/diffs/viewers/_text.html.haml
@@ -1,5 +1,5 @@
+- diff_file = viewer.diff_file
 - blob = diff_file.blob
-- blob.load_all_data!
 - total_lines = blob.lines.size
 - total_lines -= 1 if total_lines > 0 && blob.lines.last.blank?
 - if diff_view == :parallel