From 0072c23e65530815a95d2287654551a49d7df3fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
 <avarab@gmail.com>
Date: Mon, 14 Dec 2020 18:23:02 +0100
Subject: [PATCH] Updater: dispatch to update-ref create/delete/update

This doesn't make any difference in the result as git does the same
dispatching internally, but makes things a bit more obvious if you're
tracing the output.

While I'm at it move the oldid-less Delete() use to use oldid. It was
easy, and making the API safe by default makes sense.
---
 internal/git/updateref/updateref.go            | 18 ++++++++++++++++--
 internal/git/updateref/updateref_test.go       |  4 +++-
 .../service/cleanup/internalrefs/cleaner.go    |  2 +-
 .../service/operations/update_with_hooks.go    |  2 +-
 internal/gitaly/service/ref/delete_refs.go     |  2 +-
 5 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index 7230acc05e4..f820986262e 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -65,6 +65,20 @@ func New(ctx context.Context, repo repository.GitRepo, opts ...UpdaterOpt) (*Upd
 	return &Updater{repo: repo, cmd: cmd}, nil
 }
 
+// CreateUpdateDelete dispatches to Create()/Update()/Delete() as needed
+//
+// The underlying "update-ref" does the same dispatching depending on
+// what it get passed.
+func (u *Updater) CreateUpdateDelete(ref, newvalue, oldvalue string) error {
+	if newvalue == git.NullSHA {
+		return u.Delete(ref, oldvalue)
+	} else if oldvalue == git.NullSHA {
+		return u.Create(ref, newvalue)
+	} else {
+		return u.Update(ref, newvalue, oldvalue)
+	}
+}
+
 // 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)
@@ -79,8 +93,8 @@ func (u *Updater) Update(ref, newvalue, oldvalue string) error {
 }
 
 // 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(ref string, oldvalue string) error {
+	_, err := fmt.Fprintf(u.cmd, "delete %s\x00%s\x00", ref, oldvalue)
 	return err
 }
 
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 270c04de340..ba6074a2754 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -105,7 +105,9 @@ func TestDelete(t *testing.T) {
 
 	ref := "refs/heads/feature"
 
-	require.NoError(t, updater.Delete(ref))
+	refValue, err := git.NewRepository(testRepo).GetReference(ctx, ref)
+	require.NoError(t, err)
+	require.NoError(t, updater.Delete(ref, refValue.Target))
 	require.NoError(t, updater.Wait())
 
 	// check the ref was removed
diff --git a/internal/gitaly/service/cleanup/internalrefs/cleaner.go b/internal/gitaly/service/cleanup/internalrefs/cleaner.go
index 8e75649da86..85f4c763bb6 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); err != nil {
+		if err := c.updater.Delete(ref, git.NullSHA); 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 13446623622..f14be027b13 100644
--- a/internal/gitaly/service/operations/update_with_hooks.go
+++ b/internal/gitaly/service/operations/update_with_hooks.go
@@ -81,7 +81,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.CreateUpdateDelete(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 6444c5743a0..c4d7716901e 100644
--- a/internal/gitaly/service/ref/delete_refs.go
+++ b/internal/gitaly/service/ref/delete_refs.go
@@ -34,7 +34,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest)
 	}
 
 	for _, ref := range refnames {
-		if err := updater.Delete(ref); err != nil {
+		if err := updater.Delete(ref, git.NullSHA); err != nil {
 			return &gitalypb.DeleteRefsResponse{GitError: err.Error()}, nil
 		}
 	}
-- 
GitLab