Skip to content

Commit 1ed5f37

Browse files
lunnylunny
authored
Move GetFeeds to service layer (#32526)
Move GetFeeds from models to service layer, no code change.
1 parent 9364099 commit 1ed5f37

File tree

16 files changed

+250
-211
lines changed

16 files changed

+250
-211
lines changed

‎models/activities/action.go

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -448,65 +448,13 @@ type GetFeedsOptions struct {
448448
Date string // the day we want activity for: YYYY-MM-DD
449449
}
450450

451-
// GetFeeds returns actions according to the provided options
452-
func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, error) {
453-
if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil {
454-
return nil, 0, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo")
455-
}
456-
457-
cond, err := activityQueryCondition(ctx, opts)
458-
if err != nil {
459-
return nil, 0, err
460-
}
461-
462-
actions := make([]*Action, 0, opts.PageSize)
463-
var count int64
464-
opts.SetDefaultValues()
465-
466-
if opts.Page < 10 { // TODO: why it's 10 but other values? It's an experience value.
467-
sess := db.GetEngine(ctx).Where(cond)
468-
sess = db.SetSessionPagination(sess, &opts)
469-
470-
count, err = sess.Desc("`action`.created_unix").FindAndCount(&actions)
471-
if err != nil {
472-
return nil, 0, fmt.Errorf("FindAndCount: %w", err)
473-
}
474-
} else {
475-
// First, only query which IDs are necessary, and only then query all actions to speed up the overall query
476-
sess := db.GetEngine(ctx).Where(cond).Select("`action`.id")
477-
sess = db.SetSessionPagination(sess, &opts)
478-
479-
actionIDs := make([]int64, 0, opts.PageSize)
480-
if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil {
481-
return nil, 0, fmt.Errorf("Find(actionsIDs): %w", err)
482-
}
483-
484-
count, err = db.GetEngine(ctx).Where(cond).
485-
Table("action").
486-
Cols("`action`.id").Count()
487-
if err != nil {
488-
return nil, 0, fmt.Errorf("Count: %w", err)
489-
}
490-
491-
if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Desc("`action`.created_unix").Find(&actions); err != nil {
492-
return nil, 0, fmt.Errorf("Find: %w", err)
493-
}
494-
}
495-
496-
if err := ActionList(actions).LoadAttributes(ctx); err != nil {
497-
return nil, 0, fmt.Errorf("LoadAttributes: %w", err)
498-
}
499-
500-
return actions, count, nil
501-
}
502-
503451
// ActivityReadable return whether doer can read activities of user
504452
func ActivityReadable(user, doer *user_model.User) bool {
505453
return !user.KeepActivityPrivate ||
506454
doer != nil && (doer.IsAdmin || user.ID == doer.ID)
507455
}
508456

509-
func activityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.Cond, error) {
457+
func ActivityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.Cond, error) {
510458
cond := builder.NewCond()
511459

512460
if opts.RequestedTeam != nil && opts.RequestedUser == nil {

‎models/activities/action_list.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,55 @@ func (actions ActionList) LoadIssues(ctx context.Context) error {
201201
}
202202
return nil
203203
}
204+
205+
// GetFeeds returns actions according to the provided options
206+
func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, error) {
207+
if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil {
208+
return nil, 0, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo")
209+
}
210+
211+
cond, err := ActivityQueryCondition(ctx, opts)
212+
if err != nil {
213+
return nil, 0, err
214+
}
215+
216+
actions := make([]*Action, 0, opts.PageSize)
217+
var count int64
218+
opts.SetDefaultValues()
219+
220+
if opts.Page < 10 { // TODO: why it's 10 but other values? It's an experience value.
221+
sess := db.GetEngine(ctx).Where(cond)
222+
sess = db.SetSessionPagination(sess, &opts)
223+
224+
count, err = sess.Desc("`action`.created_unix").FindAndCount(&actions)
225+
if err != nil {
226+
return nil, 0, fmt.Errorf("FindAndCount: %w", err)
227+
}
228+
} else {
229+
// First, only query which IDs are necessary, and only then query all actions to speed up the overall query
230+
sess := db.GetEngine(ctx).Where(cond).Select("`action`.id")
231+
sess = db.SetSessionPagination(sess, &opts)
232+
233+
actionIDs := make([]int64, 0, opts.PageSize)
234+
if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil {
235+
return nil, 0, fmt.Errorf("Find(actionsIDs): %w", err)
236+
}
237+
238+
count, err = db.GetEngine(ctx).Where(cond).
239+
Table("action").
240+
Cols("`action`.id").Count()
241+
if err != nil {
242+
return nil, 0, fmt.Errorf("Count: %w", err)
243+
}
244+
245+
if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Desc("`action`.created_unix").Find(&actions); err != nil {
246+
return nil, 0, fmt.Errorf("Find: %w", err)
247+
}
248+
}
249+
250+
if err := ActionList(actions).LoadAttributes(ctx); err != nil {
251+
return nil, 0, fmt.Errorf("LoadAttributes: %w", err)
252+
}
253+
254+
return actions, count, nil
255+
}

‎models/activities/action_test.go

Lines changed: 0 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -42,114 +42,6 @@ func TestAction_GetRepoLink(t *testing.T) {
4242
assert.Equal(t, comment.HTMLURL(db.DefaultContext), action.GetCommentHTMLURL(db.DefaultContext))
4343
}
4444

45-
func TestGetFeeds(t *testing.T) {
46-
// test with an individual user
47-
assert.NoError(t, unittest.PrepareTestDatabase())
48-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
49-
50-
actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
51-
RequestedUser: user,
52-
Actor: user,
53-
IncludePrivate: true,
54-
OnlyPerformedBy: false,
55-
IncludeDeleted: true,
56-
})
57-
assert.NoError(t, err)
58-
if assert.Len(t, actions, 1) {
59-
assert.EqualValues(t, 1, actions[0].ID)
60-
assert.EqualValues(t, user.ID, actions[0].UserID)
61-
}
62-
assert.Equal(t, int64(1), count)
63-
64-
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
65-
RequestedUser: user,
66-
Actor: user,
67-
IncludePrivate: false,
68-
OnlyPerformedBy: false,
69-
})
70-
assert.NoError(t, err)
71-
assert.Len(t, actions, 0)
72-
assert.Equal(t, int64(0), count)
73-
}
74-
75-
func TestGetFeedsForRepos(t *testing.T) {
76-
assert.NoError(t, unittest.PrepareTestDatabase())
77-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
78-
privRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
79-
pubRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8})
80-
81-
// private repo & no login
82-
actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
83-
RequestedRepo: privRepo,
84-
IncludePrivate: true,
85-
})
86-
assert.NoError(t, err)
87-
assert.Len(t, actions, 0)
88-
assert.Equal(t, int64(0), count)
89-
90-
// public repo & no login
91-
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
92-
RequestedRepo: pubRepo,
93-
IncludePrivate: true,
94-
})
95-
assert.NoError(t, err)
96-
assert.Len(t, actions, 1)
97-
assert.Equal(t, int64(1), count)
98-
99-
// private repo and login
100-
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
101-
RequestedRepo: privRepo,
102-
IncludePrivate: true,
103-
Actor: user,
104-
})
105-
assert.NoError(t, err)
106-
assert.Len(t, actions, 1)
107-
assert.Equal(t, int64(1), count)
108-
109-
// public repo & login
110-
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
111-
RequestedRepo: pubRepo,
112-
IncludePrivate: true,
113-
Actor: user,
114-
})
115-
assert.NoError(t, err)
116-
assert.Len(t, actions, 1)
117-
assert.Equal(t, int64(1), count)
118-
}
119-
120-
func TestGetFeeds2(t *testing.T) {
121-
// test with an organization user
122-
assert.NoError(t, unittest.PrepareTestDatabase())
123-
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
124-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
125-
126-
actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
127-
RequestedUser: org,
128-
Actor: user,
129-
IncludePrivate: true,
130-
OnlyPerformedBy: false,
131-
IncludeDeleted: true,
132-
})
133-
assert.NoError(t, err)
134-
assert.Len(t, actions, 1)
135-
if assert.Len(t, actions, 1) {
136-
assert.EqualValues(t, 2, actions[0].ID)
137-
assert.EqualValues(t, org.ID, actions[0].UserID)
138-
}
139-
assert.Equal(t, int64(1), count)
140-
141-
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
142-
RequestedUser: org,
143-
Actor: user,
144-
IncludePrivate: false,
145-
OnlyPerformedBy: false,
146-
IncludeDeleted: true,
147-
})
148-
assert.NoError(t, err)
149-
assert.Len(t, actions, 0)
150-
assert.Equal(t, int64(0), count)
151-
}
152-
15345
func TestActivityReadable(t *testing.T) {
15446
tt := []struct {
15547
desc string
@@ -227,26 +119,6 @@ func TestNotifyWatchers(t *testing.T) {
227119
})
228120
}
229121

230-
func TestGetFeedsCorrupted(t *testing.T) {
231-
// Now we will not check for corrupted data in the feeds
232-
// users should run doctor to fix their data
233-
assert.NoError(t, unittest.PrepareTestDatabase())
234-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
235-
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
236-
ID: 8,
237-
RepoID: 1700,
238-
})
239-
240-
actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
241-
RequestedUser: user,
242-
Actor: user,
243-
IncludePrivate: true,
244-
})
245-
assert.NoError(t, err)
246-
assert.Len(t, actions, 1)
247-
assert.Equal(t, int64(1), count)
248-
}
249-
250122
func TestConsistencyUpdateAction(t *testing.T) {
251123
if !setting.Database.Type.IsSQLite3() {
252124
t.Skip("Test is only for SQLite database.")
@@ -322,24 +194,3 @@ func TestDeleteIssueActions(t *testing.T) {
322194
assert.NoError(t, activities_model.DeleteIssueActions(db.DefaultContext, issue.RepoID, issue.ID, issue.Index))
323195
unittest.AssertCount(t, &activities_model.Action{}, 0)
324196
}
325-
326-
func TestRepoActions(t *testing.T) {
327-
assert.NoError(t, unittest.PrepareTestDatabase())
328-
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
329-
_ = db.TruncateBeans(db.DefaultContext, &activities_model.Action{})
330-
for i := 0; i < 3; i++ {
331-
_ = db.Insert(db.DefaultContext, &activities_model.Action{
332-
UserID: 2 + int64(i),
333-
ActUserID: 2,
334-
RepoID: repo.ID,
335-
OpType: activities_model.ActionCommentIssue,
336-
})
337-
}
338-
count, _ := db.Count[activities_model.Action](db.DefaultContext, &db.ListOptions{})
339-
assert.EqualValues(t, 3, count)
340-
actions, _, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
341-
RequestedRepo: repo,
342-
})
343-
assert.NoError(t, err)
344-
assert.Len(t, actions, 1)
345-
}

‎models/activities/user_heatmap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func getUserHeatmapData(ctx context.Context, user *user_model.User, team *organi
4747
groupByName = groupBy
4848
}
4949

50-
cond, err := activityQueryCondition(ctx, GetFeedsOptions{
50+
cond, err := ActivityQueryCondition(ctx, GetFeedsOptions{
5151
RequestedUser: user,
5252
RequestedTeam: team,
5353
Actor: doer,

‎routers/api/v1/org/org.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"code.gitea.io/gitea/routers/api/v1/utils"
2020
"code.gitea.io/gitea/services/context"
2121
"code.gitea.io/gitea/services/convert"
22+
feed_service "code.gitea.io/gitea/services/feed"
2223
"code.gitea.io/gitea/services/org"
2324
user_service "code.gitea.io/gitea/services/user"
2425
)
@@ -447,7 +448,7 @@ func ListOrgActivityFeeds(ctx *context.APIContext) {
447448
ListOptions: listOptions,
448449
}
449450

450-
feeds, count, err := activities_model.GetFeeds(ctx, opts)
451+
feeds, count, err := feed_service.GetFeeds(ctx, opts)
451452
if err != nil {
452453
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
453454
return

‎routers/api/v1/org/team.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"code.gitea.io/gitea/routers/api/v1/utils"
2323
"code.gitea.io/gitea/services/context"
2424
"code.gitea.io/gitea/services/convert"
25+
feed_service "code.gitea.io/gitea/services/feed"
2526
org_service "code.gitea.io/gitea/services/org"
2627
repo_service "code.gitea.io/gitea/services/repository"
2728
)
@@ -882,7 +883,7 @@ func ListTeamActivityFeeds(ctx *context.APIContext) {
882883
ListOptions: listOptions,
883884
}
884885

885-
feeds, count, err := activities_model.GetFeeds(ctx, opts)
886+
feeds, count, err := feed_service.GetFeeds(ctx, opts)
886887
if err != nil {
887888
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
888889
return

‎routers/api/v1/repo/repo.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
actions_service "code.gitea.io/gitea/services/actions"
3535
"code.gitea.io/gitea/services/context"
3636
"code.gitea.io/gitea/services/convert"
37+
feed_service "code.gitea.io/gitea/services/feed"
3738
"code.gitea.io/gitea/services/issue"
3839
repo_service "code.gitea.io/gitea/services/repository"
3940
)
@@ -1313,7 +1314,7 @@ func ListRepoActivityFeeds(ctx *context.APIContext) {
13131314
ListOptions: listOptions,
13141315
}
13151316

1316-
feeds, count, err := activities_model.GetFeeds(ctx, opts)
1317+
feeds, count, err := feed_service.GetFeeds(ctx, opts)
13171318
if err != nil {
13181319
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
13191320
return

‎routers/api/v1/user/user.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"code.gitea.io/gitea/routers/api/v1/utils"
1414
"code.gitea.io/gitea/services/context"
1515
"code.gitea.io/gitea/services/convert"
16+
feed_service "code.gitea.io/gitea/services/feed"
1617
)
1718

1819
// Search search users
@@ -214,7 +215,7 @@ func ListUserActivityFeeds(ctx *context.APIContext) {
214215
ListOptions: listOptions,
215216
}
216217

217-
feeds, count, err := activities_model.GetFeeds(ctx, opts)
218+
feeds, count, err := feed_service.GetFeeds(ctx, opts)
218219
if err != nil {
219220
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
220221
return

‎routers/web/feed/profile.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"code.gitea.io/gitea/models/renderhelper"
1111
"code.gitea.io/gitea/modules/markup/markdown"
1212
"code.gitea.io/gitea/services/context"
13+
feed_service "code.gitea.io/gitea/services/feed"
1314

1415
"github.com/gorilla/feeds"
1516
)
@@ -28,7 +29,7 @@ func ShowUserFeedAtom(ctx *context.Context) {
2829
func showUserFeed(ctx *context.Context, formatType string) {
2930
includePrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
3031

31-
actions, _, err := activities_model.GetFeeds(ctx, activities_model.GetFeedsOptions{
32+
actions, _, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
3233
RequestedUser: ctx.ContextUser,
3334
Actor: ctx.Doer,
3435
IncludePrivate: includePrivate,

‎routers/web/feed/repo.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@ import (
99
activities_model "code.gitea.io/gitea/models/activities"
1010
repo_model "code.gitea.io/gitea/models/repo"
1111
"code.gitea.io/gitea/services/context"
12+
feed_service "code.gitea.io/gitea/services/feed"
1213

1314
"github.com/gorilla/feeds"
1415
)
1516

1617
// ShowRepoFeed shows user activity on the repo as RSS / Atom feed
1718
func ShowRepoFeed(ctx *context.Context, repo *repo_model.Repository, formatType string) {
18-
actions, _, err := activities_model.GetFeeds(ctx, activities_model.GetFeedsOptions{
19+
actions, _, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
1920
RequestedRepo: repo,
2021
Actor: ctx.Doer,
2122
IncludePrivate: true,

0 commit comments

Comments
 (0)