Skip to content

Commit 4eb2a29

Browse files
lunnylunny
authored
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.
1 parent 128eac9 commit 4eb2a29

39 files changed

+109
-168
lines changed

‎cmd/hook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ Gitea or set your environment appropriately.`, "")
377377
newCommitIDs[count] = string(fields[1])
378378
refFullNames[count] = git.RefName(fields[2])
379379

380-
commitID, _ := git.IDFromString(newCommitIDs[count])
380+
commitID, _ := git.NewIDFromString(newCommitIDs[count])
381381
if refFullNames[count] == git.BranchPrefix+"master" && !commitID.IsZero() && count == total {
382382
masterPushed = true
383383
}
@@ -671,7 +671,7 @@ Gitea or set your environment appropriately.`, "")
671671
if err != nil {
672672
return err
673673
}
674-
commitID, _ := git.IDFromString(rs.OldOID)
674+
commitID, _ := git.NewIDFromString(rs.OldOID)
675675
if !commitID.IsZero() {
676676
err = writeDataPktLine(ctx, os.Stdout, []byte("option old-oid "+rs.OldOID))
677677
if err != nil {

‎models/git/branch_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ func TestAddDeletedBranch(t *testing.T) {
3030
secondBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "branch2"})
3131
assert.True(t, secondBranch.IsDeleted)
3232

33-
objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName)
3433
commit := &git.Commit{
35-
ID: objectFormat.MustIDFromString(secondBranch.CommitID),
34+
ID: git.MustIDFromString(secondBranch.CommitID),
3635
CommitMessage: secondBranch.CommitMessage,
3736
Committer: &git.Signature{
3837
When: secondBranch.CommitTime.AsLocalTime(),

‎models/git/commit_status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ WHEN NOT MATCHED
114114

115115
// GetNextCommitStatusIndex retried 3 times to generate a resource index
116116
func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) {
117-
_, err := git.IDFromString(sha)
117+
_, err := git.NewIDFromString(sha)
118118
if err != nil {
119119
return 0, git.ErrInvalidSHA{SHA: sha}
120120
}

‎modules/git/commit_info_nogogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string,
153153
if typ != "commit" {
154154
return nil, fmt.Errorf("unexpected type: %s for commit id: %s", typ, commitID)
155155
}
156-
c, err = CommitFromReader(commit.repo, commit.ID.Type().MustIDFromString(commitID), io.LimitReader(batchReader, size))
156+
c, err = CommitFromReader(commit.repo, MustIDFromString(commitID), io.LimitReader(batchReader, size))
157157
if err != nil {
158158
return nil, err
159159
}

‎modules/git/commit_reader.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,10 @@ readLoop:
7171

7272
switch string(split[0]) {
7373
case "tree":
74-
commit.Tree = *NewTree(gitRepo, objectID.Type().MustIDFromString(string(data)))
74+
commit.Tree = *NewTree(gitRepo, MustIDFromString(string(data)))
7575
_, _ = payloadSB.Write(line)
7676
case "parent":
77-
commit.Parents = append(commit.Parents, objectID.Type().MustIDFromString(string(data)))
77+
commit.Parents = append(commit.Parents, MustIDFromString(string(data)))
7878
_, _ = payloadSB.Write(line)
7979
case "author":
8080
commit.Author = &Signature{}

‎modules/git/commit_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ func TestHasPreviousCommit(t *testing.T) {
135135
commit, err := repo.GetCommit("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0")
136136
assert.NoError(t, err)
137137

138-
parentSHA := repo.objectFormat.MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2")
139-
notParentSHA := repo.objectFormat.MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3")
138+
parentSHA := MustIDFromString("8d92fc957a4d7cfd98bc375f0b7bb189a0d6c9f2")
139+
notParentSHA := MustIDFromString("2839944139e0de9737a044f78b0e4b40d989a9e3")
140140

141141
haz, err := commit.HasPreviousCommit(parentSHA)
142142
assert.NoError(t, err)

‎modules/git/last_commit_cache.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,7 @@ func (c *LastCommitCache) Get(ref, entryPath string) (*Commit, error) {
9292

9393
// GetCommitByPath gets the last commit for the entry in the provided commit
9494
func (c *LastCommitCache) GetCommitByPath(commitID, entryPath string) (*Commit, error) {
95-
objectFormat, err := c.repo.GetObjectFormat()
96-
if err != nil {
97-
return nil, err
98-
}
99-
sha, err := objectFormat.NewIDFromString(commitID)
95+
sha, err := NewIDFromString(commitID)
10096
if err != nil {
10197
return nil, err
10298
}

‎modules/git/object_format.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package git
66
import (
77
"crypto/sha1"
88
"regexp"
9+
"strconv"
910
)
1011

1112
// sha1Pattern can be used to determine if a string is an valid sha
@@ -20,14 +21,12 @@ type ObjectFormat interface {
2021
EmptyTree() ObjectID
2122
// FullLength is the length of the hash's hex string
2223
FullLength() int
23-
24+
// IsValid returns true if the input is a valid hash
2425
IsValid(input string) bool
26+
// MustID creates a new ObjectID from a byte slice
2527
MustID(b []byte) ObjectID
26-
MustIDFromString(s string) ObjectID
27-
NewID(b []byte) (ObjectID, error)
28-
NewIDFromString(s string) (ObjectID, error)
29-
30-
NewHasher() HasherInterface
28+
// ComputeHash compute the hash for a given ObjectType and content
29+
ComputeHash(t ObjectType, content []byte) ObjectID
3130
}
3231

3332
type Sha1ObjectFormatImpl struct{}
@@ -59,20 +58,18 @@ func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID {
5958
return &id
6059
}
6160

62-
func (h Sha1ObjectFormatImpl) MustIDFromString(s string) ObjectID {
63-
return MustIDFromString(h, s)
64-
}
65-
66-
func (h Sha1ObjectFormatImpl) NewID(b []byte) (ObjectID, error) {
67-
return IDFromRaw(h, b)
68-
}
69-
70-
func (h Sha1ObjectFormatImpl) NewIDFromString(s string) (ObjectID, error) {
71-
return genericIDFromString(h, s)
72-
}
73-
74-
func (h Sha1ObjectFormatImpl) NewHasher() HasherInterface {
75-
return &Sha1Hasher{sha1.New()}
61+
// ComputeHash compute the hash for a given ObjectType and content
62+
func (h Sha1ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID {
63+
hasher := sha1.New()
64+
_, _ = hasher.Write(t.Bytes())
65+
_, _ = hasher.Write([]byte(" "))
66+
_, _ = hasher.Write([]byte(strconv.FormatInt(int64(len(content)), 10)))
67+
_, _ = hasher.Write([]byte{0})
68+
69+
// HashSum generates a SHA1 for the provided hash
70+
var sha1 Sha1Hash
71+
copy(sha1[:], hasher.Sum(nil))
72+
return &sha1
7673
}
7774

7875
var Sha1ObjectFormat ObjectFormat = Sha1ObjectFormatImpl{}

‎modules/git/object_id.go

Lines changed: 22 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,7 @@ package git
66
import (
77
"bytes"
88
"encoding/hex"
9-
"errors"
109
"fmt"
11-
"hash"
12-
"strconv"
13-
"strings"
1410
)
1511

1612
type ObjectID interface {
@@ -35,94 +31,54 @@ func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat }
3531

3632
var _ ObjectID = &Sha1Hash{}
3733

38-
// EmptyObjectID creates a new ObjectID from an object format hash name
39-
func EmptyObjectID(objectFormatName string) (ObjectID, error) {
40-
objectFormat := ObjectFormatFromName(objectFormatName)
41-
if objectFormat != nil {
42-
return objectFormat.EmptyObjectID(), nil
34+
func MustIDFromString(hexHash string) ObjectID {
35+
id, err := NewIDFromString(hexHash)
36+
if err != nil {
37+
panic(err)
4338
}
44-
45-
return nil, errors.New("unsupported hash type")
39+
return id
4640
}
4741

48-
func IDFromRaw(h ObjectFormat, b []byte) (ObjectID, error) {
49-
if len(b) != h.FullLength()/2 {
50-
return h.EmptyObjectID(), fmt.Errorf("length must be %d: %v", h.FullLength(), b)
42+
func NewIDFromString(hexHash string) (ObjectID, error) {
43+
var theObjectFormat ObjectFormat
44+
for _, objectFormat := range SupportedObjectFormats {
45+
if len(hexHash) == objectFormat.FullLength() {
46+
theObjectFormat = objectFormat
47+
break
48+
}
5149
}
52-
return h.MustID(b), nil
53-
}
5450

55-
func MustIDFromString(h ObjectFormat, s string) ObjectID {
56-
b, _ := hex.DecodeString(s)
57-
return h.MustID(b)
58-
}
59-
60-
func genericIDFromString(h ObjectFormat, s string) (ObjectID, error) {
61-
s = strings.TrimSpace(s)
62-
if len(s) != h.FullLength() {
63-
return h.EmptyObjectID(), fmt.Errorf("length must be %d: %s", h.FullLength(), s)
51+
if theObjectFormat == nil {
52+
return nil, fmt.Errorf("length %d has no matched object format: %s", len(hexHash), hexHash)
6453
}
65-
b, err := hex.DecodeString(s)
54+
55+
b, err := hex.DecodeString(hexHash)
6656
if err != nil {
67-
return h.EmptyObjectID(), err
57+
return nil, err
6858
}
69-
return h.NewID(b)
70-
}
7159

72-
func IDFromString(hexHash string) (ObjectID, error) {
73-
for _, objectFormat := range SupportedObjectFormats {
74-
if len(hexHash) == objectFormat.FullLength() {
75-
return objectFormat.NewIDFromString(hexHash)
76-
}
60+
if len(b) != theObjectFormat.FullLength()/2 {
61+
return theObjectFormat.EmptyObjectID(), fmt.Errorf("length must be %d: %v", theObjectFormat.FullLength(), b)
7762
}
78-
79-
return nil, fmt.Errorf("invalid hash hex string: '%s' len: %d", hexHash, len(hexHash))
63+
return theObjectFormat.MustID(b), nil
8064
}
8165

8266
func IsEmptyCommitID(commitID string) bool {
8367
if commitID == "" {
8468
return true
8569
}
8670

87-
id, err := IDFromString(commitID)
71+
id, err := NewIDFromString(commitID)
8872
if err != nil {
8973
return false
9074
}
9175

9276
return id.IsZero()
9377
}
9478

95-
// HasherInterface is a struct that will generate a Hash
96-
type HasherInterface interface {
97-
hash.Hash
98-
99-
HashSum() ObjectID
100-
}
101-
102-
type Sha1Hasher struct {
103-
hash.Hash
104-
}
105-
10679
// ComputeBlobHash compute the hash for a given blob content
10780
func ComputeBlobHash(hashType ObjectFormat, content []byte) ObjectID {
108-
return ComputeHash(hashType, ObjectBlob, content)
109-
}
110-
111-
// ComputeHash compute the hash for a given ObjectType and content
112-
func ComputeHash(hashType ObjectFormat, t ObjectType, content []byte) ObjectID {
113-
h := hashType.NewHasher()
114-
_, _ = h.Write(t.Bytes())
115-
_, _ = h.Write([]byte(" "))
116-
_, _ = h.Write([]byte(strconv.FormatInt(int64(len(content)), 10)))
117-
_, _ = h.Write([]byte{0})
118-
return h.HashSum()
119-
}
120-
121-
// HashSum generates a SHA1 for the provided hash
122-
func (h *Sha1Hasher) HashSum() ObjectID {
123-
var sha1 Sha1Hash
124-
copy(sha1[:], h.Hash.Sum(nil))
125-
return &sha1
81+
return hashType.ComputeHash(ObjectBlob, content)
12682
}
12783

12884
type ErrInvalidSHA struct {

‎modules/git/parse_gogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func parseTreeEntries(data []byte, ptree *Tree) ([]*TreeEntry, error) {
5757
return nil, fmt.Errorf("Invalid ls-tree output: %s", string(data))
5858
}
5959
var err error
60-
entry.ID, err = IDFromString(string(data[pos : pos+hash.Size*2]))
60+
entry.ID, err = NewIDFromString(string(data[pos : pos+hash.Size*2]))
6161
if err != nil {
6262
return nil, fmt.Errorf("invalid ls-tree output: %w", err)
6363
}

‎modules/git/parse_gogit_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ func TestParseTreeEntries(t *testing.T) {
2828
Input: "100644 blob 61ab7345a1a3bbc590068ccae37b8515cfc5843c 1022\texample/file2.txt\n",
2929
Expected: []*TreeEntry{
3030
{
31-
ID: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"),
31+
ID: MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"),
3232
gogitTreeEntry: &object.TreeEntry{
33-
Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()),
33+
Hash: plumbing.Hash(MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()),
3434
Name: "example/file2.txt",
3535
Mode: filemode.Regular,
3636
},
@@ -44,20 +44,20 @@ func TestParseTreeEntries(t *testing.T) {
4444
"040000 tree 1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8 -\texample\n",
4545
Expected: []*TreeEntry{
4646
{
47-
ID: Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"),
47+
ID: MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c"),
4848
gogitTreeEntry: &object.TreeEntry{
49-
Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()),
49+
Hash: plumbing.Hash(MustIDFromString("61ab7345a1a3bbc590068ccae37b8515cfc5843c").RawValue()),
5050
Name: "example/\n.txt",
5151
Mode: filemode.Symlink,
5252
},
5353
size: 234131,
5454
sized: true,
5555
},
5656
{
57-
ID: Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"),
57+
ID: MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8"),
5858
sized: true,
5959
gogitTreeEntry: &object.TreeEntry{
60-
Hash: plumbing.Hash(Sha1ObjectFormat.MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()),
60+
Hash: plumbing.Hash(MustIDFromString("1d01fb729fb0db5881daaa6030f9f2d3cd3d5ae8").RawValue()),
6161
Name: "example",
6262
Mode: filemode.Dir,
6363
},

‎modules/git/parse_nogogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func parseTreeEntries(objectFormat ObjectFormat, data []byte, ptree *Tree) ([]*T
7272
return nil, fmt.Errorf("unknown type: %v", string(entryMode))
7373
}
7474

75-
entry.ID, err = objectFormat.NewIDFromString(string(entryObjectID))
75+
entry.ID, err = NewIDFromString(string(entryObjectID))
7676
if err != nil {
7777
return nil, fmt.Errorf("invalid ls-tree output (invalid object id): %q, err: %w", line, err)
7878
}

‎modules/git/parse_nogogit_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,28 @@ func TestParseTreeEntriesLong(t *testing.T) {
2626
`,
2727
Expected: []*TreeEntry{
2828
{
29-
ID: objectFormat.MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"),
29+
ID: MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"),
3030
name: "README.md",
3131
entryMode: EntryModeBlob,
3232
size: 8218,
3333
sized: true,
3434
},
3535
{
36-
ID: objectFormat.MustIDFromString("037f27dc9d353ae4fd50f0474b2194c593914e35"),
36+
ID: MustIDFromString("037f27dc9d353ae4fd50f0474b2194c593914e35"),
3737
name: "README_ZH.md",
3838
entryMode: EntryModeBlob,
3939
size: 4681,
4040
sized: true,
4141
},
4242
{
43-
ID: objectFormat.MustIDFromString("9846a94f7e8350a916632929d0fda38c90dd2ca8"),
43+
ID: MustIDFromString("9846a94f7e8350a916632929d0fda38c90dd2ca8"),
4444
name: "SECURITY.md",
4545
entryMode: EntryModeBlob,
4646
size: 429,
4747
sized: true,
4848
},
4949
{
50-
ID: objectFormat.MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"),
50+
ID: MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"),
5151
name: "assets",
5252
entryMode: EntryModeTree,
5353
sized: true,
@@ -78,12 +78,12 @@ func TestParseTreeEntriesShort(t *testing.T) {
7878
`,
7979
Expected: []*TreeEntry{
8080
{
81-
ID: objectFormat.MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"),
81+
ID: MustIDFromString("ea0d83c9081af9500ac9f804101b3fd0a5c293af"),
8282
name: "README.md",
8383
entryMode: EntryModeBlob,
8484
},
8585
{
86-
ID: objectFormat.MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"),
86+
ID: MustIDFromString("84b90550547016f73c5dd3f50dea662389e67b6d"),
8787
name: "assets",
8888
entryMode: EntryModeTree,
8989
},

‎modules/git/pipeline/lfs_nogogit.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,7 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err
115115
continue
116116
case "commit":
117117
// Read in the commit to get its tree and in case this is one of the last used commits
118-
objectFormat, err := repo.GetObjectFormat()
119-
if err != nil {
120-
return nil, err
121-
}
122-
curCommit, err = git.CommitFromReader(repo, objectFormat.MustIDFromString(string(commitID)), io.LimitReader(batchReader, size))
118+
curCommit, err = git.CommitFromReader(repo, git.MustIDFromString(string(commitID)), io.LimitReader(batchReader, size))
123119
if err != nil {
124120
return nil, err
125121
}

‎modules/git/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func GetObjectFormatOfRepo(ctx context.Context, repoPath string) (ObjectFormat,
8181
return nil, errors.New(stderr.String())
8282
}
8383

84-
h, err := IDFromString(strings.TrimRight(stdout.String(), "\n"))
84+
h, err := NewIDFromString(strings.TrimRight(stdout.String(), "\n"))
8585
if err != nil {
8686
return nil, err
8787
}

0 commit comments

Comments
 (0)