diff --git a/models/issues/comment.go b/models/issues/comment.go index fd0500833e751..a0d1730f6b6ba 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -330,12 +330,6 @@ func init() { db.RegisterModel(new(Comment)) } -// PushActionContent is content of push pull comment -type PushActionContent struct { - IsForcePush bool `json:"is_force_push"` - CommitIDs []string `json:"commit_ids"` -} - // LoadIssue loads the issue reference for the comment func (c *Comment) LoadIssue(ctx context.Context) (err error) { if c.Issue != nil { diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 82a776db5bdb7..869366c5f0ee4 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -765,7 +765,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } } else if comment.Type == issues_model.CommentTypePullRequestPush { participants = addParticipant(comment.Poster, participants) - if err = issue_service.LoadCommentPushCommits(ctx, comment); err != nil { + if err = pull_service.LoadCommentPushCommits(ctx, comment); err != nil { ctx.ServerError("LoadCommentPushCommits", err) return } diff --git a/services/issue/comments.go b/services/issue/comments.go index 3ce2e2a5e13db..34673447eeae2 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -13,11 +13,7 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/json" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" - git_service "code.gitea.io/gitea/services/git" notify_service "code.gitea.io/gitea/services/notify" ) @@ -150,47 +146,3 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m return nil } - -// LoadCommentPushCommits Load push commits -func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error { - if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush { - return nil - } - - var data issues_model.PushActionContent - if err := json.Unmarshal([]byte(c.Content), &data); err != nil { - log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken - return nil - } - - c.IsForcePush = data.IsForcePush - - if c.IsForcePush { - if len(data.CommitIDs) != 2 { - return nil - } - c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1] - } else { - if err := c.LoadIssue(ctx); err != nil { - return err - } - if err := c.Issue.LoadRepo(ctx); err != nil { - return err - } - - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo) - if err != nil { - return err - } - defer closer.Close() - - c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) - if err != nil { - log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist - } else { - c.CommitsNum = int64(len(c.Commits)) - } - } - - return nil -} diff --git a/services/mailer/notify.go b/services/mailer/notify.go index 5921021ce8292..26f943ab2235c 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -13,8 +13,8 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" - issue_service "code.gitea.io/gitea/services/issue" notify_service "code.gitea.io/gitea/services/notify" + pull_service "code.gitea.io/gitea/services/pull" ) type mailNotifier struct { @@ -173,7 +173,7 @@ func (m *mailNotifier) PullRequestPushCommits(ctx context.Context, doer *user_mo log.Error("comment.Issue.PullRequest.LoadBaseRepo: %v", err) return } - if err := issue_service.LoadCommentPushCommits(ctx, comment); err != nil { + if err := pull_service.LoadCommentPushCommits(ctx, comment); err != nil { log.Error("comment.LoadPushCommits: %v", err) } m.CreateIssueComment(ctx, doer, comment.Issue.Repo, comment.Issue, comment, nil) diff --git a/services/pull/comment.go b/services/pull/comment.go index f24e8128e9483..c0561c03a0913 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -6,11 +6,15 @@ package pull import ( "context" + asymkey_model "code.gitea.io/gitea/models/asymkey" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/log" + git_service "code.gitea.io/gitea/services/git" ) // getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID @@ -46,6 +50,140 @@ func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldC return commitIDs, err } +type CachedCommitUser struct { + ID int64 + Name string + FullName string + Email string + Avatar string +} + +func convertUserToCachedCommitUser(u *user_model.User) *CachedCommitUser { + if u == nil { + return nil + } + return &CachedCommitUser{ + ID: u.ID, + Name: u.Name, + FullName: u.FullName, + Email: u.Email, + Avatar: u.Avatar, + } +} + +func convertCachedUserToUser(cu *CachedCommitUser) *user_model.User { + if cu == nil { + return nil + } + return &user_model.User{ + ID: cu.ID, + Name: cu.Name, + FullName: cu.FullName, + Email: cu.Email, + Avatar: cu.Avatar, + } +} + +// CachedCommit will be stored in database as part of push comment content to reduce +// disk read when loading push commits later. +// it will only keep necessary information to display in the timeline of the pull request +type CachedCommit struct { + CommitID string + Author *git.Signature + Committer *git.Signature + CommitMessage string + User *CachedCommitUser + + Verified bool + Warning bool + Reason string + SigningUser *CachedCommitUser // if Verified, then SigningUser is non-nil + CommittingUser *CachedCommitUser // if Verified, then CommittingUser is non-nil + SigningEmail string + SigningGPGKeyID string + SigningSSHKeyFingerprint string + TrustStatus string +} + +func convertCachedCommitsToGitCommits(cachedCommits []CachedCommit) []*asymkey_model.SignCommit { + var gitCommits []*asymkey_model.SignCommit + for _, cc := range cachedCommits { + objectID := git.MustIDFromString(cc.CommitID) + signedCommit := &asymkey_model.SignCommit{ + UserCommit: &user_model.UserCommit{ + User: convertCachedUserToUser(cc.User), + Commit: &git.Commit{ + ID: objectID, + Author: cc.Author, + Committer: cc.Committer, + CommitMessage: cc.CommitMessage, + }, + }, + Verification: &asymkey_model.CommitVerification{ + Verified: cc.Verified, + Warning: cc.Warning, + Reason: cc.Reason, + SigningEmail: cc.SigningEmail, + TrustStatus: cc.TrustStatus, + SigningUser: convertCachedUserToUser(cc.SigningUser), + CommittingUser: convertCachedUserToUser(cc.CommittingUser), + }, + } + + if cc.SigningGPGKeyID != "" { + signedCommit.Verification.SigningKey = &asymkey_model.GPGKey{ + KeyID: cc.SigningGPGKeyID, + } + } else if cc.SigningSSHKeyFingerprint != "" { + signedCommit.Verification.SigningSSHKey = &asymkey_model.PublicKey{ + Fingerprint: cc.SigningSSHKeyFingerprint, + } + } + + gitCommits = append(gitCommits, signedCommit) + } + return gitCommits +} + +func convertGitCommitsToCachedCommits(commits []*asymkey_model.SignCommit) []CachedCommit { + var cachedCommits []CachedCommit + for _, c := range commits { + var signingGPGKeyID, signingSSHKeyFingerprint string + if c.Verification != nil { + if c.Verification.SigningKey != nil { + signingGPGKeyID = c.Verification.SigningKey.KeyID + } else if c.Verification.SigningSSHKey != nil { + signingSSHKeyFingerprint = c.Verification.SigningSSHKey.Fingerprint + } + } + cachedCommits = append(cachedCommits, CachedCommit{ + CommitID: c.ID.String(), + Author: c.Author, + Committer: c.Committer, + CommitMessage: c.CommitMessage, + User: convertUserToCachedCommitUser(c.User), + + Verified: c.Verification.Verified, + Warning: c.Verification.Warning, + Reason: c.Verification.Reason, + SigningUser: convertUserToCachedCommitUser(c.Verification.SigningUser), + CommittingUser: convertUserToCachedCommitUser(c.Verification.CommittingUser), + SigningEmail: c.Verification.SigningEmail, + SigningGPGKeyID: signingGPGKeyID, + SigningSSHKeyFingerprint: signingSSHKeyFingerprint, + TrustStatus: c.Verification.TrustStatus, + }) + } + return cachedCommits +} + +// PushActionContent is content of push pull comment +type PushActionContent struct { + IsForcePush bool `json:"is_force_push"` + CommitIDs []string `json:"commit_ids"` + CachedCommits []CachedCommit `json:"cached_commits"` +} + // CreatePushPullComment create push code to pull base comment func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string, isForcePush bool) (comment *issues_model.Comment, err error) { if pr.HasMerged || oldCommitID == "" || newCommitID == "" { @@ -60,7 +198,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss Issue: pr.Issue, } - var data issues_model.PushActionContent + var data PushActionContent if opts.IsForcePush { data.CommitIDs = []string{oldCommitID, newCommitID} data.IsForcePush = true @@ -73,6 +211,28 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss if len(data.CommitIDs) == 0 { return nil, nil } + + gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo) + if err != nil { + return nil, err + } + defer closer.Close() + + validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs)) + if err != nil { + return nil, err + } + signedCommits, err := git_service.ParseCommitsWithSignature( + ctx, + pr.BaseRepo, + validatedCommits, + pr.BaseRepo.GetTrustModel(), + ) + if err != nil { + return nil, err + } + + data.CachedCommits = convertGitCommitsToCachedCommits(signedCommits) } dataJSON, err := json.Marshal(data) @@ -85,3 +245,53 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss return comment, err } + +// LoadCommentPushCommits Load push commits +func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error { + if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush { + return nil + } + + var data PushActionContent + if err := json.Unmarshal([]byte(c.Content), &data); err != nil { + log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken + return nil + } + + c.IsForcePush = data.IsForcePush + + if c.IsForcePush { + if len(data.CommitIDs) != 2 { + return nil + } + c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1] + } else { + if err := c.LoadIssue(ctx); err != nil { + return err + } + if err := c.Issue.LoadRepo(ctx); err != nil { + return err + } + + gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo) + if err != nil { + return err + } + defer closer.Close() + + if data.CachedCommits != nil { + convertedCommits := convertCachedCommitsToGitCommits(data.CachedCommits) + c.Commits, err = git_service.ParseCommitsWithStatus(ctx, convertedCommits, c.Issue.Repo) + } else { + c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) + } + + if err != nil { + log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist + } else { + c.CommitsNum = int64(len(c.Commits)) + } + } + + return nil +} diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go index abab65247ba74..9e8d6f88e87fa 100644 --- a/tests/integration/pull_comment_test.go +++ b/tests/integration/pull_comment_test.go @@ -13,7 +13,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" - issues_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -70,7 +70,7 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) { }) require.NoError(t, err) lastComment := comments[len(comments)-1] - assert.NoError(t, issues_service.LoadCommentPushCommits(t.Context(), lastComment)) + assert.NoError(t, pull_service.LoadCommentPushCommits(t.Context(), lastComment)) assert.True(t, lastComment.IsForcePush) }