MR Diffs Refactor Part 01: Backend changes._mr-refactor-part-1

12 files changed, 170 insertions, 17 deletions

diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 0c1c286a0a4..a366559ae1d 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -3,8 +3,8 @@ class Projects::BlobController < Projects::ApplicationController
   include ExtractsPath
   include CreatesCommit
   include RendersBlob
+  include NotesHelper
   include ActionView::Helpers::SanitizeHelper
-
   prepend_before_action :authenticate_user!, only: [:edit]
 
   before_action :require_non_empty_project, except: [:new, :create]
@@ -92,6 +92,7 @@ class Projects::BlobController < Projects::ApplicationController
     @lines = Gitlab::Highlight.highlight(@blob.path, @blob.data, repository: @repository).lines
 
     @form = UnfoldForm.new(params)
+
     @lines = @lines[@form.since - 1..@form.to - 1].map(&:html_safe)
 
     if @form.bottom?
@@ -102,11 +103,45 @@ class Projects::BlobController < Projects::ApplicationController
       @match_line = "@@ -#{line}+#{line} @@"
     end
 
-    render layout: false
+    if has_vue_discussions_cookie?
+      render_diff_lines
+    else
+      render layout: false
+    end
   end
 
   private
 
+  # Converts a String array to Gitlab::Diff::Line array
+  def render_diff_lines
+    @lines.map! do |line|
+      diff_line = Gitlab::Diff::Line.new(line ,'context', nil, nil, nil)
+      diff_line.rich_text = line
+      diff_line
+    end
+
+    add_match_line
+
+    render json: @lines
+  end
+
+  def add_match_line
+    return unless @form.unfold?
+
+    if @form.bottom? && @form.to < @blob.lines.size
+      old_pos = @form.to - @form.offset
+      new_pos = @form.to
+    elsif @form.since != 1
+      old_pos = new_pos = @form.since
+    end
+
+    return unless new_pos
+
+    @match_line = Gitlab::Diff::Line.new(@match_line ,'match', nil, old_pos, new_pos)
+
+    @form.bottom? ? @lines.push(@match_line) : @lines.unshift(@match_line)
+  end
+
   def blob
     @blob ||= @repository.blob_at(@commit.id, @path)
 
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index fe8525a488c..242bcfb574d 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -1,6 +1,7 @@
 class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController
   include DiffForPath
   include DiffHelper
+  include NotesHelper
   include RendersNotes
 
   before_action :apply_diff_view_cookie!
@@ -11,7 +12,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   def show
     @environment = @merge_request.environments_for(current_user).last
 
-    render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") }
+    if has_vue_discussions_cookie?
+      render json: DiffsSerializer.new.represent(@diffs, merge_request: @merge_request)
+    else
+      render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") }
+    end
   end
 
   def diff_for_path
diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb
index 6e68d275047..715b58a7295 100644
--- a/app/serializers/diff_file_entity.rb
+++ b/app/serializers/diff_file_entity.rb
@@ -1,7 +1,9 @@
 class DiffFileEntity < Grape::Entity
+  include RequestAwareEntity
   include DiffHelper
   include SubmoduleHelper
   include BlobHelper
+  include TreeHelper
   include IconsHelper
   include ActionView::Helpers::TagHelper
 
@@ -15,11 +17,20 @@ class DiffFileEntity < Grape::Entity
     diff_file.blob.path
   end
 
+  expose :blob_name do |diff_file|
+    diff_file.blob.name
+  end
+
   expose :blob_icon do |diff_file|
     blob_icon(diff_file.b_mode, diff_file.file_path)
   end
 
+  expose :file_hash do |diff_file|
+    Digest::SHA1.hexdigest(diff_file.file_path)
+  end
+
   expose :file_path
+  expose :new_file?, as: :new_file
   expose :deleted_file?, as: :deleted_file
   expose :renamed_file?, as: :renamed_file
   expose :old_path
@@ -28,6 +39,9 @@ class DiffFileEntity < Grape::Entity
   expose :a_mode
   expose :b_mode
   expose :text?, as: :text
+  expose :added_lines
+  expose :removed_lines
+  expose :diff_refs
 
   expose :old_path_html do |diff_file|
     old_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
@@ -38,4 +52,57 @@ class DiffFileEntity < Grape::Entity
     _, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
     new_path
   end
+
+  # TODO check if these are not creating a n+1 call
+  # we should probably also pass project as parameter
+  expose :edit_path, if: -> (_, options) { options[:merge_request] } do |diff_file|
+    merge_request = options[:merge_request]
+
+    edit_blob_path(merge_request.source_project, merge_request.source_branch, diff_file.new_path)
+  end
+
+  expose :view_path, if: -> (_, options) { options[:merge_request] } do |diff_file|
+    merge_request = options[:merge_request]
+
+    project_blob_path(merge_request.source_project, tree_join(merge_request.source_branch, diff_file.new_path))
+  end
+
+  expose :context_lines_path, if: -> (diff_file, _) { diff_file.text? } do |diff_file|
+    project_blob_diff_path(diff_file.repository.project, tree_join(diff_file.file_path, diff_file.content_sha))
+  end
+
+  # Used for inline diffs
+  expose :highlighted_diff_lines, if: -> (diff_file, _) { diff_file.text? } do |diff_file|
+    lines = diff_file.highlighted_diff_lines
+
+    add_bottom_match_line(lines, diff_file)
+
+    lines
+  end
+
+  # Used for parallel diffs
+  expose :parallel_diff_lines, if: -> (diff_file, _) { diff_file.text? }
+
+  private
+
+  # This adds the bottom line to the array which contains
+  # the data to load more context lines.
+  def add_bottom_match_line(lines, diff_file)
+    return if lines.empty?
+
+    last_line = lines.last
+
+    return unless last_line.new_pos < total_lines(diff_file.blob)
+
+    match_line = Gitlab::Diff::Line.new("" ,'match', nil, last_line.old_pos, last_line.new_pos)
+    lines.push(match_line)
+  end
+
+  def total_lines(blob)
+    @total_lines ||= begin
+      line_count = blob.lines.size
+      line_count -= 1 if line_count > 0 && blob.lines.last.blank?
+      line_count
+    end
+  end
 end
diff --git a/app/serializers/diff_file_serializer.rb b/app/serializers/diff_file_serializer.rb
new file mode 100644
index 00000000000..8fcb7d67e34
--- /dev/null
+++ b/app/serializers/diff_file_serializer.rb
@@ -0,0 +1,3 @@
+class DiffFileSerializer < BaseSerializer
+  entity DiffFileEntity
+end
diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb
new file mode 100644
index 00000000000..fb8027059cb
--- /dev/null
+++ b/app/serializers/diffs_entity.rb
@@ -0,0 +1,13 @@
+class DiffsEntity < Grape::Entity
+  expose :real_size
+
+  expose :added_lines do |diffs|
+    diffs.diff_files.sum(&:added_lines)
+  end
+
+  expose :removed_lines do |diffs|
+    diffs.diff_files.sum(&:removed_lines)
+  end
+
+  expose :diff_files, using: DiffFileEntity
+end
diff --git a/app/serializers/diffs_serializer.rb b/app/serializers/diffs_serializer.rb
new file mode 100644
index 00000000000..6771e10c5ac
--- /dev/null
+++ b/app/serializers/diffs_serializer.rb
@@ -0,0 +1,3 @@
+class DiffsSerializer < BaseSerializer
+  entity DiffsEntity
+end
diff --git a/app/serializers/discussion_entity.rb b/app/serializers/discussion_entity.rb
index bbbcf6a97c1..6af0973c1dd 100644
--- a/app/serializers/discussion_entity.rb
+++ b/app/serializers/discussion_entity.rb
@@ -2,6 +2,8 @@ class DiscussionEntity < Grape::Entity
   include RequestAwareEntity
 
   expose :id, :reply_id
+  expose :position, if: -> (d, _) { defined? d.diff_file }
+  expose :line_code, if: -> (d, _) { defined? d.diff_file }
   expose :expanded?, as: :expanded
 
   expose :notes, using: NoteEntity
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 34b070dd375..51772260323 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -126,11 +126,13 @@ module Gitlab
 
       # Array of Gitlab::Diff::Line objects
       def diff_lines
-        @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
+        @diff_lines ||=
+          Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a
       end
 
       def highlighted_diff_lines
-        @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
+        @highlighted_diff_lines ||=
+          Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
       end
 
       # Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 269016daac2..16da773bb4a 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -5,7 +5,7 @@ module Gitlab
 
       delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
 
-      def initialize(diff_lines, repository: nil)
+      def initialize(diff_lines, since: nil, from: nil, repository: nil)
         @repository = repository
 
         if diff_lines.is_a?(Gitlab::Diff::File)
diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb
index 0603141e441..b8966110f5a 100644
--- a/lib/gitlab/diff/line.rb
+++ b/lib/gitlab/diff/line.rb
@@ -1,22 +1,26 @@
 module Gitlab
   module Diff
     class Line
-      attr_reader :type, :index, :old_pos, :new_pos
+      attr_reader :line_code, :type, :index, :old_pos, :new_pos, :meta_data
       attr_writer :rich_text
       attr_accessor :text
 
-      def initialize(text, type, index, old_pos, new_pos, parent_file: nil)
+      def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil)
         @text, @type, @index = text, type, index
         @old_pos, @new_pos = old_pos, new_pos
         @parent_file = parent_file
+
+        # When line code is not provided from cache store we build it
+        # using the parent_file(Diff::File or Conflict::File).
+        @line_code = line_code || set_line_code
       end
 
       def self.init_from_hash(hash)
-        new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos])
+        new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code])
       end
 
       def serialize_keys
-        @serialize_keys ||= %i(text type index old_pos new_pos)
+        @serialize_keys ||= %i(line_code text type index old_pos new_pos)
       end
 
       def to_hash
@@ -58,20 +62,39 @@ module Gitlab
       end
 
       def rich_text
-        @parent_file.highlight_lines! if @parent_file && !@rich_text
+        @parent_file.try(:highlight_lines!) if @parent_file && !@rich_text
 
         @rich_text
       end
 
+      def meta_data
+        return unless meta?
+
+        {
+          old_pos: old_pos,
+          new_pos: new_pos
+        }
+      end
+
       def as_json(opts = nil)
         {
+          line_code: line_code,
           type: type,
           old_line: old_line,
           new_line: new_line,
           text: text,
-          rich_text: rich_text || text
+          rich_text: rich_text || text,
+          meta_data: meta_data
         }
       end
+
+      private
+
+      def set_line_code
+        return nil unless @parent_file
+
+        @parent_file.line_code(self)
+      end
     end
   end
 end
diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb
index 8302f30a0a2..7ae7ed286ed 100644
--- a/lib/gitlab/diff/parser.rb
+++ b/lib/gitlab/diff/parser.rb
@@ -3,7 +3,7 @@ module Gitlab
     class Parser
       include Enumerable
 
-      def parse(lines)
+      def parse(lines, diff_file: nil)
         return [] if lines.blank?
 
         @lines = lines
@@ -31,17 +31,17 @@ module Gitlab
 
               next if line_old <= 1 && line_new <= 1 # top of file
 
-              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
+              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file)
               line_obj_index += 1
               next
             elsif line[0] == '\\'
               type = "#{context}-nonewline"
 
-              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
+              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file)
               line_obj_index += 1
             else
               type = identification_type(line)
-              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
+              yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file)
               line_obj_index += 1
             end
 
diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb
index 7ee8e38af1c..c0db71c693f 100644
--- a/spec/serializers/discussion_entity_spec.rb
+++ b/spec/serializers/discussion_entity_spec.rb
@@ -30,7 +30,7 @@ describe DiscussionEntity do
     let(:note) { create(:diff_note_on_merge_request) }
 
     it 'exposes diff file attributes' do
-      expect(subject).to include(:diff_file, :truncated_diff_lines, :image_diff_html)
+      expect(subject).to include(:diff_file, :truncated_diff_lines, :image_diff_html, :diff_lines, :highlighted_diff_lines)
     end
   end
 end