git: Move `ValidateObjectID()` into `ObjectHash` structure

Move the `ValidateObjectID()` function into the `ObjectHash` structure
to make it dependent on the hash function used.

8 files changed, 67 insertions, 55 deletions

diff --git a/internal/git/object_id.go b/internal/git/object_id.go
index d47efc7c8..5cc0095d7 100644
--- a/internal/git/object_id.go
+++ b/internal/git/object_id.go
@@ -86,13 +86,6 @@ func (oid ObjectID) Revision() Revision {
 	return Revision(oid.String())
 }
 
-// ValidateObjectID checks if id is a syntactically correct object ID. Abbreviated
-// object IDs are not deemed to be valid. Returns an ErrInvalidObjectID if the
-// id is not valid.
-func ValidateObjectID(id string) error {
-	return ObjectHashSHA1.ValidateHex(id)
-}
-
 // IsZeroOID is a shortcut for `something == git.ZeroOID.String()`
 func (oid ObjectID) IsZeroOID() bool {
 	return string(oid) == string(ZeroOID)
diff --git a/internal/git/object_id_test.go b/internal/git/object_id_test.go
index 3d4374134..6a0ac6e85 100644
--- a/internal/git/object_id_test.go
+++ b/internal/git/object_id_test.go
@@ -12,50 +12,69 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestValidateObjectID(t *testing.T) {
-	for _, tc := range []struct {
-		desc  string
-		oid   string
-		valid bool
+func TestObjectHash_ValidateHex(t *testing.T) {
+	for _, hash := range []struct {
+		desc     string
+		hash     ObjectHash
+		validHex string
 	}{
 		{
-			desc:  "valid object ID",
-			oid:   "356e7793f9654d51dfb27312a1464062bceb9fa3",
-			valid: true,
-		},
-		{
-			desc:  "object ID with non-hex characters fails",
-			oid:   "x56e7793f9654d51dfb27312a1464062bceb9fa3",
-			valid: false,
+			desc:     "SHA1",
+			hash:     ObjectHashSHA1,
+			validHex: "356e7793f9654d51dfb27312a1464062bceb9fa3",
 		},
 		{
-			desc:  "object ID with upper-case letters fails",
-			oid:   "356E7793F9654D51DFB27312A1464062BCEB9FA3",
-			valid: false,
-		},
-		{
-			desc:  "too short object ID fails",
-			oid:   "356e7793f9654d51dfb27312a1464062bceb9fa",
-			valid: false,
-		},
-		{
-			desc:  "too long object ID fails",
-			oid:   "356e7793f9654d51dfb27312a1464062bceb9fa33",
-			valid: false,
-		},
-		{
-			desc:  "empty string fails",
-			oid:   "",
-			valid: false,
+			desc:     "SHA256",
+			hash:     ObjectHashSHA256,
+			validHex: "aec070645fe53ee3b3763059376134f058cc337247c978add178b6ccdfb0019f",
 		},
 	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			err := ValidateObjectID(tc.oid)
-			if tc.valid {
-				require.NoError(t, err)
-			} else {
-				require.Error(t, err)
-				require.EqualError(t, err, fmt.Sprintf("invalid object ID: %q", tc.oid))
+		t.Run(hash.desc, func(t *testing.T) {
+			for _, tc := range []struct {
+				desc  string
+				hex   string
+				valid bool
+			}{
+				{
+					desc:  "valid object ID",
+					hex:   hash.validHex,
+					valid: true,
+				},
+				{
+					desc:  "object ID with non-hex characters fails",
+					hex:   "x" + hash.validHex[1:],
+					valid: false,
+				},
+				{
+					desc:  "object ID with upper-case letters fails",
+					hex:   strings.ToUpper(hash.validHex),
+					valid: false,
+				},
+				{
+					desc:  "too short object ID fails",
+					hex:   hash.validHex[:len(hash.validHex)-1],
+					valid: false,
+				},
+				{
+					desc:  "too long object ID fails",
+					hex:   hash.validHex + "3",
+					valid: false,
+				},
+				{
+					desc:  "empty string fails",
+					hex:   "",
+					valid: false,
+				},
+			} {
+				t.Run(tc.desc, func(t *testing.T) {
+					err := hash.hash.ValidateHex(tc.hex)
+					if tc.valid {
+						require.NoError(t, err)
+					} else {
+						require.Error(t, err)
+						require.EqualError(t, err, fmt.Sprintf("invalid object ID: %q", tc.hex))
+					}
+				})
 			}
 		})
 	}
diff --git a/internal/git/updateref/update_with_hooks.go b/internal/git/updateref/update_with_hooks.go
index 23155bc40..d100f1b40 100644
--- a/internal/git/updateref/update_with_hooks.go
+++ b/internal/git/updateref/update_with_hooks.go
@@ -162,10 +162,10 @@ func (u *UpdaterWithHooks) UpdateReference(
 	if reference == "" {
 		return fmt.Errorf("reference cannot be empty")
 	}
-	if err := git.ValidateObjectID(oldrev.String()); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(oldrev.String()); err != nil {
 		return fmt.Errorf("validating old value: %w", err)
 	}
-	if err := git.ValidateObjectID(newrev.String()); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(newrev.String()); err != nil {
 		return fmt.Errorf("validating new value: %w", err)
 	}
 
diff --git a/internal/gitaly/hook/update.go b/internal/gitaly/hook/update.go
index 564e1550a..81251f6ca 100644
--- a/internal/gitaly/hook/update.go
+++ b/internal/gitaly/hook/update.go
@@ -39,10 +39,10 @@ func (m *GitLabHookManager) updateHook(ctx context.Context, payload git.HooksPay
 	if ref == "" {
 		return helper.ErrInternalf("hook got no reference")
 	}
-	if err := git.ValidateObjectID(oldValue); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(oldValue); err != nil {
 		return helper.ErrInternalf("hook got invalid old value: %w", err)
 	}
-	if err := git.ValidateObjectID(newValue); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(newValue); err != nil {
 		return helper.ErrInternalf("hook got invalid new value: %w", err)
 	}
 	if payload.UserDetails == nil {
diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go
index 17adf5c8a..9045e428c 100644
--- a/internal/gitaly/service/commit/commit_signatures.go
+++ b/internal/gitaly/service/commit/commit_signatures.go
@@ -138,7 +138,7 @@ func validateGetCommitSignaturesRequest(request *gitalypb.GetCommitSignaturesReq
 
 	// Do not support shorthand or invalid commit SHAs
 	for _, commitID := range request.CommitIds {
-		if err := git.ValidateObjectID(commitID); err != nil {
+		if err := git.ObjectHashSHA1.ValidateHex(commitID); err != nil {
 			return err
 		}
 	}
diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go
index f764d3593..c6984586d 100644
--- a/internal/gitaly/service/conflicts/resolve_conflicts.go
+++ b/internal/gitaly/service/conflicts/resolve_conflicts.go
@@ -168,8 +168,8 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader
 		authorDate = header.Timestamp.AsTime()
 	}
 
-	if git.ValidateObjectID(header.GetOurCommitOid()) != nil ||
-		git.ValidateObjectID(header.GetTheirCommitOid()) != nil {
+	if git.ObjectHashSHA1.ValidateHex(header.GetOurCommitOid()) != nil ||
+		git.ObjectHashSHA1.ValidateHex(header.GetTheirCommitOid()) != nil {
 		return errors.New("Rugged::InvalidError: unable to parse OID - contains invalid characters")
 	}
 
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index ba4514fc1..a1e4b2ad1 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -434,7 +434,7 @@ func validateUserCommitFilesHeader(header *gitalypb.UserCommitFilesRequestHeader
 
 	startSha := header.GetStartSha()
 	if len(startSha) > 0 {
-		err := git.ValidateObjectID(startSha)
+		err := git.ObjectHashSHA1.ValidateHex(startSha)
 		if err != nil {
 			return err
 		}
diff --git a/internal/gitaly/service/ref/refnames_containing.go b/internal/gitaly/service/ref/refnames_containing.go
index 278316afc..420dbda83 100644
--- a/internal/gitaly/service/ref/refnames_containing.go
+++ b/internal/gitaly/service/ref/refnames_containing.go
@@ -15,7 +15,7 @@ import (
 // ListBranchNamesContainingCommit returns a maximum of in.GetLimit() Branch names
 // which contain the SHA1 passed as argument
 func (s *server) ListBranchNamesContainingCommit(in *gitalypb.ListBranchNamesContainingCommitRequest, stream gitalypb.RefService_ListBranchNamesContainingCommitServer) error {
-	if err := git.ValidateObjectID(in.GetCommitId()); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil {
 		return helper.ErrInvalidArgument(err)
 	}
 
@@ -58,7 +58,7 @@ func (bs *branchNamesContainingCommitSender) Send() error {
 // ListTagNamesContainingCommit returns a maximum of in.GetLimit() Tag names
 // which contain the SHA1 passed as argument
 func (s *server) ListTagNamesContainingCommit(in *gitalypb.ListTagNamesContainingCommitRequest, stream gitalypb.RefService_ListTagNamesContainingCommitServer) error {
-	if err := git.ValidateObjectID(in.GetCommitId()); err != nil {
+	if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil {
 		return helper.ErrInvalidArgument(err)
 	}