Add file_identifier_hash to identify diff_file more reliably

This will replace file_hash where we need to identify diff_files
especially when there are two diff_files with the same file_name in case
of regular file -> symlink or symlink -> regular file.

app/serializers/diff_file_base_entity.rb

@@ -67,10 +67,8 @@ class DiffFileBaseEntity < Grape::Entity
     end
   end
 
-  expose :file_hash do |diff_file|
-    Digest::SHA1.hexdigest(diff_file.file_path)
-  end
-
+  expose :file_identifier_hash
+  expose :file_hash
   expose :file_path
   expose :old_path
   expose :new_path

app/serializers/diff_file_metadata_entity.rb

@@ -7,7 +7,6 @@ class DiffFileMetadataEntity < Grape::Entity
   expose :old_path
   expose :new_file?, as: :new_file
   expose :deleted_file?, as: :deleted_file
-  expose :file_hash do |diff_file|
-    Digest::SHA1.hexdigest(diff_file.file_path)
-  end
+  expose :file_identifier_hash
+  expose :file_hash
 end

ee/app/models/draft_note.rb

@@ -28,6 +28,8 @@ class DraftNote < ApplicationRecord
 
   scope :authored_by, ->(u) { where(author_id: u.id) }
 
+  delegate :file_path, :file_hash, :file_identifier_hash, to: :diff_file, allow_nil: true
+
   def self.positions
     where.not(position: nil)
       .select(:position)
@@ -90,18 +92,6 @@ class DraftNote < ApplicationRecord
     @line_code ||= diff_file&.line_code_for_position(original_position)
   end
 
-  def file_hash
-    return unless diff_file
-
-    Digest::SHA1.hexdigest(diff_file.file_path)
-  end
-
-  def file_path
-    return unless diff_file
-
-    diff_file.file_path
-  end
-
   def publish_params
     attrs = PUBLISH_ATTRS.dup
     attrs.concat(DIFF_ATTRS) if on_diff?

ee/app/serializers/draft_note_entity.rb

@@ -7,6 +7,7 @@ class DraftNoteEntity < Grape::Entity
   expose :merge_request_id
   expose :position, if: -> (note, _) { note.on_diff? }
   expose :line_code
+  expose :file_identifier_hash
   expose :file_hash
   expose :file_path
   expose :note

ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb

@@ -79,6 +79,7 @@ describe Projects::MergeRequests::DraftsController do
       expect(response).to have_gitlab_http_status(:ok)
       expect(json_response['position']).to be_present
       expect(json_response['file_hash']).to be_present
+      expect(json_response['file_identifier_hash']).to be_present
       expect(json_response['line_code']).to match(/\w+_\d+_\d+/)
       expect(json_response['note_html']).to eq('<p dir="auto">This is a unpublished comment</p>')
     end

ee/spec/models/draft_note_spec.rb

@@ -5,7 +5,38 @@ require 'spec_helper'
 describe DraftNote do
   include RepoHelpers
 
+  let(:project)       { create(:project, :repository) }
+  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+
   describe 'validations' do
     it_behaves_like 'a valid diff positionable note', :draft_note
   end
+
+  describe 'delegations' do
+    it { is_expected.to delegate_method(:file_path).to(:diff_file).allow_nil }
+    it { is_expected.to delegate_method(:file_hash).to(:diff_file).allow_nil }
+    it { is_expected.to delegate_method(:file_identifier_hash).to(:diff_file).allow_nil }
+  end
+
+  describe '#diff_file' do
+    let(:draft_note) { build(:draft_note, merge_request: merge_request) }
+
+    context 'when diff_file exists' do
+      it "returns an unfolded diff_file" do
+        diff_file = instance_double(Gitlab::Diff::File)
+        expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(diff_file)
+        expect(diff_file).to receive(:unfold_diff_lines).with(draft_note.original_position)
+
+        expect(draft_note.diff_file).to be diff_file
+      end
+    end
+
+    context 'when diff_file does not exist' do
+      it 'returns nil' do
+        expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(nil)
+
+        expect(draft_note.diff_file).to be_nil
+      end
+    end
+  end
 end

lib/gitlab/diff/file.rb

@@ -225,6 +225,10 @@ module Gitlab
         new_path.presence || old_path
       end
 
+      def file_hash
+        Digest::SHA1.hexdigest(file_path)
+      end
+
       def added_lines
         @stats&.additions || diff_lines.count(&:added?)
       end
@@ -237,6 +241,10 @@ module Gitlab
         "#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
       end
 
+      def file_identifier_hash
+        Digest::SHA1.hexdigest(file_identifier)
+      end
+
       def diffable?
         repository.attributes(file_path).fetch('diff') { true }
       end

spec/lib/gitlab/diff/file_spec.rb

@@ -282,6 +282,18 @@ describe Gitlab::Diff::File do
     end
   end
 
+  describe '#file_hash' do
+    it 'returns a hash of file_path' do
+      expect(diff_file.file_hash).to eq(Digest::SHA1.hexdigest(diff_file.file_path))
+    end
+  end
+
+  describe '#file_identifier_hash' do
+    it 'returns a hash of file_identifier' do
+      expect(diff_file.file_identifier_hash).to eq(Digest::SHA1.hexdigest(diff_file.file_identifier))
+    end
+  end
+
   context 'diff file stats' do
     let(:diff_file) do
       described_class.new(diff,

spec/support/shared_examples/serializers/diff_file_entity_shared_examples.rb

@@ -8,7 +8,7 @@ RSpec.shared_examples 'diff file base entity' do
                                :file_hash, :file_path, :old_path, :new_path,
                                :viewer, :diff_refs, :stored_externally,
                                :external_storage, :renamed_file, :deleted_file,
-                               :a_mode, :b_mode, :new_file)
+                               :a_mode, :b_mode, :new_file, :file_identifier_hash)
   end
 
   # Converted diff files from GitHub import does not contain blob file