Link to previous path when viewing blame on renamed files

On blame page, the "View blame prior to this change" button
fail if the file has been renamed.

To resolve that, when reading the raw blame from git,
the lines beginning with "previous" are read
to retrieve the previous path.

Also, the button has been removed for the initial commit
on the file.

Changelog: changed

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