Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use complete SHA to create and query commit status #22244

Merged
merged 12 commits into from
Dec 27, 2022
4 changes: 4 additions & 0 deletions models/git/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
}

if _, err := git.NewIDFromString(opts.SHA); err != nil {
return fmt.Errorf("NewCommitStatus[%s, %s]: invalid sha: %w", repoPath, opts.SHA, err)
}

ctx, committer, err := db.TxContext(db.DefaultContext)
if err != nil {
return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)
Expand Down
1 change: 1 addition & 0 deletions routers/api/v1/repo/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func getCommitStatuses(ctx *context.APIContext, sha string) {
ctx.Error(http.StatusBadRequest, "ref/sha not given", nil)
return
}
sha = utils.MustConvertToSHA1(ctx.Context, sha)
repo := ctx.Repo.Repository

listOptions := utils.GetListOptions(ctx)
Expand Down
29 changes: 29 additions & 0 deletions routers/api/v1/utils/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ func ResolveRefOrSha(ctx *context.APIContext, ref string) string {
}
}

sha = MustConvertToSHA1(ctx.Context, sha)

if ctx.Repo.GitRepo != nil {
err := ctx.Repo.GitRepo.AddLastCommitCache(ctx.Repo.Repository.GetCommitsCountCacheKey(ref, ref != sha), ctx.Repo.Repository.FullName(), sha)
if err != nil {
Expand Down Expand Up @@ -65,3 +67,30 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str
}
return "", "", nil
}

// ConvertToSHA1 returns a SHA1 from a potential ID string
wolfogre marked this conversation as resolved.
Show resolved Hide resolved
func ConvertToSHA1(ctx *context.Context, commitID string) (git.SHA1, error) {
if len(commitID) == 40 && git.IsValidSHAPattern(commitID) {
wolfogre marked this conversation as resolved.
Show resolved Hide resolved
sha1, err := git.NewIDFromString(commitID)
if err == nil {
return sha1, nil
}
}

gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.Repo.Repository.RepoPath())
if err != nil {
return git.SHA1{}, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
}
defer closer.Close()

return gitRepo.ConvertToSHA1(commitID)
}

// MustConvertToSHA1 returns a SHA1 string from a potential ID string, or returns origin input if it can't convert to SHA1
func MustConvertToSHA1(ctx *context.Context, commitID string) string {
sha, err := ConvertToSHA1(ctx, commitID)
if err != nil {
return commitID
}
return sha.String()
}
5 changes: 4 additions & 1 deletion services/repository/files/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato
}
defer closer.Close()

if _, err := gitRepo.GetCommit(sha); err != nil {
if commit, err := gitRepo.GetCommit(sha); err != nil {
gitRepo.Close()
return fmt.Errorf("GetCommit[%s]: %w", sha, err)
} else if len(sha) != 40 {
wolfogre marked this conversation as resolved.
Show resolved Hide resolved
// use complete commit sha
sha = commit.ID.String()
}
gitRepo.Close()

Expand Down
5 changes: 5 additions & 0 deletions tests/integration/repo_commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
reqOne := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/status")
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)

// By short SHA
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/statuses")
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/status")
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)

// By Ref
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses")
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/status")
Expand Down