localrepo: Use ObjectIDs when reading/writing git objects

Both the `ReadObject()` and `WriteBlob()` functions currently accept,
respectively return, an untyped string. Convert these functions to use an
ObjectID to better convey its meaning.

6 files changed, 48 insertions, 43 deletions

diff --git a/internal/git/localrepo/objects.go b/internal/git/localrepo/objects.go
index d4874a413..6d3c9a3bb 100644
--- a/internal/git/localrepo/objects.go
+++ b/internal/git/localrepo/objects.go
@@ -24,7 +24,7 @@ var (
 // 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) (string, error) {
+func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) (git.ObjectID, error) {
 	stdout := &bytes.Buffer{}
 	stderr := &bytes.Buffer{}
 
@@ -49,7 +49,12 @@ func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader)
 		return "", errorWithStderr(err, stderr.Bytes())
 	}
 
-	return text.ChompBytes(stdout.Bytes()), nil
+	oid, err := git.NewObjectIDFromHex(text.ChompBytes(stdout.Bytes()))
+	if err != nil {
+		return "", err
+	}
+
+	return oid, nil
 }
 
 // FormatTagError is used by FormatTag() below
@@ -164,7 +169,7 @@ func (err InvalidObjectError) Error() string { return fmt.Sprintf("invalid objec
 
 // ReadObject reads an object from the repository's object database. InvalidObjectError
 // is returned if the oid does not refer to a valid object.
-func (repo *Repo) ReadObject(ctx context.Context, oid string) ([]byte, error) {
+func (repo *Repo) ReadObject(ctx context.Context, oid git.ObjectID) ([]byte, error) {
 	const msgInvalidObject = "fatal: Not a valid object name "
 
 	stdout := &bytes.Buffer{}
@@ -173,7 +178,7 @@ func (repo *Repo) ReadObject(ctx context.Context, oid string) ([]byte, error) {
 		git.SubCmd{
 			Name:  "cat-file",
 			Flags: []git.Option{git.Flag{"-p"}},
-			Args:  []string{oid},
+			Args:  []string{oid.String()},
 		},
 		git.WithStdout(stdout),
 		git.WithStderr(stderr),
diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go
index 893c54428..91bbcb7c0 100644
--- a/internal/git/localrepo/objects_test.go
+++ b/internal/git/localrepo/objects_test.go
@@ -231,13 +231,13 @@ func TestRepo_ReadObject(t *testing.T) {
 
 	for _, tc := range []struct {
 		desc    string
-		oid     string
+		oid     git.ObjectID
 		content string
 		error   error
 	}{
 		{
 			desc:  "invalid object",
-			oid:   git.ZeroOID.String(),
+			oid:   git.ZeroOID,
 			error: InvalidObjectError(git.ZeroOID.String()),
 		},
 		{
diff --git a/internal/git2go/apply_test.go b/internal/git2go/apply_test.go
index e3ed37d8c..6f303257c 100644
--- a/internal/git2go/apply_test.go
+++ b/internal/git2go/apply_test.go
@@ -42,7 +42,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Author:     author,
 		Committer:  committer,
 		Message:    "base commit",
-		Actions:    []Action{CreateFile{Path: "file", OID: oidBase}},
+		Actions:    []Action{CreateFile{Path: "file", OID: oidBase.String()}},
 	})
 	require.NoError(t, err)
 
@@ -51,7 +51,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Author:     author,
 		Committer:  committer,
 		Message:    "commit with ab",
-		Actions:    []Action{CreateFile{Path: "file", OID: oidA}},
+		Actions:    []Action{CreateFile{Path: "file", OID: oidA.String()}},
 	})
 	require.NoError(t, err)
 
@@ -61,7 +61,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Committer:  committer,
 		Message:    "commit with a",
 		Parent:     parentCommitSHA,
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidA}},
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidA.String()}},
 	})
 	require.NoError(t, err)
 
@@ -71,7 +71,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Committer:  committer,
 		Message:    "commit to b",
 		Parent:     parentCommitSHA,
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidB}},
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidB.String()}},
 	})
 	require.NoError(t, err)
 
@@ -81,7 +81,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Committer:  committer,
 		Message:    "commit a -> b",
 		Parent:     updateToA,
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidB}},
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidB.String()}},
 	})
 	require.NoError(t, err)
 
@@ -90,7 +90,7 @@ func TestExecutor_Apply(t *testing.T) {
 		Author:     author,
 		Committer:  committer,
 		Message:    "commit with other-file",
-		Actions:    []Action{CreateFile{Path: "other-file", OID: oidA}},
+		Actions:    []Action{CreateFile{Path: "other-file", OID: oidA.String()}},
 	})
 	require.NoError(t, err)
 
diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go
index 6e689a314..c4c8427f7 100644
--- a/internal/git2go/commit_test.go
+++ b/internal/git2go/commit_test.go
@@ -120,7 +120,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file", Content: "original"},
@@ -139,7 +139,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file", Content: "original"},
@@ -152,8 +152,8 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
-						CreateFile{Path: "file", OID: updatedFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
+						CreateFile{Path: "file", OID: updatedFile.String()},
 					},
 					error: FileExistsError("file"),
 				},
@@ -165,7 +165,7 @@ func TestExecutor_Commit(t *testing.T) {
 				{
 					actions: []Action{
 						CreateDirectory{Path: "directory"},
-						CreateFile{Path: "directory", OID: originalFile},
+						CreateFile{Path: "directory", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "directory", Content: "original"},
@@ -178,8 +178,8 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
-						UpdateFile{Path: "file", OID: updatedFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
+						UpdateFile{Path: "file", OID: updatedFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file", Content: "updated"},
@@ -192,7 +192,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file", Content: "original"},
@@ -200,7 +200,7 @@ func TestExecutor_Commit(t *testing.T) {
 				},
 				{
 					actions: []Action{
-						UpdateFile{Path: "file", OID: updatedFile},
+						UpdateFile{Path: "file", OID: updatedFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file", Content: "updated"},
@@ -213,7 +213,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						UpdateFile{Path: "non-existing", OID: updatedFile},
+						UpdateFile{Path: "non-existing", OID: updatedFile.String()},
 					},
 					error: FileNotFoundError("non-existing"),
 				},
@@ -224,8 +224,8 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "original-file", OID: originalFile},
-						MoveFile{Path: "original-file", NewPath: "moved-file", OID: originalFile},
+						CreateFile{Path: "original-file", OID: originalFile.String()},
+						MoveFile{Path: "original-file", NewPath: "moved-file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "moved-file", Content: "original"},
@@ -250,7 +250,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "original-file", OID: originalFile},
+						CreateFile{Path: "original-file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "original-file", Content: "original"},
@@ -282,8 +282,8 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "source-file", OID: originalFile},
-						CreateFile{Path: "already-existing", OID: updatedFile},
+						CreateFile{Path: "source-file", OID: originalFile.String()},
+						CreateFile{Path: "already-existing", OID: updatedFile.String()},
 						MoveFile{Path: "source-file", NewPath: "already-existing"},
 					},
 					error: FileExistsError("already-existing"),
@@ -297,7 +297,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file", OID: originalFile},
+						CreateFile{Path: "file", OID: originalFile.String()},
 						CreateDirectory{Path: "already-existing"},
 						MoveFile{Path: "file", NewPath: "already-existing"},
 					},
@@ -312,7 +312,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "original-file", OID: originalFile},
+						CreateFile{Path: "original-file", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "original-file", Content: "original"},
@@ -320,7 +320,7 @@ func TestExecutor_Commit(t *testing.T) {
 				},
 				{
 					actions: []Action{
-						MoveFile{Path: "original-file", NewPath: "moved-file", OID: updatedFile},
+						MoveFile{Path: "original-file", NewPath: "moved-file", OID: updatedFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "moved-file", Content: "updated"},
@@ -344,7 +344,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
 						ChangeFileMode{Path: "file-1", ExecutableMode: true},
 					},
 					treeEntries: []gittest.TreeEntry{
@@ -366,7 +366,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
 						ChangeFileMode{Path: "file-1", ExecutableMode: true},
 					},
 					treeEntries: []gittest.TreeEntry{
@@ -380,7 +380,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file-1", Content: "original"},
@@ -412,8 +412,8 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
-						CreateFile{Path: "file-2", OID: updatedFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
+						CreateFile{Path: "file-2", OID: updatedFile.String()},
 						MoveFile{Path: "file-1", NewPath: "file-2"},
 					},
 					error: FileExistsError("file-2"),
@@ -436,7 +436,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
 						DeleteFile{Path: "file-1"},
 					},
 				},
@@ -447,7 +447,7 @@ func TestExecutor_Commit(t *testing.T) {
 			steps: []step{
 				{
 					actions: []Action{
-						CreateFile{Path: "file-1", OID: originalFile},
+						CreateFile{Path: "file-1", OID: originalFile.String()},
 					},
 					treeEntries: []gittest.TreeEntry{
 						{Mode: DefaultMode, Path: "file-1", Content: "original"},
@@ -500,7 +500,7 @@ func TestExecutor_Commit(t *testing.T) {
 func getCommit(t testing.TB, ctx context.Context, repo *localrepo.Repo, oid string) commit {
 	t.Helper()
 
-	data, err := repo.ReadObject(ctx, oid)
+	data, err := repo.ReadObject(ctx, git.ObjectID(oid))
 	require.NoError(t, err)
 
 	var commit commit
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts_test.go b/internal/gitaly/service/conflicts/resolve_conflicts_test.go
index 6b2bf0e74..bee92f864 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts_test.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts_test.go
@@ -121,7 +121,7 @@ func testSuccessfulResolveConflictsRequest(t *testing.T, ctx context.Context) {
 		testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "read-tree", branch)
 		testhelper.MustRunCommand(t, nil,
 			"git", "-C", testRepoPath,
-			"update-index", "--add", "--cacheinfo", "100644", blobID, missingAncestorPath,
+			"update-index", "--add", "--cacheinfo", "100644", blobID.String(), missingAncestorPath,
 		)
 		treeID := bytes.TrimSpace(
 			testhelper.MustRunCommand(t, nil,
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index 8aa3791bf..e01bcf3cd 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -270,7 +270,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 			}
 
 			actions = append(actions, git2go.CreateFile{
-				OID:            blobID,
+				OID:            blobID.String(),
 				Path:           path,
 				ExecutableMode: pbAction.header.ExecuteFilemode,
 			})
@@ -285,7 +285,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 				return fmt.Errorf("validate previous path: %w", err)
 			}
 
-			var oid string
+			var oid git.ObjectID
 			if !pbAction.header.InferContent {
 				var err error
 				oid, err = localRepo.WriteBlob(ctx, path, content)
@@ -297,7 +297,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 			actions = append(actions, git2go.MoveFile{
 				Path:    prevPath,
 				NewPath: path,
-				OID:     oid,
+				OID:     oid.String(),
 			})
 		case gitalypb.UserCommitFilesActionHeader_UPDATE:
 			oid, err := localRepo.WriteBlob(ctx, path, content)
@@ -307,7 +307,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi
 
 			actions = append(actions, git2go.UpdateFile{
 				Path: path,
-				OID:  oid,
+				OID:  oid.String(),
 			})
 		case gitalypb.UserCommitFilesActionHeader_DELETE:
 			actions = append(actions, git2go.DeleteFile{