Improve ObjectFormat interface (#28496)

The 4 functions are duplicated, especially as interface methods. I think
we just need to keep `MustID` the only one and remove other 3.

```
MustID(b []byte) ObjectID
MustIDFromString(s string) ObjectID
NewID(b []byte) (ObjectID, error)
NewIDFromString(s string) (ObjectID, error)
```

Introduced the new interfrace method `ComputeHash` which will replace
the interface `HasherInterface`. Now we don't need to keep two
interfaces.

Reintroduced `git.NewIDFromString` and `git.MustIDFromString`. The new
function will detect the hash length to decide which objectformat of it.
If it's 40, then it's SHA1. If it's 64, then it's SHA256. This will be
right if the commitID is a full one. So the parameter should be always a
full commit id.

@AdamMajer Please review.

Author: lunny

cmd/hook.go

@@ -377,7 +377,7 @@ Gitea or set your environment appropriately.`, "")
 		newCommitIDs[count] = string(fields[1])
 		refFullNames[count] = git.RefName(fields[2])
 
-		commitID, _ := git.IDFromString(newCommitIDs[count])
+		commitID, _ := git.NewIDFromString(newCommitIDs[count])
 		if refFullNames[count] == git.BranchPrefix+"master" && !commitID.IsZero() && count == total {
 			masterPushed = true
 		}
@@ -671,7 +671,7 @@ Gitea or set your environment appropriately.`, "")
 		if err != nil {
 			return err
 		}
-		commitID, _ := git.IDFromString(rs.OldOID)
+		commitID, _ := git.NewIDFromString(rs.OldOID)
 		if !commitID.IsZero() {
 			err = writeDataPktLine(ctx, os.Stdout, []byte("option old-oid "+rs.OldOID))
 			if err != nil {

models/git/branch_test.go

@@ -30,9 +30,8 @@ func TestAddDeletedBranch(t *testing.T) {
 	secondBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "branch2"})
 	assert.True(t, secondBranch.IsDeleted)
 
-	objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName)
 	commit := &git.Commit{
-		ID:            objectFormat.MustIDFromString(secondBranch.CommitID),
+		ID:            git.MustIDFromString(secondBranch.CommitID),
 		CommitMessage: secondBranch.CommitMessage,
 		Committer: &git.Signature{
 			When: secondBranch.CommitTime.AsLocalTime(),

models/git/commit_status.go

@@ -114,7 +114,7 @@ WHEN NOT MATCHED
 
 // GetNextCommitStatusIndex retried 3 times to generate a resource index
 func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) {
-	_, err := git.IDFromString(sha)
+	_, err := git.NewIDFromString(sha)
 	if err != nil {
 		return 0, git.ErrInvalidSHA{SHA: sha}
 	}

modules/git/commit_info_nogogit.go

@@ -153,7 +153,7 @@ func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string,
 		if typ != "commit" {
 			return nil, fmt.Errorf("unexpected type: %s for commit id: %s", typ, commitID)
 		}
-		c, err = CommitFromReader(commit.repo, commit.ID.Type().MustIDFromString(commitID), io.LimitReader(batchReader, size))
+		c, err = CommitFromReader(commit.repo, MustIDFromString(commitID), io.LimitReader(batchReader, size))
 		if err != nil {
 			return nil, err
 		}

modules/git/commit_reader.go

@@ -71,10 +71,10 @@ readLoop:
 
 			switch string(split[0]) {
 			case "tree":
-				commit.Tree = *NewTree(gitRepo, objectID.Type().MustIDFromString(string(data)))
+				commit.Tree = *NewTree(gitRepo, MustIDFromString(string(data)))
 				_, _ = payloadSB.Write(line)
 			case "parent":
-				commit.Parents = append(commit.Parents, objectID.Type().MustIDFromString(string(data)))
+				commit.Parents = append(commit.Parents, MustIDFromString(string(data)))
 				_, _ = payloadSB.Write(line)
 			case "author":
 				commit.Author = &Signature{}

modules/git/commit_test.go

@@ -135,8 +135,8 @@ func TestHasPreviousCommit(t *testing.T) {
 	commit, err := repo.GetCommit("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0")
 	assert.NoError(t, err)
 
-	parentSHA := repo.objectFormat.MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2")
-	notParentSHA := repo.objectFormat.MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3")
+	parentSHA := MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2")
+	notParentSHA := MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3")
 
 	haz, err := commit.HasPreviousCommit(parentSHA)
 	assert.NoError(t, err)

modules/git/last_commit_cache.go

@@ -92,11 +92,7 @@ func (c *LastCommitCache) Get(ref, entryPath string) (*Commit, error) {
 
 // GetCommitByPath gets the last commit for the entry in the provided commit
 func (c *LastCommitCache) GetCommitByPath(commitID, entryPath string) (*Commit, error) {
-	objectFormat, err := c.repo.GetObjectFormat()
-	if err != nil {
-		return nil, err
-	}
-	sha, err := objectFormat.NewIDFromString(commitID)
+	sha, err := NewIDFromString(commitID)
 	if err != nil {
 		return nil, err
 	}

modules/git/object_format.go

@@ -6,6 +6,7 @@ package git
 import (
 	"crypto/sha1"
 	"regexp"
+	"strconv"
 )
 
 // sha1Pattern can be used to determine if a string is an valid sha
@@ -20,14 +21,12 @@ type ObjectFormat interface {
 	EmptyTree() ObjectID
 	// FullLength is the length of the hash's hex string
 	FullLength() int
-
+	// IsValid returns true if the input is a valid hash
 	IsValid(input string) bool
+	// MustID creates a new ObjectID from a byte slice
 	MustID(b []byte) ObjectID
-	MustIDFromString(s string) ObjectID
-	NewID(b []byte) (ObjectID, error)
-	NewIDFromString(s string) (ObjectID, error)
-
-	NewHasher() HasherInterface
+	// ComputeHash compute the hash for a given ObjectType and content
+	ComputeHash(t ObjectType, content []byte) ObjectID
 }
 
 type Sha1ObjectFormatImpl struct{}
@@ -59,20 +58,18 @@ func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID {
 	return &id
 }
 
-func (h Sha1ObjectFormatImpl) MustIDFromString(s string) ObjectID {
-	return MustIDFromString(h, s)
-}
-
-func (h Sha1ObjectFormatImpl) NewID(b []byte) (ObjectID, error) {
-	return IDFromRaw(h, b)
-}
-
-func (h Sha1ObjectFormatImpl) NewIDFromString(s string) (ObjectID, error) {
-	return genericIDFromString(h, s)
-}
-
-func (h Sha1ObjectFormatImpl) NewHasher() HasherInterface {
-	return &Sha1Hasher{sha1.New()}
+// ComputeHash compute the hash for a given ObjectType and content
+func (h Sha1ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID {
+	hasher := sha1.New()
+	_, _ = hasher.Write(t.Bytes())
+	_, _ = hasher.Write([]byte(" "))
+	_, _ = hasher.Write([]byte(strconv.FormatInt(int64(len(content)), 10)))
+	_, _ = hasher.Write([]byte{0})
+
+	// HashSum generates a SHA1 for the provided hash
+	var sha1 Sha1Hash
+	copy(sha1[:], hasher.Sum(nil))
+	return &sha1
 }
 
 var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{}

modules/git/object_id.go

@@ -6,11 +6,7 @@ package git
 import (
 	"bytes"
 	"encoding/hex"
-	"errors"
 	"fmt"
-	"hash"
-	"strconv"
-	"strings"
 )
 
 type ObjectID interface {
@@ -35,94 +31,54 @@ func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat }
 
 var _ ObjectID = &Sha1Hash{}
 
-// EmptyObjectID creates a new ObjectID from an object format hash name
-func EmptyObjectID(objectFormatName string) (ObjectID, error) {
-	objectFormat := ObjectFormatFromName(objectFormatName)
-	if objectFormat != nil {
-		return objectFormat.EmptyObjectID(), nil
+func MustIDFromString(hexHash string) ObjectID {
+	id, err := NewIDFromString(hexHash)
+	if err != nil {
+		panic(err)
 	}
-
-	return nil, errors.New("unsupported hash type")
+	return id
 }
 
-func IDFromRaw(h ObjectFormat, b []byte) (ObjectID, error) {
-	if len(b) != h.FullLength()/2 {
-		return h.EmptyObjectID(), fmt.Errorf("length must be %d: %v", h.FullLength(), b)
+func NewIDFromString(hexHash string) (ObjectID, error) {
+	var theObjectFormat ObjectFormat
+	for _, objectFormat := range SupportedObjectFormats {
+		if len(hexHash) == objectFormat.FullLength() {
+			theObjectFormat = objectFormat
+			break
+		}
 	}
-	return h.MustID(b), nil
-}
 
-func MustIDFromString(h ObjectFormat, s string) ObjectID {
-	b, _ := hex.DecodeString(s)
-	return h.MustID(b)
-}
-
-func genericIDFromString(h ObjectFormat, s string) (ObjectID, error) {
-	s = strings.TrimSpace(s)
-	if len(s) != h.FullLength() {
-		return h.EmptyObjectID(), fmt.Errorf("length must be %d: %s", h.FullLength(), s)
+	if theObjectFormat == nil {
+		return nil, fmt.Errorf("length %d has no matched object format: %s", len(hexHash), hexHash)
 	}
-	b, err := hex.DecodeString(s)
+
+	b, err := hex.DecodeString(hexHash)
 	if err != nil {
-		return h.EmptyObjectID(), err
+		return nil, err
 	}
-	return h.NewID(b)
-}
 
-func IDFromString(hexHash string) (ObjectID, error) {
-	for _, objectFormat := range SupportedObjectFormats {
-		if len(hexHash) == objectFormat.FullLength() {
-			return objectFormat.NewIDFromString(hexHash)
-		}
+	if len(b) != theObjectFormat.FullLength()/2 {
+		return theObjectFormat.EmptyObjectID(), fmt.Errorf("length must be %d: %v", theObjectFormat.FullLength(), b)
 	}
-
-	return nil, fmt.Errorf("invalid hash hex string: '%s' len: %d", hexHash, len(hexHash))
+	return theObjectFormat.MustID(b), nil
 }
 
 func IsEmptyCommitID(commitID string) bool {
 	if commitID == "" {
 		return true
 	}
 
-	id, err := IDFromString(commitID)
+	id, err := NewIDFromString(commitID)
 	if err != nil {
 		return false
 	}
 
 	return id.IsZero()
 }
 
-// HasherInterface is a struct that will generate a Hash
-type HasherInterface interface {
-	hash.Hash
-
-	HashSum() ObjectID
-}
-
-type Sha1Hasher struct {
-	hash.Hash
-}
-
 // ComputeBlobHash compute the hash for a given blob content
 func ComputeBlobHash(hashType ObjectFormat, content []byte) ObjectID {
-	return ComputeHash(hashType, ObjectBlob, content)
-}
-
-// ComputeHash compute the hash for a given ObjectType and content
-func ComputeHash(hashType ObjectFormat, t ObjectType, content []byte) ObjectID {
-	h := hashType.NewHasher()
-	_, _ = h.Write(t.Bytes())
-	_, _ = h.Write([]byte(" "))
-	_, _ = h.Write([]byte(strconv.FormatInt(int64(len(content)), 10)))
-	_, _ = h.Write([]byte{0})
-	return h.HashSum()
-}
-
-// HashSum generates a SHA1 for the provided hash
-func (h *Sha1Hasher) HashSum() ObjectID {
-	var sha1 Sha1Hash
-	copy(sha1[:], h.Hash.Sum(nil))
-	return &sha1
+	return hashType.ComputeHash(ObjectBlob, content)
 }
 
 type ErrInvalidSHA struct {

modules/git/parse_gogit.go

@@ -57,7 +57,7 @@ func parseTreeEntries(data []byte, ptree *Tree) ([]*TreeEntry, error) {
 			return nil, fmt.Errorf("Invalid ls-tree output: %s", string(data))
 		}
 		var err error
-		entry.ID, err = IDFromString(string(data[pos : pos+hash.Size*2]))
+		entry.ID, err = NewIDFromString(string(data[pos : pos+hash.Size*2]))
 		if err != nil {
 			return nil, fmt.Errorf("invalid ls-tree output: %w", err)
 		}