updateref: Convert interface to accept ReferenceNames

Instead of accepting untyped strings, this commit refactors the
updateref interface to accept ReferenceNames instead.

10 files changed, 23 insertions, 21 deletions

diff --git a/cmd/gitaly-ssh/upload_pack_test.go b/cmd/gitaly-ssh/upload_pack_test.go
index 126561bed..88d60ae9b 100644
--- a/cmd/gitaly-ssh/upload_pack_test.go
+++ b/cmd/gitaly-ssh/upload_pack_test.go
@@ -40,7 +40,7 @@ func TestVisibilityOfHiddenRefs(t *testing.T) {
 	updater, err := updateref.New(ctx, testRepo)
 
 	require.NoError(t, err)
-	require.NoError(t, updater.Create(keepAroundRef, existingSha))
+	require.NoError(t, updater.Create(git.ReferenceName(keepAroundRef), existingSha))
 	require.NoError(t, updater.Wait())
 
 	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "transfer.hideRefs", keepAroundNamespace)
diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go
index ea49c223c..cc782fd6c 100644
--- a/internal/git/objectpool/fetch.go
+++ b/internal/git/objectpool/fetch.go
@@ -168,7 +168,7 @@ func rescueDanglingObjects(ctx context.Context, repo repository.GitRepo) error {
 			continue
 		}
 
-		ref := danglingObjectNamespace + "/" + split[2]
+		ref := git.ReferenceName(danglingObjectNamespace + "/" + split[2])
 		if err := updater.Create(ref, split[2]); err != nil {
 			return err
 		}
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index e20a40b78..348c00ed5 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -73,21 +73,21 @@ func New(ctx context.Context, repo repository.GitRepo, opts ...UpdaterOpt) (*Upd
 }
 
 // Create commands the reference to be created with the sha specified in value
-func (u *Updater) Create(ref, value string) error {
-	_, err := fmt.Fprintf(u.cmd, "create %s\x00%s\x00", ref, value)
+func (u *Updater) Create(reference git.ReferenceName, value string) error {
+	_, err := fmt.Fprintf(u.cmd, "create %s\x00%s\x00", reference.String(), value)
 	return err
 }
 
 // Update commands the reference to be updated to point at the sha specified in
 // newvalue
-func (u *Updater) Update(ref, newvalue, oldvalue string) error {
-	_, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00%s\x00", ref, newvalue, oldvalue)
+func (u *Updater) Update(reference git.ReferenceName, newvalue, oldvalue string) error {
+	_, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00%s\x00", reference.String(), newvalue, oldvalue)
 	return err
 }
 
 // Delete commands the reference to be removed from the repository
-func (u *Updater) Delete(ref string) error {
-	_, err := fmt.Fprintf(u.cmd, "delete %s\x00\x00", ref)
+func (u *Updater) Delete(reference git.ReferenceName) error {
+	_, err := fmt.Fprintf(u.cmd, "delete %s\x00\x00", reference.String())
 	return err
 }
 
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 905b9e470..a7e1d3cff 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -52,7 +52,7 @@ func TestCreate(t *testing.T) {
 	ref := "refs/heads/_create"
 	sha := headCommit.Id
 
-	require.NoError(t, updater.Create(ref, sha))
+	require.NoError(t, updater.Create(git.ReferenceName(ref), sha))
 	require.NoError(t, updater.Wait())
 
 	// check the ref was created
@@ -80,7 +80,7 @@ func TestUpdate(t *testing.T) {
 	require.NoError(t, logErr)
 	require.NotEqual(t, commit.Id, sha, "%s points to HEAD: %s in the test repository", ref, sha)
 
-	require.NoError(t, updater.Update(ref, sha, ""))
+	require.NoError(t, updater.Update(git.ReferenceName(ref), sha, ""))
 	require.NoError(t, updater.Wait())
 
 	// check the ref was updated
@@ -91,7 +91,7 @@ func TestUpdate(t *testing.T) {
 	// since ref has been updated to HEAD, we know that it does not point to HEAD^. So, HEAD^ is an invalid "old value" for updating ref
 	parentCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD^")
 	require.NoError(t, err)
-	require.Error(t, updater.Update(ref, parentCommit.Id, parentCommit.Id))
+	require.Error(t, updater.Update(git.ReferenceName(ref), parentCommit.Id, parentCommit.Id))
 
 	// check the ref was not updated
 	commit, logErr = log.GetCommit(ctx, locator, testRepo, ref)
@@ -108,7 +108,7 @@ func TestDelete(t *testing.T) {
 
 	ref := "refs/heads/feature"
 
-	require.NoError(t, updater.Delete(ref))
+	require.NoError(t, updater.Delete(git.ReferenceName(ref)))
 	require.NoError(t, updater.Wait())
 
 	locator := config.NewLocator(config.Config)
@@ -132,7 +132,7 @@ func TestBulkOperation(t *testing.T) {
 
 	for i := 0; i < 1000; i++ {
 		ref := fmt.Sprintf("refs/head/_test_%d", i)
-		require.NoError(t, updater.Create(ref, headCommit.Id), "Failed to create ref %d", i)
+		require.NoError(t, updater.Create(git.ReferenceName(ref), headCommit.Id), "Failed to create ref %d", i)
 	}
 
 	require.NoError(t, updater.Wait())
@@ -157,7 +157,7 @@ func TestContextCancelAbortsRefChanges(t *testing.T) {
 
 	ref := "refs/heads/_shouldnotexist"
 
-	require.NoError(t, updater.Create(ref, headCommit.Id))
+	require.NoError(t, updater.Create(git.ReferenceName(ref), headCommit.Id))
 
 	// Force the update-ref process to terminate early
 	childCancel()
@@ -184,7 +184,7 @@ func TestUpdater_closingStdinAbortsChanges(t *testing.T) {
 
 	updater, err := New(ctx, testRepo)
 	require.NoError(t, err)
-	require.NoError(t, updater.Create(ref, headCommit.Id))
+	require.NoError(t, updater.Create(git.ReferenceName(ref), headCommit.Id))
 
 	// Note that we call `Wait()` on the command, not on the updater. This
 	// circumvents our usual semantics of sending "commit" and thus
diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
index 5ee728709..f2add93cf 100644
--- a/internal/gitaly/service/cleanup/internalrefs/cleaner.go
+++ b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
@@ -117,7 +117,7 @@ func (c *Cleaner) processEntry(ctx context.Context, oldSHA, newSHA string) error
 
 	// Remove the internal refs pointing to oldSHA
 	for _, ref := range refs {
-		if err := c.updater.Delete(ref.String()); err != nil {
+		if err := c.updater.Delete(ref); err != nil {
 			return err
 		}
 	}
diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go
index 2d023a1ae..431c14ff3 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -93,7 +93,7 @@ func (s *Server) updateReferenceWithHooks(ctx context.Context, repo *gitalypb.Re
 		return err
 	}
 
-	if err := updater.Update(reference, newrev, oldrev); err != nil {
+	if err := updater.Update(git.ReferenceName(reference), newrev, oldrev); err != nil {
 		return err
 	}
 
diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go
index 7d4d7ef8c..c205e7dca 100644
--- a/internal/gitaly/service/ref/delete_refs.go
+++ b/internal/gitaly/service/ref/delete_refs.go
@@ -33,7 +33,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest)
 	}
 
 	for _, ref := range refnames {
-		if err := updater.Delete(ref.String()); err != nil {
+		if err := updater.Delete(ref); err != nil {
 			return &gitalypb.DeleteRefsResponse{GitError: err.Error()}, nil
 		}
 	}
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index fbee01fbf..874902408 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
@@ -97,7 +98,7 @@ func TestFindAllBranchNamesVeryLargeResponse(t *testing.T) {
 		refName := fmt.Sprintf("refs/heads/test-%0100d", i)
 		require.True(t, len(refName) > refSizeLowerBound, "ref %q must be larger than %d", refName, refSizeLowerBound)
 
-		require.NoError(t, updater.Create(refName, "HEAD"))
+		require.NoError(t, updater.Create(git.ReferenceName(refName), "HEAD"))
 		testRefs = append(testRefs, refName)
 	}
 
diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go
index 274dd4bf7..be58f282c 100644
--- a/internal/gitaly/service/repository/optimize_test.go
+++ b/internal/gitaly/service/repository/optimize_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/stats"
 	"gitlab.com/gitlab-org/gitaly/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
@@ -105,7 +106,7 @@ func TestOptimizeRepository(t *testing.T) {
 
 	for _, blobID := range blobIDs {
 		commitID := testhelper.CommitBlobWithName(t, testRepoPath, blobID, blobID, "adding another blob....")
-		require.NoError(t, updater.Create("refs/heads/"+blobID, commitID))
+		require.NoError(t, updater.Create(git.ReferenceName("refs/heads/"+blobID), commitID))
 	}
 
 	require.NoError(t, updater.Wait())
diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go
index edf7a724b..d7d33e8dc 100644
--- a/internal/gitaly/service/repository/write_ref.go
+++ b/internal/gitaly/service/repository/write_ref.go
@@ -51,7 +51,7 @@ func updateRef(ctx context.Context, req *gitalypb.WriteRefRequest) error {
 	if err != nil {
 		return fmt.Errorf("error when running creating new updater: %v", err)
 	}
-	if err = u.Update(string(req.GetRef()), string(req.GetRevision()), string(req.GetOldRevision())); err != nil {
+	if err = u.Update(git.ReferenceName(req.GetRef()), string(req.GetRevision()), string(req.GetOldRevision())); err != nil {
 		return fmt.Errorf("error when creating update-ref command: %v", err)
 	}
 	if err = u.Wait(); err != nil {