service/cleanup: Improve validation of input

Gitaly should return the same error for all RPCs where the
Repository input parameter is missing.
The test coverage extended to cover changed code.

2 files changed, 27 insertions, 6 deletions

diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
index 84b39aca9..9b65eae67 100644
--- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
+++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go
@@ -1,9 +1,9 @@
 package cleanup
 
 import (
-	"fmt"
 	"io"
 
+	gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/internalrefs"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/notifier"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
@@ -66,7 +66,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg
 
 func validateFirstRequest(req *gitalypb.ApplyBfgObjectMapStreamRequest) error {
 	if repo := req.GetRepository(); repo == nil {
-		return fmt.Errorf("first request: repository not set")
+		return gitalyerrors.ErrEmptyRepository
 	}
 
 	return nil
diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
index ab97c0e3f..388c4b973 100644
--- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
+++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go
@@ -18,6 +18,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestApplyBfgObjectMapStreamSuccess(t *testing.T) {
@@ -107,11 +108,31 @@ func requireEntry(t *testing.T, entry *gitalypb.ApplyBfgObjectMapStreamResponse_
 func TestApplyBfgObjectMapStreamFailsOnInvalidInput(t *testing.T) {
 	ctx := testhelper.Context(t)
 
-	_, repo, _, client := setupCleanupService(t, ctx)
+	cfg, protoRepo, _, client := setupCleanupService(t, ctx)
 
-	entries, err := doStreamingRequest(t, ctx, repo, client, "invalid-data here as you can see")
-	require.Empty(t, entries)
-	testhelper.RequireGrpcCode(t, err, codes.InvalidArgument)
+	t.Run("invalid object map", func(t *testing.T) {
+		entries, err := doStreamingRequest(t, ctx, protoRepo, client, "invalid-data here as you can see")
+		require.Empty(t, entries)
+		testhelper.RequireGrpcCode(t, err, codes.InvalidArgument)
+	})
+
+	repo := localrepo.NewTestRepo(t, cfg, protoRepo)
+	headCommit, err := repo.ReadCommit(ctx, "HEAD")
+	require.NoError(t, err)
+
+	objectMapData := fmt.Sprintf(
+		"old                                      new\n%s %s\n",
+		headCommit.Id, git.ObjectHashSHA1.ZeroOID.String(),
+	)
+
+	t.Run("no repository provided", func(t *testing.T) {
+		entries, err := doStreamingRequest(t, ctx, nil, client, objectMapData)
+		require.Empty(t, entries)
+		testhelper.RequireGrpcError(t, status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect(
+			"empty Repository",
+			"repo scoped: empty Repository",
+		)), err)
+	})
 }
 
 func doStreamingRequest(t *testing.T, ctx context.Context, repo *gitalypb.Repository, client gitalypb.CleanupServiceClient, objectMap string) ([]*gitalypb.ApplyBfgObjectMapStreamResponse_Entry, error) {