Allow Commit.RawBlame to take a Range parameter

Git supports a wide array of range options for this option, but for now
we restrict the formats we support to the very simplest, which is of
the form "1,100".

We can use this to implement limits on how many lines of the blame we
retrieve GitLab-side, which should help to reduce the resource usage
of this particular RPC.

Changelog: added

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)