Skip to content

Implemented head_commit for webhooks #16282

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

Merged
merged 6 commits into from
Jun 29, 2021
Merged
Changes from all 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
@@ -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
@@ -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,
@@ -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
@@ -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,
131 changes: 74 additions & 57 deletions modules/repository/commits.go
Original file line number Diff line number Diff line change
@@ -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
@@ -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) {
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
@@ -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
@@ -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)
@@ -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) {
@@ -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",
@@ -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)
19 changes: 10 additions & 9 deletions routers/api/v1/repo/hook.go
Original file line number Diff line number Diff line change
@@ -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
44 changes: 23 additions & 21 deletions routers/web/repo/webhook.go
Original file line number Diff line number Diff line change
@@ -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())
4 changes: 2 additions & 2 deletions services/repository/push.go
Original file line number Diff line number Diff line change
@@ -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
@@ -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)
2 changes: 1 addition & 1 deletion templates/user/dashboard/feeds.tmpl
Original file line number Diff line number Diff line change
@@ -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}}