symbols: replace usage of gitserver client's DiffSymbols method with …

…new ChangedFiles method

cmd/symbols/fetcher/BUILD.bazel

@@ -32,6 +32,7 @@ go_test(
     deps = [
         "//cmd/symbols/gitserver",
         "//internal/api",
+        "//internal/gitserver",
         "//internal/gitserver/gitdomain",
         "//internal/observation",
         "//internal/search",

cmd/symbols/gitserver/BUILD.bazel

@@ -33,7 +33,6 @@ go_test(
         "//internal/gitserver",
         "//internal/gitserver/gitdomain",
         "//internal/observation",
-        "@com_github_google_go_cmp//cmp",
         "@com_github_stretchr_testify//require",
     ],
 )

cmd/symbols/gitserver/client.go

@@ -22,8 +22,8 @@ type GitserverClient interface {
 	// determine if the error is a bad request (eg invalid repo).
 	FetchTar(context.Context, api.RepoName, api.CommitID, []string) (io.ReadCloser, error)
 
-	// GitDiff returns the paths that have changed between two commits.
-	GitDiff(context.Context, api.RepoName, api.CommitID, api.CommitID) (Changes, error)
+	// ChangedFiles returns an iterator that yields the paths that have changed between two commits.
+	ChangedFiles(context.Context, api.RepoName, api.CommitID, api.CommitID) (gitserver.ChangedFilesIterator, error)
 
 	// NewFileReader returns an io.ReadCloser reading from the named file at commit.
 	// The caller should always close the reader after use.
@@ -75,22 +75,15 @@ func (c *gitserverClient) FetchTar(ctx context.Context, repo api.RepoName, commi
 	return c.innerClient.ArchiveReader(ctx, repo, opts)
 }
 
-func (c *gitserverClient) GitDiff(ctx context.Context, repo api.RepoName, commitA, commitB api.CommitID) (_ Changes, err error) {
+func (c *gitserverClient) ChangedFiles(ctx context.Context, repo api.RepoName, commitA, commitB api.CommitID) (iterator gitserver.ChangedFilesIterator, err error) {
 	ctx, _, endObservation := c.operations.gitDiff.With(ctx, &err, observation.Args{Attrs: []attribute.KeyValue{
 		repo.Attr(),
 		attribute.String("commitA", string(commitA)),
 		attribute.String("commitB", string(commitB)),
 	}})
 	defer endObservation(1, observation.Args{})
 
-	output, err := c.innerClient.DiffSymbols(ctx, repo, commitA, commitB)
-
-	changes, err := parseGitDiffOutput(output)
-	if err != nil {
-		return Changes{}, errors.Wrap(err, "failed to parse git diff output")
-	}
-
-	return changes, nil
+	return c.innerClient.ChangedFiles(ctx, repo, string(commitA), string(commitB))
 }
 
 func (c *gitserverClient) NewFileReader(ctx context.Context, repoCommitPath types.RepoCommitPath) (io.ReadCloser, error) {

cmd/symbols/gitserver/client_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
 
 	"github.com/sourcegraph/sourcegraph/internal/api"
@@ -13,67 +12,6 @@ import (
 	"github.com/sourcegraph/sourcegraph/internal/observation"
 )
 
-func TestParseGitDiffOutput(t *testing.T) {
-	testCases := []struct {
-		output          []byte
-		expectedChanges Changes
-		shouldError     bool
-	}{
-		{
-			output: combineBytes(
-				[]byte("A"), NUL, []byte("added1.json"), NUL,
-				[]byte("M"), NUL, []byte("modified1.json"), NUL,
-				[]byte("D"), NUL, []byte("deleted1.json"), NUL,
-				[]byte("A"), NUL, []byte("added2.json"), NUL,
-				[]byte("M"), NUL, []byte("modified2.json"), NUL,
-				[]byte("D"), NUL, []byte("deleted2.json"), NUL,
-				[]byte("A"), NUL, []byte("added3.json"), NUL,
-				[]byte("M"), NUL, []byte("modified3.json"), NUL,
-				[]byte("D"), NUL, []byte("deleted3.json"), NUL,
-			),
-			expectedChanges: Changes{
-				Added:    []string{"added1.json", "added2.json", "added3.json"},
-				Modified: []string{"modified1.json", "modified2.json", "modified3.json"},
-				Deleted:  []string{"deleted1.json", "deleted2.json", "deleted3.json"},
-			},
-		},
-		{
-			output: combineBytes(
-				[]byte("A"), NUL, []byte("added1.json"), NUL,
-				[]byte("M"), NUL, []byte("modified1.json"), NUL,
-				[]byte("D"), NUL,
-			),
-			shouldError: true,
-		},
-		{
-			output: []byte{},
-		},
-	}
-
-	for _, testCase := range testCases {
-		changes, err := parseGitDiffOutput(testCase.output)
-		if err != nil {
-			if !testCase.shouldError {
-				t.Fatalf("unexpected error parsing git diff output: %s", err)
-			}
-		} else if testCase.shouldError {
-			t.Fatalf("expected error, got none")
-		}
-
-		if diff := cmp.Diff(testCase.expectedChanges, changes); diff != "" {
-			t.Errorf("unexpected changes (-want +got):\n%s", diff)
-		}
-	}
-}
-
-func combineBytes(bss ...[]byte) (combined []byte) {
-	for _, bs := range bss {
-		combined = append(combined, bs...)
-	}
-
-	return combined
-}
-
 func TestGitserverClient_PaginatedRevList(t *testing.T) {
 	allCommits := []*gitdomain.Commit{
 		{ID: "4ac04f2761285633cd35188c696a6e08de03c00c"},

cmd/symbols/gitserver/observability.go

@@ -30,6 +30,6 @@ func newOperations(observationCtx *observation.Context) *operations {
 
 	return &operations{
 		fetchTar: op("FetchTar"),
-		gitDiff:  op("GitDiff"),
+		gitDiff:  op("ChangedFiles"),
 	}
 }

cmd/symbols/internal/api/BUILD.bazel

@@ -58,6 +58,7 @@ go_test(
         "//internal/database/dbmocks",
         "//internal/diskcache",
         "//internal/endpoint",
+        "//internal/gitserver",
         "//internal/gitserver/gitdomain",
         "//internal/grpc/defaults",
         "//internal/observation",