From 6bfb4b57698fe1281e30b11f020bf7ec022243a6 Mon Sep 17 00:00:00 2001 From: Quang-Minh Nguyen Date: Mon, 15 Apr 2024 09:41:50 +0700 Subject: [PATCH] Make SetFullPath no-op in WAL transaction SetFullPath RPC writes the repository's full path into the git config. This is not supported by WALtransaction. That RPC was scheduled for removal in some next versions. Until then, we should mark that RPC as a no-op when being executed in the context of a WAL transaction. This commit returns Unimplemented status code if the RPC handler detects an active WAL partition manager. From the definition, Unimplemented status code is for "The operation is not implemented or is not supported/enabled in this service". That's a perfect status code for this purpose. --- .../gitaly/service/repository/fullpath.go | 4 + .../service/repository/fullpath_test.go | 76 ++++++++++++++----- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/internal/gitaly/service/repository/fullpath.go b/internal/gitaly/service/repository/fullpath.go index d2fcd97745..3079f190bb 100644 --- a/internal/gitaly/service/repository/fullpath.go +++ b/internal/gitaly/service/repository/fullpath.go @@ -17,6 +17,10 @@ func (s *server) SetFullPath( ctx context.Context, request *gitalypb.SetFullPathRequest, ) (*gitalypb.SetFullPathResponse, error) { + if s.walPartitionManager != nil { + return nil, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL") + } + repository := request.GetRepository() if err := s.locator.ValidateRepository(repository); err != nil { return nil, structerr.NewInvalidArgument("%w", err) diff --git a/internal/gitaly/service/repository/fullpath_test.go b/internal/gitaly/service/repository/fullpath_test.go index b31a56f470..da23a1aff8 100644 --- a/internal/gitaly/service/repository/fullpath_test.go +++ b/internal/gitaly/service/repository/fullpath_test.go @@ -28,8 +28,13 @@ func TestSetFullPath(t *testing.T) { Repository: nil, Path: "my/repo", }) + require.Nil(t, response) - testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet), err) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet), err) + } }) t.Run("missing path", func(t *testing.T) { @@ -40,7 +45,11 @@ func TestSetFullPath(t *testing.T) { Path: "", }) require.Nil(t, response) - testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("no path provided"), err) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("no path provided"), err) + } }) t.Run("invalid storage", func(t *testing.T) { @@ -68,9 +77,14 @@ func TestSetFullPath(t *testing.T) { Path: "my/repo", }) require.Nil(t, response) - testhelper.RequireGrpcError(t, testhelper.ToInterceptedMetadata( - structerr.New("%w", storage.NewRepositoryNotFoundError(repo.StorageName, repo.RelativePath)), - ), err) + + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + testhelper.RequireGrpcError(t, testhelper.ToInterceptedMetadata( + structerr.New("%w", storage.NewRepositoryNotFoundError(repo.StorageName, repo.RelativePath)), + ), err) + } }) t.Run("normal repo", func(t *testing.T) { @@ -80,11 +94,15 @@ func TestSetFullPath(t *testing.T) { Repository: repo, Path: "foo/bar", }) - require.NoError(t, err) - testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + require.NoError(t, err) + testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) - fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", fullPathKey) - require.Equal(t, "foo/bar", text.ChompBytes(fullPath)) + fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", fullPathKey) + require.Equal(t, "foo/bar", text.ChompBytes(fullPath)) + } }) t.Run("missing config", func(t *testing.T) { @@ -97,11 +115,16 @@ func TestSetFullPath(t *testing.T) { Repository: repo, Path: "foo/bar", }) - require.NoError(t, err) - testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) - fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", fullPathKey) - require.Equal(t, "foo/bar", text.ChompBytes(fullPath)) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + require.NoError(t, err) + testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) + + fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", fullPathKey) + require.Equal(t, "foo/bar", text.ChompBytes(fullPath)) + } }) t.Run("multiple times", func(t *testing.T) { @@ -112,12 +135,18 @@ func TestSetFullPath(t *testing.T) { Repository: repo, Path: fmt.Sprintf("foo/%d", i), }) - require.NoError(t, err) - testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + require.NoError(t, err) + testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) + } } - fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", "--get-all", fullPathKey) - require.Equal(t, "foo/4", text.ChompBytes(fullPath)) + if !testhelper.IsWALEnabled() { + fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", "--get-all", fullPathKey) + require.Equal(t, "foo/4", text.ChompBytes(fullPath)) + } }) t.Run("multiple preexisting paths", func(t *testing.T) { @@ -131,11 +160,16 @@ func TestSetFullPath(t *testing.T) { Repository: repo, Path: "replace", }) - require.NoError(t, err) - testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) - fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", "--get-all", fullPathKey) - require.Equal(t, "replace", text.ChompBytes(fullPath)) + if testhelper.IsWALEnabled() { + testhelper.RequireGrpcError(t, structerr.NewUnimplemented("SetFullPath RPC is not supported by WAL"), err) + } else { + require.NoError(t, err) + testhelper.ProtoEqual(t, &gitalypb.SetFullPathResponse{}, response) + + fullPath := gittest.Exec(t, cfg, "-C", repoPath, "config", "--get-all", fullPathKey) + require.Equal(t, "replace", text.ChompBytes(fullPath)) + } }) } -- GitLab