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

Implemented head_commit for webhooks #16282

Merged
merged 6 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions modules/notification/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m

func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
apiPusher := convert.ToUser(pusher, nil)
apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
Expand All @@ -574,6 +574,7 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Re
After: opts.NewCommitID,
CompareURL: setting.AppURL + commits.CompareURL,
Commits: apiCommits,
HeadCommit: apiHeadCommit,
Repo: convert.ToRepo(repo, models.AccessModeOwner),
Pusher: apiPusher,
Sender: apiPusher,
Expand Down Expand Up @@ -790,7 +791,7 @@ func (m *webhookNotifier) NotifyDeleteRelease(doer *models.User, rel *models.Rel

func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
apiPusher := convert.ToUser(pusher, nil)
apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
Expand All @@ -802,6 +803,7 @@ func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *model
After: opts.NewCommitID,
CompareURL: setting.AppURL + commits.CompareURL,
Commits: apiCommits,
HeadCommit: apiHeadCommit,
Repo: convert.ToRepo(repo, models.AccessModeOwner),
Pusher: apiPusher,
Sender: apiPusher,
Expand Down
131 changes: 74 additions & 57 deletions modules/repository/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ type PushCommit struct {

// PushCommits represents list of commits in a push operation.
type PushCommits struct {
Len int
Commits []*PushCommit
HeadCommit *PushCommit
CompareURL string

avatars map[string]string
Expand All @@ -44,67 +44,88 @@ func NewPushCommits() *PushCommits {
}
}

// ToAPIPayloadCommits converts a PushCommits object to
// api.PayloadCommit format.
func (pc *PushCommits) ToAPIPayloadCommits(repoPath, repoLink string) ([]*api.PayloadCommit, error) {
commits := make([]*api.PayloadCommit, len(pc.Commits))

if pc.emailUsers == nil {
pc.emailUsers = make(map[string]*models.User)
}
// toAPIPayloadCommit converts a single PushCommit to an api.PayloadCommit object.
func (pc *PushCommits) toAPIPayloadCommit(repoPath, repoLink string, commit *PushCommit) (*api.PayloadCommit, error) {
var err error
for i, commit := range pc.Commits {
authorUsername := ""
author, ok := pc.emailUsers[commit.AuthorEmail]
if !ok {
author, err = models.GetUserByEmail(commit.AuthorEmail)
if err == nil {
authorUsername = author.Name
pc.emailUsers[commit.AuthorEmail] = author
}
} else {
authorUsername := ""
author, ok := pc.emailUsers[commit.AuthorEmail]
if !ok {
author, err = models.GetUserByEmail(commit.AuthorEmail)
if err == nil {
authorUsername = author.Name
pc.emailUsers[commit.AuthorEmail] = author
}
} else {
authorUsername = author.Name
}

committerUsername := ""
committer, ok := pc.emailUsers[commit.CommitterEmail]
if !ok {
committer, err = models.GetUserByEmail(commit.CommitterEmail)
if err == nil {
// TODO: check errors other than email not found.
committerUsername = committer.Name
pc.emailUsers[commit.CommitterEmail] = committer
}
} else {
committerUsername := ""
committer, ok := pc.emailUsers[commit.CommitterEmail]
if !ok {
committer, err = models.GetUserByEmail(commit.CommitterEmail)
if err == nil {
// TODO: check errors other than email not found.
committerUsername = committer.Name
pc.emailUsers[commit.CommitterEmail] = committer
}
} else {
committerUsername = committer.Name
}

fileStatus, err := git.GetCommitFileStatus(repoPath, commit.Sha1)
fileStatus, err := git.GetCommitFileStatus(repoPath, commit.Sha1)
if err != nil {
return nil, fmt.Errorf("FileStatus [commit_sha1: %s]: %v", commit.Sha1, err)
}

return &api.PayloadCommit{
ID: commit.Sha1,
Message: commit.Message,
URL: fmt.Sprintf("%s/commit/%s", repoLink, commit.Sha1),
Author: &api.PayloadUser{
Name: commit.AuthorName,
Email: commit.AuthorEmail,
UserName: authorUsername,
},
Committer: &api.PayloadUser{
Name: commit.CommitterName,
Email: commit.CommitterEmail,
UserName: committerUsername,
},
Added: fileStatus.Added,
Removed: fileStatus.Removed,
Modified: fileStatus.Modified,
Timestamp: commit.Timestamp,
}, nil
}

// ToAPIPayloadCommits converts a PushCommits object to api.PayloadCommit format.
// It returns all converted commits and, if provided, the head commit or an error otherwise.
func (pc *PushCommits) ToAPIPayloadCommits(repoPath, repoLink string) ([]*api.PayloadCommit, *api.PayloadCommit, error) {
KN4CK3R marked this conversation as resolved.
Show resolved Hide resolved
commits := make([]*api.PayloadCommit, len(pc.Commits))
var headCommit *api.PayloadCommit

if pc.emailUsers == nil {
pc.emailUsers = make(map[string]*models.User)
}
for i, commit := range pc.Commits {
apiCommit, err := pc.toAPIPayloadCommit(repoPath, repoLink, commit)
if err != nil {
return nil, fmt.Errorf("FileStatus [commit_sha1: %s]: %v", commit.Sha1, err)
return nil, nil, err
}

commits[i] = &api.PayloadCommit{
ID: commit.Sha1,
Message: commit.Message,
URL: fmt.Sprintf("%s/commit/%s", repoLink, commit.Sha1),
Author: &api.PayloadUser{
Name: commit.AuthorName,
Email: commit.AuthorEmail,
UserName: authorUsername,
},
Committer: &api.PayloadUser{
Name: commit.CommitterName,
Email: commit.CommitterEmail,
UserName: committerUsername,
},
Added: fileStatus.Added,
Removed: fileStatus.Removed,
Modified: fileStatus.Modified,
Timestamp: commit.Timestamp,
commits[i] = apiCommit
if pc.HeadCommit != nil && pc.HeadCommit.Sha1 == commits[i].ID {
headCommit = apiCommit
}
}
return commits, nil
if pc.HeadCommit != nil && headCommit == nil {
var err error
headCommit, err = pc.toAPIPayloadCommit(repoPath, repoLink, pc.HeadCommit)
if err != nil {
return nil, nil, err
}
}
return commits, headCommit, nil
}

// AvatarLink tries to match user in database with e-mail
Expand Down Expand Up @@ -157,13 +178,9 @@ func CommitToPushCommit(commit *git.Commit) *PushCommit {
// ListToPushCommits transforms a list.List to PushCommits type.
func ListToPushCommits(l *list.List) *PushCommits {
var commits []*PushCommit
var actEmail string
for e := l.Front(); e != nil; e = e.Next() {
commit := e.Value.(*git.Commit)
if actEmail == "" {
actEmail = commit.Committer.Email
}
commits = append(commits, CommitToPushCommit(commit))
commit := CommitToPushCommit(e.Value.(*git.Commit))
commits = append(commits, commit)
}
return &PushCommits{l.Len(), commits, "", make(map[string]string), make(map[string]*models.User)}
return &PushCommits{commits, nil, "", make(map[string]string), make(map[string]*models.User)}
}
18 changes: 14 additions & 4 deletions modules/repository/commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
Message: "good signed commit",
},
}
pushCommits.Len = len(pushCommits.Commits)
pushCommits.HeadCommit = &PushCommit{Sha1: "69554a6"}

repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 16}).(*models.Repository)
payloadCommits, err := pushCommits.ToAPIPayloadCommits(repo.RepoPath(), "/user2/repo16")
payloadCommits, headCommit, err := pushCommits.ToAPIPayloadCommits(repo.RepoPath(), "/user2/repo16")
assert.NoError(t, err)
assert.Len(t, payloadCommits, 3)
assert.NotNil(t, headCommit)

assert.Equal(t, "69554a6", payloadCommits[0].ID)
assert.Equal(t, "not signed commit", payloadCommits[0].Message)
Expand Down Expand Up @@ -85,6 +86,17 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
assert.EqualValues(t, []string{"readme.md"}, payloadCommits[2].Added)
assert.EqualValues(t, []string{}, payloadCommits[2].Removed)
assert.EqualValues(t, []string{}, payloadCommits[2].Modified)

assert.Equal(t, "69554a6", headCommit.ID)
assert.Equal(t, "not signed commit", headCommit.Message)
assert.Equal(t, "/user2/repo16/commit/69554a6", headCommit.URL)
assert.Equal(t, "User2", headCommit.Committer.Name)
assert.Equal(t, "user2", headCommit.Committer.UserName)
assert.Equal(t, "User2", headCommit.Author.Name)
assert.Equal(t, "user2", headCommit.Author.UserName)
assert.EqualValues(t, []string{}, headCommit.Added)
assert.EqualValues(t, []string{}, headCommit.Removed)
assert.EqualValues(t, []string{"readme.md"}, headCommit.Modified)
}

func TestPushCommits_AvatarLink(t *testing.T) {
Expand All @@ -109,7 +121,6 @@ func TestPushCommits_AvatarLink(t *testing.T) {
Message: "message2",
},
}
pushCommits.Len = len(pushCommits.Commits)

assert.Equal(t,
"https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s=112",
Expand Down Expand Up @@ -177,7 +188,6 @@ func TestListToPushCommits(t *testing.T) {
})

pushCommits := ListToPushCommits(l)
assert.Equal(t, 2, pushCommits.Len)
if assert.Len(t, pushCommits.Commits, 2) {
assert.Equal(t, "Message1", pushCommits.Commits[0].Message)
assert.Equal(t, hexString1, pushCommits.Commits[0].Sha1)
Expand Down
19 changes: 10 additions & 9 deletions routers/api/v1/repo/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,17 @@ func TestHook(ctx *context.APIContext) {
return
}

commit := convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit)

if err := webhook.PrepareWebhook(hook, ctx.Repo.Repository, models.HookEventPush, &api.PushPayload{
Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
Before: ctx.Repo.Commit.ID.String(),
After: ctx.Repo.Commit.ID.String(),
Commits: []*api.PayloadCommit{
convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit),
},
Repo: convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
Pusher: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
Sender: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
Before: ctx.Repo.Commit.ID.String(),
After: ctx.Repo.Commit.ID.String(),
Commits: []*api.PayloadCommit{commit},
HeadCommit: commit,
Repo: convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
Pusher: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
Sender: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
}); err != nil {
ctx.Error(http.StatusInternalServerError, "PrepareWebhook: ", err)
return
Expand Down
44 changes: 23 additions & 21 deletions routers/web/repo/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -1085,28 +1085,30 @@ func TestWebhook(ctx *context.Context) {
}

apiUser := convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone)
p := &api.PushPayload{
Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
Before: commit.ID.String(),
After: commit.ID.String(),
Commits: []*api.PayloadCommit{
{
ID: commit.ID.String(),
Message: commit.Message(),
URL: ctx.Repo.Repository.HTMLURL() + "/commit/" + commit.ID.String(),
Author: &api.PayloadUser{
Name: commit.Author.Name,
Email: commit.Author.Email,
},
Committer: &api.PayloadUser{
Name: commit.Committer.Name,
Email: commit.Committer.Email,
},
},

apiCommit := &api.PayloadCommit{
ID: commit.ID.String(),
Message: commit.Message(),
URL: ctx.Repo.Repository.HTMLURL() + "/commit/" + commit.ID.String(),
Author: &api.PayloadUser{
Name: commit.Author.Name,
Email: commit.Author.Email,
},
Committer: &api.PayloadUser{
Name: commit.Committer.Name,
Email: commit.Committer.Email,
},
Repo: convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
Pusher: apiUser,
Sender: apiUser,
}

p := &api.PushPayload{
Ref: git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
Before: commit.ID.String(),
After: commit.ID.String(),
Commits: []*api.PayloadCommit{apiCommit},
HeadCommit: apiCommit,
Repo: convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
Pusher: apiUser,
Sender: apiUser,
}
if err := webhook.PrepareWebhook(w, ctx.Repo.Repository, models.HookEventPush, p); err != nil {
ctx.Flash.Error("PrepareWebhook: " + err.Error())
Expand Down
4 changes: 2 additions & 2 deletions services/repository/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
if opts.IsNewRef() && opts.IsDelRef() {
return fmt.Errorf("Old and new revisions are both %s", git.EmptySHA)
}
var commits = &repo_module.PushCommits{}
if opts.IsTag() { // If is tag reference
if pusher == nil || pusher.ID != opts.PusherID {
var err error
Expand Down Expand Up @@ -192,7 +191,8 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
}
}

commits = repo_module.ListToPushCommits(l)
commits := repo_module.ListToPushCommits(l)
commits.HeadCommit = repo_module.CommitToPushCommit(newCommit)

if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil {
log.Error("updateIssuesCommit: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion templates/user/dashboard/feeds.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
</li>
{{end}}
{{end}}
{{if and (gt $push.Len 1) $push.CompareURL}}<li><a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{$.i18n.Tr "action.compare_commits" $push.Len}} »</a></li>{{end}}
{{if and (gt (len $push.Commits) 1) $push.CompareURL}}<li><a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{$.i18n.Tr "action.compare_commits" (len $push.Commits)}} »</a></li>{{end}}
</ul>
</div>
{{else if eq .GetOpType 6}}
Expand Down