localrepo: Refactor `WriteBlob()` to accept a configuration

Refactor `WriteBlob()` to accept a configuration. This makes the
function more readily extensible and also makes it clearer that the
path argument is in fact optional.

10 files changed, 44 insertions, 23 deletions

diff --git a/internal/git/localrepo/blob.go b/internal/git/localrepo/blob.go
index bd52a202b..992d9f090 100644
--- a/internal/git/localrepo/blob.go
+++ b/internal/git/localrepo/blob.go
@@ -10,21 +10,30 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
 )
 
+// WriteBlobConfig is the configuration used to write blobs into the repository.
+type WriteBlobConfig struct {
+	//  Path is used by git to decide which filters to run on the content.
+	Path string
+}
+
 // WriteBlob writes a blob to the repository's object database and
-// returns its object ID. Path is used by git to decide which filters to
-// run on the content.
-func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) (git.ObjectID, error) {
+// returns its object ID.
+func (repo *Repo) WriteBlob(ctx context.Context, content io.Reader, cfg WriteBlobConfig) (git.ObjectID, error) {
 	stdout := &bytes.Buffer{}
 	stderr := &bytes.Buffer{}
 
+	options := []git.Option{
+		git.Flag{Name: "--stdin"},
+		git.Flag{Name: "-w"},
+	}
+	if cfg.Path != "" {
+		options = append(options, git.ValueFlag{Name: "--path", Value: cfg.Path})
+	}
+
 	cmd, err := repo.Exec(ctx,
 		git.Command{
-			Name: "hash-object",
-			Flags: []git.Option{
-				git.ValueFlag{Name: "--path", Value: path},
-				git.Flag{Name: "--stdin"},
-				git.Flag{Name: "-w"},
-			},
+			Name:  "hash-object",
+			Flags: options,
 		},
 		git.WithStdin(content),
 		git.WithStdout(stdout),
diff --git a/internal/git/localrepo/blob_test.go b/internal/git/localrepo/blob_test.go
index d57cf472b..f8cf695b9 100644
--- a/internal/git/localrepo/blob_test.go
+++ b/internal/git/localrepo/blob_test.go
@@ -68,7 +68,9 @@ func TestRepo_WriteBlob(t *testing.T) {
 			require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.SharedDir))
 			require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PublicFile))
 
-			sha, err := repo.WriteBlob(ctx, "file-path", tc.input)
+			sha, err := repo.WriteBlob(ctx, tc.input, WriteBlobConfig{
+				Path: "file-path",
+			})
 			require.Equal(t, tc.error, err)
 			if tc.error != nil {
 				return
diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go
index 688b223e9..425798965 100644
--- a/internal/git/localrepo/repo_test.go
+++ b/internal/git/localrepo/repo_test.go
@@ -60,11 +60,11 @@ func TestRepo_Quarantine(t *testing.T) {
 	require.NoError(t, err)
 
 	quarantinedBlob := []byte("quarantined blob")
-	quarantinedBlobOID, err := quarantinedRepo.WriteBlob(ctx, "", bytes.NewReader(quarantinedBlob))
+	quarantinedBlobOID, err := quarantinedRepo.WriteBlob(ctx, bytes.NewReader(quarantinedBlob), WriteBlobConfig{})
 	require.NoError(t, err)
 
 	unquarantinedBlob := []byte("unquarantined blob")
-	unquarantinedBlobOID, err := unquarantinedRepo.WriteBlob(ctx, "", bytes.NewReader(unquarantinedBlob))
+	unquarantinedBlobOID, err := unquarantinedRepo.WriteBlob(ctx, bytes.NewReader(unquarantinedBlob), WriteBlobConfig{})
 	require.NoError(t, err)
 
 	for _, tc := range []struct {
diff --git a/internal/git/quarantine/quarantine_ext_test.go b/internal/git/quarantine/quarantine_ext_test.go
index 1de9038eb..b3aff0548 100644
--- a/internal/git/quarantine/quarantine_ext_test.go
+++ b/internal/git/quarantine/quarantine_ext_test.go
@@ -39,7 +39,7 @@ func TestQuarantine_localrepo(t *testing.T) {
 	})
 
 	t.Run("writes are not visible in parent repo", func(t *testing.T) {
-		blobID, err := quarantined.WriteBlob(ctx, "", strings.NewReader("contents"))
+		blobID, err := quarantined.WriteBlob(ctx, strings.NewReader("contents"), localrepo.WriteBlobConfig{})
 		require.NoError(t, err)
 
 		_, err = repo.ReadObject(ctx, blobID)
@@ -51,7 +51,7 @@ func TestQuarantine_localrepo(t *testing.T) {
 	})
 
 	t.Run("writes are visible after migrating", func(t *testing.T) {
-		blobID, err := quarantined.WriteBlob(ctx, "", strings.NewReader("contents"))
+		blobID, err := quarantined.WriteBlob(ctx, strings.NewReader("contents"), localrepo.WriteBlobConfig{})
 		require.NoError(t, err)
 
 		_, err = repo.ReadObject(ctx, blobID)
diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go
index 60c1544c8..9d8227979 100644
--- a/internal/gitaly/hook/postreceive_test.go
+++ b/internal/gitaly/hook/postreceive_test.go
@@ -409,7 +409,7 @@ func TestPostReceive_quarantine(t *testing.T) {
 	require.NoError(t, err)
 
 	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
-	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	blobID, err := quarantinedRepo.WriteBlob(ctx, strings.NewReader("allyourbasearebelongtous"), localrepo.WriteBlobConfig{})
 	require.NoError(t, err)
 
 	hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient(
diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go
index 930ca8a87..1525e096b 100644
--- a/internal/gitaly/hook/prereceive_test.go
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -218,7 +218,7 @@ func TestPrereceive_quarantine(t *testing.T) {
 	require.NoError(t, err)
 
 	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
-	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	blobID, err := quarantinedRepo.WriteBlob(ctx, strings.NewReader("allyourbasearebelongtous"), localrepo.WriteBlobConfig{})
 	require.NoError(t, err)
 
 	hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient(
diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go
index ab3b9116c..898317fe9 100644
--- a/internal/gitaly/hook/update_test.go
+++ b/internal/gitaly/hook/update_test.go
@@ -248,7 +248,7 @@ func TestUpdate_quarantine(t *testing.T) {
 	require.NoError(t, err)
 
 	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
-	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("allyourbasearebelongtous"))
+	blobID, err := quarantinedRepo.WriteBlob(ctx, strings.NewReader("allyourbasearebelongtous"), localrepo.WriteBlobConfig{})
 	require.NoError(t, err)
 
 	hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient(
diff --git a/internal/gitaly/hook/updateref/update_with_hooks_test.go b/internal/gitaly/hook/updateref/update_with_hooks_test.go
index edaeaa914..629818778 100644
--- a/internal/gitaly/hook/updateref/update_with_hooks_test.go
+++ b/internal/gitaly/hook/updateref/update_with_hooks_test.go
@@ -320,7 +320,7 @@ func TestUpdaterWithHooks_quarantine(t *testing.T) {
 	quarantine, err := quarantine.New(ctx, repoProto, locator)
 	require.NoError(t, err)
 	quarantinedRepo := localrepo.NewTestRepo(t, cfg, quarantine.QuarantinedRepo())
-	blobID, err := quarantinedRepo.WriteBlob(ctx, "", strings.NewReader("1834298812398123"))
+	blobID, err := quarantinedRepo.WriteBlob(ctx, strings.NewReader("1834298812398123"), localrepo.WriteBlobConfig{})
 	require.NoError(t, err)
 
 	expectQuarantined := func(t *testing.T, env []string, quarantined bool) {
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go
index 9a442276e..493a2a24e 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts.go
@@ -308,7 +308,9 @@ func (s *server) resolveConflictsWithGit(
 				return "", structerr.NewInternal("%w", err)
 			}
 
-			blobOID, err := repo.WriteBlob(ctx, filepath.Base(path), resolvedContent)
+			blobOID, err := repo.WriteBlob(ctx, resolvedContent, localrepo.WriteBlobConfig{
+				Path: filepath.Base(path),
+			})
 			if err != nil {
 				return "", structerr.NewInternal("writing blob: %w", err)
 			}
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index 401ee8d29..c6ed6f5dc 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -314,7 +314,9 @@ func applyAction(
 			}
 		}
 
-		blobID, err := repo.WriteBlob(ctx, filepath.Join(action.Path, ".gitkeep"), strings.NewReader(""))
+		blobID, err := repo.WriteBlob(ctx, strings.NewReader(""), localrepo.WriteBlobConfig{
+			Path: filepath.Join(action.Path, ".gitkeep"),
+		})
 		if err != nil {
 			return err
 		}
@@ -621,7 +623,9 @@ func (s *Server) userCommitFiles(
 
 		switch pbAction.header.Action {
 		case gitalypb.UserCommitFilesActionHeader_CREATE:
-			blobID, err := quarantineRepo.WriteBlob(ctx, path, content)
+			blobID, err := quarantineRepo.WriteBlob(ctx, content, localrepo.WriteBlobConfig{
+				Path: path,
+			})
 			if err != nil {
 				return fmt.Errorf("write created blob: %w", err)
 			}
@@ -645,7 +649,9 @@ func (s *Server) userCommitFiles(
 			var oid git.ObjectID
 			if !pbAction.header.InferContent {
 				var err error
-				oid, err = quarantineRepo.WriteBlob(ctx, path, content)
+				oid, err = quarantineRepo.WriteBlob(ctx, content, localrepo.WriteBlobConfig{
+					Path: path,
+				})
 				if err != nil {
 					return err
 				}
@@ -656,7 +662,9 @@ func (s *Server) userCommitFiles(
 				OID:     oid.String(),
 			})
 		case gitalypb.UserCommitFilesActionHeader_UPDATE:
-			oid, err := quarantineRepo.WriteBlob(ctx, path, content)
+			oid, err := quarantineRepo.WriteBlob(ctx, content, localrepo.WriteBlobConfig{
+				Path: path,
+			})
 			if err != nil {
 				return fmt.Errorf("write updated blob: %w", err)
 			}