Merge branch 'add-git-blame-range' into 'master'

Allow Commit.RawBlame to take a Range parameter

See merge request gitlab-org/gitaly!4433

2 files changed, 49 insertions, 11 deletions

diff --git a/internal/gitaly/service/commit/raw_blame.go b/internal/gitaly/service/commit/raw_blame.go
index 860e511b7..8bd76ef10 100644
--- a/internal/gitaly/service/commit/raw_blame.go
+++ b/internal/gitaly/service/commit/raw_blame.go
@@ -3,6 +3,7 @@ package commit
 import (
 	"fmt"
 	"io"
+	"regexp"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
@@ -12,6 +13,8 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+var validBlameRange = regexp.MustCompile(`\A\d+,\d+\z`)
+
 func (s *server) RawBlame(in *gitalypb.RawBlameRequest, stream gitalypb.CommitService_RawBlameServer) error {
 	if err := validateRawBlameRequest(in); err != nil {
 		return status.Errorf(codes.InvalidArgument, "RawBlame: %v", err)
@@ -20,10 +23,16 @@ func (s *server) RawBlame(in *gitalypb.RawBlameRequest, stream gitalypb.CommitSe
 	ctx := stream.Context()
 	revision := string(in.GetRevision())
 	path := string(in.GetPath())
+	blameRange := string(in.GetRange())
+
+	flags := []git.Option{git.Flag{Name: "-p"}}
+	if blameRange != "" {
+		flags = append(flags, git.ValueFlag{Name: "-L", Value: blameRange})
+	}
 
 	cmd, err := s.gitCmdFactory.New(ctx, in.Repository, git.SubCmd{
 		Name:        "blame",
-		Flags:       []git.Option{git.Flag{Name: "-p"}},
+		Flags:       flags,
 		Args:        []string{revision},
 		PostSepArgs: []string{path},
 	})
@@ -59,5 +68,10 @@ func validateRawBlameRequest(in *gitalypb.RawBlameRequest) error {
 		return fmt.Errorf("empty Path")
 	}
 
+	blameRange := in.GetRange()
+	if len(blameRange) > 0 && !validBlameRange.Match(blameRange) {
+		return fmt.Errorf("invalid Range")
+	}
+
 	return nil
 }
diff --git a/internal/gitaly/service/commit/raw_blame_test.go b/internal/gitaly/service/commit/raw_blame_test.go
index 5de17dd91..5de729b04 100644
--- a/internal/gitaly/service/commit/raw_blame_test.go
+++ b/internal/gitaly/service/commit/raw_blame_test.go
@@ -19,22 +19,31 @@ func TestSuccessfulRawBlameRequest(t *testing.T) {
 	_, repo, _, client := setupCommitServiceWithRepo(ctx, t, true)
 
 	testCases := []struct {
-		revision, path, data []byte
+		revision, path, data, blameRange []byte
 	}{
 		{
-			revision: []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
-			path:     []byte("files/ruby/popen.rb"),
-			data:     testhelper.MustReadFile(t, "testdata/files-ruby-popen-e63f41f-blame.txt"),
+			revision:   []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
+			path:       []byte("files/ruby/popen.rb"),
+			data:       testhelper.MustReadFile(t, "testdata/files-ruby-popen-e63f41f-blame.txt"),
+			blameRange: []byte{},
 		},
 		{
-			revision: []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
-			path:     []byte("files/ruby/../ruby/popen.rb"),
-			data:     testhelper.MustReadFile(t, "testdata/files-ruby-popen-e63f41f-blame.txt"),
+			revision:   []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
+			path:       []byte("files/ruby/popen.rb"),
+			data:       testhelper.MustReadFile(t, "testdata/files-ruby-popen-e63f41f-blame.txt")[0:956],
+			blameRange: []byte("1,5"),
 		},
 		{
-			revision: []byte("93dcf076a236c837dd47d61f86d95a6b3d71b586"),
-			path:     []byte("gitaly/empty-file"),
-			data:     []byte{},
+			revision:   []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
+			path:       []byte("files/ruby/../ruby/popen.rb"),
+			data:       testhelper.MustReadFile(t, "testdata/files-ruby-popen-e63f41f-blame.txt"),
+			blameRange: []byte{},
+		},
+		{
+			revision:   []byte("93dcf076a236c837dd47d61f86d95a6b3d71b586"),
+			path:       []byte("gitaly/empty-file"),
+			data:       []byte{},
+			blameRange: []byte{},
 		},
 	}
 
@@ -44,6 +53,7 @@ func TestSuccessfulRawBlameRequest(t *testing.T) {
 				Repository: repo,
 				Revision:   testCase.revision,
 				Path:       testCase.path,
+				Range:      testCase.blameRange,
 			}
 			c, err := client.RawBlame(ctx, request)
 			require.NoError(t, err)
@@ -73,6 +83,7 @@ func TestFailedRawBlameRequest(t *testing.T) {
 		description    string
 		repo           *gitalypb.Repository
 		revision, path []byte
+		blameRange     []byte
 		code           codes.Code
 	}{
 		{
@@ -80,6 +91,7 @@ func TestFailedRawBlameRequest(t *testing.T) {
 			repo:        invalidRepo,
 			revision:    []byte("master"),
 			path:        []byte("a/b/c"),
+			blameRange:  []byte{},
 			code:        codes.InvalidArgument,
 		},
 		{
@@ -87,6 +99,7 @@ func TestFailedRawBlameRequest(t *testing.T) {
 			repo:        repo,
 			revision:    []byte(""),
 			path:        []byte("a/b/c"),
+			blameRange:  []byte{},
 			code:        codes.InvalidArgument,
 		},
 		{
@@ -94,6 +107,7 @@ func TestFailedRawBlameRequest(t *testing.T) {
 			repo:        repo,
 			revision:    []byte("abcdef"),
 			path:        []byte(""),
+			blameRange:  []byte{},
 			code:        codes.InvalidArgument,
 		},
 		{
@@ -101,6 +115,15 @@ func TestFailedRawBlameRequest(t *testing.T) {
 			repo:        repo,
 			revision:    []byte("--output=/meow"),
 			path:        []byte("a/b/c"),
+			blameRange:  []byte{},
+			code:        codes.InvalidArgument,
+		},
+		{
+			description: "Invalid range",
+			repo:        repo,
+			revision:    []byte("abcdef"),
+			path:        []byte("a/b/c"),
+			blameRange:  []byte("foo"),
 			code:        codes.InvalidArgument,
 		},
 	}
@@ -111,6 +134,7 @@ func TestFailedRawBlameRequest(t *testing.T) {
 				Repository: testCase.repo,
 				Revision:   testCase.revision,
 				Path:       testCase.path,
+				Range:      testCase.blameRange,
 			}
 			c, err := client.RawBlame(ctx, &request)
 			require.NoError(t, err)