diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-09-18 11:36:35 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-09-19 09:51:43 +0300 |
commit | 6b700f087809d4cc8e6ce901751ef5df7ba1841f (patch) | |
tree | 46609e494133e4f1aa6265a6ef6120b9ce6457dd | |
parent | 0c64aeeef171df62628ee25049d11e0a542870e4 (diff) |
service: Simplify injection of dependencies for Hook server
While we already have a `service.Dependencies` type around for quite a
long time, we still pass in dependencies explicitly when constructing
the actual server. This makes it harder than necessary to make a server
require more dependencies as you will have to adjust all callsites where
the server is currently getting constructed.
Simplify the code to instead inject the `service.Dependencies` type into
the server directly. This will allow us to propagate dependencies more
readily in the future.
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 8 | ||||
-rw-r--r-- | internal/git/localrepo/remote_extra_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/hook/updateref/update_with_hooks_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/cleanup/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/conflicts/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/hook/server.go | 19 | ||||
-rw-r--r-- | internal/gitaly/service/hook/testhelper_test.go | 9 | ||||
-rw-r--r-- | internal/gitaly/service/objectpool/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/operations/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/operations/user_create_branch_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/ref/delete_refs_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/ref/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/repository/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/setup/register.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/testhelper_test.go | 8 | ||||
-rw-r--r-- | internal/gitaly/service/ssh/testhelper_test.go | 8 |
16 files changed, 22 insertions, 118 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index ead3f1a7b..1503e7077 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -863,13 +863,7 @@ func runHookServiceWithGitlabClient(t *testing.T, cfg config.Cfg, assertUserDeta gitalypb.RegisterHookServiceServer(srv, baggageAsserter{ t: t, assertUserDetails: assertUserDetails, - wrapped: hook.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - ), + wrapped: hook.NewServer(deps), }) }, append(serverOpts, testserver.WithGitLabClient(gitlabClient))...) } diff --git a/internal/git/localrepo/remote_extra_test.go b/internal/git/localrepo/remote_extra_test.go index 056cab827..0efb3cea7 100644 --- a/internal/git/localrepo/remote_extra_test.go +++ b/internal/git/localrepo/remote_extra_test.go @@ -30,13 +30,7 @@ func TestRepo_FetchInternal(t *testing.T) { cfg.SocketPath = testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterSSHServiceServer(srv, ssh.NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hook.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hook.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) }, testserver.WithGitCommandFactory(protocolDetectingFactory)) diff --git a/internal/gitaly/hook/updateref/update_with_hooks_test.go b/internal/gitaly/hook/updateref/update_with_hooks_test.go index 629818778..45306cde1 100644 --- a/internal/gitaly/hook/updateref/update_with_hooks_test.go +++ b/internal/gitaly/hook/updateref/update_with_hooks_test.go @@ -98,13 +98,7 @@ func TestUpdaterWithHooks_UpdateReference(t *testing.T) { // We need to set up a separate "real" hook service here, as it will be used in // git-update-ref(1) spawned by `updateRefWithHooks()` testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) }) repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ diff --git a/internal/gitaly/service/cleanup/testhelper_test.go b/internal/gitaly/service/cleanup/testhelper_test.go index de87469d4..3f488eba8 100644 --- a/internal/gitaly/service/cleanup/testhelper_test.go +++ b/internal/gitaly/service/cleanup/testhelper_test.go @@ -36,13 +36,7 @@ func setupCleanupService(t *testing.T, ctx context.Context) (config.Cfg, *gitaly func runCleanupServiceServer(t *testing.T, cfg config.Cfg) string { return testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterCleanupServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) }) } diff --git a/internal/gitaly/service/conflicts/testhelper_test.go b/internal/gitaly/service/conflicts/testhelper_test.go index ae8d5e1df..382c2f39a 100644 --- a/internal/gitaly/service/conflicts/testhelper_test.go +++ b/internal/gitaly/service/conflicts/testhelper_test.go @@ -39,13 +39,7 @@ func runConflictsServer(tb testing.TB, cfg config.Cfg, hookManager hook.Manager) gitalypb.RegisterConflictsServiceServer(srv, NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) gitalypb.RegisterSSHServiceServer(srv, ssh.NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterCommitServiceServer(srv, commit.NewServer(deps)) }, testserver.WithHookManager(hookManager)) } diff --git a/internal/gitaly/service/hook/server.go b/internal/gitaly/service/hook/server.go index d99f66aa9..3d2977616 100644 --- a/internal/gitaly/service/hook/server.go +++ b/internal/gitaly/service/hook/server.go @@ -6,6 +6,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git" gitalyhook "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/limiter" "gitlab.com/gitlab-org/gitaly/v16/internal/streamcache" @@ -31,19 +32,13 @@ type server struct { } // NewServer creates a new instance of a gRPC namespace server -func NewServer( - manager gitalyhook.Manager, - locator storage.Locator, - gitCmdFactory git.CommandFactory, - packObjectsCache streamcache.Cache, - packObjectsLimiter limiter.Limiter, -) gitalypb.HookServiceServer { +func NewServer(deps *service.Dependencies) gitalypb.HookServiceServer { srv := &server{ - manager: manager, - locator: locator, - gitCmdFactory: gitCmdFactory, - packObjectsCache: packObjectsCache, - packObjectsLimiter: packObjectsLimiter, + manager: deps.GetHookManager(), + locator: deps.GetLocator(), + gitCmdFactory: deps.GetGitCmdFactory(), + packObjectsCache: deps.GetPackObjectsCache(), + packObjectsLimiter: deps.GetPackObjectsLimiter(), runPackObjectsFn: runPackObjects, } diff --git a/internal/gitaly/service/hook/testhelper_test.go b/internal/gitaly/service/hook/testhelper_test.go index 8246ebe5d..f55cbea57 100644 --- a/internal/gitaly/service/hook/testhelper_test.go +++ b/internal/gitaly/service/hook/testhelper_test.go @@ -4,7 +4,6 @@ import ( "testing" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" - gitalyhook "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/repository" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" @@ -54,13 +53,7 @@ func runHooksServer(tb testing.TB, cfg config.Cfg, opts []serverOption, serverOp serverOpts = append(serverOpts, testserver.WithDisablePraefect()) return testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) { - hookServer := NewServer( - gitalyhook.NewManager(deps.GetCfg(), deps.GetLocator(), deps.GetGitCmdFactory(), deps.GetTxManager(), deps.GetGitlabClient()), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - ) + hookServer := NewServer(deps) for _, opt := range opts { opt(hookServer.(*server)) } diff --git a/internal/gitaly/service/objectpool/testhelper_test.go b/internal/gitaly/service/objectpool/testhelper_test.go index 816936fc7..3310a3c45 100644 --- a/internal/gitaly/service/objectpool/testhelper_test.go +++ b/internal/gitaly/service/objectpool/testhelper_test.go @@ -64,13 +64,7 @@ func setupWithConfig(t *testing.T, ctx context.Context, cfg config.Cfg, opts ... func runObjectPoolServer(t *testing.T, cfg config.Cfg, locator storage.Locator, logger log.Logger, opts ...testserver.GitalyServerOpt) string { return testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterObjectPoolServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) }, append(opts, testserver.WithLocator(locator), testserver.WithLogger(logger))...) } diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go index 1fbbcc8da..f2b11a0e3 100644 --- a/internal/gitaly/service/operations/testhelper_test.go +++ b/internal/gitaly/service/operations/testhelper_test.go @@ -80,13 +80,7 @@ func runOperationServiceServer(tb testing.TB, cfg config.Cfg, options ...testser return testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterOperationServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hook.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hook.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) gitalypb.RegisterRefServiceServer(srv, ref.NewServer(deps)) gitalypb.RegisterCommitServiceServer(srv, commit.NewServer(deps)) diff --git a/internal/gitaly/service/operations/user_create_branch_test.go b/internal/gitaly/service/operations/user_create_branch_test.go index c7feddde0..964ab5239 100644 --- a/internal/gitaly/service/operations/user_create_branch_test.go +++ b/internal/gitaly/service/operations/user_create_branch_test.go @@ -124,13 +124,7 @@ func TestUserCreateBranch_transactions(t *testing.T) { cfg.ListenAddr = "127.0.0.1:0" // runs gitaly on the TCP address addr := testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterOperationServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hook.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hook.NewServer(deps)) // Praefect proxy execution disabled as praefect runs only on the UNIX socket, but // the test requires a TCP listening address. }, testserver.WithDisablePraefect()) diff --git a/internal/gitaly/service/ref/delete_refs_test.go b/internal/gitaly/service/ref/delete_refs_test.go index 2eec6b652..6ec7ead6a 100644 --- a/internal/gitaly/service/ref/delete_refs_test.go +++ b/internal/gitaly/service/ref/delete_refs_test.go @@ -107,13 +107,7 @@ func TestDeleteRefs_transaction(t *testing.T) { addr := testserver.RunGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterRefServiceServer(srv, NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) }, testserver.WithTransactionManager(txManager)) cfg.SocketPath = addr diff --git a/internal/gitaly/service/ref/testhelper_test.go b/internal/gitaly/service/ref/testhelper_test.go index e35c925b6..c64ff480a 100644 --- a/internal/gitaly/service/ref/testhelper_test.go +++ b/internal/gitaly/service/ref/testhelper_test.go @@ -47,13 +47,7 @@ func setupRefService(tb testing.TB) (config.Cfg, gitalypb.RefServiceClient) { func runRefServiceServer(tb testing.TB, cfg config.Cfg) string { return testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterRefServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) }) } diff --git a/internal/gitaly/service/repository/testhelper_test.go b/internal/gitaly/service/repository/testhelper_test.go index 73ba330b5..39b618ce5 100644 --- a/internal/gitaly/service/repository/testhelper_test.go +++ b/internal/gitaly/service/repository/testhelper_test.go @@ -57,13 +57,7 @@ func newMuxedRepositoryClient(t *testing.T, ctx context.Context, cfg config.Cfg, func runRepositoryService(tb testing.TB, cfg config.Cfg, opts ...testserver.GitalyServerOpt) (gitalypb.RepositoryServiceClient, string) { serverSocketPath := testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterRepositoryServiceServer(srv, NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterRemoteServiceServer(srv, remote.NewServer(deps)) gitalypb.RegisterSSHServiceServer(srv, ssh.NewServer(deps)) gitalypb.RegisterRefServiceServer(srv, ref.NewServer(deps)) diff --git a/internal/gitaly/service/setup/register.go b/internal/gitaly/service/setup/register.go index 3f2cd51cf..07b839493 100644 --- a/internal/gitaly/service/setup/register.go +++ b/internal/gitaly/service/setup/register.go @@ -70,13 +70,7 @@ func RegisterAll(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterRemoteServiceServer(srv, remote.NewServer(deps)) gitalypb.RegisterServerServiceServer(srv, server.NewServer(deps.GetGitCmdFactory(), deps.GetCfg().Storages)) gitalypb.RegisterObjectPoolServiceServer(srv, objectpool.NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hook.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hook.NewServer(deps)) gitalypb.RegisterInternalGitalyServer(srv, internalgitaly.NewServer( deps.GetCfg().Storages, deps.GetLocator(), diff --git a/internal/gitaly/service/smarthttp/testhelper_test.go b/internal/gitaly/service/smarthttp/testhelper_test.go index 3b341810e..c9487106d 100644 --- a/internal/gitaly/service/smarthttp/testhelper_test.go +++ b/internal/gitaly/service/smarthttp/testhelper_test.go @@ -29,13 +29,7 @@ func startSmartHTTPServerWithOptions(t *testing.T, cfg config.Cfg, opts []Server gitalypb.RegisterSmartHTTPServiceServer(srv, NewServer(deps, opts...)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) gitalypb.RegisterObjectPoolServiceServer(srv, objectpool.NewServer(deps)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) }, serverOpts...) } diff --git a/internal/gitaly/service/ssh/testhelper_test.go b/internal/gitaly/service/ssh/testhelper_test.go index e81ea6e05..eb3b5fe40 100644 --- a/internal/gitaly/service/ssh/testhelper_test.go +++ b/internal/gitaly/service/ssh/testhelper_test.go @@ -31,13 +31,7 @@ func runSSHServerWithOptions(t *testing.T, cfg config.Cfg, opts []ServerOpt, ser func startSSHServerWithOptions(t *testing.T, cfg config.Cfg, opts []ServerOpt, serverOpts ...testserver.GitalyServerOpt) testserver.GitalyServer { return testserver.StartGitalyServer(t, cfg, func(srv *grpc.Server, deps *service.Dependencies) { gitalypb.RegisterSSHServiceServer(srv, NewServer(deps, opts...)) - gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer( - deps.GetHookManager(), - deps.GetLocator(), - deps.GetGitCmdFactory(), - deps.GetPackObjectsCache(), - deps.GetPackObjectsLimiter(), - )) + gitalypb.RegisterHookServiceServer(srv, hookservice.NewServer(deps)) gitalypb.RegisterRepositoryServiceServer(srv, repository.NewServer(deps)) gitalypb.RegisterObjectPoolServiceServer(srv, objectpool.NewServer(deps)) }, serverOpts...) |