Merge branch 'blame-on-renamed-files' into 'master'

Link to previous path when viewing blame on renamed files

See merge request gitlab-org/gitlab!80231

app/presenters/gitlab/blame_presenter.rb

@@ -32,8 +32,8 @@ module Gitlab
       @groups ||= blame.groups
     end
 
-    def commit_data(commit)
-      @commits[commit.id] ||= get_commit_data(commit)
+    def commit_data(commit, previous_path = nil)
+      @commits[commit.id] ||= get_commit_data(commit, previous_path)
     end
 
     private
@@ -44,25 +44,25 @@ module Gitlab
     # to avoid recalculating it multiple times.
     # For such files, it could significantly improve the performance of the Blame.
     def precalculate_data_by_commit!
-      groups.each { |group| commit_data(group[:commit]) }
+      groups.each { |group| commit_data(group[:commit], group[:previous_path]) }
     end
 
-    def get_commit_data(commit)
+    def get_commit_data(commit, previous_path = nil)
       CommitData.new.tap do |data|
         data.author_avatar = author_avatar(commit, size: 36, has_tooltip: false, lazy: true)
         data.age_map_class = age_map_class(commit.committed_date, project_duration)
         data.commit_link = link_to commit.title, project_commit_path(project, commit.id), class: "cdark", title: commit.title
         data.commit_author_link = commit_author_link(commit, avatar: false)
-        data.project_blame_link = project_blame_link(commit)
+        data.project_blame_link = project_blame_link(commit, previous_path)
         data.time_ago_tooltip = time_ago_with_tooltip(commit.committed_date)
       end
     end
 
-    def project_blame_link(commit)
+    def project_blame_link(commit, previous_path = nil)
       previous_commit_id = commit.parent_id
-      return unless previous_commit_id
+      return unless previous_commit_id && !previous_path.nil?
 
-      link_to project_blame_path(project, tree_join(previous_commit_id, path)),
+      link_to project_blame_path(project, tree_join(previous_commit_id, previous_path)),
         title: _('View blame prior to this change'),
         aria: { label: _('View blame prior to this change') },
         class: 'version-link',

lib/gitlab/blame.rb

@@ -15,13 +15,13 @@ module Gitlab
       current_group = nil
 
       i = 0
-      blame.each do |commit, line|
+      blame.each do |commit, line, previous_path|
         commit = Commit.new(commit, project)
         commit.lazy_author # preload author
 
         if prev_sha != commit.sha
           groups << current_group if current_group
-          current_group = { commit: commit, lines: [] }
+          current_group = { commit: commit, lines: [], previous_path: previous_path }
         end
 
         current_group[:lines] << (highlight ? highlighted_lines[i].html_safe : line)

lib/gitlab/encoding_helper.rb

@@ -105,6 +105,35 @@ module Gitlab
       io.tap { |io| io.set_encoding(Encoding::ASCII_8BIT) }
     end
 
+    ESCAPED_CHARS = {
+      "a" => "\a", "b" => "\b", "e" => "\e", "f" => "\f",
+      "n" => "\n", "r" => "\r", "t" => "\t", "v" => "\v",
+      "\"" => "\""
+    }.freeze
+
+    # rubocop:disable Style/AsciiComments
+    # `unquote_path` decode filepaths that are returned by some git commands.
+    # The path may be returned in double-quotes if it contains special characters,
+    # that are encoded in octal. Also, some characters (see `ESCAPED_CHARS`) are escaped.
+    # eg. "\311\240\304\253\305\247\305\200\310\247\306\200" (quotes included) is decoded as ɠīŧŀȧƀ
+    #
+    # Based on `unquote_c_style` from git source
+    # https://github.com/git/git/blob/v2.35.1/quote.c#L399
+    # rubocop:enable Style/AsciiComments
+    def unquote_path(filename)
+      return filename unless filename[0] == '"'
+
+      filename = filename[1..-2].gsub(/\\(?:([#{ESCAPED_CHARS.keys.join}\\])|(\d{3}))/) do
+        if c = Regexp.last_match(1)
+          c == "\\" ? "\\" : ESCAPED_CHARS[c]
+        elsif c = Regexp.last_match(2)
+          c.to_i(8).chr
+        end
+      end
+
+      filename.force_encoding("UTF-8")
+    end
+
     private
 
     def force_encode_utf8(message)

lib/gitlab/git/blame.rb

@@ -17,7 +17,7 @@ module Gitlab
 
       def each
         @blames.each do |blame|
-          yield(blame.commit, blame.line)
+          yield(blame.commit, blame.line, blame.previous_path)
         end
       end
 
@@ -34,6 +34,8 @@ module Gitlab
         final = []
         info = {}
         commits = {}
+        commit_id = nil
+        previous_paths = {}
 
         # process the output
         output.split("\n").each do |line|
@@ -45,6 +47,11 @@ module Gitlab
             commit_id = m[1]
             commits[commit_id] = nil unless commits.key?(commit_id)
             info[m[3].to_i] = [commit_id, m[2].to_i]
+          elsif line.start_with?("previous ")
+            # previous 1485b69e7b839a21436e81be6d3aa70def5ed341 initial-commit
+            # previous 9521e52704ee6100e7d2a76896a4ef0eb53ff1b8 "\303\2511\\\303\251\\303\\251\n"
+            #                                                   ^ char index 50
+            previous_paths[commit_id] = unquote_path(line[50..])
           end
         end
 
@@ -54,7 +61,7 @@ module Gitlab
 
         # get it together
         info.sort.each do |lineno, (commit_id, old_lineno)|
-          final << BlameLine.new(lineno, old_lineno, commits[commit_id], lines[lineno - 1])
+          final << BlameLine.new(lineno, old_lineno, commits[commit_id], lines[lineno - 1], previous_paths[commit_id])
         end
 
         @lines = final
@@ -62,13 +69,14 @@ module Gitlab
     end
 
     class BlameLine
-      attr_accessor :lineno, :oldlineno, :commit, :line
+      attr_accessor :lineno, :oldlineno, :commit, :line, :previous_path
 
-      def initialize(lineno, oldlineno, commit, line)
+      def initialize(lineno, oldlineno, commit, line, previous_path)
         @lineno = lineno
         @oldlineno = oldlineno
         @commit = commit
         @line = line
+        @previous_path = previous_path
       end
     end
   end

spec/lib/gitlab/blame_spec.rb

@@ -22,5 +22,18 @@ RSpec.describe Gitlab::Blame do
       expect(subject[-1][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
       expect(subject[-1][:lines]).to eq(["  end", "end"])
     end
+
+    context 'renamed file' do
+      let(:path) { 'files/plain_text/renamed' }
+      let(:commit) { project.commit('blame-on-renamed') }
+
+      it 'adds previous path' do
+        expect(subject[0][:previous_path]).to be nil
+        expect(subject[0][:lines]).to match_array(['Initial commit', 'Initial commit'])
+
+        expect(subject[1][:previous_path]).to eq('files/plain_text/initial-commit')
+        expect(subject[1][:lines]).to match_array(['Renamed as "filename"'])
+      end
+    end
   end
 end

spec/lib/gitlab/encoding_helper_spec.rb

@@ -265,4 +265,14 @@ RSpec.describe Gitlab::EncodingHelper do
       end
     end
   end
+
+  describe '#unquote_path' do
+    it do
+      expect(described_class.unquote_path('unquoted')).to eq('unquoted')
+      expect(described_class.unquote_path('"quoted"')).to eq('quoted')
+      expect(described_class.unquote_path('"\\311\\240\\304\\253\\305\\247\\305\\200\\310\\247\\306\\200"')).to eq('ɠīŧŀȧƀ')
+      expect(described_class.unquote_path('"\\\\303\\\\251"')).to eq('\303\251')
+      expect(described_class.unquote_path('"\a\b\e\f\n\r\t\v\""')).to eq("\a\b\e\f\n\r\t\v\"")
+    end
+  end
 end

spec/lib/gitlab/git/blame_spec.rb

@@ -71,5 +71,40 @@ RSpec.describe Gitlab::Git::Blame, :seed_helper do
         expect(data.first[:line]).to be_utf8
       end
     end
+
+    context "renamed file" do
+      let(:project) { create(:project, :repository) }
+      let(:commit) { project.commit('blame-on-renamed') }
+      let(:path) { 'files/plain_text/renamed' }
+
+      let(:blame) { described_class.new(project.repository, commit.id, path) }
+
+      it do
+        data = []
+        blame.each do |commit, line, previous_path|
+          data << {
+            commit: commit,
+            line: line,
+            previous_path: previous_path
+          }
+        end
+
+        expect(data.size).to eq(5)
+
+        expect(data[0][:line]).to eq('Initial commit')
+        expect(data[0][:previous_path]).to be nil
+        expect(data[1][:line]).to eq('Initial commit')
+        expect(data[1][:previous_path]).to be nil
+
+        expect(data[2][:line]).to eq('Renamed as "filename"')
+        expect(data[2][:previous_path]).to eq('files/plain_text/initial-commit')
+
+        expect(data[3][:line]).to eq('Renamed as renamed')
+        expect(data[3][:previous_path]).to eq('files/plain_text/"filename"')
+
+        expect(data[4][:line]).to eq('Last edit, no rename')
+        expect(data[4][:previous_path]).to eq('files/plain_text/renamed')
+      end
+    end
   end
 end

spec/presenters/gitlab/blame_presenter_spec.rb

@@ -37,9 +37,28 @@ RSpec.describe Gitlab::BlamePresenter do
         expect(data.age_map_class).to include('blame-commit-age-')
         expect(data.commit_link.to_s).to include '913c66a37b4a45b9769037c55c2d238bd0942d2e">Files, encoding and much more</a>'
         expect(data.commit_author_link.to_s).to include('<a class="commit-author-link" href=')
-        expect(data.project_blame_link.to_s).to include('<a title="View blame prior to this change"')
         expect(data.time_ago_tooltip.to_s).to include('data-container="body">Feb 27, 2014</time>')
       end
     end
+
+    context 'renamed file' do
+      let(:path) { 'files/plain_text/renamed' }
+      let(:commit) { project.commit('blame-on-renamed') }
+
+      it 'does not generate link to previous blame on initial commit' do
+        commit = blame.groups[0][:commit]
+        data = subject.commit_data(commit)
+
+        expect(data.project_blame_link.to_s).to eq('')
+      end
+
+      it 'generates link link to previous blame' do
+        commit = blame.groups[1][:commit]
+        data = subject.commit_data(commit)
+
+        expect(data.project_blame_link.to_s).to include('<a title="View blame prior to this change"')
+        expect(data.project_blame_link.to_s).to include('/blame/405a45736a75e439bb059e638afaa9a3c2eeda79/files/plain_text/initial-commit')
+      end
+    end
   end
 end

spec/support/helpers/test_env.rb

@@ -80,7 +80,8 @@ module TestEnv
     'compare-with-merge-head-source'     => 'f20a03d',
     'compare-with-merge-head-target'     => '2f1e176',
     'trailers'                           => 'f0a5ed6',
-    'add_commit_with_5mb_subject'        => '8cf8e80'
+    'add_commit_with_5mb_subject'        => '8cf8e80',
+    'blame-on-renamed'                   => '32c33da'
   }.freeze
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily