Skip to content

Commit 736b728

Browse files
lunnylunny
committed
Cache the push commits of comment into database to make the pull request with many push commits load fast
1 parent 46d7ade commit 736b728

File tree

6 files changed

+216
-60
lines changed

6 files changed

+216
-60
lines changed

‎models/issues/comment.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,6 @@ func init() {
330330
db.RegisterModel(new(Comment))
331331
}
332332

333-
// PushActionContent is content of push pull comment
334-
type PushActionContent struct {
335-
IsForcePush bool `json:"is_force_push"`
336-
CommitIDs []string `json:"commit_ids"`
337-
}
338-
339333
// LoadIssue loads the issue reference for the comment
340334
func (c *Comment) LoadIssue(ctx context.Context) (err error) {
341335
if c.Issue != nil {

‎routers/web/repo/issue_view.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
765765
}
766766
} else if comment.Type == issues_model.CommentTypePullRequestPush {
767767
participants = addParticipant(comment.Poster, participants)
768-
if err = issue_service.LoadCommentPushCommits(ctx, comment); err != nil {
768+
if err = pull_service.LoadCommentPushCommits(ctx, comment); err != nil {
769769
ctx.ServerError("LoadCommentPushCommits", err)
770770
return
771771
}

‎services/issue/comments.go‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@ import (
1313
access_model "code.gitea.io/gitea/models/perm/access"
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
user_model "code.gitea.io/gitea/models/user"
16-
"code.gitea.io/gitea/modules/gitrepo"
17-
"code.gitea.io/gitea/modules/json"
18-
"code.gitea.io/gitea/modules/log"
1916
"code.gitea.io/gitea/modules/timeutil"
20-
git_service "code.gitea.io/gitea/services/git"
2117
notify_service "code.gitea.io/gitea/services/notify"
2218
)
2319

@@ -150,47 +146,3 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
150146

151147
return nil
152148
}
153-
154-
// LoadCommentPushCommits Load push commits
155-
func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error {
156-
if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush {
157-
return nil
158-
}
159-
160-
var data issues_model.PushActionContent
161-
if err := json.Unmarshal([]byte(c.Content), &data); err != nil {
162-
log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken
163-
return nil
164-
}
165-
166-
c.IsForcePush = data.IsForcePush
167-
168-
if c.IsForcePush {
169-
if len(data.CommitIDs) != 2 {
170-
return nil
171-
}
172-
c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1]
173-
} else {
174-
if err := c.LoadIssue(ctx); err != nil {
175-
return err
176-
}
177-
if err := c.Issue.LoadRepo(ctx); err != nil {
178-
return err
179-
}
180-
181-
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo)
182-
if err != nil {
183-
return err
184-
}
185-
defer closer.Close()
186-
187-
c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
188-
if err != nil {
189-
log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist
190-
} else {
191-
c.CommitsNum = int64(len(c.Commits))
192-
}
193-
}
194-
195-
return nil
196-
}

‎services/mailer/notify.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313
repo_model "code.gitea.io/gitea/models/repo"
1414
user_model "code.gitea.io/gitea/models/user"
1515
"code.gitea.io/gitea/modules/log"
16-
issue_service "code.gitea.io/gitea/services/issue"
1716
notify_service "code.gitea.io/gitea/services/notify"
17+
pull_service "code.gitea.io/gitea/services/pull"
1818
)
1919

2020
type mailNotifier struct {
@@ -173,7 +173,7 @@ func (m *mailNotifier) PullRequestPushCommits(ctx context.Context, doer *user_mo
173173
log.Error("comment.Issue.PullRequest.LoadBaseRepo: %v", err)
174174
return
175175
}
176-
if err := issue_service.LoadCommentPushCommits(ctx, comment); err != nil {
176+
if err := pull_service.LoadCommentPushCommits(ctx, comment); err != nil {
177177
log.Error("comment.LoadPushCommits: %v", err)
178178
}
179179
m.CreateIssueComment(ctx, doer, comment.Issue.Repo, comment.Issue, comment, nil)

‎services/pull/comment.go‎

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ package pull
66
import (
77
"context"
88

9+
asymkey_model "code.gitea.io/gitea/models/asymkey"
910
issues_model "code.gitea.io/gitea/models/issues"
1011
repo_model "code.gitea.io/gitea/models/repo"
1112
user_model "code.gitea.io/gitea/models/user"
13+
"code.gitea.io/gitea/modules/git"
1214
"code.gitea.io/gitea/modules/gitrepo"
1315
"code.gitea.io/gitea/modules/json"
16+
"code.gitea.io/gitea/modules/log"
17+
git_service "code.gitea.io/gitea/services/git"
1418
)
1519

1620
// 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
4650
return commitIDs, err
4751
}
4852

53+
type CachedCommitUser struct {
54+
ID int64
55+
Name string
56+
FullName string
57+
Email string
58+
Avatar string
59+
}
60+
61+
func convertUserToCachedCommitUser(u *user_model.User) *CachedCommitUser {
62+
if u == nil {
63+
return nil
64+
}
65+
return &CachedCommitUser{
66+
ID: u.ID,
67+
Name: u.Name,
68+
FullName: u.FullName,
69+
Email: u.Email,
70+
Avatar: u.Avatar,
71+
}
72+
}
73+
74+
func convertCachedUserToUser(cu *CachedCommitUser) *user_model.User {
75+
if cu == nil {
76+
return nil
77+
}
78+
return &user_model.User{
79+
ID: cu.ID,
80+
Name: cu.Name,
81+
FullName: cu.FullName,
82+
Email: cu.Email,
83+
Avatar: cu.Avatar,
84+
}
85+
}
86+
87+
// CachedCommit will be stored in database as part of push comment content to reduce
88+
// disk read when loading push commits later.
89+
// it will only keep necessary information to display in the timeline of the pull request
90+
type CachedCommit struct {
91+
CommitID string
92+
Author *git.Signature
93+
Committer *git.Signature
94+
CommitMessage string
95+
User *CachedCommitUser
96+
97+
Verified bool
98+
Warning bool
99+
Reason string
100+
SigningUser *CachedCommitUser // if Verified, then SigningUser is non-nil
101+
CommittingUser *CachedCommitUser // if Verified, then CommittingUser is non-nil
102+
SigningEmail string
103+
SigningGPGKeyID string
104+
SigningSSHKeyFingerprint string
105+
TrustStatus string
106+
}
107+
108+
func convertCachedCommitsToGitCommits(cachedCommits []CachedCommit, repo *repo_model.Repository) []*asymkey_model.SignCommit {
109+
var gitCommits []*asymkey_model.SignCommit
110+
for _, cc := range cachedCommits {
111+
objectID := git.MustIDFromString(cc.CommitID)
112+
signedCommit := &asymkey_model.SignCommit{
113+
UserCommit: &user_model.UserCommit{
114+
User: convertCachedUserToUser(cc.User),
115+
Commit: &git.Commit{
116+
ID: objectID,
117+
Author: cc.Author,
118+
Committer: cc.Committer,
119+
CommitMessage: cc.CommitMessage,
120+
},
121+
},
122+
Verification: &asymkey_model.CommitVerification{
123+
Verified: cc.Verified,
124+
Warning: cc.Warning,
125+
Reason: cc.Reason,
126+
SigningEmail: cc.SigningEmail,
127+
TrustStatus: cc.TrustStatus,
128+
SigningUser: convertCachedUserToUser(cc.SigningUser),
129+
CommittingUser: convertCachedUserToUser(cc.CommittingUser),
130+
},
131+
}
132+
133+
if cc.SigningGPGKeyID != "" {
134+
signedCommit.Verification.SigningKey = &asymkey_model.GPGKey{
135+
KeyID: cc.SigningGPGKeyID,
136+
}
137+
} else if cc.SigningSSHKeyFingerprint != "" {
138+
signedCommit.Verification.SigningSSHKey = &asymkey_model.PublicKey{
139+
Fingerprint: cc.SigningSSHKeyFingerprint,
140+
}
141+
}
142+
143+
gitCommits = append(gitCommits, signedCommit)
144+
}
145+
return gitCommits
146+
}
147+
148+
func convertGitCommitsToCachedCommits(commits []*asymkey_model.SignCommit) []CachedCommit {
149+
var cachedCommits []CachedCommit
150+
for _, c := range commits {
151+
var signingGPGKeyID, signingSSHKeyFingerprint string
152+
if c.Verification != nil {
153+
if c.Verification.SigningKey != nil {
154+
signingGPGKeyID = c.Verification.SigningKey.KeyID
155+
} else if c.Verification.SigningSSHKey != nil {
156+
signingSSHKeyFingerprint = c.Verification.SigningSSHKey.Fingerprint
157+
}
158+
}
159+
cachedCommits = append(cachedCommits, CachedCommit{
160+
CommitID: c.ID.String(),
161+
Author: c.Author,
162+
Committer: c.Committer,
163+
CommitMessage: c.CommitMessage,
164+
User: convertUserToCachedCommitUser(c.User),
165+
166+
Verified: c.Verification.Verified,
167+
Warning: c.Verification.Warning,
168+
Reason: c.Verification.Reason,
169+
SigningUser: convertUserToCachedCommitUser(c.Verification.SigningUser),
170+
CommittingUser: convertUserToCachedCommitUser(c.Verification.CommittingUser),
171+
SigningEmail: c.Verification.SigningEmail,
172+
SigningGPGKeyID: signingGPGKeyID,
173+
SigningSSHKeyFingerprint: signingSSHKeyFingerprint,
174+
TrustStatus: c.Verification.TrustStatus,
175+
})
176+
}
177+
return cachedCommits
178+
}
179+
180+
// PushActionContent is content of push pull comment
181+
type PushActionContent struct {
182+
IsForcePush bool `json:"is_force_push"`
183+
CommitIDs []string `json:"commit_ids"`
184+
CachedCommits []CachedCommit `json:"cached_commits"`
185+
}
186+
49187
// CreatePushPullComment create push code to pull base comment
50188
func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string, isForcePush bool) (comment *issues_model.Comment, err error) {
51189
if pr.HasMerged || oldCommitID == "" || newCommitID == "" {
@@ -60,7 +198,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
60198
Issue: pr.Issue,
61199
}
62200

63-
var data issues_model.PushActionContent
201+
var data PushActionContent
64202
if opts.IsForcePush {
65203
data.CommitIDs = []string{oldCommitID, newCommitID}
66204
data.IsForcePush = true
@@ -73,6 +211,28 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
73211
if len(data.CommitIDs) == 0 {
74212
return nil, nil
75213
}
214+
215+
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
216+
if err != nil {
217+
return nil, err
218+
}
219+
defer closer.Close()
220+
221+
validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs))
222+
if err != nil {
223+
return nil, err
224+
}
225+
signedCommits, err := git_service.ParseCommitsWithSignature(
226+
ctx,
227+
pr.BaseRepo,
228+
validatedCommits,
229+
pr.BaseRepo.GetTrustModel(),
230+
)
231+
if err != nil {
232+
return nil, err
233+
}
234+
235+
data.CachedCommits = convertGitCommitsToCachedCommits(signedCommits)
76236
}
77237

78238
dataJSON, err := json.Marshal(data)
@@ -85,3 +245,53 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
85245

86246
return comment, err
87247
}
248+
249+
// LoadCommentPushCommits Load push commits
250+
func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error {
251+
if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush {
252+
return nil
253+
}
254+
255+
var data PushActionContent
256+
if err := json.Unmarshal([]byte(c.Content), &data); err != nil {
257+
log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken
258+
return nil
259+
}
260+
261+
c.IsForcePush = data.IsForcePush
262+
263+
if c.IsForcePush {
264+
if len(data.CommitIDs) != 2 {
265+
return nil
266+
}
267+
c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1]
268+
} else {
269+
if err := c.LoadIssue(ctx); err != nil {
270+
return err
271+
}
272+
if err := c.Issue.LoadRepo(ctx); err != nil {
273+
return err
274+
}
275+
276+
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo)
277+
if err != nil {
278+
return err
279+
}
280+
defer closer.Close()
281+
282+
if data.CachedCommits != nil {
283+
convertedCommits := convertCachedCommitsToGitCommits(data.CachedCommits, c.Issue.Repo)
284+
c.Commits, err = git_service.ParseCommitsWithStatus(ctx, convertedCommits, c.Issue.Repo)
285+
} else {
286+
c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
287+
}
288+
289+
if err != nil {
290+
log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist
291+
} else {
292+
c.CommitsNum = int64(len(c.Commits))
293+
}
294+
}
295+
296+
return nil
297+
}

‎tests/integration/pull_comment_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313

1414
issues_model "code.gitea.io/gitea/models/issues"
1515
"code.gitea.io/gitea/models/unittest"
16-
issues_service "code.gitea.io/gitea/services/issue"
1716

17+
pull_service "code.gitea.io/gitea/services/pull"
1818
"github.com/stretchr/testify/assert"
1919
"github.com/stretchr/testify/require"
2020
)
@@ -70,7 +70,7 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) {
7070
})
7171
require.NoError(t, err)
7272
lastComment := comments[len(comments)-1]
73-
assert.NoError(t, issues_service.LoadCommentPushCommits(t.Context(), lastComment))
73+
assert.NoError(t, pull_service.LoadCommentPushCommits(t.Context(), lastComment))
7474
assert.True(t, lastComment.IsForcePush)
7575
}
7676

0 commit comments

Comments
 (0)