From 1ad960e9d6c88125c8b64ca8ada057d8cf79589d Mon Sep 17 00:00:00 2001
From: Sami Hiltunen <shiltunen@gitlab.com>
Date: Wed, 30 Mar 2022 17:29:23 +0300
Subject: Add test case for missing pool in DeleteObjectPool

DeleteObjectPool's tests do not currently test what happens if the
request is missing the pool repository. This commit adds the missing
test case. While at it, it exports the functionality to extract a pool
from the request so Praefect's DeleteObjectPool handler can reuse it
later and pass the same test.
---
 internal/gitaly/service/objectpool/create.go      | 20 +++++++++++++++-----
 internal/gitaly/service/objectpool/create_test.go | 23 +++++++++++++++++++++--
 2 files changed, 36 insertions(+), 7 deletions(-)

(limited to 'internal/gitaly/service')

diff --git a/internal/gitaly/service/objectpool/create.go b/internal/gitaly/service/objectpool/create.go
index 1e97f8c88..2dfd35f0a 100644
--- a/internal/gitaly/service/objectpool/create.go
+++ b/internal/gitaly/service/objectpool/create.go
@@ -57,18 +57,28 @@ func (s *server) DeleteObjectPool(ctx context.Context, in *gitalypb.DeleteObject
 	return &gitalypb.DeleteObjectPoolResponse{}, nil
 }
 
-type poolRequest interface {
+// PoolRequest is the interface of a gRPC request that carries an object pool.
+type PoolRequest interface {
 	GetObjectPool() *gitalypb.ObjectPool
 }
 
-func (s *server) poolForRequest(req poolRequest) (*objectpool.ObjectPool, error) {
-	reqPool := req.GetObjectPool()
-
-	poolRepo := reqPool.GetRepository()
+// ExtractPool returns the pool repository from the request or an error if the
+// request did no contain a pool.
+func ExtractPool(req PoolRequest) (*gitalypb.Repository, error) {
+	poolRepo := req.GetObjectPool().GetRepository()
 	if poolRepo == nil {
 		return nil, errMissingPool
 	}
 
+	return poolRepo, nil
+}
+
+func (s *server) poolForRequest(req PoolRequest) (*objectpool.ObjectPool, error) {
+	poolRepo, err := ExtractPool(req)
+	if err != nil {
+		return nil, err
+	}
+
 	pool, err := objectpool.NewObjectPool(s.locator, s.gitCmdFactory, s.catfileCache, s.txManager, s.housekeepingManager, poolRepo.GetStorageName(), poolRepo.GetRelativePath())
 	if err != nil {
 		if err == objectpool.ErrInvalidPoolDir {
diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go
index 09ebca2a9..35afaaf0a 100644
--- a/internal/gitaly/service/objectpool/create_test.go
+++ b/internal/gitaly/service/objectpool/create_test.go
@@ -156,9 +156,20 @@ func TestDelete(t *testing.T) {
 
 	for _, tc := range []struct {
 		desc         string
+		noPool       bool
 		relativePath string
 		error        error
 	}{
+		{
+			desc:   "no pool in request fails",
+			noPool: true,
+			error:  errMissingPool,
+		},
+		{
+			desc:         "deleting outside pools directory fails",
+			relativePath: ".",
+			error:        errInvalidPoolDir,
+		},
 		{
 			desc:         "deleting outside pools directory fails",
 			relativePath: ".",
@@ -199,20 +210,28 @@ func TestDelete(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			_, err := client.DeleteObjectPool(ctx, &gitalypb.DeleteObjectPoolRequest{ObjectPool: &gitalypb.ObjectPool{
+			request := &gitalypb.DeleteObjectPoolRequest{ObjectPool: &gitalypb.ObjectPool{
 				Repository: &gitalypb.Repository{
 					StorageName:  repo.GetStorageName(),
 					RelativePath: tc.relativePath,
 				},
-			}})
+			}}
+
+			if tc.noPool {
+				request.ObjectPool = nil
+			}
+
 			expectedErr := tc.error
 			if tc.error == errInvalidPoolDir && testhelper.IsPraefectEnabled() {
 				expectedErr = helper.ErrNotFound(fmt.Errorf(
 					"mutator call: route repository mutator: get repository id: %w",
 					commonerr.NewRepositoryNotFoundError(repo.GetStorageName(), tc.relativePath),
 				))
+			} else if tc.error == errMissingPool && testhelper.IsPraefectEnabled() {
+				expectedErr = helper.ErrInvalidArgumentf("repo scoped: unable to descend OID [1 1] into message gitaly.DeleteObjectPoolRequest: proto field is empty")
 			}
 
+			_, err := client.DeleteObjectPool(ctx, request)
 			testhelper.RequireGrpcError(t, expectedErr, err)
 
 			response, err := repositoryClient.RepositoryExists(ctx, &gitalypb.RepositoryExistsRequest{
-- 
cgit v1.2.3