Skip to content

Commit ae63568

Browse files
lunnylunny
authored
Move notifywatch to service layer (#33825)
No logic change.
1 parent 31ddbe1 commit ae63568

File tree

6 files changed

+189
-190
lines changed

6 files changed

+189
-190
lines changed

‎models/activities/action.go

+5-133
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ import (
1616
"code.gitea.io/gitea/models/db"
1717
issues_model "code.gitea.io/gitea/models/issues"
1818
"code.gitea.io/gitea/models/organization"
19-
access_model "code.gitea.io/gitea/models/perm/access"
2019
repo_model "code.gitea.io/gitea/models/repo"
21-
"code.gitea.io/gitea/models/unit"
2220
user_model "code.gitea.io/gitea/models/user"
2321
"code.gitea.io/gitea/modules/git"
2422
"code.gitea.io/gitea/modules/log"
@@ -200,15 +198,13 @@ func (a *Action) LoadActUser(ctx context.Context) {
200198
}
201199
}
202200

203-
func (a *Action) LoadRepo(ctx context.Context) {
201+
func (a *Action) LoadRepo(ctx context.Context) error {
204202
if a.Repo != nil {
205-
return
203+
return nil
206204
}
207205
var err error
208206
a.Repo, err = repo_model.GetRepositoryByID(ctx, a.RepoID)
209-
if err != nil {
210-
log.Error("repo_model.GetRepositoryByID(%d): %v", a.RepoID, err)
211-
}
207+
return err
212208
}
213209

214210
// GetActFullName gets the action's user full name.
@@ -250,7 +246,7 @@ func (a *Action) GetActDisplayNameTitle(ctx context.Context) string {
250246

251247
// GetRepoUserName returns the name of the action repository owner.
252248
func (a *Action) GetRepoUserName(ctx context.Context) string {
253-
a.LoadRepo(ctx)
249+
_ = a.LoadRepo(ctx)
254250
if a.Repo == nil {
255251
return "(non-existing-repo)"
256252
}
@@ -265,7 +261,7 @@ func (a *Action) ShortRepoUserName(ctx context.Context) string {
265261

266262
// GetRepoName returns the name of the action repository.
267263
func (a *Action) GetRepoName(ctx context.Context) string {
268-
a.LoadRepo(ctx)
264+
_ = a.LoadRepo(ctx)
269265
if a.Repo == nil {
270266
return "(non-existing-repo)"
271267
}
@@ -567,130 +563,6 @@ func DeleteOldActions(ctx context.Context, olderThan time.Duration) (err error)
567563
return err
568564
}
569565

570-
// NotifyWatchers creates batch of actions for every watcher.
571-
// It could insert duplicate actions for a repository action, like this:
572-
// * Original action: UserID=1 (the real actor), ActUserID=1
573-
// * Organization action: UserID=100 (the repo's org), ActUserID=1
574-
// * Watcher action: UserID=20 (a user who is watching a repo), ActUserID=1
575-
func NotifyWatchers(ctx context.Context, actions ...*Action) error {
576-
var watchers []*repo_model.Watch
577-
var repo *repo_model.Repository
578-
var err error
579-
var permCode []bool
580-
var permIssue []bool
581-
var permPR []bool
582-
583-
e := db.GetEngine(ctx)
584-
585-
for _, act := range actions {
586-
repoChanged := repo == nil || repo.ID != act.RepoID
587-
588-
if repoChanged {
589-
// Add feeds for user self and all watchers.
590-
watchers, err = repo_model.GetWatchers(ctx, act.RepoID)
591-
if err != nil {
592-
return fmt.Errorf("get watchers: %w", err)
593-
}
594-
}
595-
596-
// Add feed for actioner.
597-
act.UserID = act.ActUserID
598-
if _, err = e.Insert(act); err != nil {
599-
return fmt.Errorf("insert new actioner: %w", err)
600-
}
601-
602-
if repoChanged {
603-
act.LoadRepo(ctx)
604-
repo = act.Repo
605-
606-
// check repo owner exist.
607-
if err := act.Repo.LoadOwner(ctx); err != nil {
608-
return fmt.Errorf("can't get repo owner: %w", err)
609-
}
610-
} else if act.Repo == nil {
611-
act.Repo = repo
612-
}
613-
614-
// Add feed for organization
615-
if act.Repo.Owner.IsOrganization() && act.ActUserID != act.Repo.Owner.ID {
616-
act.ID = 0
617-
act.UserID = act.Repo.Owner.ID
618-
if err = db.Insert(ctx, act); err != nil {
619-
return fmt.Errorf("insert new actioner: %w", err)
620-
}
621-
}
622-
623-
if repoChanged {
624-
permCode = make([]bool, len(watchers))
625-
permIssue = make([]bool, len(watchers))
626-
permPR = make([]bool, len(watchers))
627-
for i, watcher := range watchers {
628-
user, err := user_model.GetUserByID(ctx, watcher.UserID)
629-
if err != nil {
630-
permCode[i] = false
631-
permIssue[i] = false
632-
permPR[i] = false
633-
continue
634-
}
635-
perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
636-
if err != nil {
637-
permCode[i] = false
638-
permIssue[i] = false
639-
permPR[i] = false
640-
continue
641-
}
642-
permCode[i] = perm.CanRead(unit.TypeCode)
643-
permIssue[i] = perm.CanRead(unit.TypeIssues)
644-
permPR[i] = perm.CanRead(unit.TypePullRequests)
645-
}
646-
}
647-
648-
for i, watcher := range watchers {
649-
if act.ActUserID == watcher.UserID {
650-
continue
651-
}
652-
act.ID = 0
653-
act.UserID = watcher.UserID
654-
act.Repo.Units = nil
655-
656-
switch act.OpType {
657-
case ActionCommitRepo, ActionPushTag, ActionDeleteTag, ActionPublishRelease, ActionDeleteBranch:
658-
if !permCode[i] {
659-
continue
660-
}
661-
case ActionCreateIssue, ActionCommentIssue, ActionCloseIssue, ActionReopenIssue:
662-
if !permIssue[i] {
663-
continue
664-
}
665-
case ActionCreatePullRequest, ActionCommentPull, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest, ActionAutoMergePullRequest:
666-
if !permPR[i] {
667-
continue
668-
}
669-
}
670-
671-
if err = db.Insert(ctx, act); err != nil {
672-
return fmt.Errorf("insert new action: %w", err)
673-
}
674-
}
675-
}
676-
return nil
677-
}
678-
679-
// NotifyWatchersActions creates batch of actions for every watcher.
680-
func NotifyWatchersActions(ctx context.Context, acts []*Action) error {
681-
ctx, committer, err := db.TxContext(ctx)
682-
if err != nil {
683-
return err
684-
}
685-
defer committer.Close()
686-
for _, act := range acts {
687-
if err := NotifyWatchers(ctx, act); err != nil {
688-
return err
689-
}
690-
}
691-
return committer.Commit()
692-
}
693-
694566
// DeleteIssueActions delete all actions related with issueID
695567
func DeleteIssueActions(ctx context.Context, repoID, issueID, issueIndex int64) error {
696568
// delete actions assigned to this issue

‎models/activities/action_test.go

-37
Original file line numberDiff line numberDiff line change
@@ -82,43 +82,6 @@ func TestActivityReadable(t *testing.T) {
8282
}
8383
}
8484

85-
func TestNotifyWatchers(t *testing.T) {
86-
assert.NoError(t, unittest.PrepareTestDatabase())
87-
88-
action := &activities_model.Action{
89-
ActUserID: 8,
90-
RepoID: 1,
91-
OpType: activities_model.ActionStarRepo,
92-
}
93-
assert.NoError(t, activities_model.NotifyWatchers(db.DefaultContext, action))
94-
95-
// One watchers are inactive, thus action is only created for user 8, 1, 4, 11
96-
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
97-
ActUserID: action.ActUserID,
98-
UserID: 8,
99-
RepoID: action.RepoID,
100-
OpType: action.OpType,
101-
})
102-
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
103-
ActUserID: action.ActUserID,
104-
UserID: 1,
105-
RepoID: action.RepoID,
106-
OpType: action.OpType,
107-
})
108-
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
109-
ActUserID: action.ActUserID,
110-
UserID: 4,
111-
RepoID: action.RepoID,
112-
OpType: action.OpType,
113-
})
114-
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
115-
ActUserID: action.ActUserID,
116-
UserID: 11,
117-
RepoID: action.RepoID,
118-
OpType: action.OpType,
119-
})
120-
}
121-
12285
func TestConsistencyUpdateAction(t *testing.T) {
12386
if !setting.Database.Type.IsSQLite3() {
12487
t.Skip("Test is only for SQLite database.")

‎routers/web/feed/convert.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func toReleaseLink(ctx *context.Context, act *activities_model.Action) string {
5050

5151
// renderCommentMarkdown renders the comment markdown to html
5252
func renderCommentMarkdown(ctx *context.Context, act *activities_model.Action, content string) template.HTML {
53-
act.LoadRepo(ctx)
53+
_ = act.LoadRepo(ctx)
5454
if act.Repo == nil {
5555
return ""
5656
}

‎services/feed/feed.go

+127
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,138 @@ package feed
55

66
import (
77
"context"
8+
"fmt"
89

910
activities_model "code.gitea.io/gitea/models/activities"
11+
"code.gitea.io/gitea/models/db"
12+
access_model "code.gitea.io/gitea/models/perm/access"
13+
repo_model "code.gitea.io/gitea/models/repo"
14+
"code.gitea.io/gitea/models/unit"
15+
user_model "code.gitea.io/gitea/models/user"
16+
"code.gitea.io/gitea/modules/setting"
1017
)
1118

1219
// GetFeeds returns actions according to the provided options
1320
func GetFeeds(ctx context.Context, opts activities_model.GetFeedsOptions) (activities_model.ActionList, int64, error) {
1421
return activities_model.GetFeeds(ctx, opts)
1522
}
23+
24+
// notifyWatchers creates batch of actions for every watcher.
25+
// It could insert duplicate actions for a repository action, like this:
26+
// * Original action: UserID=1 (the real actor), ActUserID=1
27+
// * Organization action: UserID=100 (the repo's org), ActUserID=1
28+
// * Watcher action: UserID=20 (a user who is watching a repo), ActUserID=1
29+
func notifyWatchers(ctx context.Context, act *activities_model.Action, watchers []*repo_model.Watch, permCode, permIssue, permPR []bool) error {
30+
// Add feed for actioner.
31+
act.UserID = act.ActUserID
32+
if err := db.Insert(ctx, act); err != nil {
33+
return fmt.Errorf("insert new actioner: %w", err)
34+
}
35+
36+
// Add feed for organization
37+
if act.Repo.Owner.IsOrganization() && act.ActUserID != act.Repo.Owner.ID {
38+
act.ID = 0
39+
act.UserID = act.Repo.Owner.ID
40+
if err := db.Insert(ctx, act); err != nil {
41+
return fmt.Errorf("insert new actioner: %w", err)
42+
}
43+
}
44+
45+
for i, watcher := range watchers {
46+
if act.ActUserID == watcher.UserID {
47+
continue
48+
}
49+
act.ID = 0
50+
act.UserID = watcher.UserID
51+
act.Repo.Units = nil
52+
53+
switch act.OpType {
54+
case activities_model.ActionCommitRepo, activities_model.ActionPushTag, activities_model.ActionDeleteTag, activities_model.ActionPublishRelease, activities_model.ActionDeleteBranch:
55+
if !permCode[i] {
56+
continue
57+
}
58+
case activities_model.ActionCreateIssue, activities_model.ActionCommentIssue, activities_model.ActionCloseIssue, activities_model.ActionReopenIssue:
59+
if !permIssue[i] {
60+
continue
61+
}
62+
case activities_model.ActionCreatePullRequest, activities_model.ActionCommentPull, activities_model.ActionMergePullRequest, activities_model.ActionClosePullRequest, activities_model.ActionReopenPullRequest, activities_model.ActionAutoMergePullRequest:
63+
if !permPR[i] {
64+
continue
65+
}
66+
}
67+
68+
if err := db.Insert(ctx, act); err != nil {
69+
return fmt.Errorf("insert new action: %w", err)
70+
}
71+
}
72+
73+
return nil
74+
}
75+
76+
// NotifyWatchersActions creates batch of actions for every watcher.
77+
func NotifyWatchers(ctx context.Context, acts ...*activities_model.Action) error {
78+
return db.WithTx(ctx, func(ctx context.Context) error {
79+
if len(acts) == 0 {
80+
return nil
81+
}
82+
83+
repoID := acts[0].RepoID
84+
if repoID == 0 {
85+
setting.PanicInDevOrTesting("action should belong to a repo")
86+
return nil
87+
}
88+
if err := acts[0].LoadRepo(ctx); err != nil {
89+
return err
90+
}
91+
repo := acts[0].Repo
92+
if err := repo.LoadOwner(ctx); err != nil {
93+
return err
94+
}
95+
96+
actUserID := acts[0].ActUserID
97+
98+
// Add feeds for user self and all watchers.
99+
watchers, err := repo_model.GetWatchers(ctx, repoID)
100+
if err != nil {
101+
return fmt.Errorf("get watchers: %w", err)
102+
}
103+
104+
permCode := make([]bool, len(watchers))
105+
permIssue := make([]bool, len(watchers))
106+
permPR := make([]bool, len(watchers))
107+
for i, watcher := range watchers {
108+
user, err := user_model.GetUserByID(ctx, watcher.UserID)
109+
if err != nil {
110+
permCode[i] = false
111+
permIssue[i] = false
112+
permPR[i] = false
113+
continue
114+
}
115+
perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
116+
if err != nil {
117+
permCode[i] = false
118+
permIssue[i] = false
119+
permPR[i] = false
120+
continue
121+
}
122+
permCode[i] = perm.CanRead(unit.TypeCode)
123+
permIssue[i] = perm.CanRead(unit.TypeIssues)
124+
permPR[i] = perm.CanRead(unit.TypePullRequests)
125+
}
126+
127+
for _, act := range acts {
128+
if act.RepoID != repoID {
129+
setting.PanicInDevOrTesting("action should belong to the same repo, expected[%d], got[%d] ", repoID, act.RepoID)
130+
}
131+
if act.ActUserID != actUserID {
132+
setting.PanicInDevOrTesting("action should have the same actor, expected[%d], got[%d] ", actUserID, act.ActUserID)
133+
}
134+
135+
act.Repo = repo
136+
if err := notifyWatchers(ctx, act, watchers, permCode, permIssue, permPR); err != nil {
137+
return err
138+
}
139+
}
140+
return nil
141+
})
142+
}

0 commit comments

Comments
 (0)