gitserver: Move implementation of HasCommitAfter to caller

This is only used in one place, so we're moving the implementation to the caller. Gitserver client only exposes gRPC methods without additional logic on top.

Test plan:

Kept the existing tests but rewrote them to test Commits instead so we don't lose coverage for this.

internal/gitserver/client.go

@@ -371,10 +371,6 @@ type Client interface {
 	// GetObject fetches git object data in the supplied repo
 	GetObject(ctx context.Context, repo api.RepoName, objectName string) (*gitdomain.GitObject, error)
 
-	// HasCommitAfter indicates the staleness of a repository. It returns a boolean indicating if a repository
-	// contains a commit past a specified date.
-	HasCommitAfter(ctx context.Context, repo api.RepoName, date string, revspec string) (bool, error)
-
 	// IsRepoCloneable returns nil if the repository is cloneable.
 	IsRepoCloneable(context.Context, api.RepoName) error
 

internal/gitserver/commands.go

@@ -1051,35 +1051,6 @@ func hasAccessToCommit(ctx context.Context, commit *wrappedCommit, repoName api.
 	return false, nil
 }
 
-// HasCommitAfter indicates the staleness of a repository. It returns a boolean indicating if a repository
-// contains a commit past a specified date.
-func (c *clientImplementor) HasCommitAfter(ctx context.Context, repo api.RepoName, date string, revspec string) (_ bool, err error) {
-	ctx, _, endObservation := c.operations.hasCommitAfter.With(ctx, &err, observation.Args{
-		MetricLabelValues: []string{c.scope},
-		Attrs: []attribute.KeyValue{
-			repo.Attr(),
-			attribute.String("date", date),
-			attribute.String("revSpec", revspec),
-		},
-	})
-	defer endObservation(1, observation.Args{})
-
-	if revspec == "" {
-		revspec = "HEAD"
-	}
-
-	// TODO: Because N: 1 currently has a special meaning because of `isRequestForSingleCommit`,
-	// we ask for two commits here, but the second one we never actually need.
-	// One we figure out why `isRequestForSingleCommit` exists in the first place,
-	// we should update this.
-	commits, err := c.Commits(ctx, repo, CommitsOptions{N: 2, After: date, Range: revspec})
-	if err != nil {
-		return false, err
-	}
-
-	return len(commits) > 0, nil
-}
-
 func isBadObjectErr(output, obj string) bool {
 	return output == "fatal: bad object "+obj
 }

internal/gitserver/commands_test.go

@@ -144,7 +144,7 @@ func TestLogPartsPerCommitInSync(t *testing.T) {
 	require.Equal(t, partsPerCommit-1, strings.Count(logFormatWithoutRefs, "%x00"))
 }
 
-func TestRepository_HasCommitAfter(t *testing.T) {
+func TestCommits_After(t *testing.T) {
 	ClientMocks.LocalGitserver = true
 	defer ResetClientMocks()
 	ctx := actor.WithActor(context.Background(), &actor.Actor{
@@ -230,9 +230,15 @@ func TestRepository_HasCommitAfter(t *testing.T) {
 					gitCommands[i] = fmt.Sprintf("GIT_COMMITTER_NAME=a GIT_COMMITTER_EMAIL=a@a.com GIT_COMMITTER_DATE=%s git commit --allow-empty -m foo --author='a <a@a.com>'", date)
 				}
 				repo := MakeGitRepository(t, gitCommands...)
-				got, err := client.HasCommitAfter(ctx, repo, tc.after, tc.revspec)
-				if err != nil || got != tc.want {
-					t.Errorf("got %t hascommitafter, want %t", got, tc.want)
+				got, err := client.Commits(ctx, repo, CommitsOptions{
+					N:     2,
+					Range: tc.revspec,
+					After: tc.after,
+				})
+				require.NoError(t, err)
+
+				if len(got) > 0 != tc.want {
+					t.Errorf("got %t commits, want %t", len(got) > 0, tc.want)
 				}
 			})
 		}
@@ -251,23 +257,31 @@ func TestRepository_HasCommitAfter(t *testing.T) {
 				checker := getTestSubRepoPermsChecker("file2")
 				client := NewTestClient(t).WithChecker(checker)
 				repo := MakeGitRepository(t, gitCommands...)
-				got, err := client.HasCommitAfter(ctx, repo, tc.after, tc.revspec)
+				got, err := client.Commits(ctx, repo, CommitsOptions{
+					N:     2,
+					After: tc.after,
+					Range: tc.revspec,
+				})
 				if err != nil {
 					t.Errorf("got error: %s", err)
 				}
-				if got != tc.want {
-					t.Errorf("got %t hascommitafter, want %t", got, tc.want)
+				if len(got) > 0 != tc.want {
+					t.Errorf("got %t commits, want %t", len(got) > 0, tc.want)
 				}
 
-				// Case where user can't view commit 1 or commit 2, which will mean in some cases since HasCommitAfter will be false due to those commits not being visible.
+				// Case where user can't view commit 1 or commit 2, which will mean in some cases since len(Commits)>0 will be false due to those commits not being visible.
 				checker = getTestSubRepoPermsChecker("file1", "file2")
 				client = NewTestClient(t).WithChecker(checker)
-				got, err = client.HasCommitAfter(ctx, repo, tc.after, tc.revspec)
+				got, err = client.Commits(ctx, repo, CommitsOptions{
+					N:     2,
+					After: tc.after,
+					Range: tc.revspec,
+				})
 				if err != nil {
 					t.Errorf("got error: %s", err)
 				}
-				if got != tc.wantSubRepoTest {
-					t.Errorf("got %t hascommitafter, want %t", got, tc.wantSubRepoTest)
+				if len(got) > 0 != tc.wantSubRepoTest {
+					t.Errorf("got %t commits, want %t", len(got) > 0, tc.wantSubRepoTest)
 				}
 			})
 		}